From bab72441c6e98b85b5f8dde9eebc5308af9deaf5 Mon Sep 17 00:00:00 2001 From: djordon Date: Wed, 18 Sep 2024 10:54:41 -0400 Subject: [PATCH 01/11] Make the serde implementations transparent --- signer/src/keys.rs | 2 ++ signer/src/storage/model.rs | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/signer/src/keys.rs b/signer/src/keys.rs index ea75741c..2ebfb0b9 100644 --- a/signer/src/keys.rs +++ b/signer/src/keys.rs @@ -41,6 +41,7 @@ use crate::error::Error; /// The public key type for the secp256k1 elliptic curve. #[derive(Copy, Clone, Debug, PartialOrd, Ord, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[serde(transparent)] pub struct PublicKey(secp256k1::PublicKey); impl From<&secp256k1::PublicKey> for PublicKey { @@ -231,6 +232,7 @@ impl std::fmt::Display for PublicKey { /// A private key type for the secp256k1 elliptic curve. #[derive(Copy, Clone, Debug, PartialEq, Eq, Deserialize)] +#[serde(transparent)] pub struct PrivateKey(secp256k1::SecretKey); impl FromStr for PrivateKey { diff --git a/signer/src/storage/model.rs b/signer/src/storage/model.rs index 600a65f2..3c18dc4e 100644 --- a/signer/src/storage/model.rs +++ b/signer/src/storage/model.rs @@ -352,7 +352,8 @@ impl From for bitcoin::Transaction { } /// The bitcoin transaction ID -#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] +#[serde(transparent)] pub struct BitcoinTxId(bitcoin::Txid); impl BitcoinTxId { @@ -382,6 +383,7 @@ impl From<[u8; 32]> for BitcoinTxId { /// Bitcoin block hash #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] +#[serde(transparent)] pub struct BitcoinBlockHash(bitcoin::BlockHash); impl BitcoinBlockHash { @@ -471,6 +473,7 @@ impl From<[u8; 32]> for StacksBlockHash { /// Stacks transaction ID #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] +#[serde(transparent)] pub struct StacksTxId(blockstack_lib::burnchains::Txid); impl Deref for StacksTxId { @@ -547,6 +550,7 @@ impl PartialOrd for StacksPrincipal { /// A ScriptPubkey of a UTXO. #[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] +#[serde(transparent)] pub struct ScriptPubKey(bitcoin::ScriptBuf); impl Deref for ScriptPubKey { From 1691c8f7a4c9fdc7e9da37b4894fa1d172d0f3ad Mon Sep 17 00:00:00 2001 From: djordon Date: Wed, 18 Sep 2024 11:09:49 -0400 Subject: [PATCH 02/11] Change the name of a struct --- signer/src/storage/in_memory.rs | 4 ++-- signer/src/storage/mod.rs | 2 +- signer/src/storage/model.rs | 6 +++--- signer/src/storage/postgres.rs | 6 ++---- signer/src/testing/storage/model.rs | 6 +++--- 5 files changed, 11 insertions(+), 13 deletions(-) diff --git a/signer/src/storage/in_memory.rs b/signer/src/storage/in_memory.rs index 5a46ff95..84b7816e 100644 --- a/signer/src/storage/in_memory.rs +++ b/signer/src/storage/in_memory.rs @@ -540,7 +540,7 @@ impl super::DbWrite for SharedStore { txs: Vec, ) -> Result<(), Self::Error> { for tx in txs { - let bitcoin_transaction = model::BitcoinTransaction { + let bitcoin_transaction = model::BitcoinTxRef { txid: tx.txid.into(), block_hash: tx.block_hash.into(), }; @@ -652,7 +652,7 @@ impl super::DbWrite for SharedStore { async fn write_bitcoin_transaction( &self, - bitcoin_transaction: &model::BitcoinTransaction, + bitcoin_transaction: &model::BitcoinTxRef, ) -> Result<(), Self::Error> { let mut store = self.lock().await; diff --git a/signer/src/storage/mod.rs b/signer/src/storage/mod.rs index 14112156..4b2d4dea 100644 --- a/signer/src/storage/mod.rs +++ b/signer/src/storage/mod.rs @@ -224,7 +224,7 @@ pub trait DbWrite { /// Write a connection between a bitcoin block and a transaction fn write_bitcoin_transaction( &self, - bitcoin_transaction: &model::BitcoinTransaction, + bitcoin_transaction: &model::BitcoinTxRef, ) -> impl Future> + Send; /// Write the bitcoin transactions to the data store. diff --git a/signer/src/storage/model.rs b/signer/src/storage/model.rs index 3c18dc4e..df852252 100644 --- a/signer/src/storage/model.rs +++ b/signer/src/storage/model.rs @@ -63,12 +63,12 @@ pub struct DepositRequest { pub recipient: StacksPrincipal, /// The amount in the deposit UTXO. #[sqlx(try_from = "i64")] - #[cfg_attr(feature = "testing", dummy(faker = "100..1_000_000_000"))] + #[cfg_attr(feature = "testing", dummy(faker = "1_000_000..1_000_000_000"))] pub amount: u64, /// The maximum portion of the deposited amount that may /// be used to pay for transaction fees. #[sqlx(try_from = "i64")] - #[cfg_attr(feature = "testing", dummy(faker = "100..1_000_000_000"))] + #[cfg_attr(feature = "testing", dummy(faker = "100..100_000"))] pub max_fee: u64, /// The addresses of the input UTXOs funding the deposit request. #[cfg_attr( @@ -198,7 +198,7 @@ impl WithdrawalSigner { /// A connection between a bitcoin block and a bitcoin transaction. #[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord, sqlx::FromRow)] -pub struct BitcoinTransaction { +pub struct BitcoinTxRef { /// Transaction ID. pub txid: BitcoinTxId, /// The block in which the transaction exists. diff --git a/signer/src/storage/postgres.rs b/signer/src/storage/postgres.rs index 93aa5565..099cf02e 100644 --- a/signer/src/storage/postgres.rs +++ b/signer/src/storage/postgres.rs @@ -851,7 +851,7 @@ impl super::DbRead for PgStore { &self, txid: &model::BitcoinTxId, ) -> Result, Self::Error> { - sqlx::query_as::<_, model::BitcoinTransaction>( + sqlx::query_as::<_, model::BitcoinTxRef>( "SELECT txid, block_hash FROM sbtc_signer.bitcoin_transactions WHERE txid = $1", ) .bind(txid) @@ -982,7 +982,6 @@ impl super::DbRead for PgStore { WITH RECURSIVE tx_block_chain AS ( SELECT block_hash - , block_height , parent_hash , 0 AS counter FROM sbtc_signer.bitcoin_blocks @@ -992,7 +991,6 @@ impl super::DbRead for PgStore { SELECT child.block_hash - , child.block_height , child.parent_hash , parent.counter + 1 FROM sbtc_signer.bitcoin_blocks AS child @@ -1312,7 +1310,7 @@ impl super::DbWrite for PgStore { async fn write_bitcoin_transaction( &self, - bitcoin_transaction: &model::BitcoinTransaction, + bitcoin_transaction: &model::BitcoinTxRef, ) -> Result<(), Self::Error> { sqlx::query( "INSERT INTO sbtc_signer.bitcoin_transactions (txid, block_hash) diff --git a/signer/src/testing/storage/model.rs b/signer/src/testing/storage/model.rs index 29c80b53..2cc209a2 100644 --- a/signer/src/testing/storage/model.rs +++ b/signer/src/testing/storage/model.rs @@ -34,7 +34,7 @@ pub struct TestData { pub transactions: Vec, /// Connection between bitcoin blocks and transactions - pub bitcoin_transactions: Vec, + pub bitcoin_transactions: Vec, /// Connection between bitcoin blocks and transactions pub stacks_transactions: Vec, @@ -270,7 +270,7 @@ struct DepositData { pub deposit_requests: Vec, pub deposit_signers: Vec, pub transactions: Vec, - pub bitcoin_transactions: Vec, + pub bitcoin_transactions: Vec, } impl DepositData { @@ -310,7 +310,7 @@ impl DepositData { }) .collect(); - let bitcoin_transaction = model::BitcoinTransaction { + let bitcoin_transaction = model::BitcoinTxRef { txid: raw_transaction.txid.into(), block_hash: bitcoin_block.block_hash, }; From 3862c3506ed5392be0c6e6079d4fe55bde27c037 Mon Sep 17 00:00:00 2001 From: djordon Date: Wed, 18 Sep 2024 11:13:04 -0400 Subject: [PATCH 03/11] Add validation to the CompleteDepositV1 struct. Update the number of arguments that it needs. Update the trait validation function to take in a context object. --- signer/src/error.rs | 5 + signer/src/stacks/contracts.rs | 293 ++++++++++++++++++++++++-- signer/src/stacks/wallet.rs | 5 +- signer/src/testing/wallet.rs | 10 +- signer/src/testing/wsts.rs | 16 ++ signer/src/transaction_signer.rs | 48 +++-- signer/tests/integration/contracts.rs | 11 + signer/tests/integration/postgres.rs | 46 ++-- 8 files changed, 366 insertions(+), 68 deletions(-) diff --git a/signer/src/error.rs b/signer/src/error.rs index 8e5472b5..e10f94f8 100644 --- a/signer/src/error.rs +++ b/signer/src/error.rs @@ -3,6 +3,7 @@ use std::borrow::Cow; use blockstack_lib::types::chainstate::StacksBlockId; +use crate::stacks::contracts::DepositValidationError; use crate::{codec, ecdsa, network}; /// Top-level signer error @@ -79,6 +80,10 @@ pub enum Error { #[error("could not decode Nakamoto block from tenure with block: {1}; {0}")] DecodeNakamotoTenure(#[source] blockstack_lib::codec::Error, StacksBlockId), + /// Failed to validate the complete-deposit contract call transaction. + #[error("{0}")] + DepositValidation(#[from] Box), + /// An error when serializing an object to JSON #[error("{0}")] JsonSerialize(#[source] serde_json::Error), diff --git a/signer/src/stacks/contracts.rs b/signer/src/stacks/contracts.rs index aacbc3b4..cb375c88 100644 --- a/signer/src/stacks/contracts.rs +++ b/signer/src/stacks/contracts.rs @@ -18,6 +18,7 @@ use std::collections::BTreeSet; use std::future::Future; +use std::ops::Deref; use std::sync::OnceLock; use bitcoin::hashes::Hash as _; @@ -42,8 +43,38 @@ use blockstack_lib::types::chainstate::StacksAddress; use crate::error::Error; use crate::keys::PublicKey; use crate::stacks::wallet::SignerWallet; +use crate::storage::model::BitcoinBlockHash; +use crate::storage::model::BitcoinBlockRef; +use crate::storage::model::BitcoinTxId; use crate::storage::DbRead; +/// This struct is used as supplemental data to help validate a request to +/// sign a contract call transaction. +/// +/// With the exception of the origin, this data is not fetched from the +/// signer that sent the request, but is instead internal to the current +/// signer. +#[derive(Debug, Clone, Copy)] +pub struct ReqContext { + /// This signers current view of the chain tip of the canonical bitcoin + /// blockchain. It is the block hash and height of the block on the + /// bitcoin blockchain with the greatest height. On ties, we sort by + /// the block hash descending and take the first one. + pub chain_tip: BitcoinBlockRef, + /// How many bitcoin blocks back from the chain tip the signer will + /// look for requests. + pub context_window: u16, + /// The public key of the signer that created the deposit request + /// transaction. This is very unlikely to ever be used in the + /// [`AsContractCall::validate`] function, but is here for logging and + /// tracking purposes. + pub origin: PublicKey, + /// The number of signatures required for an accepted deposit request. + pub signatures_required: u16, + /// The expected deployer of the sBTC smart contract. + pub deployer: StacksAddress, +} + /// A struct describing any transaction post-execution conditions that we'd /// like to enforce. /// @@ -128,7 +159,11 @@ pub trait AsContractCall { /// Validate that it is okay to sign this contract call transaction, /// because the included data matches what this signer knows from the /// stacks and bitcoin blockchains. - fn validate(&self, _: &S) -> impl Future> + Send + fn validate( + &self, + db: &S, + ctx: &ReqContext, + ) -> impl Future> + Send where S: DbRead + Send + Sync, Error: From<::Error>; @@ -179,12 +214,27 @@ pub struct CompleteDepositV1 { /// The outpoint of the bitcoin UTXO that was spent as a deposit for /// sBTC. pub outpoint: OutPoint, - /// The amount of sats associated with the above UTXO. + /// The amount of sats swept in by the signers when they moved in the + /// above UTXO. This amount is less than the amount associated with the + /// above UTXO because of bitcoin mining fees. pub amount: u64, /// The address where the newly minted sBTC will be deposited. pub recipient: PrincipalData, /// The address that deployed the contract. pub deployer: StacksAddress, + /// The transaction ID for the sweep transaction that moved the deposit + /// UTXO into the signers' UTXO. One of the inputs to this transaction + /// must be the above `outpoint`. + pub sweep_txid: BitcoinTxId, + /// The block hash of the bitcoin block that contains a sweep + /// transaction with the above `outpoint` as one of its inputs. This + /// field, with the `sweep_block_height` field, is used by the + /// `complete-deposit-wrapper` clarity function to ensure that we do + /// not mint in case a bitcoin reorg affects the sweep transaction + /// that is included in this block. + pub sweep_block_hash: BitcoinBlockHash, + /// The block height associated with the above bitcoin block hash. + pub sweep_block_height: u64, } impl AsContractCall for CompleteDepositV1 { @@ -210,22 +260,224 @@ impl AsContractCall for CompleteDepositV1 { /// Validates that the Complete deposit request satisfies the following /// criteria: /// - /// 1. That the outpoint exists on the canonical bitcoin blockchain. - /// 2. That the outpoint was used as an input into a signer sweep - /// transaction. - /// 3. That the signer sweep transaction exists on the canonical + /// 1. That the smart contract deployer matches the deployer in our + /// context. + /// 2. That the signer has a record of the deposit request in its list + /// of pending and accepted deposit requests. + /// 3. That the signer sweep transaction is on the canonical bitcoin + /// blockchain. + /// 4. That the sweep transaction uses the indicated deposit outpoint + /// as an input. + /// 5. That the recipients in the transaction matches that of the + /// deposit request. + /// 6. That the amount to mint does not exceed the deposit amount. + /// 7. That the max-fee is less than the desired max-fee. + /// + /// # Notes + /// + /// The `complete-deposit-wrapper` public function will not mint to the + /// user again if we mistakenly submit two transactions for the same + /// deposit outpoint. This means we do not need to do a check for + /// existence of a similar transaction in the stacks mempool. This is + /// fortunate, because even if we wanted to, the only view into the + /// stacks-core mempool is through the `POST /new_mempool_tx` webhooks, + /// which we do not currently ingest. + async fn validate(&self, db: &S, ctx: &ReqContext) -> Result<(), Error> + where + S: DbRead + Send + Sync, + Error: From<::Error>, + { + self.validate_sweep_tx(db, ctx).await?; + self.validate_deposit_vars(db, ctx).await + } +} + +impl CompleteDepositV1 { + /// Validate the the variables in this transaction match the input in + /// the deposit request. + /// + /// Specifically, this function checks: + /// 1. That the smart contract deployer matches the deployer in our + /// context. + /// 2. That the signer has a record of the deposit request in its list + /// of pending and accepted deposit requests. + /// 5. That the recipients in the transaction matches that of the + /// deposit request. + /// 6. That the amount to mint does not exceed the deposit amount. + /// 7. That the max-fee is less than the desired max-fee. + async fn validate_deposit_vars(&self, db: &S, ctx: &ReqContext) -> Result<(), Error> + where + S: DbRead + Send + Sync, + Error: From<::Error>, + { + // 1. That the smart contract deployer matches the deployer in our + // context. + if self.deployer != ctx.deployer { + return Err(DepositValidationMsg::DeployerMismatch.into_error(ctx, self)); + } + // 2. Check that the signer has a record of the deposit request + // from our list of pending and accepted deposit requests. + // + // Check that this is actually a pending and accepted deposit + // request. + let deposit_requests = db + .get_pending_accepted_deposit_requests( + &ctx.chain_tip.block_hash, + ctx.context_window, + ctx.signatures_required, + ) + .await?; + + let deposit_request = deposit_requests + .into_iter() + .find(|req| req.outpoint() == self.outpoint) + .ok_or_else(|| DepositValidationMsg::DepositRequestMissing.into_error(ctx, self))?; + + // 5. Check that the recipients in the transaction matches that of + // the deposit request. + if &self.recipient != deposit_request.recipient.deref() { + // Whoa, how is the outpoint right but the recipient is wrong?. + // TODO: serialize this struct and the deposit request to a + // JSON and put it in the logs. Or store. + tracing::warn!("Recipient in transaction does not match deposit request"); + return Err(DepositValidationMsg::RecipientMismatch.into_error(ctx, self)); + } + // 6. Check that the amount to mint does not exceed the deposit + // amount. + if self.amount > deposit_request.amount { + return Err(DepositValidationMsg::InvalidMintAmount.into_error(ctx, self)); + } + // 7. Check that the max-fee is less than the desired max-fee. + // + // The smart contract cannot check if we exceed the max fee, so we + // do a check ourselves. + if deposit_request.amount - self.amount > deposit_request.max_fee { + return Err(DepositValidationMsg::InvalidFee.into_error(ctx, self)); + } + + Ok(()) + } + + /// This function validates the sweep transaction. + /// + /// Specifically, this function: + /// 3. Check that the signer sweep transaction is on the canonical /// bitcoin blockchain. - /// 5. That the `amount` matches the amount in the `outpoint` less - /// their portion of fees spent in the sweep transaction. - /// 4. That the principal matches the principal embedded in the deposit - /// script locked in the outpoint. - async fn validate(&self, _storage: &S) -> Result + /// 4. Check that the sweep transaction uses the indicated deposit + /// outpoint as an input. + async fn validate_sweep_tx(&self, db: &S, ctx: &ReqContext) -> Result<(), Error> where S: DbRead + Send + Sync, Error: From<::Error>, { - // TODO(255): Add validation implementation - Ok(false) + // First we check that we have a record of the transaction. + let sweep_tx = db + .get_bitcoin_tx(&self.sweep_txid, &self.sweep_block_hash) + .await? + .ok_or_else(|| DepositValidationMsg::SweepTransactionMissing.into_error(ctx, self))?; + // 3. Check that the signer sweep transaction is on the canonical + // bitcoin blockchain. + // + // From the above, we know that the sweep transaction is in the + // `sweep_block_hash`. Now we just need to check that this block is + // on the canonical bitcoin blockchain. + let block_ref = BitcoinBlockRef { + block_hash: self.sweep_block_hash, + block_height: self.sweep_block_height, + }; + + let in_canonical_bitcoin_blockchain = db + .in_canonical_bitcoin_blockchain(&ctx.chain_tip, &block_ref) + .await?; + if !in_canonical_bitcoin_blockchain { + return Err(DepositValidationMsg::SweepTransactionReorged.into_error(ctx, self)); + } + // 4. Check that the sweep transaction uses the indicated deposit + // outpoint as an input. + // + // Okay great, we know that the sweep transaction exists on the + // canonical bitcoin blockchain, we just need to do a simple check + // of the transaction inputs. + let mut tx_inputs = sweep_tx.input.iter(); + if !tx_inputs.any(|tx_in| tx_in.previous_output == self.outpoint) { + return Err(DepositValidationMsg::DepositMissingFromSweep.into_error(ctx, self)); + } + + Ok(()) + } +} + +/// A struct for a validation error containing all of the necessary +/// context. +#[derive(Debug)] +pub struct DepositValidationError { + /// The specific error that happened during validation. + pub error: DepositValidationMsg, + /// The additional information that was used when trying to + /// validate the complete-deposit contract call. This includes the + /// public key of the signer that was attempting to generate the + /// `complete-deposit` transaction. + pub context: ReqContext, + /// The specific transaction that was being validated. + pub tx: CompleteDepositV1, +} + +impl std::fmt::Display for DepositValidationError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + // TODO: Add the other variables to the error message. + self.error.fmt(f) + } +} + +impl std::error::Error for DepositValidationError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + Some(&self.error) + } +} + +/// The responses for validation of a complete-deposit smart contract call +/// transactions. +#[derive(Debug, thiserror::Error, PartialEq, Eq)] +pub enum DepositValidationMsg { + /// The smart contract deployer is fixed, so this should always match. + #[error("The deployer in the transaction does not match the expected deployer")] + DeployerMismatch, + /// The deposit outpoint is missing from the indicated sweep + /// transaction. + #[error("deposit outpoint is missing from the indicated sweep transaction")] + DepositMissingFromSweep, + /// We do not have a record of the deposit request in our list of + /// pending and accepted deposit requests. + #[error("no record of deposit request in pending and accepted deposit requests")] + DepositRequestMissing, + /// The fee paid to the bitcoin miners exceeded the max fee. + #[error("fee paid to the bitcoin miners exceeded the max fee")] + InvalidFee, + /// The amount to mint must not exceed the amount in the deposit + /// request. + #[error("amount to mint exceeded the amount in the deposit request")] + InvalidMintAmount, + /// The recipient did not match the recipient in our deposit request + /// records. + #[error("recipient did not match the recipient in our deposit request")] + RecipientMismatch, + /// The sweep transaction that included the deposit request is missing + /// from our records. + #[error("sweep transaction not found")] + SweepTransactionMissing, + /// The sweep transaction has been affected by a reorg. Submitting this + /// transaction now will likely lead to a failed stacks transaction. + #[error("sweep transaction has been affected by a reorg")] + SweepTransactionReorged, +} + +impl DepositValidationMsg { + fn into_error(self, ctx: &ReqContext, tx: &CompleteDepositV1) -> Error { + Error::DepositValidation(Box::new(DepositValidationError { + error: self, + context: *ctx, + tx: tx.clone(), + })) } } @@ -281,13 +533,13 @@ impl AsContractCall for AcceptWithdrawalV1 { /// 3. That the signer bitmap matches the signer decisions stored in /// this signer's database. /// 4. That the `tx_fee` matches the amount spent to the bitcoin miner. - async fn validate(&self, _storage: &S) -> Result + async fn validate(&self, _db: &S, _ctx: &ReqContext) -> Result<(), Error> where S: DbRead + Send + Sync, Error: From<::Error>, { // TODO(255): Add validation implementation - Ok(false) + Ok(()) } } @@ -328,13 +580,13 @@ 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, _storage: &S) -> Result + async fn validate(&self, _db: &S, _ctx: &ReqContext) -> Result<(), Error> where S: DbRead + Send + Sync, Error: From<::Error>, { // TODO(255): Add validation implementation - Ok(false) + Ok(()) } } @@ -436,13 +688,13 @@ impl AsContractCall for RotateKeysV1 { /// threshold is different from the last signature threshold. /// 4. That the number of required signatures is strictly greater than /// `new_keys as f64 / 2.0`. - async fn validate(&self, _storage: &S) -> Result + async fn validate(&self, _db: &S, _ctx: &ReqContext) -> Result<(), Error> where S: DbRead + Send + Sync, Error: From<::Error>, { // TODO(255): Add validation implementation - Ok(false) + Ok(()) } } @@ -466,6 +718,9 @@ mod tests { amount: 15000, recipient: PrincipalData::from(StacksAddress::burn_address(true)), deployer: StacksAddress::burn_address(false), + sweep_txid: BitcoinTxId::from([0; 32]), + sweep_block_hash: BitcoinBlockHash::from([0; 32]), + sweep_block_height: 7, }; let _ = call.as_contract_call(); diff --git a/signer/src/stacks/wallet.rs b/signer/src/stacks/wallet.rs index a1c4d5ac..3a68570c 100644 --- a/signer/src/stacks/wallet.rs +++ b/signer/src/stacks/wallet.rs @@ -317,6 +317,7 @@ mod tests { use test_case::test_case; use crate::signature::sign_stacks_tx; + use crate::stacks::contracts::ReqContext; use crate::storage::DbRead; use super::*; @@ -335,12 +336,12 @@ mod tests { fn as_contract_args(&self) -> Vec { Vec::new() } - async fn validate(&self, _: &S) -> Result + async fn validate(&self, _db: &S, _ctx: &ReqContext) -> Result<(), Error> where S: DbRead + Send + Sync, Error: From<::Error>, { - Ok(true) + Ok(()) } } diff --git a/signer/src/testing/wallet.rs b/signer/src/testing/wallet.rs index 43bb9727..beefe59a 100644 --- a/signer/src/testing/wallet.rs +++ b/signer/src/testing/wallet.rs @@ -19,8 +19,10 @@ use crate::config::NetworkKind; use crate::error::Error; use crate::stacks::contracts::AsContractCall; use crate::stacks::contracts::AsTxPayload; +use crate::stacks::contracts::ReqContext; use crate::stacks::contracts::StacksTxPostConditions; use crate::stacks::wallet::SignerWallet; +use crate::storage::DbRead; /// A static for a test 2-3 multi-sig wallet. This wallet is loaded with /// funds in the local devnet environment. @@ -151,11 +153,11 @@ impl AsContractCall for InitiateWithdrawalRequest { ClarityValue::UInt(self.max_fee as u128), ] } - async fn validate(&self, _: &S) -> Result + async fn validate(&self, _db: &S, _ctx: &ReqContext) -> Result<(), Error> where - S: crate::storage::DbRead + Send + Sync, - Error: From<::Error>, + S: DbRead + Send + Sync, + Error: From<::Error>, { - Ok(true) + Ok(()) } } diff --git a/signer/src/testing/wsts.rs b/signer/src/testing/wsts.rs index 715161f4..c51f76e9 100644 --- a/signer/src/testing/wsts.rs +++ b/signer/src/testing/wsts.rs @@ -30,6 +30,22 @@ pub struct SignerInfo { pub signer_public_keys: BTreeSet, } +/// Generate a set of public keys for a group of signers +pub fn generate_signer_set(rng: &mut R, num_signers: usize) -> Vec +where + R: rand::RngCore + rand::CryptoRng, +{ + // Generate the signer set. Each SignerInfo object returned from the + // `generate_signer_info` function the public keys of + // other signers, so we take one of them and get the signing set from + // that one. + generate_signer_info(rng, num_signers) + .into_iter() + .take(1) + .flat_map(|signer_info| signer_info.signer_public_keys.into_iter()) + .collect() +} + /// Generate a new signer set pub fn generate_signer_info( rng: &mut Rng, diff --git a/signer/src/transaction_signer.rs b/signer/src/transaction_signer.rs index d0bdbc45..43b0b95c 100644 --- a/signer/src/transaction_signer.rs +++ b/signer/src/transaction_signer.rs @@ -21,12 +21,15 @@ use crate::message::StacksTransactionSignRequest; use crate::network; use crate::stacks::contracts::AsContractCall; use crate::stacks::contracts::ContractCall; +use crate::stacks::contracts::ReqContext; use crate::stacks::wallet::MultisigTx; use crate::stacks::wallet::SignerWallet; use crate::storage; use crate::storage::model; +use crate::storage::model::BitcoinBlockRef; use crate::wsts_state_machine; +use clarity::types::chainstate::StacksAddress; use futures::StreamExt; use wsts::net::DkgEnd; use wsts::net::DkgStatus; @@ -391,24 +394,18 @@ where request: &message::StacksTransactionSignRequest, bitcoin_chain_tip: &model::BitcoinBlockHash, ) -> Result<(), Error> { - let is_valid_sign_request = self - .is_valid_stackstransaction_sign_request(request, bitcoin_chain_tip) + self.is_valid_stackstransaction_sign_request(request, bitcoin_chain_tip) .await?; let wallet = self.load_wallet(request, bitcoin_chain_tip).await?; let multi_sig = MultisigTx::new_tx(&request.contract_call, &wallet, request.tx_fee); let txid = multi_sig.tx().txid(); - if is_valid_sign_request { - let signature = - crate::signature::sign_stacks_tx(multi_sig.tx(), &self.signer_private_key); + let signature = crate::signature::sign_stacks_tx(multi_sig.tx(), &self.signer_private_key); - let msg = message::StacksTransactionSignature { txid, signature }; + let msg = message::StacksTransactionSignature { txid, signature }; - self.send_message(msg, bitcoin_chain_tip).await?; - } else { - tracing::warn!(%txid, "received invalid sign request for stacks tx"); - } + self.send_message(msg, bitcoin_chain_tip).await?; Ok(()) } @@ -444,14 +441,33 @@ where async fn is_valid_stackstransaction_sign_request( &mut self, request: &message::StacksTransactionSignRequest, - _bitcoin_chain_tip: &model::BitcoinBlockHash, - ) -> Result { + chain_tip: &model::BitcoinBlockHash, + ) -> Result<(), Error> { // TODO(255): Finish the implementation + let ctx = ReqContext { + chain_tip: BitcoinBlockRef { + block_hash: *chain_tip, + // This is wrong + block_height: 0, + }, + context_window: self.context_window, + // This is wrong + origin: self.signer_pub_key(), + signatures_required: self.threshold as u16, + // This is wrong + deployer: StacksAddress::burn_address(false), + }; match &request.contract_call { - ContractCall::AcceptWithdrawalV1(contract) => contract.validate(&self.storage).await, - ContractCall::CompleteDepositV1(contract) => contract.validate(&self.storage).await, - ContractCall::RejectWithdrawalV1(contract) => contract.validate(&self.storage).await, - ContractCall::RotateKeysV1(contract) => contract.validate(&self.storage).await, + ContractCall::AcceptWithdrawalV1(contract) => { + contract.validate(&self.storage, &ctx).await + } + ContractCall::CompleteDepositV1(contract) => { + contract.validate(&self.storage, &ctx).await + } + ContractCall::RejectWithdrawalV1(contract) => { + contract.validate(&self.storage, &ctx).await + } + ContractCall::RotateKeysV1(contract) => contract.validate(&self.storage, &ctx).await, } } diff --git a/signer/tests/integration/contracts.rs b/signer/tests/integration/contracts.rs index deb505c5..2f4d3ec1 100644 --- a/signer/tests/integration/contracts.rs +++ b/signer/tests/integration/contracts.rs @@ -14,6 +14,8 @@ use signer::stacks::contracts::AsContractCall; use signer::stacks::contracts::RejectWithdrawalV1; use signer::stacks::contracts::RotateKeysV1; use signer::stacks::wallet::SignerWallet; +use signer::storage::model::BitcoinBlockHash; +use signer::storage::model::BitcoinTxId; use signer::testing::wallet::ContractCallWrapper; use tokio::sync::OnceCell; @@ -204,12 +206,18 @@ pub async fn deploy_smart_contracts() -> &'static SignerStxState { amount: 123654789, recipient: PrincipalData::parse("SN2V7WTJ7BHR03MPHZ1C9A9ZR6NZGR4WM8HT4V67Y").unwrap(), deployer: testing::wallet::WALLET.0.address(), + sweep_txid: BitcoinTxId::from([0; 32]), + sweep_block_hash: BitcoinBlockHash::from([0; 32]), + sweep_block_height: 7, }); "complete-deposit standard recipient")] #[test_case(ContractCallWrapper(CompleteDepositV1 { outpoint: bitcoin::OutPoint::null(), amount: 123654, recipient: PrincipalData::parse("ST1RQHF4VE5CZ6EK3MZPZVQBA0JVSMM9H5PMHMS1Y.my-contract-name").unwrap(), deployer: testing::wallet::WALLET.0.address(), + sweep_txid: BitcoinTxId::from([0; 32]), + sweep_block_hash: BitcoinBlockHash::from([0; 32]), + sweep_block_height: 7, }); "complete-deposit contract recipient")] #[test_case(ContractCallWrapper(AcceptWithdrawalV1 { request_id: 1, @@ -300,6 +308,9 @@ async fn estimate_tx_fees() { amount: 123654, recipient: PrincipalData::parse("ST1RQHF4VE5CZ6EK3MZPZVQBA0JVSMM9H5PMHMS1Y").unwrap(), deployer: StacksAddress::burn_address(false), + sweep_txid: BitcoinTxId::from([0; 32]), + sweep_block_hash: BitcoinBlockHash::from([0; 32]), + sweep_block_height: 7, }; let payload = ContractCallWrapper(contract_call); diff --git a/signer/tests/integration/postgres.rs b/signer/tests/integration/postgres.rs index 69d203bf..d009eff9 100644 --- a/signer/tests/integration/postgres.rs +++ b/signer/tests/integration/postgres.rs @@ -20,6 +20,7 @@ use signer::stacks::contracts::AsContractCall; use signer::stacks::contracts::AsTxPayload as _; use signer::stacks::contracts::CompleteDepositV1; use signer::stacks::contracts::RejectWithdrawalV1; +use signer::stacks::contracts::ReqContext; use signer::stacks::contracts::RotateKeysV1; use signer::stacks::events::CompletedDepositEvent; use signer::stacks::events::WithdrawalAcceptEvent; @@ -48,21 +49,6 @@ use test_case::test_case; use crate::DATABASE_NUM; -fn generate_signer_set(rng: &mut R, num_signers: usize) -> Vec -where - R: rand::RngCore + rand::CryptoRng, -{ - // Generate the signer set. Each SignerInfo object returned from the - // `testing::wsts::generate_signer_info` function the public keys of - // other signers, so we take one of them and get the signing set from - // that one. - testing::wsts::generate_signer_info(rng, num_signers) - .into_iter() - .take(1) - .flat_map(|signer_info| signer_info.signer_public_keys.into_iter()) - .collect() -} - #[cfg_attr(not(feature = "integration-tests"), ignore)] #[tokio::test] async fn should_be_able_to_query_bitcoin_blocks() { @@ -78,7 +64,7 @@ async fn should_be_able_to_query_bitcoin_blocks() { num_signers_per_request: 0, }; - let signer_set = generate_signer_set(&mut rng, 7); + let signer_set = testing::wsts::generate_signer_set(&mut rng, 7); let persisted_model = TestData::generate(&mut rng, &signer_set, &test_model_params); let not_persisted_model = TestData::generate(&mut rng, &signer_set, &test_model_params); @@ -121,12 +107,12 @@ impl AsContractCall for InitiateWithdrawalRequest { fn as_contract_args(&self) -> Vec { Vec::new() } - async fn validate(&self, _: &S) -> Result + async fn validate(&self, _db: &S, _ctx: &ReqContext) -> Result<(), Error> where S: DbRead + Send + Sync, Error: From<::Error>, { - Ok(true) + Ok(()) } } @@ -140,12 +126,18 @@ impl AsContractCall for InitiateWithdrawalRequest { amount: 123654, recipient: PrincipalData::parse("ST1RQHF4VE5CZ6EK3MZPZVQBA0JVSMM9H5PMHMS1Y").unwrap(), deployer: testing::wallet::WALLET.0.address(), + sweep_txid: BitcoinTxId::from([0; 32]), + sweep_block_hash: BitcoinBlockHash::from([0; 32]), + sweep_block_height: 7, }); "complete-deposit standard recipient")] #[test_case(ContractCallWrapper(CompleteDepositV1 { outpoint: bitcoin::OutPoint::null(), amount: 123654, recipient: PrincipalData::parse("ST1RQHF4VE5CZ6EK3MZPZVQBA0JVSMM9H5PMHMS1Y.my-contract-name").unwrap(), deployer: testing::wallet::WALLET.0.address(), + sweep_txid: BitcoinTxId::from([0; 32]), + sweep_block_hash: BitcoinBlockHash::from([0; 32]), + sweep_block_height: 7, }); "complete-deposit contract recipient")] #[test_case(ContractCallWrapper(AcceptWithdrawalV1 { request_id: 0, @@ -319,7 +311,7 @@ async fn should_return_the_same_pending_deposit_requests_as_in_memory_store() { num_withdraw_requests_per_block: 5, num_signers_per_request: 0, }; - let signer_set = generate_signer_set(&mut rng, num_signers); + let signer_set = testing::wsts::generate_signer_set(&mut rng, num_signers); let test_data = TestData::generate(&mut rng, &signer_set, &test_model_params); test_data.write_to(&mut in_memory_store).await; @@ -379,7 +371,7 @@ async fn should_return_the_same_pending_withdraw_requests_as_in_memory_store() { num_signers_per_request: 0, }; - let signer_set = generate_signer_set(&mut rng, num_signers); + let signer_set = testing::wsts::generate_signer_set(&mut rng, num_signers); let test_data = TestData::generate(&mut rng, &signer_set, &test_model_params); test_data.write_to(&mut in_memory_store).await; @@ -440,7 +432,7 @@ async fn should_return_the_same_pending_accepted_deposit_requests_as_in_memory_s }; let threshold = 4; - let signer_set = generate_signer_set(&mut rng, num_signers); + let signer_set = testing::wsts::generate_signer_set(&mut rng, num_signers); let test_data = TestData::generate(&mut rng, &signer_set, &test_model_params); test_data.write_to(&mut in_memory_store).await; @@ -509,7 +501,7 @@ async fn should_return_the_same_pending_accepted_withdraw_requests_as_in_memory_ num_signers_per_request: num_signers, }; let threshold = 4; - let signer_set = generate_signer_set(&mut rng, num_signers); + let signer_set = testing::wsts::generate_signer_set(&mut rng, num_signers); let test_data = TestData::generate(&mut rng, &signer_set, &test_model_params); test_data.write_to(&mut in_memory_store).await; @@ -574,7 +566,7 @@ async fn should_return_the_same_last_key_rotation_as_in_memory_store() { }; let num_signers = 7; let threshold = 4; - let signer_set = generate_signer_set(&mut rng, num_signers); + let signer_set = testing::wsts::generate_signer_set(&mut rng, num_signers); let test_data = TestData::generate(&mut rng, &signer_set, &test_model_params); test_data.write_to(&mut in_memory_store).await; @@ -1145,7 +1137,7 @@ async fn fetching_withdrawal_request_votes() { #[tokio::test] async fn block_in_canonical_bitcoin_blockchain_in_other_block_chain() { let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let pg_store = signer::testing::storage::new_test_database(db_num).await; + let pg_store = signer::testing::storage::new_test_database(db_num, true).await; let mut rng = rand::rngs::StdRng::seed_from_u64(51); // This is just a sql test, where we use the `TestData` struct to help @@ -1160,7 +1152,7 @@ async fn block_in_canonical_bitcoin_blockchain_in_other_block_chain() { num_signers_per_request: num_signers, }; - let signer_set = generate_signer_set(&mut rng, num_signers); + let signer_set = testing::wsts::generate_signer_set(&mut rng, num_signers); // Okay now we generate one blockchain and get its chain tip let test_data1 = TestData::generate(&mut rng, &signer_set, &test_model_params); // And we generate another blockchain and get its chain tip @@ -1220,7 +1212,7 @@ async fn block_in_canonical_bitcoin_blockchain_in_other_block_chain() { #[tokio::test] async fn we_can_fetch_bitcoin_txs_from_db() { let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let pg_store = signer::testing::storage::new_test_database(db_num).await; + let pg_store = signer::testing::storage::new_test_database(db_num, true).await; let mut rng = rand::rngs::StdRng::seed_from_u64(51); // This is just a sql test, where we use the `TestData` struct to help @@ -1235,7 +1227,7 @@ async fn we_can_fetch_bitcoin_txs_from_db() { num_signers_per_request: num_signers, }; - let signer_set = generate_signer_set(&mut rng, num_signers); + let signer_set = testing::wsts::generate_signer_set(&mut rng, num_signers); let test_data = TestData::generate(&mut rng, &signer_set, &test_model_params); test_data.write_to(&pg_store).await; From a7c65759e3bec778d170206a78c162235881c427 Mon Sep 17 00:00:00 2001 From: djordon Date: Wed, 18 Sep 2024 11:15:17 -0400 Subject: [PATCH 04/11] Add integration tests to the validation function --- signer/src/keys.rs | 6 + signer/src/storage/postgres.rs | 20 + signer/src/testing/dummy.rs | 59 ++ signer/tests/integration/complete_deposit.rs | 732 +++++++++++++++++++ signer/tests/integration/main.rs | 1 + 5 files changed, 818 insertions(+) create mode 100644 signer/tests/integration/complete_deposit.rs diff --git a/signer/src/keys.rs b/signer/src/keys.rs index 2ebfb0b9..f81fc4b1 100644 --- a/signer/src/keys.rs +++ b/signer/src/keys.rs @@ -183,6 +183,12 @@ impl From<&PublicKey> for stacks_common::util::secp256k1::Secp256k1PublicKey { } } +impl From for stacks_common::util::secp256k1::Secp256k1PublicKey { + fn from(value: PublicKey) -> Self { + Self::from(&value) + } +} + impl From<&stacks_common::util::secp256k1::Secp256k1PublicKey> for PublicKey { fn from(value: &stacks_common::util::secp256k1::Secp256k1PublicKey) -> Self { let key = secp256k1::PublicKey::from_slice(&value.to_bytes_compressed()) diff --git a/signer/src/storage/postgres.rs b/signer/src/storage/postgres.rs index 099cf02e..36275db1 100644 --- a/signer/src/storage/postgres.rs +++ b/signer/src/storage/postgres.rs @@ -321,6 +321,26 @@ impl PgStore { Ok(model::TransactionIds { tx_ids, block_hashes }) } + + /// Get the full block + #[cfg(any(test, feature = "testing"))] + pub async fn get_bitcoin_canonical_chain_tip_block( + &self, + ) -> Result, Error> { + sqlx::query_as::<_, model::BitcoinBlock>( + "SELECT + block_hash + , block_height + , parent_hash + , confirms + FROM sbtc_signer.bitcoin_blocks + ORDER BY block_height DESC, block_hash DESC + LIMIT 1", + ) + .fetch_optional(&self.0) + .await + .map_err(Error::SqlxQuery) + } } impl From for PgStore { diff --git a/signer/src/testing/dummy.rs b/signer/src/testing/dummy.rs index 65fcdd18..ee479add 100644 --- a/signer/src/testing/dummy.rs +++ b/signer/src/testing/dummy.rs @@ -19,6 +19,7 @@ use rand::Rng; use sbtc::deposits::DepositScriptInputs; use sbtc::deposits::ReclaimScriptInputs; use secp256k1::ecdsa::RecoverableSignature; +use secp256k1::SECP256K1; use stacks_common::types::chainstate::StacksAddress; use crate::keys::PrivateKey; @@ -470,3 +471,61 @@ impl fake::Dummy for model::Transaction { } } } + +/// A struct to aid in the generation of bitcoin sweep transactions. +/// +/// BitcoinTx is created with this config, then it will have a UTXO that is +/// locked with a valid scriptPubKey that the signers can spend. +#[derive(Debug, Clone)] +pub struct SweepTxConfig { + /// The public key of the signer. + pub signer_public_key: PublicKey, + /// The amount of the signers UTXO afterwards. + pub amounts: std::ops::Range, + /// The outpoints to use as inputs. + pub inputs: Vec, +} + +impl fake::Dummy for BitcoinTx { + fn dummy_with_rng(config: &SweepTxConfig, rng: &mut R) -> Self { + let internal_key = config.signer_public_key.into(); + let outpoints = config.inputs.iter().copied(); + + let deposit_tx = bitcoin::Transaction { + version: bitcoin::transaction::Version::TWO, + lock_time: bitcoin::absolute::LockTime::ZERO, + input: outpoints + .map(|previous_output| TxIn { + previous_output, + sequence: bitcoin::Sequence::ZERO, + script_sig: ScriptBuf::new(), + witness: bitcoin::Witness::new(), + }) + .collect(), + output: vec![TxOut { + value: Amount::from_sat(config.amounts.clone().choose(rng).unwrap_or_default()), + script_pubkey: ScriptBuf::new_p2tr(SECP256K1, internal_key, None), + }], + }; + + Self::from(deposit_tx) + } +} + +impl fake::Dummy for model::Transaction { + fn dummy_with_rng(config: &SweepTxConfig, rng: &mut R) -> Self { + let mut tx = Vec::new(); + + let bitcoin_tx: BitcoinTx = config.fake_with_rng(rng); + bitcoin_tx + .consensus_encode(&mut tx) + .expect("In-memory writers never fail"); + + model::Transaction { + tx, + txid: bitcoin_tx.compute_txid().to_byte_array(), + tx_type: model::TransactionType::DepositRequest, + block_hash: fake::Faker.fake_with_rng(rng), + } + } +} diff --git a/signer/tests/integration/complete_deposit.rs b/signer/tests/integration/complete_deposit.rs new file mode 100644 index 00000000..cf51ae7d --- /dev/null +++ b/signer/tests/integration/complete_deposit.rs @@ -0,0 +1,732 @@ +use std::sync::atomic::Ordering; + +use bitcoin::hashes::Hash as _; +use bitcoin::OutPoint; +use blockstack_lib::types::chainstate::StacksAddress; + +use rand::rngs::OsRng; +use signer::error::Error; +use signer::keys::PublicKey; +use signer::stacks::contracts::AsContractCall as _; +use signer::stacks::contracts::CompleteDepositV1; +use signer::stacks::contracts::DepositValidationMsg; +use signer::stacks::contracts::ReqContext; +use signer::storage::model; +use signer::storage::model::BitcoinBlock; +use signer::storage::model::BitcoinTxRef; +use signer::storage::model::DepositRequest; +use signer::storage::model::StacksPrincipal; +use signer::storage::postgres::PgStore; +use signer::storage::DbRead as _; +use signer::storage::DbWrite as _; +use signer::testing; +use signer::testing::dummy::SweepTxConfig; +use signer::testing::storage::model::TestData; + +use fake::Fake; +use rand::SeedableRng; + +use crate::DATABASE_NUM; + +/// Create a "proper" [`CompleteDepositV1`] object and context with the +/// given information. If the information here is correct then the returned +/// [`CompleteDepositV1`] object will pass validation with the given +/// context. +fn make_complete_deposit( + req: &DepositRequest, + sweep_tx: &model::Transaction, + chain_tip: &BitcoinBlock, +) -> (CompleteDepositV1, ReqContext) { + // Okay now we get ready to create the transaction using the + // `CompleteDepositV1` type. + let complete_deposit_tx = CompleteDepositV1 { + // This outpoint points to the deposit UTXO. + outpoint: req.outpoint(), + // This amount must not exceed the amount in the deposit request. + amount: req.amount, + // The recipient must match what was indicated in the deposit + // request. + recipient: req.recipient.clone().into(), + // The deployer must match what is in the signers' context. + deployer: StacksAddress::burn_address(false), + // The sweep transaction ID must point to a transaction on + // the canonical bitcoin blockchain. + sweep_txid: sweep_tx.txid.into(), + // The block hash of the block that includes the above sweep + // transaction. It must be on the canonical bitcoin blockchain. + sweep_block_hash: chain_tip.block_hash, + // This must be the height of the above block. + sweep_block_height: chain_tip.block_height, + }; + + // This is what the current signer things of the state of things. + let ctx = ReqContext { + chain_tip: chain_tip.into(), + // This value means that the signer will go back 10 blocks when + // looking for pending and accepted deposit requests. + context_window: 10, + // The value here doesn't matter. + origin: fake::Faker.fake_with_rng(&mut OsRng), + // This value affects how many deposit transactions are consider + // accepted. + signatures_required: 2, + // This is who the current signer thinks deployed the sBTC + // contracts. + deployer: StacksAddress::burn_address(false), + }; + + (complete_deposit_tx, ctx) +} + +/// Generate a signer set, deposit requests and store them into the +/// database. +async fn depossit_setup(rng: &mut R, pg_store: &PgStore) -> Vec +where + R: rand::RngCore + rand::CryptoRng, +{ + // This is just a sql test, where we use the `TestData` struct to help + // populate the database with test data. We set all of the other + // unnecessary parameters to zero. + let num_signers = 7; + let test_model_params = testing::storage::model::Params { + num_bitcoin_blocks: 20, + num_stacks_blocks_per_bitcoin_block: 0, + num_deposit_requests_per_block: 2, + num_withdraw_requests_per_block: 0, + num_signers_per_request: num_signers, + }; + + // Normal: this generates the blockchain as well as deposit request + // transactions in each bitcoin block. + let signer_set = testing::wsts::generate_signer_set(rng, num_signers); + let test_data = TestData::generate(rng, &signer_set, &test_model_params); + test_data.write_to(pg_store).await; + signer_set +} + +/// For this test we check that the `CompleteDepositV1::validate` function +/// returns okay when everything matches the way that it is supposed to. +#[cfg_attr(not(feature = "integration-tests"), ignore)] +#[tokio::test] +async fn complete_deposit_validation_happy_path() { + // Normal: this generates the blockchain as well as deposit request + // transactions in each bitcoin block. + let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); + let pg_store = signer::testing::storage::new_test_database(db_num, true).await; + let mut rng = rand::rngs::StdRng::seed_from_u64(51); + let signatures_required = 3; + + let signer_set = depossit_setup(&mut rng, &pg_store).await; + // Get the chain tip. + let chain_tip = pg_store + .get_bitcoin_canonical_chain_tip_block() + .await + .unwrap() + .unwrap(); + // Normal: Get an existing deposit request on the canonical bitcoin + // blockchain. + let deposit_req = pg_store + .get_pending_accepted_deposit_requests(&chain_tip.block_hash, 20, signatures_required) + .await + .unwrap() + .last() + .cloned() + .unwrap(); + + // Normal: we generate a transaction that sweeps in the deposit. + let sweep_config = SweepTxConfig { + signer_public_key: PublicKey::combine_keys(&signer_set).unwrap(), + amounts: 3000..1_000_000_000, + inputs: vec![deposit_req.outpoint()], + }; + let mut sweep_tx: model::Transaction = sweep_config.fake_with_rng(&mut rng); + // Normal: make sure the sweep transaction is on the canonical bitcoin + // blockchain and is in our database. + sweep_tx.block_hash = chain_tip.block_hash.into_bytes(); + // Normal: make sure that we have a record of the sweep transaction in + // our database. + let bitcoin_tx_ref = BitcoinTxRef { + txid: sweep_tx.txid.into(), + block_hash: sweep_tx.block_hash.into(), + }; + pg_store.write_transaction(&sweep_tx).await.unwrap(); + pg_store + .write_bitcoin_transaction(&bitcoin_tx_ref) + .await + .unwrap(); + + // Generate the transaction and corresponding request context. + let (complete_deposit_tx, ctx) = make_complete_deposit(&deposit_req, &sweep_tx, &chain_tip); + + // This should not return an Err. + complete_deposit_tx.validate(&pg_store, &ctx).await.unwrap(); + + signer::testing::storage::drop_db(pg_store).await; +} + +/// For this test we check that the `CompleteDepositV1::validate` function +/// returns a deposit validation error with a DeployerMismatch message when +/// the deployer doesn't match but everything else is okay. +#[cfg_attr(not(feature = "integration-tests"), ignore)] +#[tokio::test] +async fn complete_deposit_validation_deployer_mismatch() { + // Normal: this generates the blockchain as well as deposit request + // transactions in each bitcoin block. + let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); + let pg_store = signer::testing::storage::new_test_database(db_num, true).await; + let mut rng = rand::rngs::StdRng::seed_from_u64(51); + let signatures_required = 3; + + let signer_set = depossit_setup(&mut rng, &pg_store).await; + + // Get the chain tip + let chain_tip = pg_store + .get_bitcoin_canonical_chain_tip_block() + .await + .unwrap() + .unwrap(); + // Normal: Get an existing deposit request on the canonical bitcoin + // blockchain. + let deposit_req = pg_store + .get_pending_accepted_deposit_requests(&chain_tip.block_hash, 20, signatures_required) + .await + .unwrap() + .last() + .cloned() + .unwrap(); + + // Normal: we generate a transaction that sweeps in the deposit. + let sweep_config = SweepTxConfig { + signer_public_key: PublicKey::combine_keys(&signer_set).unwrap(), + amounts: 3000..1_000_000_000, + inputs: vec![deposit_req.outpoint()], + }; + let mut sweep_tx: model::Transaction = sweep_config.fake_with_rng(&mut rng); + // Normal: make sure the sweep transaction is on the canonical bitcoin + // blockchain and is in our database. + sweep_tx.block_hash = chain_tip.block_hash.into_bytes(); + // Normal: make sure that we have a record of the sweep transaction in + // our database. + let bitcoin_tx_ref = BitcoinTxRef { + txid: sweep_tx.txid.into(), + block_hash: sweep_tx.block_hash.into(), + }; + pg_store.write_transaction(&sweep_tx).await.unwrap(); + pg_store + .write_bitcoin_transaction(&bitcoin_tx_ref) + .await + .unwrap(); + + // Generate the transaction and corresponding request context. + let (mut complete_deposit_tx, mut ctx) = + make_complete_deposit(&deposit_req, &sweep_tx, &chain_tip); + // Different: Okay, let's make sure we get the deployers do not match. + complete_deposit_tx.deployer = StacksAddress::p2pkh(false, &signer_set[0].into()); + ctx.deployer = StacksAddress::p2pkh(false, &signer_set[1].into()); + + let validate_future = complete_deposit_tx.validate(&pg_store, &ctx); + match validate_future.await.unwrap_err() { + Error::DepositValidation(ref err) => { + assert_eq!(err.error, DepositValidationMsg::DeployerMismatch) + } + err => panic!("unexpected error during validation {err}"), + } + + signer::testing::storage::drop_db(pg_store).await; +} + +/// For this test we check that the `CompleteDepositV1::validate` function +/// returns a deposit validation error with a DepositRequestMissing message +/// when the signer does not have a record of the deposit request doesn't +/// match but everything else is okay. +#[cfg_attr(not(feature = "integration-tests"), ignore)] +#[tokio::test] +async fn complete_deposit_validation_missing_deposit_request() { + // Normal: this generates the blockchain as well as deposit request + // transactions in each bitcoin block. + let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); + let pg_store = signer::testing::storage::new_test_database(db_num, true).await; + let mut rng = rand::rngs::StdRng::seed_from_u64(51); + + let signer_set = depossit_setup(&mut rng, &pg_store).await; + + // Normal: Get the chain tip and any pending deposit request in the blockchain + // identified by the chain tip. + let chain_tip = pg_store + .get_bitcoin_canonical_chain_tip_block() + .await + .unwrap() + .unwrap(); + // Different: Let's use a random deposit request instead of one that + // exists in the database. + let deposit_req: DepositRequest = fake::Faker.fake_with_rng(&mut rng); + + // Normal: we generate a transaction that sweeps in the deposit. + let sweep_config = SweepTxConfig { + signer_public_key: PublicKey::combine_keys(&signer_set).unwrap(), + amounts: 3000..1_000_000_000, + inputs: vec![deposit_req.outpoint()], + }; + let mut sweep_tx: model::Transaction = sweep_config.fake_with_rng(&mut rng); + // Normal: make sure the sweep transaction is on the canonical bitcoin + // blockchain and is in our database. + sweep_tx.block_hash = chain_tip.block_hash.into_bytes(); + // Normal: make sure that we have a record of the sweep transaction in + // our database. + let bitcoin_tx_ref = BitcoinTxRef { + txid: sweep_tx.txid.into(), + block_hash: sweep_tx.block_hash.into(), + }; + pg_store.write_transaction(&sweep_tx).await.unwrap(); + pg_store + .write_bitcoin_transaction(&bitcoin_tx_ref) + .await + .unwrap(); + + let (complete_deposit_tx, ctx) = make_complete_deposit(&deposit_req, &sweep_tx, &chain_tip); + + let validation_result = complete_deposit_tx.validate(&pg_store, &ctx).await; + match validation_result.unwrap_err() { + Error::DepositValidation(ref err) => { + assert_eq!(err.error, DepositValidationMsg::DepositRequestMissing) + } + err => panic!("unexpected error during validation {err}"), + } + + signer::testing::storage::drop_db(pg_store).await; +} + +/// For this test we check that the `CompleteDepositV1::validate` function +/// returns a deposit validation error with a RecipientMismatch message +/// when the recipient in the complete-deposit transaction does not match +/// the recipient in our records. +#[cfg_attr(not(feature = "integration-tests"), ignore)] +#[tokio::test] +async fn complete_deposit_validation_recipient_mismatch() { + // Normal: this generates the blockchain as well as deposit request + // transactions in each bitcoin block. + let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); + let pg_store = signer::testing::storage::new_test_database(db_num, true).await; + let mut rng = rand::rngs::StdRng::seed_from_u64(51); + let signatures_required = 3; + + let signer_set = depossit_setup(&mut rng, &pg_store).await; + + // Get the chain tip. + let chain_tip = pg_store + .get_bitcoin_canonical_chain_tip_block() + .await + .unwrap() + .unwrap(); + // Normal: Get an existing deposit request on the canonical bitcoin + // blockchain. + let deposit_req = pg_store + .get_pending_accepted_deposit_requests(&chain_tip.block_hash, 20, signatures_required) + .await + .unwrap() + .last() + .cloned() + .unwrap(); + + // Normal: we generate a transaction that sweeps in the deposit. + let sweep_config = SweepTxConfig { + signer_public_key: PublicKey::combine_keys(&signer_set).unwrap(), + amounts: 3000..1_000_000_000, + inputs: vec![deposit_req.outpoint()], + }; + let mut sweep_tx: model::Transaction = sweep_config.fake_with_rng(&mut rng); + // Normal: make sure the sweep transaction is on the canonical bitcoin + // blockchain and is in our database. + sweep_tx.block_hash = chain_tip.block_hash.into_bytes(); + // Normal: make sure that we have a record of the sweep transaction in + // our database. + let bitcoin_tx_ref = BitcoinTxRef { + txid: sweep_tx.txid.into(), + block_hash: sweep_tx.block_hash.into(), + }; + pg_store.write_transaction(&sweep_tx).await.unwrap(); + pg_store + .write_bitcoin_transaction(&bitcoin_tx_ref) + .await + .unwrap(); + + // Generate the transaction and corresponding request context. + let (mut complete_deposit_tx, ctx) = make_complete_deposit(&deposit_req, &sweep_tx, &chain_tip); + // Different: Okay, let's make sure we get the deployers do not match. + complete_deposit_tx.recipient = fake::Faker + .fake_with_rng::(&mut rng) + .into(); + + let validate_future = complete_deposit_tx.validate(&pg_store, &ctx); + match validate_future.await.unwrap_err() { + Error::DepositValidation(ref err) => { + assert_eq!(err.error, DepositValidationMsg::RecipientMismatch) + } + err => panic!("unexpected error during validation {err}"), + } + + signer::testing::storage::drop_db(pg_store).await; +} + +/// For this test we check that the `CompleteDepositV1::validate` function +/// returns a deposit validation error with a InvalidMintAmount message +/// when the amount of sBTC to mint exceeds the amount in the signer's +/// deposit request record. +#[cfg_attr(not(feature = "integration-tests"), ignore)] +#[tokio::test] +async fn complete_deposit_validation_invalid_mint_amount() { + // Normal: this generates the blockchain as well as deposit request + // transactions in each bitcoin block. + let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); + let pg_store = signer::testing::storage::new_test_database(db_num, true).await; + let mut rng = rand::rngs::StdRng::seed_from_u64(51); + let signatures_required = 3; + + let signer_set = depossit_setup(&mut rng, &pg_store).await; + + // Get the chain tip. + let chain_tip = pg_store + .get_bitcoin_canonical_chain_tip_block() + .await + .unwrap() + .unwrap(); + // Normal: Get an existing deposit request on the canonical bitcoin + // blockchain. + let deposit_req = pg_store + .get_pending_accepted_deposit_requests(&chain_tip.block_hash, 20, signatures_required) + .await + .unwrap() + .last() + .cloned() + .unwrap(); + + // Normal: we generate a transaction that sweeps in the deposit. + let sweep_config = SweepTxConfig { + signer_public_key: PublicKey::combine_keys(&signer_set).unwrap(), + amounts: 3000..1_000_000_000, + inputs: vec![deposit_req.outpoint()], + }; + let mut sweep_tx: model::Transaction = sweep_config.fake_with_rng(&mut rng); + // Normal: make sure the sweep transaction is on the canonical bitcoin + // blockchain and is in our database. + sweep_tx.block_hash = chain_tip.block_hash.into_bytes(); + // Normal: make sure that we have a record of the sweep transaction in + // our database. + let bitcoin_tx_ref = BitcoinTxRef { + txid: sweep_tx.txid.into(), + block_hash: sweep_tx.block_hash.into(), + }; + pg_store.write_transaction(&sweep_tx).await.unwrap(); + pg_store + .write_bitcoin_transaction(&bitcoin_tx_ref) + .await + .unwrap(); + + // Generate the transaction and corresponding request context. + let (mut complete_deposit_tx, ctx) = make_complete_deposit(&deposit_req, &sweep_tx, &chain_tip); + // Different: The amount cannot exceed the amount in the deposit + // request. + complete_deposit_tx.amount = deposit_req.amount + 1000; + + let validate_future = complete_deposit_tx.validate(&pg_store, &ctx); + match validate_future.await.unwrap_err() { + Error::DepositValidation(ref err) => { + assert_eq!(err.error, DepositValidationMsg::InvalidMintAmount) + } + err => panic!("unexpected error during validation {err}"), + } + + signer::testing::storage::drop_db(pg_store).await; +} + +/// For this test we check that the `CompleteDepositV1::validate` function +/// returns a deposit validation error with a InvalidFee message when the +/// amount of sBTC to mint is less than the `amount - max-fee` from in the +/// signer's deposit request record. +#[cfg_attr(not(feature = "integration-tests"), ignore)] +#[tokio::test] +async fn complete_deposit_validation_invalid_fee() { + // Normal: this generates the blockchain as well as deposit request + // transactions in each bitcoin block. + let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); + let pg_store = signer::testing::storage::new_test_database(db_num, true).await; + let mut rng = rand::rngs::StdRng::seed_from_u64(51); + let signatures_required = 3; + + let signer_set = depossit_setup(&mut rng, &pg_store).await; + + // Get the chain tip. + let chain_tip = pg_store + .get_bitcoin_canonical_chain_tip_block() + .await + .unwrap() + .unwrap(); + // Normal: Get an existing deposit request on the canonical bitcoin + // blockchain. + let deposit_req = pg_store + .get_pending_accepted_deposit_requests(&chain_tip.block_hash, 20, signatures_required) + .await + .unwrap() + .last() + .cloned() + .unwrap(); + + // Normal: we generate a transaction that sweeps in the deposit. + let sweep_config = SweepTxConfig { + signer_public_key: PublicKey::combine_keys(&signer_set).unwrap(), + amounts: 3000..1_000_000_000, + inputs: vec![deposit_req.outpoint()], + }; + let mut sweep_tx: model::Transaction = sweep_config.fake_with_rng(&mut rng); + // Normal: make sure the sweep transaction is on the canonical bitcoin + // blockchain and is in our database. + sweep_tx.block_hash = chain_tip.block_hash.into_bytes(); + // Normal: make sure that we have a record of the sweep transaction in + // our database. + let bitcoin_tx_ref = BitcoinTxRef { + txid: sweep_tx.txid.into(), + block_hash: sweep_tx.block_hash.into(), + }; + pg_store.write_transaction(&sweep_tx).await.unwrap(); + pg_store + .write_bitcoin_transaction(&bitcoin_tx_ref) + .await + .unwrap(); + + // Generate the transaction and corresponding request context. + let (mut complete_deposit_tx, ctx) = make_complete_deposit(&deposit_req, &sweep_tx, &chain_tip); + // Different: The amount cannot be less than the deposit amount less + // the max-fee. + complete_deposit_tx.amount = deposit_req.amount - deposit_req.max_fee - 1; + + let validate_future = complete_deposit_tx.validate(&pg_store, &ctx); + match validate_future.await.unwrap_err() { + Error::DepositValidation(ref err) => { + assert_eq!(err.error, DepositValidationMsg::InvalidFee) + } + err => panic!("unexpected error during validation {err}"), + } + + signer::testing::storage::drop_db(pg_store).await; +} + +/// For this test we check that the `CompleteDepositV1::validate` function +/// returns a deposit validation error with a SweepTransactionMissing +/// message when the signer does not have a record of the sweep +/// transaction. +#[cfg_attr(not(feature = "integration-tests"), ignore)] +#[tokio::test] +async fn complete_deposit_validation_sweep_tx_missing() { + // Normal: this generates the blockchain as well as deposit request + // transactions in each bitcoin block. + let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); + let pg_store = signer::testing::storage::new_test_database(db_num, true).await; + let mut rng = rand::rngs::StdRng::seed_from_u64(51); + let signatures_required = 3; + + let signer_set = depossit_setup(&mut rng, &pg_store).await; + + // Get the chain tip. + let chain_tip = pg_store + .get_bitcoin_canonical_chain_tip_block() + .await + .unwrap() + .unwrap(); + // Normal: Get an existing deposit request on the canonical bitcoin + // blockchain. + let deposit_req = pg_store + .get_pending_accepted_deposit_requests(&chain_tip.block_hash, 20, signatures_required) + .await + .unwrap() + .last() + .cloned() + .unwrap(); + + // Normal: we generate a transaction that sweeps in the deposit. + let sweep_config = SweepTxConfig { + signer_public_key: PublicKey::combine_keys(&signer_set).unwrap(), + amounts: 3000..1_000_000_000, + inputs: vec![deposit_req.outpoint()], + }; + let mut sweep_tx: model::Transaction = sweep_config.fake_with_rng(&mut rng); + // Normal: make sure the sweep transaction is on the canonical bitcoin + // blockchain and is in our database. + sweep_tx.block_hash = chain_tip.block_hash.into_bytes(); + + // Different: we are supposed to store a sweep transaction, but we do + // not do that here. + + // Generate the transaction and corresponding request context. + let (complete_deposit_tx, ctx) = make_complete_deposit(&deposit_req, &sweep_tx, &chain_tip); + + let validation_result = complete_deposit_tx.validate(&pg_store, &ctx).await; + match validation_result.unwrap_err() { + Error::DepositValidation(ref err) => { + assert_eq!(err.error, DepositValidationMsg::SweepTransactionMissing) + } + err => panic!("unexpected error during validation {err}"), + } + + signer::testing::storage::drop_db(pg_store).await; +} + +/// For this test we check that the `CompleteDepositV1::validate` function +/// returns a deposit validation error with a SweepTransactionReorged +/// message when the sweep transaction is in our records but is not on what +/// the signer thinks is the canonical bitcoin blockchain. +#[cfg_attr(not(feature = "integration-tests"), ignore)] +#[tokio::test] +async fn complete_deposit_validation_sweep_reorged() { + // Normal: this generates the blockchain as well as deposit request + // transactions in each bitcoin block. + let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); + let pg_store = signer::testing::storage::new_test_database(db_num, true).await; + let mut rng = rand::rngs::StdRng::seed_from_u64(51); + let signatures_required = 3; + + let signer_set = depossit_setup(&mut rng, &pg_store).await; + // Get the chain tip. + let chain_tip = pg_store + .get_bitcoin_canonical_chain_tip_block() + .await + .unwrap() + .unwrap(); + // Normal: Get an existing deposit request on the canonical bitcoin + // blockchain. + let deposit_req = pg_store + .get_pending_accepted_deposit_requests(&chain_tip.block_hash, 20, signatures_required) + .await + .unwrap() + .last() + .cloned() + .unwrap(); + + // Normal: we generate a transaction that sweeps in the deposit. + let sweep_config = SweepTxConfig { + signer_public_key: PublicKey::combine_keys(&signer_set).unwrap(), + amounts: 3000..1_000_000_000, + inputs: vec![deposit_req.outpoint()], + }; + let mut sweep_tx: model::Transaction = sweep_config.fake_with_rng(&mut rng); + // Different: the transaction that sweeps in the deposit gets + // confirmed, but on a bitcoin blockchain that is not the canonical + // one. We generate a new blockchain and put it there. + // + // Note that this blockchain might actually be have a greater height, + // but we get to say which one is the canonical one in our context so + // that fact doesn't matter in this test. + let test_model_params = testing::storage::model::Params { + num_bitcoin_blocks: 10, + num_stacks_blocks_per_bitcoin_block: 0, + num_deposit_requests_per_block: 0, + num_withdraw_requests_per_block: 0, + num_signers_per_request: 0, + }; + let test_data2 = TestData::generate(&mut rng, &signer_set, &test_model_params); + test_data2.write_to(&pg_store).await; + let chain_tip2 = test_data2 + .bitcoin_blocks + .iter() + .max_by_key(|x| (x.block_height, x.block_hash)) + .unwrap(); + sweep_tx.block_hash = chain_tip2.block_hash.into_bytes(); + // Normal: make sure that we have a record of the sweep transaction in + // our database. + let bitcoin_tx_ref = BitcoinTxRef { + txid: sweep_tx.txid.into(), + block_hash: sweep_tx.block_hash.into(), + }; + pg_store.write_transaction(&sweep_tx).await.unwrap(); + pg_store + .write_bitcoin_transaction(&bitcoin_tx_ref) + .await + .unwrap(); + + // Generate the transaction and corresponding request context. + let (complete_deposit_tx, mut ctx) = + make_complete_deposit(&deposit_req, &sweep_tx, &chain_tip2); + ctx.chain_tip = chain_tip.into(); + + let validation_result = complete_deposit_tx.validate(&pg_store, &ctx).await; + match validation_result.unwrap_err() { + Error::DepositValidation(ref err) => { + assert_eq!(err.error, DepositValidationMsg::SweepTransactionReorged) + } + err => panic!("unexpected error during validation {err}"), + } + + signer::testing::storage::drop_db(pg_store).await; +} + +/// For this test we check that the `CompleteDepositV1::validate` function +/// returns a deposit validation error with a DepositMissingFromSweep +/// message when the sweep transaction is in our records, is on what the +/// signer thinks is the canonical bitcoin blockchain, but it does not have +/// an input that that matches the deposit request outpoint. +#[cfg_attr(not(feature = "integration-tests"), ignore)] +#[tokio::test] +async fn complete_deposit_validation_deposit_not_in_sweep() { + // Normal: this generates the blockchain as well as deposit request + // transactions in each bitcoin block. + let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); + let pg_store = signer::testing::storage::new_test_database(db_num, true).await; + let mut rng = rand::rngs::StdRng::seed_from_u64(51); + let signatures_required = 3; + + let signer_set = depossit_setup(&mut rng, &pg_store).await; + + // Get the chain tip. + let chain_tip = pg_store + .get_bitcoin_canonical_chain_tip_block() + .await + .unwrap() + .unwrap(); + // Normal: Get an existing deposit request on the canonical bitcoin + // blockchain. + let deposit_req = pg_store + .get_pending_accepted_deposit_requests(&chain_tip.block_hash, 20, signatures_required) + .await + .unwrap() + .last() + .cloned() + .unwrap(); + + // Different: The sweep transaction does not include the deposit + // request UTXO as an input. + let sweep_config = SweepTxConfig { + signer_public_key: PublicKey::combine_keys(&signer_set).unwrap(), + amounts: 3000..1_000_000_000, + inputs: vec![OutPoint { + txid: bitcoin::Txid::from_byte_array(fake::Faker.fake_with_rng(&mut rng)), + vout: 0, + }], + }; + let mut sweep_tx: model::Transaction = sweep_config.fake_with_rng(&mut rng); + // Normal: make sure the sweep transaction is on the canonical bitcoin + // blockchain and is in our database. + sweep_tx.block_hash = chain_tip.block_hash.into_bytes(); + // Normal: make sure that we have a record of the sweep transaction in + // our database. + let bitcoin_tx_ref = BitcoinTxRef { + txid: sweep_tx.txid.into(), + block_hash: sweep_tx.block_hash.into(), + }; + pg_store.write_transaction(&sweep_tx).await.unwrap(); + pg_store + .write_bitcoin_transaction(&bitcoin_tx_ref) + .await + .unwrap(); + + // Generate the transaction and corresponding request context. + let (complete_deposit_tx, ctx) = make_complete_deposit(&deposit_req, &sweep_tx, &chain_tip); + + let validation_result = complete_deposit_tx.validate(&pg_store, &ctx).await; + match validation_result.unwrap_err() { + Error::DepositValidation(ref err) => { + assert_eq!(err.error, DepositValidationMsg::DepositMissingFromSweep) + } + err => panic!("unexpected error during validation {err}"), + } + + signer::testing::storage::drop_db(pg_store).await; +} diff --git a/signer/tests/integration/main.rs b/signer/tests/integration/main.rs index 37ad9870..c239ae15 100644 --- a/signer/tests/integration/main.rs +++ b/signer/tests/integration/main.rs @@ -4,6 +4,7 @@ mod contracts; mod postgres; mod rbf; mod transaction_signer; +mod complete_deposit; mod utxo_construction; mod zmq; From 528fde2c9fe586f9b6e7688ed13a1e268547c651 Mon Sep 17 00:00:00 2001 From: djordon Date: Wed, 18 Sep 2024 11:15:25 -0400 Subject: [PATCH 05/11] Update a stray comment --- signer/src/bitcoin/zmq.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/signer/src/bitcoin/zmq.rs b/signer/src/bitcoin/zmq.rs index a801d448..58522e59 100644 --- a/signer/src/bitcoin/zmq.rs +++ b/signer/src/bitcoin/zmq.rs @@ -131,7 +131,8 @@ pub fn parse_bitcoin_core_message(message: ZmqMessage) -> Result { let topic = core::str::from_utf8(data[0]).map(ToString::to_string); Err(Error::BitcoinCoreZmqUnsupported(topic)) From 00e5a39f18227563f2f65705822d1f7c9569e973 Mon Sep 17 00:00:00 2001 From: djordon Date: Wed, 18 Sep 2024 11:22:09 -0400 Subject: [PATCH 06/11] This makes more sense --- signer/src/storage/postgres.rs | 20 --------- signer/tests/integration/complete_deposit.rs | 46 ++++++++++++-------- signer/tests/integration/main.rs | 2 +- 3 files changed, 29 insertions(+), 39 deletions(-) diff --git a/signer/src/storage/postgres.rs b/signer/src/storage/postgres.rs index 36275db1..099cf02e 100644 --- a/signer/src/storage/postgres.rs +++ b/signer/src/storage/postgres.rs @@ -321,26 +321,6 @@ impl PgStore { Ok(model::TransactionIds { tx_ids, block_hashes }) } - - /// Get the full block - #[cfg(any(test, feature = "testing"))] - pub async fn get_bitcoin_canonical_chain_tip_block( - &self, - ) -> Result, Error> { - sqlx::query_as::<_, model::BitcoinBlock>( - "SELECT - block_hash - , block_height - , parent_hash - , confirms - FROM sbtc_signer.bitcoin_blocks - ORDER BY block_height DESC, block_hash DESC - LIMIT 1", - ) - .fetch_optional(&self.0) - .await - .map_err(Error::SqlxQuery) - } } impl From for PgStore { diff --git a/signer/tests/integration/complete_deposit.rs b/signer/tests/integration/complete_deposit.rs index cf51ae7d..fd3c013c 100644 --- a/signer/tests/integration/complete_deposit.rs +++ b/signer/tests/integration/complete_deposit.rs @@ -104,6 +104,25 @@ where signer_set } +/// Get the full block +async fn get_bitcoin_canonical_chain_tip_block( + store: &PgStore, +) -> Result, Error> { + sqlx::query_as::<_, model::BitcoinBlock>( + "SELECT + block_hash + , block_height + , parent_hash + , confirms + FROM sbtc_signer.bitcoin_blocks + ORDER BY block_height DESC, block_hash DESC + LIMIT 1", + ) + .fetch_optional(store.pool()) + .await + .map_err(Error::SqlxQuery) +} + /// For this test we check that the `CompleteDepositV1::validate` function /// returns okay when everything matches the way that it is supposed to. #[cfg_attr(not(feature = "integration-tests"), ignore)] @@ -118,8 +137,7 @@ async fn complete_deposit_validation_happy_path() { let signer_set = depossit_setup(&mut rng, &pg_store).await; // Get the chain tip. - let chain_tip = pg_store - .get_bitcoin_canonical_chain_tip_block() + let chain_tip = get_bitcoin_canonical_chain_tip_block(&pg_store) .await .unwrap() .unwrap(); @@ -180,8 +198,7 @@ async fn complete_deposit_validation_deployer_mismatch() { let signer_set = depossit_setup(&mut rng, &pg_store).await; // Get the chain tip - let chain_tip = pg_store - .get_bitcoin_canonical_chain_tip_block() + let chain_tip = get_bitcoin_canonical_chain_tip_block(&pg_store) .await .unwrap() .unwrap(); @@ -252,8 +269,7 @@ async fn complete_deposit_validation_missing_deposit_request() { // Normal: Get the chain tip and any pending deposit request in the blockchain // identified by the chain tip. - let chain_tip = pg_store - .get_bitcoin_canonical_chain_tip_block() + let chain_tip = get_bitcoin_canonical_chain_tip_block(&pg_store) .await .unwrap() .unwrap(); @@ -313,8 +329,7 @@ async fn complete_deposit_validation_recipient_mismatch() { let signer_set = depossit_setup(&mut rng, &pg_store).await; // Get the chain tip. - let chain_tip = pg_store - .get_bitcoin_canonical_chain_tip_block() + let chain_tip = get_bitcoin_canonical_chain_tip_block(&pg_store) .await .unwrap() .unwrap(); @@ -385,8 +400,7 @@ async fn complete_deposit_validation_invalid_mint_amount() { let signer_set = depossit_setup(&mut rng, &pg_store).await; // Get the chain tip. - let chain_tip = pg_store - .get_bitcoin_canonical_chain_tip_block() + let chain_tip = get_bitcoin_canonical_chain_tip_block(&pg_store) .await .unwrap() .unwrap(); @@ -456,8 +470,7 @@ async fn complete_deposit_validation_invalid_fee() { let signer_set = depossit_setup(&mut rng, &pg_store).await; // Get the chain tip. - let chain_tip = pg_store - .get_bitcoin_canonical_chain_tip_block() + let chain_tip = get_bitcoin_canonical_chain_tip_block(&pg_store) .await .unwrap() .unwrap(); @@ -527,8 +540,7 @@ async fn complete_deposit_validation_sweep_tx_missing() { let signer_set = depossit_setup(&mut rng, &pg_store).await; // Get the chain tip. - let chain_tip = pg_store - .get_bitcoin_canonical_chain_tip_block() + let chain_tip = get_bitcoin_canonical_chain_tip_block(&pg_store) .await .unwrap() .unwrap(); @@ -586,8 +598,7 @@ async fn complete_deposit_validation_sweep_reorged() { let signer_set = depossit_setup(&mut rng, &pg_store).await; // Get the chain tip. - let chain_tip = pg_store - .get_bitcoin_canonical_chain_tip_block() + let chain_tip = get_bitcoin_canonical_chain_tip_block(&pg_store) .await .unwrap() .unwrap(); @@ -676,8 +687,7 @@ async fn complete_deposit_validation_deposit_not_in_sweep() { let signer_set = depossit_setup(&mut rng, &pg_store).await; // Get the chain tip. - let chain_tip = pg_store - .get_bitcoin_canonical_chain_tip_block() + let chain_tip = get_bitcoin_canonical_chain_tip_block(&pg_store) .await .unwrap() .unwrap(); diff --git a/signer/tests/integration/main.rs b/signer/tests/integration/main.rs index c239ae15..60d1f9f4 100644 --- a/signer/tests/integration/main.rs +++ b/signer/tests/integration/main.rs @@ -1,10 +1,10 @@ use std::sync::atomic::AtomicU16; +mod complete_deposit; mod contracts; mod postgres; mod rbf; mod transaction_signer; -mod complete_deposit; mod utxo_construction; mod zmq; From 5d462c0ec9fab70fd3e4e95faab0e293d0afdb38 Mon Sep 17 00:00:00 2001 From: djordon Date: Wed, 18 Sep 2024 11:52:45 -0400 Subject: [PATCH 07/11] minor clean-ups --- signer/src/stacks/contracts.rs | 6 +----- signer/src/testing/dummy.rs | 4 +--- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/signer/src/stacks/contracts.rs b/signer/src/stacks/contracts.rs index cb375c88..36cfc2f4 100644 --- a/signer/src/stacks/contracts.rs +++ b/signer/src/stacks/contracts.rs @@ -336,10 +336,6 @@ impl CompleteDepositV1 { // 5. Check that the recipients in the transaction matches that of // the deposit request. if &self.recipient != deposit_request.recipient.deref() { - // Whoa, how is the outpoint right but the recipient is wrong?. - // TODO: serialize this struct and the deposit request to a - // JSON and put it in the logs. Or store. - tracing::warn!("Recipient in transaction does not match deposit request"); return Err(DepositValidationMsg::RecipientMismatch.into_error(ctx, self)); } // 6. Check that the amount to mint does not exceed the deposit @@ -424,7 +420,7 @@ pub struct DepositValidationError { impl std::fmt::Display for DepositValidationError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - // TODO: Add the other variables to the error message. + // TODO(191): Add the other variables to the error message. self.error.fmt(f) } } diff --git a/signer/src/testing/dummy.rs b/signer/src/testing/dummy.rs index ee479add..d0fd4e75 100644 --- a/signer/src/testing/dummy.rs +++ b/signer/src/testing/dummy.rs @@ -517,9 +517,7 @@ impl fake::Dummy for model::Transaction { let mut tx = Vec::new(); let bitcoin_tx: BitcoinTx = config.fake_with_rng(rng); - bitcoin_tx - .consensus_encode(&mut tx) - .expect("In-memory writers never fail"); + bitcoin_tx.consensus_encode(&mut tx).unwrap(); model::Transaction { tx, From 3f4e6c47e044153a609684a9406095ae0428acbb Mon Sep 17 00:00:00 2001 From: djordon Date: Wed, 18 Sep 2024 11:58:38 -0400 Subject: [PATCH 08/11] Update comments, fix typos --- signer/src/stacks/contracts.rs | 20 +++--- signer/tests/integration/complete_deposit.rs | 64 ++++++++++---------- 2 files changed, 41 insertions(+), 43 deletions(-) diff --git a/signer/src/stacks/contracts.rs b/signer/src/stacks/contracts.rs index 36cfc2f4..0c180871 100644 --- a/signer/src/stacks/contracts.rs +++ b/signer/src/stacks/contracts.rs @@ -51,15 +51,14 @@ use crate::storage::DbRead; /// This struct is used as supplemental data to help validate a request to /// sign a contract call transaction. /// -/// With the exception of the origin, this data is not fetched from the -/// signer that sent the request, but is instead internal to the current -/// signer. +/// Except for the origin, this data is not fetched from the signer that +/// sent the request, but is instead internal to the current signer. #[derive(Debug, Clone, Copy)] pub struct ReqContext { - /// This signers current view of the chain tip of the canonical bitcoin - /// blockchain. It is the block hash and height of the block on the - /// bitcoin blockchain with the greatest height. On ties, we sort by - /// the block hash descending and take the first one. + /// This signer's current view of the chain tip of the canonical + /// bitcoin blockchain. It is the block hash and height of the block on + /// the bitcoin blockchain with the greatest height. On ties, we sort + /// by the block hash descending and take the first one. pub chain_tip: BitcoinBlockRef, /// How many bitcoin blocks back from the chain tip the signer will /// look for requests. @@ -293,8 +292,8 @@ impl AsContractCall for CompleteDepositV1 { } impl CompleteDepositV1 { - /// Validate the the variables in this transaction match the input in - /// the deposit request. + /// Validate the variables in this transaction match the input in the + /// deposit request. /// /// Specifically, this function checks: /// 1. That the smart contract deployer matches the deployer in our @@ -403,8 +402,7 @@ impl CompleteDepositV1 { } } -/// A struct for a validation error containing all of the necessary -/// context. +/// A struct for a validation error containing all the necessary context. #[derive(Debug)] pub struct DepositValidationError { /// The specific error that happened during validation. diff --git a/signer/tests/integration/complete_deposit.rs b/signer/tests/integration/complete_deposit.rs index fd3c013c..4912e083 100644 --- a/signer/tests/integration/complete_deposit.rs +++ b/signer/tests/integration/complete_deposit.rs @@ -80,12 +80,12 @@ fn make_complete_deposit( /// Generate a signer set, deposit requests and store them into the /// database. -async fn depossit_setup(rng: &mut R, pg_store: &PgStore) -> Vec +async fn deposit_setup(rng: &mut R, pg_store: &PgStore) -> Vec where R: rand::RngCore + rand::CryptoRng, { // This is just a sql test, where we use the `TestData` struct to help - // populate the database with test data. We set all of the other + // populate the database with test data. We set all the other // unnecessary parameters to zero. let num_signers = 7; let test_model_params = testing::storage::model::Params { @@ -107,8 +107,8 @@ where /// Get the full block async fn get_bitcoin_canonical_chain_tip_block( store: &PgStore, -) -> Result, Error> { - sqlx::query_as::<_, model::BitcoinBlock>( +) -> Result, Error> { + sqlx::query_as::<_, BitcoinBlock>( "SELECT block_hash , block_height @@ -131,11 +131,11 @@ async fn complete_deposit_validation_happy_path() { // Normal: this generates the blockchain as well as deposit request // transactions in each bitcoin block. let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let pg_store = signer::testing::storage::new_test_database(db_num, true).await; + let pg_store = testing::storage::new_test_database(db_num, true).await; let mut rng = rand::rngs::StdRng::seed_from_u64(51); let signatures_required = 3; - let signer_set = depossit_setup(&mut rng, &pg_store).await; + let signer_set = deposit_setup(&mut rng, &pg_store).await; // Get the chain tip. let chain_tip = get_bitcoin_canonical_chain_tip_block(&pg_store) .await @@ -179,7 +179,7 @@ async fn complete_deposit_validation_happy_path() { // This should not return an Err. complete_deposit_tx.validate(&pg_store, &ctx).await.unwrap(); - signer::testing::storage::drop_db(pg_store).await; + testing::storage::drop_db(pg_store).await; } /// For this test we check that the `CompleteDepositV1::validate` function @@ -191,11 +191,11 @@ async fn complete_deposit_validation_deployer_mismatch() { // Normal: this generates the blockchain as well as deposit request // transactions in each bitcoin block. let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let pg_store = signer::testing::storage::new_test_database(db_num, true).await; + let pg_store = testing::storage::new_test_database(db_num, true).await; let mut rng = rand::rngs::StdRng::seed_from_u64(51); let signatures_required = 3; - let signer_set = depossit_setup(&mut rng, &pg_store).await; + let signer_set = deposit_setup(&mut rng, &pg_store).await; // Get the chain tip let chain_tip = get_bitcoin_canonical_chain_tip_block(&pg_store) @@ -249,7 +249,7 @@ async fn complete_deposit_validation_deployer_mismatch() { err => panic!("unexpected error during validation {err}"), } - signer::testing::storage::drop_db(pg_store).await; + testing::storage::drop_db(pg_store).await; } /// For this test we check that the `CompleteDepositV1::validate` function @@ -262,10 +262,10 @@ async fn complete_deposit_validation_missing_deposit_request() { // Normal: this generates the blockchain as well as deposit request // transactions in each bitcoin block. let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let pg_store = signer::testing::storage::new_test_database(db_num, true).await; + let pg_store = testing::storage::new_test_database(db_num, true).await; let mut rng = rand::rngs::StdRng::seed_from_u64(51); - let signer_set = depossit_setup(&mut rng, &pg_store).await; + let signer_set = deposit_setup(&mut rng, &pg_store).await; // Normal: Get the chain tip and any pending deposit request in the blockchain // identified by the chain tip. @@ -309,7 +309,7 @@ async fn complete_deposit_validation_missing_deposit_request() { err => panic!("unexpected error during validation {err}"), } - signer::testing::storage::drop_db(pg_store).await; + testing::storage::drop_db(pg_store).await; } /// For this test we check that the `CompleteDepositV1::validate` function @@ -322,11 +322,11 @@ async fn complete_deposit_validation_recipient_mismatch() { // Normal: this generates the blockchain as well as deposit request // transactions in each bitcoin block. let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let pg_store = signer::testing::storage::new_test_database(db_num, true).await; + let pg_store = testing::storage::new_test_database(db_num, true).await; let mut rng = rand::rngs::StdRng::seed_from_u64(51); let signatures_required = 3; - let signer_set = depossit_setup(&mut rng, &pg_store).await; + let signer_set = deposit_setup(&mut rng, &pg_store).await; // Get the chain tip. let chain_tip = get_bitcoin_canonical_chain_tip_block(&pg_store) @@ -380,7 +380,7 @@ async fn complete_deposit_validation_recipient_mismatch() { err => panic!("unexpected error during validation {err}"), } - signer::testing::storage::drop_db(pg_store).await; + testing::storage::drop_db(pg_store).await; } /// For this test we check that the `CompleteDepositV1::validate` function @@ -393,11 +393,11 @@ async fn complete_deposit_validation_invalid_mint_amount() { // Normal: this generates the blockchain as well as deposit request // transactions in each bitcoin block. let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let pg_store = signer::testing::storage::new_test_database(db_num, true).await; + let pg_store = testing::storage::new_test_database(db_num, true).await; let mut rng = rand::rngs::StdRng::seed_from_u64(51); let signatures_required = 3; - let signer_set = depossit_setup(&mut rng, &pg_store).await; + let signer_set = deposit_setup(&mut rng, &pg_store).await; // Get the chain tip. let chain_tip = get_bitcoin_canonical_chain_tip_block(&pg_store) @@ -450,7 +450,7 @@ async fn complete_deposit_validation_invalid_mint_amount() { err => panic!("unexpected error during validation {err}"), } - signer::testing::storage::drop_db(pg_store).await; + testing::storage::drop_db(pg_store).await; } /// For this test we check that the `CompleteDepositV1::validate` function @@ -463,11 +463,11 @@ async fn complete_deposit_validation_invalid_fee() { // Normal: this generates the blockchain as well as deposit request // transactions in each bitcoin block. let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let pg_store = signer::testing::storage::new_test_database(db_num, true).await; + let pg_store = testing::storage::new_test_database(db_num, true).await; let mut rng = rand::rngs::StdRng::seed_from_u64(51); let signatures_required = 3; - let signer_set = depossit_setup(&mut rng, &pg_store).await; + let signer_set = deposit_setup(&mut rng, &pg_store).await; // Get the chain tip. let chain_tip = get_bitcoin_canonical_chain_tip_block(&pg_store) @@ -520,7 +520,7 @@ async fn complete_deposit_validation_invalid_fee() { err => panic!("unexpected error during validation {err}"), } - signer::testing::storage::drop_db(pg_store).await; + testing::storage::drop_db(pg_store).await; } /// For this test we check that the `CompleteDepositV1::validate` function @@ -533,11 +533,11 @@ async fn complete_deposit_validation_sweep_tx_missing() { // Normal: this generates the blockchain as well as deposit request // transactions in each bitcoin block. let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let pg_store = signer::testing::storage::new_test_database(db_num, true).await; + let pg_store = testing::storage::new_test_database(db_num, true).await; let mut rng = rand::rngs::StdRng::seed_from_u64(51); let signatures_required = 3; - let signer_set = depossit_setup(&mut rng, &pg_store).await; + let signer_set = deposit_setup(&mut rng, &pg_store).await; // Get the chain tip. let chain_tip = get_bitcoin_canonical_chain_tip_block(&pg_store) @@ -579,7 +579,7 @@ async fn complete_deposit_validation_sweep_tx_missing() { err => panic!("unexpected error during validation {err}"), } - signer::testing::storage::drop_db(pg_store).await; + testing::storage::drop_db(pg_store).await; } /// For this test we check that the `CompleteDepositV1::validate` function @@ -592,11 +592,11 @@ async fn complete_deposit_validation_sweep_reorged() { // Normal: this generates the blockchain as well as deposit request // transactions in each bitcoin block. let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let pg_store = signer::testing::storage::new_test_database(db_num, true).await; + let pg_store = testing::storage::new_test_database(db_num, true).await; let mut rng = rand::rngs::StdRng::seed_from_u64(51); let signatures_required = 3; - let signer_set = depossit_setup(&mut rng, &pg_store).await; + let signer_set = deposit_setup(&mut rng, &pg_store).await; // Get the chain tip. let chain_tip = get_bitcoin_canonical_chain_tip_block(&pg_store) .await @@ -623,7 +623,7 @@ async fn complete_deposit_validation_sweep_reorged() { // confirmed, but on a bitcoin blockchain that is not the canonical // one. We generate a new blockchain and put it there. // - // Note that this blockchain might actually be have a greater height, + // Note that this blockchain might actually have a greater height, // but we get to say which one is the canonical one in our context so // that fact doesn't matter in this test. let test_model_params = testing::storage::model::Params { @@ -666,7 +666,7 @@ async fn complete_deposit_validation_sweep_reorged() { err => panic!("unexpected error during validation {err}"), } - signer::testing::storage::drop_db(pg_store).await; + testing::storage::drop_db(pg_store).await; } /// For this test we check that the `CompleteDepositV1::validate` function @@ -680,11 +680,11 @@ async fn complete_deposit_validation_deposit_not_in_sweep() { // Normal: this generates the blockchain as well as deposit request // transactions in each bitcoin block. let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let pg_store = signer::testing::storage::new_test_database(db_num, true).await; + let pg_store = testing::storage::new_test_database(db_num, true).await; let mut rng = rand::rngs::StdRng::seed_from_u64(51); let signatures_required = 3; - let signer_set = depossit_setup(&mut rng, &pg_store).await; + let signer_set = deposit_setup(&mut rng, &pg_store).await; // Get the chain tip. let chain_tip = get_bitcoin_canonical_chain_tip_block(&pg_store) @@ -738,5 +738,5 @@ async fn complete_deposit_validation_deposit_not_in_sweep() { err => panic!("unexpected error during validation {err}"), } - signer::testing::storage::drop_db(pg_store).await; + testing::storage::drop_db(pg_store).await; } From 1f1197c18d36aa0c8a7ab61470d5d9dad19b675a Mon Sep 17 00:00:00 2001 From: djordon Date: Wed, 18 Sep 2024 12:01:53 -0400 Subject: [PATCH 09/11] Fix more typos --- signer/tests/integration/postgres.rs | 68 ++++++++++++++-------------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/signer/tests/integration/postgres.rs b/signer/tests/integration/postgres.rs index d009eff9..c4ffeb67 100644 --- a/signer/tests/integration/postgres.rs +++ b/signer/tests/integration/postgres.rs @@ -53,7 +53,7 @@ use crate::DATABASE_NUM; #[tokio::test] async fn should_be_able_to_query_bitcoin_blocks() { let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let mut store = signer::testing::storage::new_test_database(db_num, true).await; + let mut store = testing::storage::new_test_database(db_num, true).await; let mut rng = rand::rngs::StdRng::seed_from_u64(42); let test_model_params = testing::storage::model::Params { @@ -159,7 +159,7 @@ impl AsContractCall for InitiateWithdrawalRequest { #[tokio::test] async fn writing_stacks_blocks_works(contract: ContractCallWrapper) { let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let store = signer::testing::storage::new_test_database(db_num, true).await; + let store = testing::storage::new_test_database(db_num, true).await; let path = "tests/fixtures/tenure-blocks-0-e5fdeb1a51ba6eb297797a1c473e715c27dc81a58ba82c698f6a32eeccee9a5b.bin"; let mut file = std::fs::File::open(path).unwrap(); @@ -254,7 +254,7 @@ async fn writing_stacks_blocks_works(contract: ContractCallWr #[tokio::test] async fn checking_stacks_blocks_exists_works() { let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let store = signer::testing::storage::new_test_database(db_num, true).await; + let store = testing::storage::new_test_database(db_num, true).await; let path = "tests/fixtures/tenure-blocks-0-e5fdeb1a51ba6eb297797a1c473e715c27dc81a58ba82c698f6a32eeccee9a5b.bin"; let mut file = std::fs::File::open(path).unwrap(); @@ -297,7 +297,7 @@ async fn checking_stacks_blocks_exists_works() { #[tokio::test] async fn should_return_the_same_pending_deposit_requests_as_in_memory_store() { let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let mut pg_store = signer::testing::storage::new_test_database(db_num, true).await; + let mut pg_store = testing::storage::new_test_database(db_num, true).await; let mut in_memory_store = storage::in_memory::Store::new_shared(); let mut rng = rand::rngs::StdRng::seed_from_u64(42); @@ -332,12 +332,12 @@ async fn should_return_the_same_pending_deposit_requests_as_in_memory_store() { chain_tip ); - let mut pending_depoist_requests = in_memory_store + let mut pending_deposit_requests = in_memory_store .get_pending_deposit_requests(&chain_tip, context_window) .await .expect("failed to get pending deposit requests"); - pending_depoist_requests.sort(); + pending_deposit_requests.sort(); let mut pg_pending_deposit_requests = pg_store .get_pending_deposit_requests(&chain_tip, context_window) @@ -346,7 +346,7 @@ async fn should_return_the_same_pending_deposit_requests_as_in_memory_store() { pg_pending_deposit_requests.sort(); - assert_eq!(pending_depoist_requests, pg_pending_deposit_requests); + assert_eq!(pending_deposit_requests, pg_pending_deposit_requests); signer::testing::storage::drop_db(pg_store).await; } @@ -356,7 +356,7 @@ async fn should_return_the_same_pending_deposit_requests_as_in_memory_store() { #[tokio::test] async fn should_return_the_same_pending_withdraw_requests_as_in_memory_store() { let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let mut pg_store = signer::testing::storage::new_test_database(db_num, true).await; + let mut pg_store = testing::storage::new_test_database(db_num, true).await; let mut in_memory_store = storage::in_memory::Store::new_shared(); let mut rng = rand::rngs::StdRng::seed_from_u64(42); @@ -416,7 +416,7 @@ async fn should_return_the_same_pending_withdraw_requests_as_in_memory_store() { #[tokio::test] async fn should_return_the_same_pending_accepted_deposit_requests_as_in_memory_store() { let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let mut pg_store = signer::testing::storage::new_test_database(db_num, true).await; + let mut pg_store = testing::storage::new_test_database(db_num, true).await; let mut in_memory_store = storage::in_memory::Store::new_shared(); let mut rng = rand::rngs::StdRng::seed_from_u64(42); @@ -482,7 +482,7 @@ async fn should_return_the_same_pending_accepted_deposit_requests_as_in_memory_s #[tokio::test] async fn should_return_the_same_pending_accepted_withdraw_requests_as_in_memory_store() { let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let mut pg_store = signer::testing::storage::new_test_database(db_num, true).await; + let mut pg_store = testing::storage::new_test_database(db_num, true).await; let mut in_memory_store = storage::in_memory::Store::new_shared(); let mut rng = rand::rngs::StdRng::seed_from_u64(42); @@ -552,7 +552,7 @@ async fn should_return_the_same_pending_accepted_withdraw_requests_as_in_memory_ #[tokio::test] async fn should_return_the_same_last_key_rotation_as_in_memory_store() { let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let mut pg_store = signer::testing::storage::new_test_database(db_num, true).await; + let mut pg_store = testing::storage::new_test_database(db_num, true).await; let mut in_memory_store = storage::in_memory::Store::new_shared(); let mut rng = rand::rngs::StdRng::seed_from_u64(42); @@ -625,7 +625,7 @@ async fn should_return_the_same_last_key_rotation_as_in_memory_store() { #[tokio::test] async fn writing_deposit_requests_postgres() { let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let store = signer::testing::storage::new_test_database(db_num, true).await; + let store = testing::storage::new_test_database(db_num, true).await; let num_rows = 15; let mut rng = rand::rngs::StdRng::seed_from_u64(51); let deposit_requests: Vec = @@ -670,7 +670,7 @@ async fn writing_deposit_requests_postgres() { #[tokio::test] async fn writing_transactions_postgres() { let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let store = signer::testing::storage::new_test_database(db_num, true).await; + let store = testing::storage::new_test_database(db_num, true).await; let num_rows = 12; let mut rng = rand::rngs::StdRng::seed_from_u64(51); let mut txs: Vec = @@ -693,7 +693,7 @@ async fn writing_transactions_postgres() { }; // We start by writing the bitcoin block because of the foreign key - // constrait + // constraint store.write_bitcoin_block(&db_block).await.unwrap(); // Let's see if we can write these transactions to the database. @@ -741,7 +741,7 @@ async fn writing_transactions_postgres() { #[tokio::test] async fn writing_completed_deposit_requests_postgres() { let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let store = signer::testing::storage::new_test_database(db_num, true).await; + let store = testing::storage::new_test_database(db_num, true).await; let mut rng = rand::rngs::StdRng::seed_from_u64(51); let event: CompletedDepositEvent = fake::Faker.fake_with_rng(&mut rng); @@ -779,7 +779,7 @@ async fn writing_completed_deposit_requests_postgres() { #[tokio::test] async fn writing_withdrawal_create_requests_postgres() { let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let store = signer::testing::storage::new_test_database(db_num, true).await; + let store = testing::storage::new_test_database(db_num, true).await; let mut rng = rand::rngs::StdRng::seed_from_u64(51); let event: WithdrawalCreateEvent = fake::Faker.fake_with_rng(&mut rng); @@ -825,7 +825,7 @@ async fn writing_withdrawal_create_requests_postgres() { #[tokio::test] async fn writing_withdrawal_accept_requests_postgres() { let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let store = signer::testing::storage::new_test_database(db_num, true).await; + let store = testing::storage::new_test_database(db_num, true).await; let mut rng = rand::rngs::StdRng::seed_from_u64(51); let event: WithdrawalAcceptEvent = fake::Faker.fake_with_rng(&mut rng); @@ -868,7 +868,7 @@ async fn writing_withdrawal_accept_requests_postgres() { #[tokio::test] async fn writing_withdrawal_reject_requests_postgres() { let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let store = signer::testing::storage::new_test_database(db_num, true).await; + let store = testing::storage::new_test_database(db_num, true).await; let mut rng = rand::rngs::StdRng::seed_from_u64(51); let event: WithdrawalRejectEvent = fake::Faker.fake_with_rng(&mut rng); @@ -907,11 +907,11 @@ async fn writing_withdrawal_reject_requests_postgres() { #[cfg_attr(not(feature = "integration-tests"), ignore)] #[tokio::test] async fn fetching_deposit_request_votes() { - // So we have 7 signers but we wiill only receive votes from 4 of them. + // So we have 7 signers, but we will only receive votes from 4 of them. // Three of the votes will be to accept and one explicit reject. The // others will be counted as rejections in the query. let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let store = signer::testing::storage::new_test_database(db_num, true).await; + let store = testing::storage::new_test_database(db_num, true).await; let mut rng = rand::rngs::StdRng::seed_from_u64(51); let signer_set_config = SignerSetConfig { @@ -920,7 +920,7 @@ async fn fetching_deposit_request_votes() { }; let rotate_keys: RotateKeysTransaction = signer_set_config.fake_with_rng(&mut rng); // Before we can write the rotate keys into the postgres database, we - // need to have a transaction in the trasnactions table. + // need to have a transaction in the transactions table. let transaction = model::Transaction { txid: rotate_keys.txid.into_bytes(), tx: Vec::new(), @@ -992,7 +992,7 @@ async fn fetching_deposit_request_votes() { .collect(); // Let's make sure that the votes are what we expected. For the votes - // that we've recieved, they should match exactly. + // that we've received, they should match exactly. for decision in signer_decisions.into_iter() { let actual_vote = actual_signer_vote_map .remove(&decision.signer_pub_key) @@ -1000,7 +1000,7 @@ async fn fetching_deposit_request_votes() { assert_eq!(actual_vote, Some(decision.is_accepted)); } - // The remianing keys, the ones were we have not received a vote, + // The remaining keys, the ones were we have not received a vote, // should be all None. assert!(actual_signer_vote_map.values().all(Option::is_none)); @@ -1015,11 +1015,11 @@ async fn fetching_deposit_request_votes() { #[cfg_attr(not(feature = "integration-tests"), ignore)] #[tokio::test] async fn fetching_withdrawal_request_votes() { - // So we have 7 signers but we wiill only receive votes from 4 of them. + // So we have 7 signers, but we will only receive votes from 4 of them. // Three of the votes will be to accept and one explicit reject. The // others will be counted as rejections in the query. let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let store = signer::testing::storage::new_test_database(db_num, true).await; + let store = testing::storage::new_test_database(db_num, true).await; let mut rng = rand::rngs::StdRng::seed_from_u64(51); let signer_set_config = SignerSetConfig { @@ -1028,7 +1028,7 @@ async fn fetching_withdrawal_request_votes() { }; let rotate_keys: RotateKeysTransaction = signer_set_config.fake_with_rng(&mut rng); // Before we can write the rotate keys into the postgres database, we - // need to have a transaction in the trasnactions table. + // need to have a transaction in the transactions table. let transaction = model::Transaction { txid: rotate_keys.txid.into_bytes(), tx: Vec::new(), @@ -1115,7 +1115,7 @@ async fn fetching_withdrawal_request_votes() { .collect(); // Let's make sure that the votes are what we expected. For the votes - // that we've recieved, they should match exactly. + // that we've received, they should match exactly. for decision in signer_decisions.into_iter() { let actual_vote = actual_signer_vote_map .remove(&decision.signer_pub_key) @@ -1123,7 +1123,7 @@ async fn fetching_withdrawal_request_votes() { assert_eq!(actual_vote, Some(decision.is_accepted)); } - // The remianing keys, the ones were we have not received a vote, + // The remaining keys, the ones were we have not received a vote, // should be all None. assert!(actual_signer_vote_map.values().all(Option::is_none)); @@ -1137,11 +1137,11 @@ async fn fetching_withdrawal_request_votes() { #[tokio::test] async fn block_in_canonical_bitcoin_blockchain_in_other_block_chain() { let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let pg_store = signer::testing::storage::new_test_database(db_num, true).await; + let pg_store = testing::storage::new_test_database(db_num, true).await; let mut rng = rand::rngs::StdRng::seed_from_u64(51); // This is just a sql test, where we use the `TestData` struct to help - // populate the database with test data. We set all of the other + // populate the database with test data. We set all the other // unnecessary parameters to zero. let num_signers = 0; let test_model_params = testing::storage::model::Params { @@ -1212,11 +1212,11 @@ async fn block_in_canonical_bitcoin_blockchain_in_other_block_chain() { #[tokio::test] async fn we_can_fetch_bitcoin_txs_from_db() { let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let pg_store = signer::testing::storage::new_test_database(db_num, true).await; + let pg_store = testing::storage::new_test_database(db_num, true).await; let mut rng = rand::rngs::StdRng::seed_from_u64(51); // This is just a sql test, where we use the `TestData` struct to help - // populate the database with test data. We set all of the other + // populate the database with test data. We set all the other // unnecessary parameters to zero. let num_signers = 0; let test_model_params = testing::storage::model::Params { @@ -1233,7 +1233,7 @@ async fn we_can_fetch_bitcoin_txs_from_db() { let tx = test_data.bitcoin_transactions.choose(&mut rng).unwrap(); - // Now let's try fecthing this transaction + // Now let's try fetching this transaction let btc_tx = pg_store .get_bitcoin_tx(&tx.txid, &tx.block_hash) .await @@ -1242,7 +1242,7 @@ async fn we_can_fetch_bitcoin_txs_from_db() { assert_eq!(btc_tx.compute_txid(), tx.txid.into()); - // Now let's try fecthing this transaction when we know it is missing. + // Now let's try fetching this transaction when we know it is missing. let txid: BitcoinTxId = fake::Faker.fake_with_rng(&mut rng); let block_hash: BitcoinBlockHash = fake::Faker.fake_with_rng(&mut rng); // Actual block but missing txid From ae8f00b3a7223ee52e6c95d07f8136b96ee4b751 Mon Sep 17 00:00:00 2001 From: djordon Date: Wed, 18 Sep 2024 16:08:41 -0400 Subject: [PATCH 10/11] cargo fmt --- signer/src/storage/postgres.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/signer/src/storage/postgres.rs b/signer/src/storage/postgres.rs index 4dffe478..7a73aafb 100644 --- a/signer/src/storage/postgres.rs +++ b/signer/src/storage/postgres.rs @@ -1304,10 +1304,7 @@ impl super::DbWrite for PgStore { Ok(()) } - async fn write_bitcoin_transaction( - &self, - tx_ref: &model::BitcoinTxRef, - ) -> Result<(), Error> { + async fn write_bitcoin_transaction(&self, tx_ref: &model::BitcoinTxRef) -> Result<(), Error> { sqlx::query( "INSERT INTO sbtc_signer.bitcoin_transactions (txid, block_hash) VALUES ($1, $2) From ae6ed9e91fc14ccfe16458abc6f4fab95cf3a19c Mon Sep 17 00:00:00 2001 From: djordon Date: Thu, 19 Sep 2024 16:19:15 -0400 Subject: [PATCH 11/11] Fix typo, and simplify error message variant names --- signer/src/stacks/contracts.rs | 45 ++++++++++---------- signer/tests/integration/complete_deposit.rs | 18 ++++---- 2 files changed, 31 insertions(+), 32 deletions(-) diff --git a/signer/src/stacks/contracts.rs b/signer/src/stacks/contracts.rs index 5b2b294e..b2e116b2 100644 --- a/signer/src/stacks/contracts.rs +++ b/signer/src/stacks/contracts.rs @@ -269,7 +269,7 @@ impl AsContractCall for CompleteDepositV1 { /// 5. That the recipients in the transaction matches that of the /// deposit request. /// 6. That the amount to mint does not exceed the deposit amount. - /// 7. That the max-fee is less than the desired max-fee. + /// 7. That the fee is less than the desired max-fee. /// /// # Notes /// @@ -309,7 +309,7 @@ impl CompleteDepositV1 { // 1. That the smart contract deployer matches the deployer in our // context. if self.deployer != ctx.deployer { - return Err(DepositValidationMsg::DeployerMismatch.into_error(ctx, self)); + return Err(DepositErrorMsg::DeployerMismatch.into_error(ctx, self)); } // 2. Check that the signer has a record of the deposit request // from our list of pending and accepted deposit requests. @@ -327,24 +327,23 @@ impl CompleteDepositV1 { let deposit_request = deposit_requests .into_iter() .find(|req| req.outpoint() == self.outpoint) - .ok_or_else(|| DepositValidationMsg::DepositRequestMissing.into_error(ctx, self))?; + .ok_or_else(|| DepositErrorMsg::RequestMissing.into_error(ctx, self))?; // 5. Check that the recipients in the transaction matches that of // the deposit request. if &self.recipient != deposit_request.recipient.deref() { - return Err(DepositValidationMsg::RecipientMismatch.into_error(ctx, self)); + return Err(DepositErrorMsg::RecipientMismatch.into_error(ctx, self)); } // 6. Check that the amount to mint does not exceed the deposit // amount. if self.amount > deposit_request.amount { - return Err(DepositValidationMsg::InvalidMintAmount.into_error(ctx, self)); + return Err(DepositErrorMsg::InvalidMintAmount.into_error(ctx, self)); } - // 7. Check that the max-fee is less than the desired max-fee. + // 7. Check that the fee is less than the desired max-fee. // - // The smart contract cannot check if we exceed the max fee, so we - // do a check ourselves. + // The smart contract cannot check if we exceed the max fee. if deposit_request.amount - self.amount > deposit_request.max_fee { - return Err(DepositValidationMsg::InvalidFee.into_error(ctx, self)); + return Err(DepositErrorMsg::InvalidFee.into_error(ctx, self)); } Ok(()) @@ -365,7 +364,7 @@ impl CompleteDepositV1 { let sweep_tx = db .get_bitcoin_tx(&self.sweep_txid, &self.sweep_block_hash) .await? - .ok_or_else(|| DepositValidationMsg::SweepTransactionMissing.into_error(ctx, self))?; + .ok_or_else(|| DepositErrorMsg::SweepTransactionMissing.into_error(ctx, self))?; // 3. Check that the signer sweep transaction is on the canonical // bitcoin blockchain. // @@ -381,7 +380,7 @@ impl CompleteDepositV1 { .in_canonical_bitcoin_blockchain(&ctx.chain_tip, &block_ref) .await?; if !in_canonical_bitcoin_blockchain { - return Err(DepositValidationMsg::SweepTransactionReorged.into_error(ctx, self)); + return Err(DepositErrorMsg::SweepTransactionReorged.into_error(ctx, self)); } // 4. Check that the sweep transaction uses the indicated deposit // outpoint as an input. @@ -391,7 +390,7 @@ impl CompleteDepositV1 { // of the transaction inputs. let mut tx_inputs = sweep_tx.input.iter(); if !tx_inputs.any(|tx_in| tx_in.previous_output == self.outpoint) { - return Err(DepositValidationMsg::DepositMissingFromSweep.into_error(ctx, self)); + return Err(DepositErrorMsg::MissingFromSweep.into_error(ctx, self)); } Ok(()) @@ -402,7 +401,7 @@ impl CompleteDepositV1 { #[derive(Debug)] pub struct DepositValidationError { /// The specific error that happened during validation. - pub error: DepositValidationMsg, + pub error: DepositErrorMsg, /// The additional information that was used when trying to /// validate the complete-deposit contract call. This includes the /// public key of the signer that was attempting to generate the @@ -428,18 +427,10 @@ impl std::error::Error for DepositValidationError { /// The responses for validation of a complete-deposit smart contract call /// transactions. #[derive(Debug, thiserror::Error, PartialEq, Eq)] -pub enum DepositValidationMsg { +pub enum DepositErrorMsg { /// The smart contract deployer is fixed, so this should always match. #[error("The deployer in the transaction does not match the expected deployer")] DeployerMismatch, - /// The deposit outpoint is missing from the indicated sweep - /// transaction. - #[error("deposit outpoint is missing from the indicated sweep transaction")] - DepositMissingFromSweep, - /// We do not have a record of the deposit request in our list of - /// pending and accepted deposit requests. - #[error("no record of deposit request in pending and accepted deposit requests")] - DepositRequestMissing, /// The fee paid to the bitcoin miners exceeded the max fee. #[error("fee paid to the bitcoin miners exceeded the max fee")] InvalidFee, @@ -447,10 +438,18 @@ pub enum DepositValidationMsg { /// request. #[error("amount to mint exceeded the amount in the deposit request")] InvalidMintAmount, + /// The deposit outpoint is missing from the indicated sweep + /// transaction. + #[error("deposit outpoint is missing from the indicated sweep transaction")] + MissingFromSweep, /// The recipient did not match the recipient in our deposit request /// records. #[error("recipient did not match the recipient in our deposit request")] RecipientMismatch, + /// We do not have a record of the deposit request in our list of + /// pending and accepted deposit requests. + #[error("no record of deposit request in pending and accepted deposit requests")] + RequestMissing, /// The sweep transaction that included the deposit request is missing /// from our records. #[error("sweep transaction not found")] @@ -461,7 +460,7 @@ pub enum DepositValidationMsg { SweepTransactionReorged, } -impl DepositValidationMsg { +impl DepositErrorMsg { fn into_error(self, ctx: &ReqContext, tx: &CompleteDepositV1) -> Error { Error::DepositValidation(Box::new(DepositValidationError { error: self, diff --git a/signer/tests/integration/complete_deposit.rs b/signer/tests/integration/complete_deposit.rs index 4912e083..f89c927d 100644 --- a/signer/tests/integration/complete_deposit.rs +++ b/signer/tests/integration/complete_deposit.rs @@ -9,7 +9,7 @@ use signer::error::Error; use signer::keys::PublicKey; use signer::stacks::contracts::AsContractCall as _; use signer::stacks::contracts::CompleteDepositV1; -use signer::stacks::contracts::DepositValidationMsg; +use signer::stacks::contracts::DepositErrorMsg; use signer::stacks::contracts::ReqContext; use signer::storage::model; use signer::storage::model::BitcoinBlock; @@ -244,7 +244,7 @@ async fn complete_deposit_validation_deployer_mismatch() { let validate_future = complete_deposit_tx.validate(&pg_store, &ctx); match validate_future.await.unwrap_err() { Error::DepositValidation(ref err) => { - assert_eq!(err.error, DepositValidationMsg::DeployerMismatch) + assert_eq!(err.error, DepositErrorMsg::DeployerMismatch) } err => panic!("unexpected error during validation {err}"), } @@ -304,7 +304,7 @@ async fn complete_deposit_validation_missing_deposit_request() { let validation_result = complete_deposit_tx.validate(&pg_store, &ctx).await; match validation_result.unwrap_err() { Error::DepositValidation(ref err) => { - assert_eq!(err.error, DepositValidationMsg::DepositRequestMissing) + assert_eq!(err.error, DepositErrorMsg::RequestMissing) } err => panic!("unexpected error during validation {err}"), } @@ -375,7 +375,7 @@ async fn complete_deposit_validation_recipient_mismatch() { let validate_future = complete_deposit_tx.validate(&pg_store, &ctx); match validate_future.await.unwrap_err() { Error::DepositValidation(ref err) => { - assert_eq!(err.error, DepositValidationMsg::RecipientMismatch) + assert_eq!(err.error, DepositErrorMsg::RecipientMismatch) } err => panic!("unexpected error during validation {err}"), } @@ -445,7 +445,7 @@ async fn complete_deposit_validation_invalid_mint_amount() { let validate_future = complete_deposit_tx.validate(&pg_store, &ctx); match validate_future.await.unwrap_err() { Error::DepositValidation(ref err) => { - assert_eq!(err.error, DepositValidationMsg::InvalidMintAmount) + assert_eq!(err.error, DepositErrorMsg::InvalidMintAmount) } err => panic!("unexpected error during validation {err}"), } @@ -515,7 +515,7 @@ async fn complete_deposit_validation_invalid_fee() { let validate_future = complete_deposit_tx.validate(&pg_store, &ctx); match validate_future.await.unwrap_err() { Error::DepositValidation(ref err) => { - assert_eq!(err.error, DepositValidationMsg::InvalidFee) + assert_eq!(err.error, DepositErrorMsg::InvalidFee) } err => panic!("unexpected error during validation {err}"), } @@ -574,7 +574,7 @@ async fn complete_deposit_validation_sweep_tx_missing() { let validation_result = complete_deposit_tx.validate(&pg_store, &ctx).await; match validation_result.unwrap_err() { Error::DepositValidation(ref err) => { - assert_eq!(err.error, DepositValidationMsg::SweepTransactionMissing) + assert_eq!(err.error, DepositErrorMsg::SweepTransactionMissing) } err => panic!("unexpected error during validation {err}"), } @@ -661,7 +661,7 @@ async fn complete_deposit_validation_sweep_reorged() { let validation_result = complete_deposit_tx.validate(&pg_store, &ctx).await; match validation_result.unwrap_err() { Error::DepositValidation(ref err) => { - assert_eq!(err.error, DepositValidationMsg::SweepTransactionReorged) + assert_eq!(err.error, DepositErrorMsg::SweepTransactionReorged) } err => panic!("unexpected error during validation {err}"), } @@ -733,7 +733,7 @@ async fn complete_deposit_validation_deposit_not_in_sweep() { let validation_result = complete_deposit_tx.validate(&pg_store, &ctx).await; match validation_result.unwrap_err() { Error::DepositValidation(ref err) => { - assert_eq!(err.error, DepositValidationMsg::DepositMissingFromSweep) + assert_eq!(err.error, DepositErrorMsg::MissingFromSweep) } err => panic!("unexpected error during validation {err}"), }