From ddecb5036d6097e35003a05dc387f8dc68814ba7 Mon Sep 17 00:00:00 2001 From: Joey Yandle Date: Mon, 10 Feb 2025 17:22:58 -0500 Subject: [PATCH 1/4] sketch out reject-withdrawal-request validation and implement first two checks call iter not into_iter for lint check check that the withdrawal-request is final on the btc chain use corrent block hash in storage requests and errors use proper chain tip errors create withdrawal-reject integration tests based on the withdrawal-accept tests log error since we don't have good error returns yet clean up error handling disable most reject tests but leave in the ones that are analaguous to the accept ones don't clone if you can copy add some temporary logging use block heights to determine if 6 blocks have passed since the withdrawal request failure to get withdrawal signers or request report should map to RequestMissing remove unused tests; use bitmap mismatch test move QualifiedRequestId into common proto; RejectWithdrawalV1 now has a QualifiedRequestId in both code and proto; deal with fallout fix tests to use QualifiedRequestId for RejectWithdrawalV1 map the DB error to the expected type remove stacks/common.proto since the linter complained it wasn't being used check reject threshold use TestSweepSetup2 to try to fix problems with the signer set length address review comments, though they seem to have made things worse use sweep block height not fake data in TestSweepSetup2 hard wire the correct signing threshold into ReqContext add TestSweep2 reject_withdrawal_request and call it from happy path test use Faker for block height if we haven't swept yet remove eprintln remove TODO comment use ctx.config().signer.public_key() not req_ctx.origin wrap comments; remove commented out code don't map errors; set request_id as i64::MAX not u64::MAX so we don't get a SQL error in the conversion pass db into make_withdrawal_reject2 and use it to get the chain tip; use make_withdrawal_reject2 in reject_withdrawal_validation_bitmap_mismatch so it will have the chain tip in req_ctx; use req_ctx to get chain tip in validate split let into two statements for readability remove Withdrawal token from error variants check to see if withdrawal request has expired in validate; add test for expiry check expired with and without rejection cases add test for not_expired_not_rejected, which tests that validate returns RequestNotRejected get the chain tip directly from the db after failing with RequestNotFinal mine an extra block and show that validate now succeeds remove unused error variants simplify validation failure conditions as per #1363 fail validation if the request is not final, i.e. expired --- protobufs/stacks/signer/v1/common.proto | 28 ++ protobufs/stacks/signer/v1/messages.proto | 25 +- protobufs/stacks/signer/v1/requests.proto | 3 +- signer/build.rs | 1 + signer/src/error.rs | 6 + signer/src/proto/convert.rs | 4 +- .../src/proto/generated/stacks.signer.v1.rs | 60 +-- signer/src/stacks/contracts.rs | 144 +++++- signer/src/testing/dummy.rs | 2 +- signer/src/testing/message.rs | 2 +- signer/tests/integration/contracts.rs | 9 +- signer/tests/integration/main.rs | 1 + signer/tests/integration/postgres.rs | 6 +- signer/tests/integration/setup.rs | 12 +- signer/tests/integration/withdrawal_reject.rs | 459 ++++++++++++++++++ 15 files changed, 695 insertions(+), 67 deletions(-) create mode 100644 protobufs/stacks/signer/v1/common.proto create mode 100644 signer/tests/integration/withdrawal_reject.rs diff --git a/protobufs/stacks/signer/v1/common.proto b/protobufs/stacks/signer/v1/common.proto new file mode 100644 index 000000000..2c996c120 --- /dev/null +++ b/protobufs/stacks/signer/v1/common.proto @@ -0,0 +1,28 @@ +syntax = "proto3"; + +package stacks.signer.v1; + +import "stacks/common.proto"; + +// An identifier for a withdrawal request, comprised of the Stacks +// transaction ID, the Stacks block ID that included the transaction, and +// the request-id generated by the clarity contract for the withdrawal +// request. +message QualifiedRequestId { + // The ID that was generated in the clarity contract call for the + // withdrawal request. + uint64 request_id = 1; + // The txid that generated the request. + stacks.StacksTxid txid = 2; + // The Stacks block ID that includes the transaction that generated + // the request. + stacks.StacksBlockId block_hash = 3; +} + +// Describes the fees for a transaction. +message Fees { + // The total fee paid in sats for the transaction. + uint64 total = 1; + // The fee rate paid in sats per virtual byte. + double rate = 2; +} diff --git a/protobufs/stacks/signer/v1/messages.proto b/protobufs/stacks/signer/v1/messages.proto index 35ef81a8d..f58c154d5 100644 --- a/protobufs/stacks/signer/v1/messages.proto +++ b/protobufs/stacks/signer/v1/messages.proto @@ -5,7 +5,7 @@ package stacks.signer.v1; import "bitcoin/bitcoin.proto"; import "crypto/common.proto"; import "crypto/wsts/wsts.proto"; -import "stacks/common.proto"; +import "stacks/signer/v1/common.proto"; import "stacks/signer/v1/decisions.proto"; import "stacks/signer/v1/requests.proto"; @@ -110,26 +110,3 @@ message TxRequestIds { // transaction. repeated QualifiedRequestId withdrawals = 2; } - -// An identifier for a withdrawal request, comprised of the Stacks -// transaction ID, the Stacks block ID that included the transaction, and -// the request-id generated by the clarity contract for the withdrawal -// request. -message QualifiedRequestId { - // The ID that was generated in the clarity contract call for the - // withdrawal request. - uint64 request_id = 1; - // The txid that generated the request. - stacks.StacksTxid txid = 2; - // The Stacks block ID that includes the transaction that generated - // the request. - stacks.StacksBlockId block_hash = 3; -} - -// Describes the fees for a transaction. -message Fees { - // The total fee paid in sats for the transaction. - uint64 total = 1; - // The fee rate paid in sats per virtual byte. - double rate = 2; -} diff --git a/protobufs/stacks/signer/v1/requests.proto b/protobufs/stacks/signer/v1/requests.proto index b30ad67ea..7c8b27d07 100644 --- a/protobufs/stacks/signer/v1/requests.proto +++ b/protobufs/stacks/signer/v1/requests.proto @@ -5,6 +5,7 @@ package stacks.signer.v1; import "bitcoin/bitcoin.proto"; import "crypto/common.proto"; import "stacks/common.proto"; +import "stacks/signer/v1/common.proto"; // Represents a request to sign a Stacks transaction. message StacksTransactionSignRequest { @@ -108,7 +109,7 @@ message RejectWithdrawal { // The ID of the withdrawal request generated by the // `initiate-withdrawal-request` function in the sbtc-withdrawal smart // contract. - uint64 request_id = 1; + QualifiedRequestId id = 1; // A bitmap of how the signers voted. The length of the list must be less // than or equal to 128. Here, we assume that a true implies that the // associated signer voted *against* the withdrawal. diff --git a/signer/build.rs b/signer/build.rs index db2e5a2b5..ebfb5e5d8 100644 --- a/signer/build.rs +++ b/signer/build.rs @@ -46,6 +46,7 @@ pub fn compile_protos() { "protobufs/crypto/wsts/state.proto", "protobufs/crypto/wsts/wsts.proto", "protobufs/stacks/common.proto", + "protobufs/stacks/signer/v1/common.proto", "protobufs/stacks/signer/v1/decisions.proto", "protobufs/stacks/signer/v1/requests.proto", "protobufs/stacks/signer/v1/messages.proto", diff --git a/signer/src/error.rs b/signer/src/error.rs index 3f1129269..38ad960e4 100644 --- a/signer/src/error.rs +++ b/signer/src/error.rs @@ -12,6 +12,7 @@ use crate::keys::PublicKeyXOnly; use crate::stacks::contracts::DepositValidationError; use crate::stacks::contracts::RotateKeysValidationError; use crate::stacks::contracts::WithdrawalAcceptValidationError; +use crate::stacks::contracts::WithdrawalRejectValidationError; use crate::storage::model::SigHash; use crate::wsts_state_machine::StateMachineId; @@ -582,6 +583,11 @@ pub enum Error { #[error("withdrawal accept validation error: {0}")] WithdrawalAcceptValidation(#[source] Box), + /// The error for when the request to sign a withdrawal-reject + /// transaction fails at the validation step. + #[error("withdrawal reject validation error: {0}")] + WithdrawalRejectValidation(#[source] Box), + /// WSTS error. #[error("WSTS error: {0}")] Wsts(#[source] wsts::state_machine::signer::Error), diff --git a/signer/src/proto/convert.rs b/signer/src/proto/convert.rs index a8548c56e..c8e24ea1a 100644 --- a/signer/src/proto/convert.rs +++ b/signer/src/proto/convert.rs @@ -454,7 +454,7 @@ impl TryFrom for AcceptWithdrawalV1 { impl From for proto::RejectWithdrawal { fn from(value: RejectWithdrawalV1) -> Self { proto::RejectWithdrawal { - request_id: value.request_id, + id: Some(value.id.into()), signer_bitmap: value.signer_bitmap.iter().map(|e| *e).collect(), deployer: Some(value.deployer.into()), } @@ -478,7 +478,7 @@ impl TryFrom for RejectWithdrawalV1 { }); Ok(RejectWithdrawalV1 { - request_id: value.request_id, + id: value.id.required()?.try_into()?, signer_bitmap, deployer: value.deployer.required()?.try_into()?, }) diff --git a/signer/src/proto/generated/stacks.signer.v1.rs b/signer/src/proto/generated/stacks.signer.v1.rs index 1bb154989..59a6a6ec2 100644 --- a/signer/src/proto/generated/stacks.signer.v1.rs +++ b/signer/src/proto/generated/stacks.signer.v1.rs @@ -1,4 +1,32 @@ // This file is @generated by prost-build. +/// An identifier for a withdrawal request, comprised of the Stacks +/// transaction ID, the Stacks block ID that included the transaction, and +/// the request-id generated by the clarity contract for the withdrawal +/// request. +#[derive(Clone, Copy, PartialEq, ::prost::Message)] +pub struct QualifiedRequestId { + /// The ID that was generated in the clarity contract call for the + /// withdrawal request. + #[prost(uint64, tag = "1")] + pub request_id: u64, + /// The txid that generated the request. + #[prost(message, optional, tag = "2")] + pub txid: ::core::option::Option, + /// The Stacks block ID that includes the transaction that generated + /// the request. + #[prost(message, optional, tag = "3")] + pub block_hash: ::core::option::Option, +} +/// Describes the fees for a transaction. +#[derive(Clone, Copy, PartialEq, ::prost::Message)] +pub struct Fees { + /// The total fee paid in sats for the transaction. + #[prost(uint64, tag = "1")] + pub total: u64, + /// The fee rate paid in sats per virtual byte. + #[prost(double, tag = "2")] + pub rate: f64, +} /// Represents a decision to accept or reject a deposit request. #[derive(Clone, Copy, PartialEq, ::prost::Message)] pub struct SignerDepositDecision { @@ -166,8 +194,8 @@ pub struct RejectWithdrawal { /// The ID of the withdrawal request generated by the /// `initiate-withdrawal-request` function in the sbtc-withdrawal smart /// contract. - #[prost(uint64, tag = "1")] - pub request_id: u64, + #[prost(message, optional, tag = "1")] + pub id: ::core::option::Option, /// A bitmap of how the signers voted. The length of the list must be less /// than or equal to 128. Here, we assume that a true implies that the /// associated signer voted *against* the withdrawal. @@ -399,31 +427,3 @@ pub struct TxRequestIds { #[prost(message, repeated, tag = "2")] pub withdrawals: ::prost::alloc::vec::Vec, } -/// An identifier for a withdrawal request, comprised of the Stacks -/// transaction ID, the Stacks block ID that included the transaction, and -/// the request-id generated by the clarity contract for the withdrawal -/// request. -#[derive(Clone, Copy, PartialEq, ::prost::Message)] -pub struct QualifiedRequestId { - /// The ID that was generated in the clarity contract call for the - /// withdrawal request. - #[prost(uint64, tag = "1")] - pub request_id: u64, - /// The txid that generated the request. - #[prost(message, optional, tag = "2")] - pub txid: ::core::option::Option, - /// The Stacks block ID that includes the transaction that generated - /// the request. - #[prost(message, optional, tag = "3")] - pub block_hash: ::core::option::Option, -} -/// Describes the fees for a transaction. -#[derive(Clone, Copy, PartialEq, ::prost::Message)] -pub struct Fees { - /// The total fee paid in sats for the transaction. - #[prost(uint64, tag = "1")] - pub total: u64, - /// The fee rate paid in sats per virtual byte. - #[prost(double, tag = "2")] - pub rate: f64, -} diff --git a/signer/src/stacks/contracts.rs b/signer/src/stacks/contracts.rs index e78c0824d..78ed39c3a 100644 --- a/signer/src/stacks/contracts.rs +++ b/signer/src/stacks/contracts.rs @@ -44,6 +44,7 @@ use blockstack_lib::types::chainstate::StacksAddress; use blockstack_lib::util_lib::strings::StacksString; use clarity::vm::ClarityVersion; +use crate::bitcoin::validation::WithdrawalRequestStatus; use crate::bitcoin::BitcoinInteract; use crate::context::Context; use crate::error::Error; @@ -53,9 +54,11 @@ use crate::storage::model::BitcoinBlockHash; use crate::storage::model::BitcoinBlockRef; use crate::storage::model::BitcoinTxId; use crate::storage::model::DkgSharesStatus; +use crate::storage::model::QualifiedRequestId; use crate::storage::model::ToLittleEndianOrder as _; use crate::storage::DbRead; use crate::DEPOSIT_DUST_LIMIT; +use crate::WITHDRAWAL_BLOCKS_EXPIRY; use super::api::StacksInteract; @@ -884,6 +887,33 @@ impl std::error::Error for WithdrawalAcceptValidationError { } } +/// A struct for a validation error containing all the necessary context. +#[derive(Debug)] +pub struct WithdrawalRejectValidationError { + /// The specific error that happened during validation. + pub error: WithdrawalRejectErrorMsg, + /// The additional information that was used when trying to validate + /// the `reject-withdrawal-request` contract call. This includes the + /// public key of the signer that was attempting to generate the + /// `reject-withdrawal-request` transaction. + pub context: ReqContext, + /// The specific transaction that was being validated. + pub tx: RejectWithdrawalV1, +} + +impl std::fmt::Display for WithdrawalRejectValidationError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + // TODO(191): Add the other variables to the error message. + self.error.fmt(f) + } +} + +impl std::error::Error for WithdrawalRejectValidationError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + Some(&self.error) + } +} + /// The responses for validation of an accept-withdrawal-request smart /// contract call transaction. #[derive(Debug, thiserror::Error, PartialEq, Eq)] @@ -941,6 +971,42 @@ impl WithdrawalErrorMsg { } } +/// The responses for validation of a reject-withdrawal-request smart +/// contract call transaction. +#[derive(Debug, thiserror::Error, PartialEq, Eq)] +pub enum WithdrawalRejectErrorMsg { + /// The bitmap set in the transaction object should match the one in + /// our database. + #[error("bitmap does not match expected bitmap from")] + BitmapMismatch, + /// The smart contract deployer is fixed, so this should always match. + #[error("the deployer in the transaction does not match the expected deployer")] + DeployerMismatch, + /// We do not have a record of the withdrawal request in our list of + /// pending and accepted withdrawal requests. + #[error("no record of withdrawal request in pending and accepted withdrawal requests")] + RequestMissing, + /// Withdrawal request fulfilled + #[error("Withdrawal request fulfilled")] + RequestFulfilled, + /// Withdrawal request unconfirmed + #[error("Withdrawal request unconfirmed")] + RequestUnconfirmed, + /// Withdrawal request is not final + #[error("Withdrawal request is not final")] + RequestNotFinal, +} + +impl WithdrawalRejectErrorMsg { + fn into_error(self, ctx: &ReqContext, tx: &RejectWithdrawalV1) -> Error { + Error::WithdrawalRejectValidation(Box::new(WithdrawalRejectValidationError { + error: self, + context: *ctx, + tx: *tx, + })) + } +} + /// This struct is used to generate a properly formatted Stacks transaction /// for calling the reject-withdrawal-request function in the /// sbtc-withdrawal smart contract. @@ -949,7 +1015,7 @@ pub struct RejectWithdrawalV1 { /// The ID of the withdrawal request generated by the /// initiate-withdrawal-request function in the sbtc-withdrawal smart /// contract. - pub request_id: u64, + pub id: QualifiedRequestId, /// A bitmap of how the signers voted. This structure supports up to /// 128 distinct signers. Here, we assume that a 1 (or true) implies /// that the signer voted *against* the transaction. @@ -976,7 +1042,7 @@ impl AsContractCall for RejectWithdrawalV1 { } fn as_contract_args(&self) -> Vec { vec![ - ClarityValue::UInt(self.request_id as u128), + ClarityValue::UInt(self.id.request_id as u128), ClarityValue::UInt(self.signer_bitmap.load_le()), ] } @@ -987,11 +1053,73 @@ impl AsContractCall for RejectWithdrawalV1 { /// an event on the canonical Stacks blockchain. /// 2. That the signer bitmap matches the signer decisions stored in /// this signer's database. - async fn validate(&self, _ctx: &C, _req_ctx: &ReqContext) -> Result<(), Error> + async fn validate(&self, ctx: &C, req_ctx: &ReqContext) -> Result<(), Error> where C: Context + Send + Sync, { - // TODO(255): Add validation implementation + if self.deployer != req_ctx.deployer { + return Err(WithdrawalRejectErrorMsg::DeployerMismatch.into_error(req_ctx, self)); + } + + // 1. The request exists. Check whether the associated withdrawal request transaction + // is confirmed on the canonical stacks blockchain. Fail the withdrawal request if + // it is not on the canonical stacks blockchain. + // + // 2. The double spend check. Check whether the qualified request ID is in the + // bitcoin_withdrawals_outputs table, and that any of the associated txids are + // confirmed on the canonical bitcoin blockchain. Fail the withdrawal request if + // such a transaction was found. + + let stacks_chain_tip = ctx + .get_storage() + .get_stacks_chain_tip(&req_ctx.chain_tip.block_hash) + .await + .unwrap() + .unwrap(); + let maybe_report = ctx + .get_storage() + .get_withdrawal_request_report( + &req_ctx.chain_tip.block_hash, + &stacks_chain_tip.block_hash, + &self.id, + &ctx.config().signer.public_key(), + ) + .await?; + + let Some(report) = maybe_report else { + return Err(WithdrawalRejectErrorMsg::RequestMissing.into_error(req_ctx, self)); + }; + + match report.status { + WithdrawalRequestStatus::Confirmed => (), + WithdrawalRequestStatus::Fulfilled(_txid) => { + // fails #2 + return Err(WithdrawalRejectErrorMsg::RequestFulfilled.into_error(req_ctx, self)); + } + WithdrawalRequestStatus::Unconfirmed => { + // fails #1 + return Err(WithdrawalRejectErrorMsg::RequestUnconfirmed.into_error(req_ctx, self)); + } + } + + let signer_votes = ctx + .get_storage() + .get_withdrawal_request_signer_votes(&self.id, &req_ctx.aggregate_key) + .await?; + let signer_bitmap = BitArray::<[u8; 16]>::from(signer_votes); + + if signer_bitmap != self.signer_bitmap { + return Err(WithdrawalRejectErrorMsg::BitmapMismatch.into_error(req_ctx, self)); + } + + let request_block_height = report.bitcoin_block_height; + let blocks_observed = req_ctx.chain_tip.block_height - request_block_height; + + // 4. The request is expired. + if blocks_observed < WITHDRAWAL_BLOCKS_EXPIRY { + return Err(WithdrawalRejectErrorMsg::RequestNotFinal.into_error(req_ctx, self)); + } + Ok(()) } } @@ -1358,6 +1486,8 @@ mod tests { use secp256k1::SECP256K1; use crate::config::NetworkKind; + use crate::storage::model::StacksBlockHash; + use crate::storage::model::StacksTxId; use super::*; @@ -1400,7 +1530,11 @@ mod tests { // This is to check that this function doesn't implicitly panic. If // it doesn't panic now, it can never panic at runtime. let call = RejectWithdrawalV1 { - request_id: 42, + id: QualifiedRequestId { + request_id: 1, + txid: StacksTxId::from([0; 32]), + block_hash: StacksBlockHash::from([0; 32]), + }, signer_bitmap: BitArray::new([1; 16]), deployer: StacksAddress::burn_address(false), }; diff --git a/signer/src/testing/dummy.rs b/signer/src/testing/dummy.rs index af1f5f8f4..ad2fa5607 100644 --- a/signer/src/testing/dummy.rs +++ b/signer/src/testing/dummy.rs @@ -704,7 +704,7 @@ impl fake::Dummy for RejectWithdrawalV1 { let address = StacksAddress::p2pkh(false, &pubkey); RejectWithdrawalV1 { - request_id: config.fake_with_rng(rng), + id: config.fake_with_rng(rng), signer_bitmap: BitArray::new(config.fake_with_rng(rng)), deployer: address, } diff --git a/signer/src/testing/message.rs b/signer/src/testing/message.rs index a8244a7f5..ed577e7c8 100644 --- a/signer/src/testing/message.rs +++ b/signer/src/testing/message.rs @@ -82,7 +82,7 @@ impl fake::Dummy for message::StacksTransactionSignRequest { let private_key = PrivateKey::new(rng); Self { contract_tx: ContractCall::RejectWithdrawalV1(RejectWithdrawalV1 { - request_id: 1, + id: config.fake_with_rng(rng), signer_bitmap: BitArray::ZERO, deployer: StacksAddress::burn_address(false), }) diff --git a/signer/tests/integration/contracts.rs b/signer/tests/integration/contracts.rs index b80aadae8..f4424b20d 100644 --- a/signer/tests/integration/contracts.rs +++ b/signer/tests/integration/contracts.rs @@ -35,6 +35,9 @@ use signer::stacks::api::SubmitTxResponse; use signer::stacks::contracts::CompleteDepositV1; use signer::stacks::wallet::MultisigTx; use signer::storage::in_memory::Store; +use signer::storage::model::QualifiedRequestId; +use signer::storage::model::StacksBlockHash; +use signer::storage::model::StacksTxId; use signer::storage::postgres; use signer::testing; use signer::testing::wallet::InitiateWithdrawalRequest; @@ -160,7 +163,11 @@ pub async fn deploy_smart_contracts() -> &'static SignerStxState { deployer: *testing::wallet::WALLET.0.address(), }); "create-withdrawal")] #[test_case(ContractCallWrapper(RejectWithdrawalV1 { - request_id: 2, + id: QualifiedRequestId { + request_id: 2, + txid: StacksTxId::from([0; 32]), + block_hash: StacksBlockHash::from([0; 32]), + }, signer_bitmap: BitArray::ZERO, deployer: *testing::wallet::WALLET.0.address(), }); "reject-withdrawal")] diff --git a/signer/tests/integration/main.rs b/signer/tests/integration/main.rs index 4a6b04939..89eea5466 100644 --- a/signer/tests/integration/main.rs +++ b/signer/tests/integration/main.rs @@ -20,6 +20,7 @@ mod transaction_coordinator; mod transaction_signer; mod utxo_construction; mod withdrawal_accept; +mod withdrawal_reject; mod zmq; /// This is needed to make sure that each test has as many isolated /// databases as it needs. diff --git a/signer/tests/integration/postgres.rs b/signer/tests/integration/postgres.rs index b641c394e..e12a0f674 100644 --- a/signer/tests/integration/postgres.rs +++ b/signer/tests/integration/postgres.rs @@ -181,7 +181,11 @@ impl AsContractCall for InitiateWithdrawalRequest { sweep_block_height: 7, }); "accept-withdrawal")] #[test_case(ContractCallWrapper(RejectWithdrawalV1 { - request_id: 0, + id: QualifiedRequestId { + request_id: 0, + txid: StacksTxId::from([0; 32]), + block_hash: StacksBlockHash::from([0; 32]), + }, signer_bitmap: BitArray::ZERO, deployer: *testing::wallet::WALLET.0.address(), }); "reject-withdrawal")] diff --git a/signer/tests/integration/setup.rs b/signer/tests/integration/setup.rs index 7d8515e41..17657d54e 100644 --- a/signer/tests/integration/setup.rs +++ b/signer/tests/integration/setup.rs @@ -541,6 +541,7 @@ pub async fn fill_signers_utxo( } /// The information about a sweep transaction that has been confirmed. +#[derive(Clone)] pub struct TestSignerSet { /// The signer object. It's public key represents the group of signers' /// public keys, allowing us to abstract away the fact that there are @@ -795,6 +796,10 @@ impl TestSweepSetup2 { Some(self.sweep_tx_info.as_ref()?.block_hash) } + pub fn sweep_block_height(&self) -> Option { + Some(self.sweep_tx_info.as_ref()?.block_height) + } + /// Store a stacks genesis block that is on the canonical Stacks /// blockchain identified by the sweep chain tip. pub async fn store_stacks_genesis_block(&self, db: &PgStore) { @@ -1054,11 +1059,16 @@ impl TestSweepSetup2 { } } + pub fn reject_withdrawal_request(&mut self) { + for i in 0..self.signers.keys.len() { + self.withdrawal_request.signer_bitmap.replace(i, true); + } + } + pub async fn store_withdrawal_request(&self, db: &PgStore) { for stacks_block in self.stacks_blocks.iter() { db.write_stacks_block(stacks_block).await.unwrap(); } - for withdrawal in self.withdrawals.iter() { let withdrawal_request = model::WithdrawalRequest { request_id: withdrawal.request.request_id, diff --git a/signer/tests/integration/withdrawal_reject.rs b/signer/tests/integration/withdrawal_reject.rs new file mode 100644 index 000000000..6503b23c9 --- /dev/null +++ b/signer/tests/integration/withdrawal_reject.rs @@ -0,0 +1,459 @@ +use std::collections::BTreeSet; + +use blockstack_lib::types::chainstate::StacksAddress; +use rand::rngs::OsRng; +use sbtc::testing::regtest; +use signer::error::Error; +use signer::keys::PublicKey; +use signer::stacks::contracts::AsContractCall as _; +use signer::stacks::contracts::RejectWithdrawalV1; +use signer::stacks::contracts::ReqContext; +use signer::stacks::contracts::WithdrawalRejectErrorMsg; +use signer::storage::model::BitcoinBlockRef; +use signer::storage::postgres::PgStore; +use signer::storage::DbRead; +use signer::testing; + +use fake::Fake; +use rand::SeedableRng; +use signer::context::Context; +use signer::testing::context::*; +use signer::WITHDRAWAL_BLOCKS_EXPIRY; + +use crate::setup::backfill_bitcoin_blocks; +use crate::setup::DepositAmounts; +use crate::setup::TestSignerSet; +use crate::setup::TestSweepSetup; +use crate::setup::TestSweepSetup2; + +/// Create a "proper" [`RejectWithdrawalV1`] object and context with the +/// given information. If the information here is correct then the returned +/// [`RejectWithdrawalV1`] object will pass validation with the given +/// context. +fn make_withdrawal_reject(data: &TestSweepSetup) -> (RejectWithdrawalV1, ReqContext) { + // Okay now we get ready to create the transaction using the + // `RejectWithdrawalV1` type. + let complete_withdrawal_tx = RejectWithdrawalV1 { + // This points to the withdrawal request transaction. + id: data.withdrawal_request.qualified_id(), + signer_bitmap: data.withdrawal_request.signer_bitmap, + // The deployer must match what is in the signers' context. + deployer: StacksAddress::burn_address(false), + }; + + // This is what the current signer thinks is the state of things. + let req_ctx = ReqContext { + chain_tip: BitcoinBlockRef { + block_hash: data.sweep_block_hash.into(), + block_height: data.sweep_block_height, + }, + // This value means that the signer will go back 20 blocks when + // looking for pending and rejected withdrawal requests. + context_window: 20, + // The value here doesn't matter. + origin: fake::Faker.fake_with_rng(&mut OsRng), + // When checking whether the transaction is from the signer, we + // check that the first "prevout" has a `scriptPubKey` that the + // signers control. + aggregate_key: data.aggregated_signer.keypair.public_key().into(), + // This value affects whether a withdrawal request is considered + // "rejected". During validation, a signer won't sign a transaction + // if it is not considered rejected but the collection of signers. + signatures_required: 2, + // This is who the current signer thinks deployed the sBTC + // contracts. + deployer: StacksAddress::burn_address(false), + }; + + (complete_withdrawal_tx, req_ctx) +} + +/// Create a "proper" [`RejectWithdrawalV1`] object and context with the +/// given information. If the information here is correct then the returned +/// [`RejectWithdrawalV1`] object will pass validation with the given +/// context. +async fn make_withdrawal_reject2( + data: &TestSweepSetup2, + db: &PgStore, +) -> (RejectWithdrawalV1, ReqContext) { + // Okay now we get ready to create the transaction using the + // `RejectWithdrawalV1` type. + let complete_withdrawal_tx = RejectWithdrawalV1 { + // This points to the withdrawal request transaction. + id: data.withdrawal_request.qualified_id(), + signer_bitmap: data.withdrawal_request.signer_bitmap, + // The deployer must match what is in the signers' context. + deployer: StacksAddress::burn_address(false), + }; + + let chain_tip = db + .get_bitcoin_canonical_chain_tip_ref() + .await + .unwrap() + .unwrap(); + + // This is what the current signer thinks is the state of things. + let req_ctx = ReqContext { + chain_tip, + // This value means that the signer will go back 20 blocks when + // looking for pending and rejected withdrawal requests. + context_window: 20, + // The value here doesn't matter. + origin: fake::Faker.fake_with_rng(&mut OsRng), + // When checking whether the transaction is from the signer, we + // check that the first "prevout" has a `scriptPubKey` that the + // signers control. + aggregate_key: data.signers.signer.keypair.public_key().into(), + // This value affects whether a withdrawal request is considered + // "rejected". During validation, a signer won't sign a transaction + // if it is not considered rejected but the collection of signers. + signatures_required: 4, + // This is who the current signer thinks deployed the sBTC + // contracts. + deployer: StacksAddress::burn_address(false), + }; + + (complete_withdrawal_tx, req_ctx) +} + +/// For this test we check that the `RejectWithdrawalV1::validate` function +/// returns okay when everything matches the way that it is supposed to. +#[tokio::test] +async fn reject_withdrawal_validation_happy_path() { + // Normal: this generates the blockchain as well as a transaction + // sweeping out the funds for a withdrawal request. This is just setup + // and should be essentially the same between tests. + let db = testing::storage::new_test_database().await; + let mut rng = rand::rngs::StdRng::seed_from_u64(51); + let (rpc, faucet) = regtest::initialize_blockchain(); + + let amount = 1_000_000; + let test_signer_set = TestSignerSet::new(&mut rng); + let deposit_amounts = DepositAmounts { amount, max_fee: amount / 2 }; + let mut setup = + TestSweepSetup2::new_setup(test_signer_set.clone(), &faucet, &[deposit_amounts]); + + setup.submit_sweep_tx(rpc, faucet, true); + + let ctx = TestContext::builder() + .with_storage(db.clone()) + .with_first_bitcoin_core_client() + .with_mocked_stacks_client() + .with_mocked_emily_client() + .build(); + + let public_keys = test_signer_set + .keys + .iter() + .cloned() + .collect::>(); + ctx.state().update_current_signer_set(public_keys); + + // Normal: the signer follows the bitcoin blockchain and event observer + // should be getting new block events from bitcoin-core. We haven't + // hooked up our block observer, so we need to manually update the + // database with new bitcoin block headers. + backfill_bitcoin_blocks(&db, rpc, &setup.sweep_block_hash().unwrap()).await; + + // Normal: we take the sweep transaction as is from the test setup and + // store it in the database. + setup.store_sweep_tx(&db).await; + + // Normal: we need to store a row in the dkg_shares table so that we + // have a record of the scriptPubKey that the signers control. + setup.store_dkg_shares(&db).await; + + // Normal: the request and how the signers voted needs to be added to + // the database. Here the bitmap in the withdrawal request object + // corresponds to how the signers voted. + setup.reject_withdrawal_request(); + setup.store_withdrawal_request(&db).await; + setup.store_withdrawal_decisions(&db).await; + + // Generate more blocks then backfill the DB + let mut hashes = faucet.generate_blocks(WITHDRAWAL_BLOCKS_EXPIRY); + let last = hashes.pop().unwrap(); + backfill_bitcoin_blocks(&db, rpc, &last).await; + + // Generate the transaction and corresponding request context. + let (reject_withdrawal_tx, req_ctx) = make_withdrawal_reject2(&setup, &db).await; + + reject_withdrawal_tx.validate(&ctx, &req_ctx).await.unwrap(); + + testing::storage::drop_db(db).await; +} + +/// For this test we check that the `RejectWithdrawalV1::validate` function +/// returns fails validation when the withdrawal request is NOT expired +#[tokio::test] +async fn reject_withdrawal_validation_not_final() { + // Normal: this generates the blockchain as well as a transaction + // sweeping out the funds for a withdrawal request. This is just setup + // and should be essentially the same between tests. + let db = testing::storage::new_test_database().await; + let mut rng = rand::rngs::StdRng::seed_from_u64(51); + let (rpc, faucet) = regtest::initialize_blockchain(); + + let amount = 1_000_000; + let test_signer_set = TestSignerSet::new(&mut rng); + let deposit_amounts = DepositAmounts { amount, max_fee: amount / 2 }; + let mut setup = + TestSweepSetup2::new_setup(test_signer_set.clone(), &faucet, &[deposit_amounts]); + + setup.submit_sweep_tx(rpc, faucet, true); + + let ctx = TestContext::builder() + .with_storage(db.clone()) + .with_first_bitcoin_core_client() + .with_mocked_stacks_client() + .with_mocked_emily_client() + .build(); + + let public_keys = test_signer_set + .keys + .iter() + .cloned() + .collect::>(); + ctx.state().update_current_signer_set(public_keys); + + // Normal: the signer follows the bitcoin blockchain and event observer + // should be getting new block events from bitcoin-core. We haven't + // hooked up our block observer, so we need to manually update the + // database with new bitcoin block headers. + backfill_bitcoin_blocks(&db, rpc, &setup.sweep_block_hash().unwrap()).await; + + // Normal: we take the sweep transaction as is from the test setup and + // store it in the database. + setup.store_sweep_tx(&db).await; + + // Normal: we need to store a row in the dkg_shares table so that we + // have a record of the scriptPubKey that the signers control. + setup.store_dkg_shares(&db).await; + + // Normal: the request and how the signers voted needs to be added to + // the database. Here the bitmap in the withdrawal request object + // corresponds to how the signers voted. + setup.store_withdrawal_request(&db).await; + setup.store_withdrawal_decisions(&db).await; + + // Generate more blocks then backfill the DB + let mut hashes = faucet.generate_blocks(WITHDRAWAL_BLOCKS_EXPIRY - 1); + let last = hashes.pop().unwrap(); + backfill_bitcoin_blocks(&db, rpc, &last).await; + + // Generate the transaction and corresponding request context. + let (reject_withdrawal_tx, req_ctx) = make_withdrawal_reject2(&setup, &db).await; + + let validate_future = reject_withdrawal_tx.validate(&ctx, &req_ctx); + match validate_future.await.unwrap_err() { + Error::WithdrawalRejectValidation(ref err) => { + assert_eq!(err.error, WithdrawalRejectErrorMsg::RequestNotFinal) + } + err => panic!("unexpected error during validation {err}"), + } + + testing::storage::drop_db(db).await; +} + +/// For this test we check that the `RejectWithdrawalV1::validate` function +/// returns a withdrawal validation error with a DeployerMismatch message +/// when the deployer doesn't match but everything else is okay. +#[tokio::test] +async fn reject_withdrawal_validation_deployer_mismatch() { + // Normal: this generates the blockchain as well as a transaction + // sweeping out the funds for a withdrawal request. + let db = testing::storage::new_test_database().await; + let mut rng = rand::rngs::StdRng::seed_from_u64(51); + let (rpc, faucet) = regtest::initialize_blockchain(); + let setup = TestSweepSetup::new_setup(&rpc, &faucet, 1_000_000, &mut rng); + + // Normal: the signer follows the bitcoin blockchain and event observer + // should be getting new block events from bitcoin-core. We haven't + // hooked up our block observer, so we need to manually update the + // database with new bitcoin block headers. + backfill_bitcoin_blocks(&db, rpc, &setup.sweep_block_hash).await; + + // Normal: we take the sweep transaction as is from the test setup and + // store it in the database. + setup.store_sweep_tx(&db).await; + + // Normal: we need to store a row in the dkg_shares table so that we + // have a record of the scriptPubKey that the signers control. + setup.store_dkg_shares(&db).await; + + // Normal: the request and how the signers voted needs to be added to + // the database. Here the bitmap in the withdrawal request object + // corresponds to how the signers voted. + setup.store_withdrawal_request(&db).await; + setup.store_withdrawal_decisions(&db).await; + + // Generate the transaction and corresponding request context. + let (mut reject_withdrawal_tx, mut req_ctx) = make_withdrawal_reject(&setup); + // Different: Okay, let's make sure the deployers do not match. + reject_withdrawal_tx.deployer = StacksAddress::p2pkh(false, &setup.signer_keys[0].into()); + req_ctx.deployer = StacksAddress::p2pkh(false, &setup.signer_keys[1].into()); + + let ctx = TestContext::builder() + .with_storage(db.clone()) + .with_first_bitcoin_core_client() + .with_mocked_stacks_client() + .with_mocked_emily_client() + .build(); + + // Generate more blocks then backfill the DB + let mut hashes = faucet.generate_blocks(6); + let last = hashes.pop().unwrap(); + backfill_bitcoin_blocks(&db, rpc, &last).await; + + let validate_future = reject_withdrawal_tx.validate(&ctx, &req_ctx); + match validate_future.await.unwrap_err() { + Error::WithdrawalRejectValidation(ref err) => { + assert_eq!(err.error, WithdrawalRejectErrorMsg::DeployerMismatch) + } + err => panic!("unexpected error during validation {err}"), + } + + testing::storage::drop_db(db).await; +} + +/// For this test we check that the `RejectWithdrawalV1::validate` function +/// returns a withdrawal validation error with a RequestMissing message +/// when the signer does not have a record of the withdrawal request +/// doesn't match but everything else is okay. +#[tokio::test] +async fn reject_withdrawal_validation_missing_withdrawal_request() { + // Normal: this generates the blockchain as well as a transaction + // sweeping out the funds for a withdrawal request. + let db = testing::storage::new_test_database().await; + let mut rng = rand::rngs::StdRng::seed_from_u64(51); + let (rpc, faucet) = regtest::initialize_blockchain(); + let setup = TestSweepSetup::new_setup(&rpc, &faucet, 1_000_000, &mut rng); + + // Normal: the signer follows the bitcoin blockchain and event observer + // should be getting new block events from bitcoin-core. We haven't + // hooked up our block observer, so we need to manually update the + // database with new bitcoin block headers. + backfill_bitcoin_blocks(&db, rpc, &setup.sweep_block_hash).await; + + // Normal: we take the sweep transaction as is from the test setup and + // store it in the database. + setup.store_sweep_tx(&db).await; + + // Normal: we need to store a row in the dkg_shares table so that we + // have a record of the scriptPubKey that the signers control. + setup.store_dkg_shares(&db).await; + + // Normal: the request and how the signers voted needs to be added to + // the database. Here the bitmap in the withdrawal request object + // corresponds to how the signers voted. + setup.store_withdrawal_request(&db).await; + setup.store_withdrawal_decisions(&db).await; + + // Generate the transaction and corresponding request context. + let (mut reject_withdrawal_tx, req_ctx) = make_withdrawal_reject(&setup); + // Different: Let's use a request_id that does not exist in our + // database. In these tests, the withdrawal id starts at 0 and + // increments by 1 for each withdrawal request generated. + reject_withdrawal_tx.id.request_id = i64::MAX as u64; + + let ctx = TestContext::builder() + .with_storage(db.clone()) + .with_first_bitcoin_core_client() + .with_mocked_stacks_client() + .with_mocked_emily_client() + .build(); + + // Generate more blocks then backfill the DB + let mut hashes = faucet.generate_blocks(6); + let last = hashes.pop().unwrap(); + backfill_bitcoin_blocks(&db, rpc, &last).await; + + let validation_result = reject_withdrawal_tx.validate(&ctx, &req_ctx).await; + match validation_result.unwrap_err() { + Error::WithdrawalRejectValidation(ref err) => { + assert_eq!(err.error, WithdrawalRejectErrorMsg::RequestMissing) + } + err => panic!("unexpected error during validation {err}"), + } + + testing::storage::drop_db(db).await; +} + +/// For this test we check that the `RejectWithdrawalV1::validate` function +/// returns a withdrawal validation error with a BitmapMismatch message +/// when bitmap in the transaction does not match what our records would +/// create for the bitmap. +#[tokio::test] +async fn reject_withdrawal_validation_bitmap_mismatch() { + // Normal: this generates the blockchain as well as a transaction + // sweeping out the funds for a withdrawal request. + let db = testing::storage::new_test_database().await; + let mut rng = rand::rngs::StdRng::seed_from_u64(51); + let (rpc, faucet) = regtest::initialize_blockchain(); + + let amount = 1_000_000; + let test_signer_set = TestSignerSet::new(&mut rng); + let deposit_amounts = DepositAmounts { amount, max_fee: amount / 2 }; + let mut setup = + TestSweepSetup2::new_setup(test_signer_set.clone(), &faucet, &[deposit_amounts]); + + setup.submit_sweep_tx(rpc, faucet, true); + + let ctx = TestContext::builder() + .with_storage(db.clone()) + .with_first_bitcoin_core_client() + .with_mocked_stacks_client() + .with_mocked_emily_client() + .build(); + + let public_keys = test_signer_set + .keys + .iter() + .cloned() + .collect::>(); + ctx.state().update_current_signer_set(public_keys); + + // Normal: the signer follows the bitcoin blockchain and event observer + // should be getting new block events from bitcoin-core. We haven't + // hooked up our block observer, so we need to manually update the + // database with new bitcoin block headers. + backfill_bitcoin_blocks(&db, rpc, &setup.sweep_block_hash().unwrap()).await; + + // Normal: we take the sweep transaction as is from the test setup and + // store it in the database. + setup.store_sweep_tx(&db).await; + + // Normal: we need to store a row in the dkg_shares table so that we + // have a record of the scriptPubKey that the signers control. + setup.store_dkg_shares(&db).await; + + // Normal: the request and how the signers voted needs to be added to + // the database. Here the bitmap in the withdrawal request object + // corresponds to how the signers voted. + setup.reject_withdrawal_request(); + setup.store_withdrawal_request(&db).await; + setup.store_withdrawal_decisions(&db).await; + + // Generate more blocks then backfill the DB + let mut hashes = faucet.generate_blocks(6); + let last = hashes.pop().unwrap(); + backfill_bitcoin_blocks(&db, rpc, &last).await; + + // Generate the transaction and corresponding request context. + let (mut reject_withdrawal_tx, req_ctx) = make_withdrawal_reject2(&setup, &db).await; + + // Different: We're going to get the bitmap that is a little different + // from what is expected. + let first_vote = *reject_withdrawal_tx.signer_bitmap.get(0).unwrap(); + reject_withdrawal_tx.signer_bitmap.set(0, !first_vote); + + let validation_result = reject_withdrawal_tx.validate(&ctx, &req_ctx).await; + match validation_result.unwrap_err() { + Error::WithdrawalRejectValidation(ref err) => { + assert_eq!(err.error, WithdrawalRejectErrorMsg::BitmapMismatch) + } + err => panic!("unexpected error during validation {err}"), + } + + testing::storage::drop_db(db).await; +} From eb22556aaf8f172b2d38a8871ecfb0d85084500c Mon Sep 17 00:00:00 2001 From: Joey Yandle Date: Tue, 18 Feb 2025 12:20:57 -0500 Subject: [PATCH 2/4] deal with fallout from changes to TestSweepSetup2 --- signer/tests/integration/setup.rs | 10 ++- signer/tests/integration/withdrawal_reject.rs | 68 ++++++++++++++----- 2 files changed, 56 insertions(+), 22 deletions(-) diff --git a/signer/tests/integration/setup.rs b/signer/tests/integration/setup.rs index 17657d54e..941c5de2e 100644 --- a/signer/tests/integration/setup.rs +++ b/signer/tests/integration/setup.rs @@ -796,10 +796,6 @@ impl TestSweepSetup2 { Some(self.sweep_tx_info.as_ref()?.block_hash) } - pub fn sweep_block_height(&self) -> Option { - Some(self.sweep_tx_info.as_ref()?.block_height) - } - /// Store a stacks genesis block that is on the canonical Stacks /// blockchain identified by the sweep chain tip. pub async fn store_stacks_genesis_block(&self, db: &PgStore) { @@ -1060,8 +1056,10 @@ impl TestSweepSetup2 { } pub fn reject_withdrawal_request(&mut self) { - for i in 0..self.signers.keys.len() { - self.withdrawal_request.signer_bitmap.replace(i, true); + for withdrawal in self.withdrawals.iter_mut() { + for i in 0..self.signers.keys.len() { + withdrawal.request.signer_bitmap.replace(i, true); + } } } diff --git a/signer/tests/integration/withdrawal_reject.rs b/signer/tests/integration/withdrawal_reject.rs index 6503b23c9..115b34742 100644 --- a/signer/tests/integration/withdrawal_reject.rs +++ b/signer/tests/integration/withdrawal_reject.rs @@ -21,7 +21,7 @@ use signer::testing::context::*; use signer::WITHDRAWAL_BLOCKS_EXPIRY; use crate::setup::backfill_bitcoin_blocks; -use crate::setup::DepositAmounts; +use crate::setup::SweepAmounts; use crate::setup::TestSignerSet; use crate::setup::TestSweepSetup; use crate::setup::TestSweepSetup2; @@ -80,8 +80,8 @@ async fn make_withdrawal_reject2( // `RejectWithdrawalV1` type. let complete_withdrawal_tx = RejectWithdrawalV1 { // This points to the withdrawal request transaction. - id: data.withdrawal_request.qualified_id(), - signer_bitmap: data.withdrawal_request.signer_bitmap, + id: data.withdrawals[0].request.qualified_id(), + signer_bitmap: data.withdrawals[0].request.signer_bitmap, // The deployer must match what is in the signers' context. deployer: StacksAddress::burn_address(false), }; @@ -129,11 +129,23 @@ async fn reject_withdrawal_validation_happy_path() { let amount = 1_000_000; let test_signer_set = TestSignerSet::new(&mut rng); - let deposit_amounts = DepositAmounts { amount, max_fee: amount / 2 }; - let mut setup = - TestSweepSetup2::new_setup(test_signer_set.clone(), &faucet, &[deposit_amounts]); + let deposit_amounts = SweepAmounts { + amount, + max_fee: amount / 2, + is_deposit: true, + }; + let withdraw_amounts = SweepAmounts { + amount, + max_fee: amount / 2, + is_deposit: false, + }; + let mut setup = TestSweepSetup2::new_setup( + test_signer_set.clone(), + &faucet, + &[deposit_amounts, withdraw_amounts], + ); - setup.submit_sweep_tx(rpc, faucet, true); + setup.submit_sweep_tx(rpc, faucet); let ctx = TestContext::builder() .with_storage(db.clone()) @@ -196,11 +208,23 @@ async fn reject_withdrawal_validation_not_final() { let amount = 1_000_000; let test_signer_set = TestSignerSet::new(&mut rng); - let deposit_amounts = DepositAmounts { amount, max_fee: amount / 2 }; - let mut setup = - TestSweepSetup2::new_setup(test_signer_set.clone(), &faucet, &[deposit_amounts]); + let deposit_amounts = SweepAmounts { + amount, + max_fee: amount / 2, + is_deposit: true, + }; + let withdraw_amounts = SweepAmounts { + amount, + max_fee: amount / 2, + is_deposit: false, + }; + let mut setup = TestSweepSetup2::new_setup( + test_signer_set.clone(), + &faucet, + &[deposit_amounts, withdraw_amounts], + ); - setup.submit_sweep_tx(rpc, faucet, true); + setup.submit_sweep_tx(rpc, faucet); let ctx = TestContext::builder() .with_storage(db.clone()) @@ -237,7 +261,7 @@ async fn reject_withdrawal_validation_not_final() { setup.store_withdrawal_decisions(&db).await; // Generate more blocks then backfill the DB - let mut hashes = faucet.generate_blocks(WITHDRAWAL_BLOCKS_EXPIRY - 1); + let mut hashes = faucet.generate_blocks(WITHDRAWAL_BLOCKS_EXPIRY - 2); let last = hashes.pop().unwrap(); backfill_bitcoin_blocks(&db, rpc, &last).await; @@ -393,11 +417,23 @@ async fn reject_withdrawal_validation_bitmap_mismatch() { let amount = 1_000_000; let test_signer_set = TestSignerSet::new(&mut rng); - let deposit_amounts = DepositAmounts { amount, max_fee: amount / 2 }; - let mut setup = - TestSweepSetup2::new_setup(test_signer_set.clone(), &faucet, &[deposit_amounts]); + let deposit_amounts = SweepAmounts { + amount, + max_fee: amount / 2, + is_deposit: true, + }; + let withdraw_amounts = SweepAmounts { + amount, + max_fee: amount / 2, + is_deposit: false, + }; + let mut setup = TestSweepSetup2::new_setup( + test_signer_set.clone(), + &faucet, + &[deposit_amounts, withdraw_amounts], + ); - setup.submit_sweep_tx(rpc, faucet, true); + setup.submit_sweep_tx(rpc, faucet); let ctx = TestContext::builder() .with_storage(db.clone()) From 2517778787824985c5a5dde5aaae30b68fff3a6e Mon Sep 17 00:00:00 2001 From: Joey Yandle Date: Tue, 18 Feb 2025 12:37:14 -0500 Subject: [PATCH 3/4] add new_sweep_setup fn --- signer/tests/integration/withdrawal_reject.rs | 57 +++++++++---------- 1 file changed, 26 insertions(+), 31 deletions(-) diff --git a/signer/tests/integration/withdrawal_reject.rs b/signer/tests/integration/withdrawal_reject.rs index 115b34742..d607ec2d8 100644 --- a/signer/tests/integration/withdrawal_reject.rs +++ b/signer/tests/integration/withdrawal_reject.rs @@ -3,6 +3,7 @@ use std::collections::BTreeSet; use blockstack_lib::types::chainstate::StacksAddress; use rand::rngs::OsRng; use sbtc::testing::regtest; +use sbtc::testing::regtest::Faucet; use signer::error::Error; use signer::keys::PublicKey; use signer::stacks::contracts::AsContractCall as _; @@ -116,19 +117,8 @@ async fn make_withdrawal_reject2( (complete_withdrawal_tx, req_ctx) } -/// For this test we check that the `RejectWithdrawalV1::validate` function -/// returns okay when everything matches the way that it is supposed to. -#[tokio::test] -async fn reject_withdrawal_validation_happy_path() { - // Normal: this generates the blockchain as well as a transaction - // sweeping out the funds for a withdrawal request. This is just setup - // and should be essentially the same between tests. - let db = testing::storage::new_test_database().await; - let mut rng = rand::rngs::StdRng::seed_from_u64(51); - let (rpc, faucet) = regtest::initialize_blockchain(); - +fn new_sweep_setup(signers: &TestSignerSet, faucet: &Faucet) -> TestSweepSetup2 { let amount = 1_000_000; - let test_signer_set = TestSignerSet::new(&mut rng); let deposit_amounts = SweepAmounts { amount, max_fee: amount / 2, @@ -139,11 +129,27 @@ async fn reject_withdrawal_validation_happy_path() { max_fee: amount / 2, is_deposit: false, }; - let mut setup = TestSweepSetup2::new_setup( - test_signer_set.clone(), + + TestSweepSetup2::new_setup( + signers.clone(), &faucet, &[deposit_amounts, withdraw_amounts], - ); + ) +} + +/// For this test we check that the `RejectWithdrawalV1::validate` function +/// returns okay when everything matches the way that it is supposed to. +#[tokio::test] +async fn reject_withdrawal_validation_happy_path() { + // Normal: this generates the blockchain as well as a transaction + // sweeping out the funds for a withdrawal request. This is just setup + // and should be essentially the same between tests. + let db = testing::storage::new_test_database().await; + let mut rng = rand::rngs::StdRng::seed_from_u64(51); + let (rpc, faucet) = regtest::initialize_blockchain(); + + let test_signer_set = TestSignerSet::new(&mut rng); + let mut setup = new_sweep_setup(&test_signer_set, &faucet); setup.submit_sweep_tx(rpc, faucet); @@ -206,23 +212,8 @@ async fn reject_withdrawal_validation_not_final() { let mut rng = rand::rngs::StdRng::seed_from_u64(51); let (rpc, faucet) = regtest::initialize_blockchain(); - let amount = 1_000_000; let test_signer_set = TestSignerSet::new(&mut rng); - let deposit_amounts = SweepAmounts { - amount, - max_fee: amount / 2, - is_deposit: true, - }; - let withdraw_amounts = SweepAmounts { - amount, - max_fee: amount / 2, - is_deposit: false, - }; - let mut setup = TestSweepSetup2::new_setup( - test_signer_set.clone(), - &faucet, - &[deposit_amounts, withdraw_amounts], - ); + let mut setup = new_sweep_setup(&test_signer_set, &faucet); setup.submit_sweep_tx(rpc, faucet); @@ -415,6 +406,10 @@ async fn reject_withdrawal_validation_bitmap_mismatch() { let mut rng = rand::rngs::StdRng::seed_from_u64(51); let (rpc, faucet) = regtest::initialize_blockchain(); + let test_signer_set = TestSignerSet::new(&mut rng); + let mut setup = new_sweep_setup(&test_signer_set, &faucet); + + setup.submit_sweep_tx(rpc, faucet); let amount = 1_000_000; let test_signer_set = TestSignerSet::new(&mut rng); let deposit_amounts = SweepAmounts { From 166423cd9846e3124d54c5e71155ac300b532670 Mon Sep 17 00:00:00 2001 From: Joey Yandle Date: Tue, 18 Feb 2025 12:40:43 -0500 Subject: [PATCH 4/4] generate more blocks and verify that test now passes --- signer/tests/integration/withdrawal_reject.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/signer/tests/integration/withdrawal_reject.rs b/signer/tests/integration/withdrawal_reject.rs index d607ec2d8..54f1538b8 100644 --- a/signer/tests/integration/withdrawal_reject.rs +++ b/signer/tests/integration/withdrawal_reject.rs @@ -267,6 +267,16 @@ async fn reject_withdrawal_validation_not_final() { err => panic!("unexpected error during validation {err}"), } + // Generate more blocks then backfill the DB + let mut hashes = faucet.generate_blocks(2); + let last = hashes.pop().unwrap(); + backfill_bitcoin_blocks(&db, rpc, &last).await; + + // Generate the transaction and corresponding request context. + let (reject_withdrawal_tx, req_ctx) = make_withdrawal_reject2(&setup, &db).await; + + reject_withdrawal_tx.validate(&ctx, &req_ctx).await.unwrap(); + testing::storage::drop_db(db).await; }