From b454665ecde9f83826b574d7bcf8847cf05836da Mon Sep 17 00:00:00 2001 From: Francesco Leacche Date: Fri, 13 Dec 2024 15:55:20 +0000 Subject: [PATCH] add per_deposit_minimum validation --- signer/src/bitcoin/utxo.rs | 39 +++++-- signer/src/bitcoin/validation.rs | 119 +++++++++++++++------ signer/src/block_observer.rs | 2 + signer/src/context/signer_state.rs | 22 +++- signer/src/emily_client.rs | 8 ++ signer/tests/integration/block_observer.rs | 4 +- 6 files changed, 148 insertions(+), 46 deletions(-) diff --git a/signer/src/bitcoin/utxo.rs b/signer/src/bitcoin/utxo.rs index d71db6739..c7b2d0e0c 100644 --- a/signer/src/bitcoin/utxo.rs +++ b/signer/src/bitcoin/utxo.rs @@ -175,15 +175,18 @@ impl SbtcRequests { // Filter deposit requests based on two constraints: // 1. The user's max fee must be >= our minimum required fee for deposits // (based on fixed deposit tx size) - // 2. The deposit amount must be less than the per-deposit limit - // 3. The total amount being minted must stay under the maximum allowed mintable amount + // 2. The deposit amount must be greater than or equal to the per-deposit minimum + // 3. The deposit amount must be less than or equal to the per-deposit cap + // 4. The total amount being minted must stay under the maximum allowed mintable amount let minimum_deposit_fee = self.compute_minimum_fee(SOLO_DEPOSIT_TX_VSIZE); let max_mintable_cap = self.sbtc_limits.max_mintable_cap().to_sat(); let per_deposit_cap = self.sbtc_limits.per_deposit_cap().to_sat(); + let per_deposit_minimum = self.sbtc_limits.per_deposit_minimum().to_sat(); let mut amount_to_mint: u64 = 0; let deposits = self.deposits.iter().filter_map(|req| { let is_fee_valid = req.max_fee.min(req.amount) >= minimum_deposit_fee; + let is_above_per_deposit_minimum = req.amount >= per_deposit_minimum; let is_within_per_deposit_cap = req.amount <= per_deposit_cap; let is_within_max_mintable_cap = if let Some(new_amount) = amount_to_mint.checked_add(req.amount) { @@ -191,7 +194,11 @@ impl SbtcRequests { } else { false }; - if is_fee_valid && is_within_per_deposit_cap && is_within_max_mintable_cap { + if is_fee_valid + && is_above_per_deposit_minimum + && is_within_per_deposit_cap + && is_within_max_mintable_cap + { amount_to_mint += req.amount; Some(RequestRef::Deposit(req)) } else { @@ -2782,30 +2789,42 @@ mod tests { create_deposit(10_000, 10_000, 0), create_deposit(10_000, 10_000, 0), create_deposit(10_000, 10_000, 0), - ], 3, 30_000, 10_000, 30_000; "should_accept_deposits_until_max_mintable_reached")] + ], 3, 30_000, 0, 10_000, 30_000; "should_accept_deposits_until_max_mintable_reached")] #[test_case(vec![ create_deposit(10_000, 10_000, 0), create_deposit(10_000, 10_000, 0), - ], 1, 10_000, 10_000, 15_000; "should_accept_all_deposits_when_under_max_mintable")] + ], 1, 10_000, 0, 10_000, 15_000; "should_accept_all_deposits_when_under_max_mintable")] #[test_case(vec![ create_deposit(10_000, 10_000, 0), - ], 0, 0, 0, 0; "should_handle_empty_deposit_list")] + ], 0, 0, 0, 0, 0; "should_handle_empty_deposit_list")] #[test_case(vec![ create_deposit(10_000, 0, 0), create_deposit(11_000, 10_000, 0), create_deposit(9_000, 10_000, 0), - ], 1, 9_000, 10_000, 10_000; "should_skip_invalid_fee_and_accept_valid_deposits")] + ], 1, 9_000, 0, 10_000, 10_000; "should_skip_invalid_fee_and_accept_valid_deposits")] #[test_case(vec![ create_deposit(10_001, 10_000, 0), - ], 0, 0, 10_001, 10_000; "should_reject_single_deposit_exceeding_max_mintable")] + ], 0, 0, 0, 10_001, 10_000; "should_reject_single_deposit_exceeding_max_mintable")] #[test_case(vec![ create_deposit(10_000, 10_000, 0), - ], 0, 0, 8_000, 10_000; "should_reject_single_deposit_exceeding_per_deposit_cap")] + ], 0, 0, 0, 8_000, 10_000; "should_reject_single_deposit_exceeding_per_deposit_cap")] + #[test_case(vec![ + create_deposit(5_000, 10_000, 0), + create_deposit(15_000, 10_000, 0), + ], 1, 15_000, 10_000, 20_000, 30_000; "should_reject_deposits_below_per_deposit_minimum")] + #[test_case(vec![ + create_deposit(10_000, 10_000, 0), // accepted + create_deposit(9_000, 10_000, 0), // rejected (below per_deposit_minimum) + create_deposit(21_000, 10_000, 0), // rejected (above per_deposit_cap) + create_deposit(20_000, 10_000, 0), // accepted + create_deposit(20_000, 10_000, 0), // rejected (above max_mintable) + ], 2, 30_000, 10_000, 20_000, 40_000; "should_respect_all_limits")] #[tokio::test] async fn test_construct_transactions_filters_deposits_over_max_mintable( deposits: Vec, num_accepted_requests: usize, accepted_amount: u64, + per_deposit_minimum: u64, per_deposit_cap: u64, max_mintable: u64, ) { @@ -2829,8 +2848,10 @@ mod tests { accept_threshold: 8, sbtc_limits: SbtcLimits::new( None, + Some(Amount::from_sat(per_deposit_minimum)), Some(Amount::from_sat(per_deposit_cap)), None, + None, Some(Amount::from_sat(max_mintable)), ), }; diff --git a/signer/src/bitcoin/validation.rs b/signer/src/bitcoin/validation.rs index dfc7be929..25b166a27 100644 --- a/signer/src/bitcoin/validation.rs +++ b/signer/src/bitcoin/validation.rs @@ -13,6 +13,7 @@ use bitcoin::XOnlyPublicKey; use crate::bitcoin::utxo::FeeAssessment; use crate::bitcoin::utxo::SignerBtcState; use crate::context::Context; +use crate::context::SbtcLimits; use crate::error::Error; use crate::keys::PublicKey; use crate::message::BitcoinPreSignRequest; @@ -296,7 +297,6 @@ impl BitcoinPreSignRequest { // their fees anymore in order for them to be accepted by the // network. signer_state.last_fees = None; - let sbtc_limits = ctx.state().get_current_limits(); let out = BitcoinTxValidationData { signer_sighash: sighashes.signer_sighash(), deposit_sighashes: sighashes.deposit_sighashes(), @@ -306,8 +306,7 @@ impl BitcoinPreSignRequest { reports, chain_tip_height: btc_ctx.chain_tip_height, // If the cap is None, then we assume that it is unlimited. - max_deposit_amount: sbtc_limits.per_deposit_cap(), - max_withdrawal_amount: sbtc_limits.per_withdrawal_cap(), + sbtc_limits: ctx.state().get_current_limits(), }; Ok((out, signer_state)) @@ -333,10 +332,8 @@ pub struct BitcoinTxValidationData { pub tx_fee: Amount, /// the chain tip height. pub chain_tip_height: u64, - /// Maximum amount of BTC allowed to be pegged-in per transaction. - pub max_deposit_amount: Amount, - /// Maximum amount of BTC allowed to be pegged-out per transaction. - pub max_withdrawal_amount: Amount, + /// The current sBTC limits. + pub sbtc_limits: SbtcLimits, } impl BitcoinTxValidationData { @@ -365,7 +362,7 @@ impl BitcoinTxValidationData { self.chain_tip_height, &self.tx, self.tx_fee, - self.max_deposit_amount, + &self.sbtc_limits, ) }); @@ -425,7 +422,7 @@ impl BitcoinTxValidationData { self.chain_tip_height, &self.tx, self.tx_fee, - self.max_withdrawal_amount, + &self.sbtc_limits, ), is_valid_tx, }) @@ -446,7 +443,7 @@ impl BitcoinTxValidationData { self.chain_tip_height, &self.tx, self.tx_fee, - self.max_deposit_amount + &self.sbtc_limits, ), InputValidationResult::Ok | InputValidationResult::CannotSignUtxo ) @@ -457,11 +454,12 @@ impl BitcoinTxValidationData { self.chain_tip_height, &self.tx, self.tx_fee, - self.max_withdrawal_amount, + &self.sbtc_limits, ) { WithdrawalValidationResult::Unsupported | WithdrawalValidationResult::Unknown - | WithdrawalValidationResult::AmountTooHigh => false, + | WithdrawalValidationResult::AmountTooHigh + | WithdrawalValidationResult::AmountTooLow => false, } }); @@ -508,6 +506,8 @@ impl SbtcReports { pub enum InputValidationResult { /// The deposit request passed validation Ok, + /// The deposit request amount is below the allowed per-deposit minimum. + AmountTooLow, /// The deposit request amount exceeds the allowed per-deposit cap. AmountTooHigh, /// The assessed fee exceeds the max-fee in the deposit request. @@ -558,6 +558,8 @@ impl InputValidationResult { #[sqlx(type_name = "TEXT", rename_all = "snake_case")] #[cfg_attr(feature = "testing", derive(fake::Dummy))] pub enum WithdrawalValidationResult { + /// The withdrawal request amount is below the allowed per-withdrawal minimum. + AmountTooLow, /// The withdrawal request amount exceeds the allowed per-withdrawal cap AmountTooHigh, /// The signer does not have a record of the withdrawal request in @@ -678,7 +680,7 @@ impl DepositRequestReport { chain_tip_height: u64, tx: &F, tx_fee: Amount, - max_deposit_amount: Amount, + sbtc_limits: &SbtcLimits, ) -> InputValidationResult where F: FeeAssessment, @@ -702,7 +704,11 @@ impl DepositRequestReport { DepositConfirmationStatus::Confirmed(block_height, _) => block_height, }; - if self.amount > max_deposit_amount.to_sat() { + if self.amount < sbtc_limits.per_deposit_minimum().to_sat() { + return InputValidationResult::AmountTooLow; + } + + if self.amount > sbtc_limits.per_deposit_cap().to_sat() { return InputValidationResult::AmountTooHigh; } @@ -823,7 +829,7 @@ impl WithdrawalRequestReport { _: u64, _: &F, _: Amount, - _max_withdrawal_amount: Amount, + _sbtc_limits: &SbtcLimits, ) -> WithdrawalValidationResult where F: FeeAssessment, @@ -886,7 +892,7 @@ mod tests { }, status: InputValidationResult::TxNotOnBestChain, chain_tip_height: 2, - } ; "deposit-reorged")] + }, 0, u64::MAX ; "deposit-reorged")] #[test_case(DepositReportErrorMapping { report: DepositRequestReport { status: DepositConfirmationStatus::Spent(BitcoinTxId::from([1; 32])), @@ -902,7 +908,7 @@ mod tests { }, status: InputValidationResult::DepositUtxoSpent, chain_tip_height: 2, - } ; "deposit-spent")] + }, 0, u64::MAX ; "deposit-spent")] #[test_case(DepositReportErrorMapping { report: DepositRequestReport { status: DepositConfirmationStatus::Confirmed(0, BitcoinBlockHash::from([0; 32])), @@ -918,7 +924,7 @@ mod tests { }, status: InputValidationResult::NoVote, chain_tip_height: 2, - } ; "deposit-no-vote")] + }, 0, u64::MAX ; "deposit-no-vote")] #[test_case(DepositReportErrorMapping { report: DepositRequestReport { status: DepositConfirmationStatus::Confirmed(0, BitcoinBlockHash::from([0; 32])), @@ -934,7 +940,7 @@ mod tests { }, status: InputValidationResult::CannotSignUtxo, chain_tip_height: 2, - } ; "cannot-sign-for-deposit")] + }, 0, u64::MAX ; "cannot-sign-for-deposit")] #[test_case(DepositReportErrorMapping { report: DepositRequestReport { status: DepositConfirmationStatus::Confirmed(0, BitcoinBlockHash::from([0; 32])), @@ -950,7 +956,7 @@ mod tests { }, status: InputValidationResult::RejectedRequest, chain_tip_height: 2, - } ; "rejected-deposit")] + }, 0, u64::MAX ; "rejected-deposit")] #[test_case(DepositReportErrorMapping { report: DepositRequestReport { status: DepositConfirmationStatus::Confirmed(0, BitcoinBlockHash::from([0; 32])), @@ -966,7 +972,7 @@ mod tests { }, status: InputValidationResult::LockTimeExpiry, chain_tip_height: 2, - } ; "lock-time-expires-soon-1")] + }, 0, u64::MAX ; "lock-time-expires-soon-1")] #[test_case(DepositReportErrorMapping { report: DepositRequestReport { status: DepositConfirmationStatus::Confirmed(0, BitcoinBlockHash::from([0; 32])), @@ -982,7 +988,7 @@ mod tests { }, status: InputValidationResult::LockTimeExpiry, chain_tip_height: 2, - } ; "lock-time-expires-soon-2")] + }, 0, u64::MAX ; "lock-time-expires-soon-2")] #[test_case(DepositReportErrorMapping { report: DepositRequestReport { status: DepositConfirmationStatus::Confirmed(0, BitcoinBlockHash::from([0; 32])), @@ -998,7 +1004,7 @@ mod tests { }, status: InputValidationResult::UnsupportedLockTime, chain_tip_height: 2, - } ; "lock-time-in-time-units-2")] + }, 0, u64::MAX ; "lock-time-in-time-units-2")] #[test_case(DepositReportErrorMapping { report: DepositRequestReport { status: DepositConfirmationStatus::Confirmed(0, BitcoinBlockHash::from([0; 32])), @@ -1014,7 +1020,7 @@ mod tests { }, status: InputValidationResult::Ok, chain_tip_height: 2, - } ; "happy-path")] + }, 0, u64::MAX ; "happy-path")] #[test_case(DepositReportErrorMapping { report: DepositRequestReport { status: DepositConfirmationStatus::Confirmed(0, BitcoinBlockHash::from([0; 32])), @@ -1030,7 +1036,7 @@ mod tests { }, status: InputValidationResult::Unknown, chain_tip_height: 2, - } ; "unknown-prevout")] + }, 0, u64::MAX ; "unknown-prevout")] #[test_case(DepositReportErrorMapping { report: DepositRequestReport { status: DepositConfirmationStatus::Confirmed(0, BitcoinBlockHash::from([0; 32])), @@ -1046,7 +1052,7 @@ mod tests { }, status: InputValidationResult::Ok, chain_tip_height: 2, - } ; "at-the-border")] + }, 0, u64::MAX ; "at-the-border")] #[test_case(DepositReportErrorMapping { report: DepositRequestReport { status: DepositConfirmationStatus::Confirmed(0, BitcoinBlockHash::from([0; 32])), @@ -1062,7 +1068,7 @@ mod tests { }, status: InputValidationResult::FeeTooHigh, chain_tip_height: 2, - } ; "one-sat-too-high-fee-amount")] + }, 0, u64::MAX ; "one-sat-too-high-fee-amount")] #[test_case(DepositReportErrorMapping { report: DepositRequestReport { status: DepositConfirmationStatus::Confirmed(0, BitcoinBlockHash::from([0; 32])), @@ -1078,8 +1084,44 @@ mod tests { }, status: InputValidationResult::FeeTooHigh, chain_tip_height: 2, - } ; "one-sat-too-high-fee")] - fn deposit_report_validation(mapping: DepositReportErrorMapping) { + }, 0, u64::MAX ; "one-sat-too-high-fee")] + #[test_case(DepositReportErrorMapping { + report: DepositRequestReport { + status: DepositConfirmationStatus::Confirmed(0, BitcoinBlockHash::from([0; 32])), + can_sign: Some(true), + can_accept: Some(true), + amount: 100_000_000, + max_fee: u64::MAX, + lock_time: LockTime::from_height(DEPOSIT_LOCKTIME_BLOCK_BUFFER + 3), + outpoint: OutPoint::null(), + deposit_script: ScriptBuf::new(), + reclaim_script: ScriptBuf::new(), + signers_public_key: *sbtc::UNSPENDABLE_TAPROOT_KEY, + }, + status: InputValidationResult::AmountTooHigh, + chain_tip_height: 2, + }, 0, 99_999_999 ; "amount-too-high")] + #[test_case(DepositReportErrorMapping { + report: DepositRequestReport { + status: DepositConfirmationStatus::Confirmed(0, BitcoinBlockHash::from([0; 32])), + can_sign: Some(true), + can_accept: Some(true), + amount: 99_999_999, + max_fee: u64::MAX, + lock_time: LockTime::from_height(DEPOSIT_LOCKTIME_BLOCK_BUFFER + 3), + outpoint: OutPoint::null(), + deposit_script: ScriptBuf::new(), + reclaim_script: ScriptBuf::new(), + signers_public_key: *sbtc::UNSPENDABLE_TAPROOT_KEY, + }, + status: InputValidationResult::AmountTooLow, + chain_tip_height: 2, + }, 100_000_000, u64::MAX ; "amount-too-low")] + fn deposit_report_validation( + mapping: DepositReportErrorMapping, + per_deposit_minimum: u64, + per_deposit_cap: u64, + ) { let mut tx = crate::testing::btc::base_signer_transaction(); tx.input.push(TxIn { previous_output: OutPoint::null(), @@ -1088,10 +1130,19 @@ mod tests { witness: Witness::new(), }); - let status = - mapping - .report - .validate(mapping.chain_tip_height, &tx, TX_FEE, Amount::MAX_MONEY); + let status = mapping.report.validate( + mapping.chain_tip_height, + &tx, + TX_FEE, + &SbtcLimits::new( + None, + Some(Amount::from_sat(per_deposit_minimum)), + Some(Amount::from_sat(per_deposit_cap)), + None, + None, + None, + ), + ); assert_eq!(status, mapping.status); } @@ -1250,6 +1301,8 @@ mod tests { Some(total_cap), None, None, + None, + None, Some(total_cap - sbtc_supply), )); // Create cache with test data diff --git a/signer/src/block_observer.rs b/signer/src/block_observer.rs index a0c46bee5..3be6a27f6 100644 --- a/signer/src/block_observer.rs +++ b/signer/src/block_observer.rs @@ -557,7 +557,9 @@ impl BlockObserver { let limits = SbtcLimits::new( Some(limits.total_cap()), + Some(limits.per_deposit_minimum()), Some(limits.per_deposit_cap()), + Some(limits.per_withdrawal_minimum()), Some(limits.per_withdrawal_cap()), Some(max_mintable), ); diff --git a/signer/src/context/signer_state.rs b/signer/src/context/signer_state.rs index edf3c108f..e6ea08b68 100644 --- a/signer/src/context/signer_state.rs +++ b/signer/src/context/signer_state.rs @@ -82,8 +82,12 @@ impl SignerState { pub struct SbtcLimits { /// Represents the total cap for all pegged-in BTC/sBTC. total_cap: Option, + /// Represents the minimum amount of BTC allowed to be pegged-in per transaction. + per_deposit_minimum: Option, /// Represents the maximum amount of BTC allowed to be pegged-in per transaction. per_deposit_cap: Option, + /// Represents the minimum amount of sBTC allowed to be pegged-out per transaction. + per_withdrawal_minimum: Option, /// Represents the maximum amount of sBTC allowed to be pegged-out per transaction. per_withdrawal_cap: Option, /// Represents the maximum amount of sBTC that can currently be minted. @@ -94,8 +98,8 @@ impl std::fmt::Display for SbtcLimits { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!( f, - "[total cap: {:?}, per-deposit cap: {:?}, per-withdrawal cap: {:?}, max-mintable cap: {:?}]", - self.total_cap, self.per_deposit_cap, self.per_withdrawal_cap, self.max_mintable_cap + "[total cap: {:?}, per-deposit min: {:?}, per-deposit cap: {:?}, per-withdrawal min: {:?}, per-withdrawal cap: {:?}, max-mintable cap: {:?}]", + self.total_cap, self.per_deposit_minimum, self.per_deposit_cap, self.per_withdrawal_minimum, self.per_withdrawal_cap, self.max_mintable_cap ) } } @@ -104,13 +108,17 @@ impl SbtcLimits { /// Create a new `SbtcLimits` object. pub fn new( total_cap: Option, + per_deposit_minimum: Option, per_deposit_cap: Option, + per_withdrawal_minimum: Option, per_withdrawal_cap: Option, max_mintable_cap: Option, ) -> Self { Self { total_cap, + per_deposit_minimum, per_deposit_cap, + per_withdrawal_minimum, per_withdrawal_cap, max_mintable_cap, } @@ -126,11 +134,21 @@ impl SbtcLimits { self.total_cap.is_some() } + /// Get the minimum amount of BTC allowed to be pegged-in per transaction. + pub fn per_deposit_minimum(&self) -> Amount { + self.per_deposit_minimum.unwrap_or(Amount::ZERO) + } + /// Get the maximum amount of BTC allowed to be pegged-in per transaction. pub fn per_deposit_cap(&self) -> Amount { self.per_deposit_cap.unwrap_or(Amount::MAX_MONEY) } + /// Get the minimum amount of sBTC allowed to be pegged-out per transaction. + pub fn per_withdrawal_minimum(&self) -> Amount { + self.per_withdrawal_minimum.unwrap_or(Amount::ZERO) + } + /// Get the maximum amount of sBTC allowed to be pegged-out per transaction. pub fn per_withdrawal_cap(&self) -> Amount { self.per_withdrawal_cap.unwrap_or(Amount::MAX_MONEY) diff --git a/signer/src/emily_client.rs b/signer/src/emily_client.rs index c71a34fef..d7c3238b7 100644 --- a/signer/src/emily_client.rs +++ b/signer/src/emily_client.rs @@ -336,16 +336,24 @@ impl EmilyInteract for EmilyClient { .map_err(Error::EmilyApi)?; let total_cap = limits.peg_cap.and_then(|cap| cap.map(Amount::from_sat)); + let per_deposit_minimum: Option = limits + .per_deposit_minimum + .and_then(|min| min.map(Amount::from_sat)); let per_deposit_cap = limits .per_deposit_cap .and_then(|cap| cap.map(Amount::from_sat)); + let per_withdrawal_minimum: Option = limits + .per_withdrawal_minimum + .and_then(|min| min.map(Amount::from_sat)); let per_withdrawal_cap = limits .per_withdrawal_cap .and_then(|cap| cap.map(Amount::from_sat)); Ok(SbtcLimits::new( total_cap, + per_deposit_minimum, per_deposit_cap, + per_withdrawal_minimum, per_withdrawal_cap, None, )) diff --git a/signer/tests/integration/block_observer.rs b/signer/tests/integration/block_observer.rs index 8e661dd80..d9c0ee1ae 100644 --- a/signer/tests/integration/block_observer.rs +++ b/signer/tests/integration/block_observer.rs @@ -661,9 +661,9 @@ async fn block_observer_stores_donation_and_sbtc_utxos() { #[cfg_attr(not(feature = "integration-tests"), ignore)] #[test_case::test_case(false, SbtcLimits::default(); "no contracts, default limits")] -#[test_case::test_case(false, SbtcLimits::new(Some(bitcoin::Amount::from_sat(1_000)), None, None, None); "no contracts, total cap limit")] +#[test_case::test_case(false, SbtcLimits::new(Some(bitcoin::Amount::from_sat(1_000)), None, None, None, None, None); "no contracts, total cap limit")] #[test_case::test_case(true, SbtcLimits::default(); "deployed contracts, default limits")] -#[test_case::test_case(true, SbtcLimits::new(Some(bitcoin::Amount::from_sat(1_000)), None, None, None); "deployed contracts, total cap limit")] +#[test_case::test_case(true, SbtcLimits::new(Some(bitcoin::Amount::from_sat(1_000)), None, None, None, None, None); "deployed contracts, total cap limit")] #[tokio::test] async fn block_observer_handles_update_limits(deployed: bool, sbtc_limits: SbtcLimits) { // We start with the typical setup with a fresh database and context