Skip to content

Commit

Permalink
simplify deposit validation. save recipients as strings
Browse files Browse the repository at this point in the history
  • Loading branch information
@Jiloc
Jiloc committed Jan 10, 2025
1 parent d0fb288 commit 447d36a
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 127 deletions.
84 changes: 5 additions & 79 deletions emily/handler/src/api/handlers/deposit.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use crate::api::models::deposit::responses::{
GetDepositsForTransactionResponse, UpdateDepositsResponse,
};
use crate::database::entries::StatusEntry;
use stacks_common::codec::StacksMessageCodec as _;
use tracing::{debug, instrument};
use warp::reply::{json, with_status, Reply};

Expand All @@ -23,7 +22,6 @@ use crate::database::entries::deposit::{
DepositEntry, DepositEntryKey, DepositEvent, DepositParametersEntry,
ValidatedUpdateDepositsRequest,
};
use bitcoin::ScriptBuf;
use warp::http::StatusCode;

/// Get deposit handler.
Expand Down Expand Up @@ -248,30 +246,19 @@ pub async fn create_deposit(
Err(e) => return Err(e),
}

let tx = (&body).try_into()?;
let limits = accessors::get_limits(&context).await?;
body.validate(&tx, &limits)?;

// Get parameters from scripts.
let script_parameters = scripts_to_resource_parameters(
&body.deposit_script,
&body.reclaim_script,
tx.tx_out(body.bitcoin_tx_output_index as usize)
.unwrap() // Safe to unwrap. We have already validated the output index in the try_into.
.value
.to_sat(),
)?;
let deposit_info = body.validate(&limits)?;

// Make table entry.
let deposit_entry: DepositEntry = DepositEntry {
key: DepositEntryKey {
bitcoin_txid: body.bitcoin_txid,
bitcoin_tx_output_index: body.bitcoin_tx_output_index,
},
recipient: script_parameters.recipient,
recipient: deposit_info.recipient.to_string(),
parameters: DepositParametersEntry {
max_fee: script_parameters.max_fee,
lock_time: script_parameters.lock_time,
max_fee: deposit_info.max_fee,
lock_time: deposit_info.lock_time.to_consensus_u32(),
},
history: vec![DepositEvent {
status: StatusEntry::Pending,
Expand All @@ -282,7 +269,7 @@ pub async fn create_deposit(
status: Status::Pending,
last_update_block_hash: stacks_block_hash,
last_update_height: stacks_block_height,
amount: script_parameters.amount,
amount: deposit_info.amount,
reclaim_script: body.reclaim_script,
deposit_script: body.deposit_script,
..Default::default()
Expand All @@ -301,40 +288,6 @@ pub async fn create_deposit(
.map_or_else(Reply::into_response, Reply::into_response)
}

/// Parameters from the deposit and reclaim scripts.
struct ScriptParameters {
amount: u64,
max_fee: u64,
recipient: String,
lock_time: u32,
}

/// Convert scripts to resource parameters.
///
/// This function is used to convert the deposit and reclaim scripts into the
/// parameters that are stored in the database.
fn scripts_to_resource_parameters(
deposit_script: &str,
reclaim_script: &str,
amount: u64,
) -> Result<ScriptParameters, Error> {
let deposit_script_buf = ScriptBuf::from_hex(deposit_script)?;
let deposit_script_inputs = sbtc::deposits::DepositScriptInputs::parse(&deposit_script_buf)?;

let reclaim_script_buf = ScriptBuf::from_hex(reclaim_script)?;
let reclaim_script_inputs = sbtc::deposits::ReclaimScriptInputs::parse(&reclaim_script_buf)?;

let recipient_bytes = deposit_script_inputs.recipient.serialize_to_vec();
let recipient_hex_string = hex::encode(&recipient_bytes);

Ok(ScriptParameters {
amount,
max_fee: deposit_script_inputs.max_fee,
recipient: recipient_hex_string,
lock_time: reclaim_script_inputs.lock_time(),
})
}

/// Update deposits handler.
#[utoipa::path(
put,
Expand Down Expand Up @@ -415,30 +368,3 @@ pub async fn update_deposits(
}

// TODO(393): Add handler unit tests.

// Test module
#[cfg(test)]
mod tests {

use super::*;
use sbtc::testing::{self, deposits::TxSetup};
use test_case::test_case;

#[test_case(15000, 500_000, 150; "All parameters are normal numbers")]
#[test_case(0, 0, 0; "All parameters are zeros")]
fn test_scripts_to_resource_parameters(max_fee: u64, amount_sats: u64, lock_time: u32) {
let setup: TxSetup = testing::deposits::tx_setup(lock_time, max_fee, amount_sats);

let deposit_script = setup.deposit.deposit_script().to_hex_string();
let reclaim_script = setup.reclaim.reclaim_script().to_hex_string();

let script_parameters: ScriptParameters =
scripts_to_resource_parameters(&deposit_script, &reclaim_script, amount_sats).unwrap();

assert_eq!(script_parameters.max_fee, max_fee);
assert_eq!(script_parameters.lock_time, lock_time);

// TODO: Test the recipient with an input value.
assert!(script_parameters.recipient.len() > 0);
}
}
65 changes: 17 additions & 48 deletions emily/handler/src/api/models/deposit/requests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use reqwest::StatusCode;
use serde::{Deserialize, Serialize};
use utoipa::ToSchema;

use sbtc::deposits::CreateDepositRequest;
use sbtc::deposits::{CreateDepositRequest, DepositInfo};

use crate::api::models::common::{Fulfillment, Status};
use crate::api::models::limits::Limits;
Expand Down Expand Up @@ -73,7 +73,7 @@ impl CreateDepositRequestBody {
/// Validates that the deposit request is valid.
/// This includes validating the request fields, if their content matches the transaction
/// and if the amount is within the limits.
pub fn validate(&self, tx: &Transaction, limits: &Limits) -> Result<(), Error> {
pub fn validate(&self, limits: &Limits) -> Result<DepositInfo, Error> {
let deposit_req = CreateDepositRequest {
outpoint: OutPoint {
txid: parse_hex(&self.bitcoin_txid, "invalid bitcoin_txid", Txid::from_str)?,
Expand All @@ -91,6 +91,12 @@ impl CreateDepositRequestBody {
)?,
};

let tx: Transaction = parse_hex(
&self.transaction_hex,
"invalid transaction_hex",
encode::deserialize_hex,
)?;

let amount = tx
.tx_out(self.bitcoin_tx_output_index as usize)
.map_err(|_| {
Expand All @@ -101,9 +107,6 @@ impl CreateDepositRequestBody {
})?
.value
.to_sat();
deposit_req
.validate_tx(tx)
.map_err(|e| Error::HttpRequest(StatusCode::BAD_REQUEST, e.to_string()))?;

// Even if no limits are set, the deposit amount should be higher than the dust limit.
let min = limits.per_deposit_minimum.unwrap_or(DEPOSIT_DUST_LIMIT);
Expand All @@ -121,21 +124,13 @@ impl CreateDepositRequestBody {
format!("deposit amount exceeds cap ({})", cap),
));
}
Ok(())
}
}

impl TryInto<Transaction> for &CreateDepositRequestBody {
type Error = Error;

fn try_into(self) -> Result<Transaction, Error> {
parse_hex(
&self.transaction_hex,
"invalid transaction_hex",
encode::deserialize_hex,
)
deposit_req
.validate_tx(&tx)
.map_err(|e| Error::HttpRequest(StatusCode::BAD_REQUEST, e.to_string()))
}
}

/// A singlular Deposit update that contains only the fields pertinent
/// to updating the status of a deposit. This includes the key related
/// data in addition to status history related data.
Expand Down Expand Up @@ -217,41 +212,19 @@ mod tests {
pub fn parse_request(json: &str) -> CreateDepositRequestBody {
serde_json::from_str(json).expect("failed to parse request")
}

pub fn parse_transaction(request: &CreateDepositRequestBody) -> Transaction {
request.try_into().expect("failed to parse transaction")
}
}

#[tokio::test]
async fn test_transaction_deserialization_valid() {
let deposit_request = helpers::parse_request(CREATE_DEPOSIT_VALID);
let result: Result<Transaction, Error> = (&deposit_request).try_into();
assert!(result.is_ok());
}

#[tokio::test]
async fn test_transaction_deserialization_invalid() {
let deposit_request = helpers::parse_request(CREATE_DEPOSIT_INVALID_TRANSACTION_HEX);
let result: Result<Transaction, Error> = (&deposit_request).try_into();
assert_eq!(
result.unwrap_err().to_string(),
"HTTP request failed with status code 400 Bad Request: invalid transaction_hex"
.to_string()
);
}

#[tokio::test]
async fn test_deposit_validate_happy_path() {
let deposit_request = helpers::parse_request(CREATE_DEPOSIT_VALID);
let tx = helpers::parse_transaction(&deposit_request);
let limits = helpers::create_test_limits(None, None);
assert!(deposit_request.validate(&tx, &limits).is_ok());
assert!(deposit_request.validate(&limits).is_ok());
}

#[test_case(CREATE_DEPOSIT_INVALID_TXID, "invalid bitcoin_txid"; "invalid_txid")]
#[test_case(CREATE_DEPOSIT_INVALID_RECLAIM_SCRIPT, "invalid reclaim_script"; "invalid_reclaim_script")]
#[test_case(CREATE_DEPOSIT_INVALID_DEPOSIT_SCRIPT, "invalid deposit_script"; "invalid_deposit_script")]
#[test_case(CREATE_DEPOSIT_INVALID_TRANSACTION_HEX, "invalid transaction_hex"; "invalid_transaction_hex")]
#[test_case(CREATE_DEPOSIT_INVALID_OUTPUT_INDEX, "invalid bitcoin_output_index"; "invalid_output_index")]
#[test_case(CREATE_DEPOSIT_MISMATCH_TXID, "The txid of the transaction did not match the given txid"; "mismatch_txid")]
#[test_case(
Expand All @@ -265,10 +238,9 @@ mod tests {
#[tokio::test]
async fn test_deposit_validate_errors(input: &str, expected_error: &str) {
let deposit_request = helpers::parse_request(input);
let tx = helpers::parse_transaction(&deposit_request);
let limits = helpers::create_test_limits(Some(DEPOSIT_DUST_LIMIT), None);

let result = deposit_request.validate(&tx, &limits);
let result = deposit_request.validate(&limits);
assert_eq!(
result.unwrap_err().to_string(),
format!("HTTP request failed with status code 400 Bad Request: {expected_error}")
Expand All @@ -283,9 +255,8 @@ mod tests {
#[tokio::test]
async fn test_deposit_validate_limits(input: &str, min: Option<u64>, max: Option<u64>) {
let deposit_request = helpers::parse_request(input);
let tx = helpers::parse_transaction(&deposit_request);
let limits = helpers::create_test_limits(min, max);
assert!(deposit_request.validate(&tx, &limits).is_ok());
assert!(deposit_request.validate(&limits).is_ok());
}

#[test_case(CREATE_DEPOSIT_VALID, Some(1_000_000 + 1), None, "deposit amount below minimum (1000001)"; "below_min_limit")]
Expand All @@ -298,12 +269,10 @@ mod tests {
expected_error: &str,
) {
let deposit_request = helpers::parse_request(input);
let tx = helpers::parse_transaction(&deposit_request);
let limits = helpers::create_test_limits(min, max);

let result = deposit_request.validate(&tx, &limits);
let result = deposit_request.validate(&limits);

assert!(result.is_err());
assert_eq!(
result.unwrap_err().to_string(),
format!("HTTP request failed with status code 400 Bad Request: {expected_error}")
Expand Down

0 comments on commit 447d36a

Please sign in to comment.