diff --git a/signer/src/block_observer.rs b/signer/src/block_observer.rs index e1a02dabd..43c620e64 100644 --- a/signer/src/block_observer.rs +++ b/signer/src/block_observer.rs @@ -81,7 +81,7 @@ impl DepositRequestValidator for CreateDepositRequest { { // Fetch the transaction from either a block or from the mempool let Some(response) = client.get_tx(&self.outpoint.txid).await? else { - return Err(Error::BitcoinTxMissing(self.outpoint.txid)); + return Err(Error::BitcoinTxMissing(self.outpoint.txid, None)); }; Ok(Deposit { @@ -288,6 +288,10 @@ where let mut tx_bytes = Vec::new(); tx.consensus_encode(&mut tx_bytes)?; + // TODO: these aren't all sBTC transactions. Some of these + // could be "donations". One way to properly label these is + // to look at the scriptPubKey of the prevouts of the + // transaction. Ok::<_, bitcoin::io::Error>(model::Transaction { txid: tx.compute_txid().to_byte_array(), tx: tx_bytes, diff --git a/signer/src/error.rs b/signer/src/error.rs index 95bc045b9..233257f76 100644 --- a/signer/src/error.rs +++ b/signer/src/error.rs @@ -32,8 +32,8 @@ pub enum Error { /// The bitcoin tranaction was not found in the mempool or on the /// bitcoin blockchain. This is thrown when we expect the transaction /// to exist in bitcoin core but it does not. - #[error("Transaction is missing from mempool")] - BitcoinTxMissing(bitcoin::Txid), + #[error("transaction is missing, txid: {0}, block hash {1:?}")] + BitcoinTxMissing(bitcoin::Txid, Option), /// Received an error in call to estimatesmartfee RPC call #[error("failed to get fee estimate from bitcoin-core for target {1}. {0}")] @@ -203,6 +203,10 @@ pub enum Error { #[error("could not parse the provided URL: {0}")] InvalidUrl(#[source] url::ParseError), + /// This should never happen. + #[error("outpoint missing from transaction when assessing fee {0}")] + OutPointMissing(bitcoin::OutPoint), + /// This is thrown when failing to parse a hex string into an integer. #[error("could not parse the hex string into an integer")] ParseHexInt(#[source] std::num::ParseIntError), diff --git a/signer/src/keys.rs b/signer/src/keys.rs index f81fc4b11..edf065085 100644 --- a/signer/src/keys.rs +++ b/signer/src/keys.rs @@ -113,11 +113,10 @@ impl From<&PublicKey> for p256k1::point::Point { // [^2]: https://github.com/bitcoin-core/secp256k1/blob/v0.3.0/src/field.h#L78-L79 let x_element = p256k1::field::Element::from(x_part); let y_element = p256k1::field::Element::from(y_part); - // You would think that you couldn't always convert two elements - // into a Point, but `p256k1::point::Point::from` uses - // `secp256k1_gej_set_ge` under the hood, which I believe does any - // reduction. But still, we have a valid public key, so no - // reductions should be necessary. + // You cannot always convert two aribtrary elements into a Point, + // and `p256k1::point::Point::from` assumes that it is being given + // two elements that from a point in affine coordinates. We have a + // valid public key, so we know that this assumption is upheld. p256k1::point::Point::from((x_element, y_element)) } } diff --git a/signer/src/main.rs b/signer/src/main.rs index c8514ff21..8d3210939 100644 --- a/signer/src/main.rs +++ b/signer/src/main.rs @@ -277,7 +277,6 @@ async fn run_transaction_signer(ctx: impl Context) -> Result<(), Error> { context_window: 10000, threshold: 2, blocklist_checker: BlocklistClient::new(&ctx), - network_kind: config.signer.network.into(), rng: rand::thread_rng(), signer_private_key: config.signer.private_key, wsts_state_machines: HashMap::new(), @@ -299,7 +298,6 @@ async fn run_transaction_coordinator(ctx: impl Context) -> Result<(), Error> { private_key: config.signer.private_key, signing_round_max_duration: Duration::from_secs(10), threshold: 2, - bitcoin_network: config.signer.network.into(), }; coord.run().await diff --git a/signer/src/stacks/api.rs b/signer/src/stacks/api.rs index d258a324f..446b91cae 100644 --- a/signer/src/stacks/api.rs +++ b/signer/src/stacks/api.rs @@ -77,7 +77,7 @@ pub enum FeePriority { /// A trait detailing the interface with the Stacks API and Stacks Nodes. #[cfg_attr(any(test, feature = "testing"), mockall::automock)] -pub trait StacksInteract { +pub trait StacksInteract: Send + Sync { /// Retrieve the current signer set from the `sbtc-registry` contract. /// /// This is done by making a `GET /v2/data_var//sbtc-registry/current-signer-set` @@ -85,18 +85,18 @@ pub trait StacksInteract { fn get_current_signer_set( &self, contract_principal: &StacksAddress, - ) -> impl Future, Error>>; + ) -> impl Future, Error>> + Send; /// Get the latest account info for the given address. fn get_account( &self, address: &StacksAddress, - ) -> impl Future>; + ) -> impl Future> + Send; /// Submit a transaction to a Stacks node. fn submit_tx( &self, tx: &StacksTransaction, - ) -> impl Future>; + ) -> impl Future> + Send; /// Fetch the raw stacks nakamoto block from a Stacks node given the /// Stacks block ID. @@ -122,9 +122,9 @@ pub trait StacksInteract { /// This function is analogous to the GET /v3/tenures/info stacks node /// endpoint for retrieving tenure information. fn get_tenure_info(&self) -> impl Future> + Send; - /// Estimate the priority transaction fees for the input transaction - /// for the current state of the mempool. The result should be and - /// estimated total fee in microSTX. + /// Estimate the priority transaction fees given the input transaction + /// and the current state of the mempool. The result will be the + /// estimated total transaction fee in microSTX. /// /// This function usually uses the POST /v2/fees/transaction endpoint /// of a stacks node. diff --git a/signer/src/stacks/contracts.rs b/signer/src/stacks/contracts.rs index bb30abe28..01deb0b42 100644 --- a/signer/src/stacks/contracts.rs +++ b/signer/src/stacks/contracts.rs @@ -926,13 +926,14 @@ pub struct RotateKeysV1 { } impl RotateKeysV1 { - /// Create a new instance of RotateKeysV1 using the provided wallet. - pub fn new(wallet: &SignerWallet, deployer: StacksAddress, signatures_required: u16) -> Self { + /// Create a new instance of a RotateKeysV1 transaction object where + /// the new keys will match those provided by the input wallet. + pub fn new(wallet: &SignerWallet, deployer: StacksAddress) -> Self { Self { - aggregate_key: wallet.aggregate_key(), + aggregate_key: *wallet.stacks_aggregate_key(), new_keys: wallet.public_keys().clone(), deployer, - signatures_required, + signatures_required: wallet.signatures_required(), } } @@ -1091,7 +1092,7 @@ mod tests { let wallet = SignerWallet::new(&public_keys, 2, NetworkKind::Testnet, 0).unwrap(); let deployer = StacksAddress::burn_address(false); - let call = RotateKeysV1::new(&wallet, deployer, 2); + let call = RotateKeysV1::new(&wallet, deployer); // This is to check that this function doesn't implicitly panic. If // it doesn't panic now, it can never panic at runtime. diff --git a/signer/src/stacks/wallet.rs b/signer/src/stacks/wallet.rs index 1c0214559..c107c22ab 100644 --- a/signer/src/stacks/wallet.rs +++ b/signer/src/stacks/wallet.rs @@ -24,12 +24,15 @@ use secp256k1::ecdsa::RecoverableSignature; use secp256k1::Message; use crate::config::NetworkKind; +use crate::context::Context; use crate::error::Error; use crate::keys::PublicKey; use crate::signature::RecoverableEcdsaSignature as _; use crate::signature::SighashDigest as _; use crate::stacks::contracts::AsContractCall; use crate::stacks::contracts::AsTxPayload; +use crate::storage::model::BitcoinBlockHash; +use crate::storage::DbRead; use crate::MAX_KEYS; /// Stacks multisig addresses are Hash160 hashes of bitcoin Scripts (more @@ -59,9 +62,6 @@ pub struct SignerWallet { address: StacksAddress, /// The next nonce for the StacksAddress associated with the address of /// the wallet. - /// - /// TODO(510): remove the nonce, it should be set when the signer - /// creates each transaction. nonce: AtomicU64, } @@ -78,7 +78,7 @@ impl SignerWallet { /// 4. The number of public keys exceeds the MAX_KEYS constant. /// 5. The combined public key would be the point at infinity. /// - /// Error condition (5) occurs when PublicKey::combine_keys errors. + /// Error condition (5) occurs when [`PublicKey::combine_keys`] errors. /// There are two other conditions where that function errors, which /// are: /// @@ -93,10 +93,11 @@ impl SignerWallet { /// /// # Notes /// - /// Now there is always a small risk that the PublicKey::combine_keys - /// function will return a Result::Err, even with perfectly fine - /// inputs. This is highly unlikely by chance, but a Byzantine actor - /// could trigger it purposefully if we aren't careful. + /// Now there is always a small risk that [`PublicKey::combine_keys`] + /// will return a `Result::Err`, even with perfectly fine inputs. This + /// is highly unlikely by chance, but a Byzantine actor could trigger + /// it purposefully if we don't require a signer to prove that they + /// control the public key that they submit. pub fn new( public_keys: &[PublicKey], signatures_required: u16, @@ -145,13 +146,61 @@ impl SignerWallet { }) } + /// Load the multi-sig wallet from storage. + /// + /// The wallet that is loaded is the one that cooresponds to the signer + /// set defined in the last confirmed key rotation contract call. + pub async fn load(ctx: &C, chain_tip: &BitcoinBlockHash) -> Result + where + C: Context, + { + // Get the key rotation transaction from the database. This maps to + // what the stacks network thinks the signers' address is. + let last_key_rotation = ctx + .get_storage() + .get_last_key_rotation(chain_tip) + .await? + .ok_or(Error::MissingKeyRotation)?; + + let public_keys = last_key_rotation.signer_set.as_slice(); + let signatures_required = last_key_rotation.signatures_required; + let network_kind = ctx.config().signer.network; + + SignerWallet::new(public_keys, signatures_required, network_kind, 0) + } + fn hash_mode() -> OrderIndependentMultisigHashMode { MULTISIG_ADDRESS_HASH_MODE } /// Return the stacks address for the signers - pub fn address(&self) -> StacksAddress { - self.address + pub fn address(&self) -> &StacksAddress { + &self.address + } + + /// The aggregate public key of the given public keys. + /// + /// # Notes + /// + /// This aggregate is almost certainly different from the aggregate key + /// that is output after DKG. + /// + /// Once gets done + /// then we will always have a unification of the Stacks and bitcoin + /// aggregate keys. + pub fn stacks_aggregate_key(&self) -> &PublicKey { + &self.aggregate_key + } + + /// Returns the number of public keys in the multi-sig wallet. + pub fn num_signers(&self) -> u16 { + // We check that the number of keys is less than or equal to the + // MAX_KEYS variable when we created this struct, and MAX_KEYS is a + // u16. So this cast should always succeed. + self.public_keys + .len() + .try_into() + .expect("BUG! the number of keys is supposed to be less than u16::MAX") } /// Return the public keys for the signers' multi-sig wallet @@ -159,9 +208,9 @@ impl SignerWallet { &self.public_keys } - /// The aggregate public key of the given public keys. - pub fn aggregate_key(&self) -> PublicKey { - self.aggregate_key + /// Return the nonce that should be used with the next transaction + pub fn get_nonce(&self) -> u64 { + self.nonce.load(Ordering::SeqCst) } /// Set the next nonce to the provided value @@ -169,6 +218,12 @@ impl SignerWallet { self.nonce.store(value, Ordering::Relaxed) } + /// The number of participants required to construct a valid signature + /// for Stacks transactions. + pub fn signatures_required(&self) -> u16 { + self.signatures_required + } + /// Convert the signers wallet to an unsigned stacks spending /// conditions. /// @@ -309,8 +364,10 @@ impl MultisigTx { #[cfg(test)] mod tests { use blockstack_lib::clarity::vm::Value as ClarityValue; + use fake::Fake; use rand::rngs::OsRng; use rand::seq::SliceRandom; + use rand::SeedableRng as _; use secp256k1::Keypair; use secp256k1::SECP256K1; @@ -319,6 +376,13 @@ mod tests { use crate::context::Context; use crate::signature::sign_stacks_tx; use crate::stacks::contracts::ReqContext; + use crate::storage::model; + use crate::storage::model::RotateKeysTransaction; + use crate::storage::DbWrite; + use crate::testing::context::ConfigureMockedClients; + use crate::testing::context::TestContext; + use crate::testing::context::*; + use crate::testing::storage::model::TestData; use super::*; @@ -495,4 +559,86 @@ mod tests { assert_eq!(wallet1.address(), wallet2.address()) } + + /// Here we test that we can load a SignerWallet from storage. To do + /// that we: + /// 1. Generate and store random bitcoin and stacks blockchains. + /// 2. Create a random wallet. + /// 3. Generate a rotate-keys transaction object using the details of + /// the random wallet from (2). + /// 4. Attempt to load the wallet from storage. This should return + /// essentially the same wallet from (2). The only difference is + /// that the nonce in the loaded wallet is fetched from the + /// "stacks-node" (in this test it just returns a nonce of zero). + #[tokio::test] + async fn loading_signer_wallet_from_context() { + let mut rng = rand::rngs::StdRng::seed_from_u64(51); + + let ctx = TestContext::builder() + .with_in_memory_storage() + .with_mocked_clients() + .build(); + let db = ctx.get_storage_mut(); + + // Create a blockchain. We do not generate any withdrawal or + // deposit requests, so we do not need to specify the signer set. + let test_params = crate::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_data = TestData::generate(&mut rng, &[], &test_params); + test_data.write_to(&db).await; + + // Let's generate a the signers' wallet. + let signer_keys: Vec = + std::iter::repeat_with(|| Keypair::new_global(&mut OsRng)) + .map(|kp| kp.public_key().into()) + .take(50) + .collect(); + let signatures_required = 5; + let network = NetworkKind::Regtest; + let wallet1 = SignerWallet::new(&signer_keys, signatures_required, network, 0).unwrap(); + + // Let's store the key information about this wallet into the database + let rotate_keys = RotateKeysTransaction { + txid: fake::Faker.fake_with_rng(&mut rng), + aggregate_key: *wallet1.stacks_aggregate_key(), + signer_set: signer_keys.clone(), + signatures_required: wallet1.signatures_required, + }; + + let bitcoin_chain_tip = db.get_bitcoin_canonical_chain_tip().await.unwrap().unwrap(); + let stacks_chain_tip = db + .get_stacks_chain_tip(&bitcoin_chain_tip) + .await + .unwrap() + .unwrap(); + + // We haven't stored any RotateKeysTransactions into the database + // yet, so loading the wallet should fail. + assert!(SignerWallet::load(&ctx, &bitcoin_chain_tip).await.is_err()); + + let tx = model::StacksTransaction { + txid: rotate_keys.txid, + block_hash: stacks_chain_tip.block_hash, + }; + + db.write_stacks_transaction(&tx).await.unwrap(); + db.write_rotate_keys_transaction(&rotate_keys) + .await + .unwrap(); + + // Okay, now let's load it up and make sure things match. + let wallet2 = SignerWallet::load(&ctx, &bitcoin_chain_tip).await.unwrap(); + + assert_eq!(wallet1.address(), wallet2.address()); + assert_eq!(wallet1.public_keys(), wallet2.public_keys()); + assert_eq!( + wallet1.stacks_aggregate_key(), + wallet2.stacks_aggregate_key() + ); + } } diff --git a/signer/src/storage/in_memory.rs b/signer/src/storage/in_memory.rs index f61f8a3bc..fda71df8e 100644 --- a/signer/src/storage/in_memory.rs +++ b/signer/src/storage/in_memory.rs @@ -585,6 +585,22 @@ impl super::DbRead for SharedStore { Ok(maybe_tx) } + + async fn get_swept_deposit_requests( + &self, + _chain_tip: &model::BitcoinBlockHash, + _context_window: u16, + ) -> Result, Error> { + unimplemented!() + } + + async fn get_swept_withdrawal_requests( + &self, + _chain_tip: &model::BitcoinBlockHash, + _context_window: u16, + ) -> Result, Error> { + unimplemented!() + } } impl super::DbWrite for SharedStore { diff --git a/signer/src/storage/mod.rs b/signer/src/storage/mod.rs index b707afa13..2a65cb60c 100644 --- a/signer/src/storage/mod.rs +++ b/signer/src/storage/mod.rs @@ -193,6 +193,30 @@ pub trait DbRead { txid: &model::BitcoinTxId, block_hash: &model::BitcoinBlockHash, ) -> impl Future, Error>> + Send; + + /// Fetch bitcoin transactions that have fulfilled a deposit request + /// but where we have not confirmed a stacks transaction finalizing the + /// request. + /// + /// These requests only require a `complete-deposit` contract call + /// before they are considered fulfilled. + fn get_swept_deposit_requests( + &self, + chain_tip: &model::BitcoinBlockHash, + context_window: u16, + ) -> impl Future, Error>> + Send; + + /// Fetch bitcoin transactions that have fulfilled a withdrawal request + /// but where we have not confirmed a stacks transaction finalizing the + /// request. + /// + /// These requests only require a `accept-withdrawal-request` contract + /// call before they are considered fulfilled. + fn get_swept_withdrawal_requests( + &self, + chain_tip: &model::BitcoinBlockHash, + context_window: u16, + ) -> impl Future, Error>> + Send; } /// Represents the ability to write data to the signer storage. diff --git a/signer/src/storage/model.rs b/signer/src/storage/model.rs index a4764bb55..ff06f2e95 100644 --- a/signer/src/storage/model.rs +++ b/signer/src/storage/model.rs @@ -238,6 +238,77 @@ pub struct Transaction { pub block_hash: [u8; 32], } +/// A deposit request with a response bitcoin transaction that has been +/// confirmed. +#[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord, sqlx::FromRow)] +#[cfg_attr(feature = "testing", derive(fake::Dummy))] +pub struct SweptDepositRequest { + /// The transaction ID of the bitcoin transaction that swept in the + /// funds into the signers' UTXO. + pub sweep_txid: BitcoinTxId, + /// The transaction sweeping in the deposit request UTXO. + pub sweep_tx: BitcoinTx, + /// The block id of the bitcoin block that includes the sweep + /// transaction. + pub sweep_block_hash: BitcoinBlockHash, + /// The block height of the block referenced by the `sweep_block_hash`. + #[sqlx(try_from = "i64")] + pub sweep_block_height: u64, + /// Transaction ID of the deposit request transaction. + pub txid: BitcoinTxId, + /// Index of the deposit request UTXO. + #[cfg_attr(feature = "testing", dummy(faker = "0..100"))] + #[sqlx(try_from = "i32")] + pub output_index: u32, + /// The address of which the sBTC should be minted, + /// can be a smart contract address. + pub recipient: StacksPrincipal, + /// The amount in the deposit UTXO. + #[sqlx(try_from = "i64")] + #[cfg_attr(feature = "testing", dummy(faker = "1_000_000..1_000_000_000"))] + pub amount: u64, +} + +/// Withdraw request. +#[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord, sqlx::FromRow)] +#[cfg_attr(feature = "testing", derive(fake::Dummy))] +pub struct SweptWithdrawalRequest { + /// The transaction ID of the bitcoin transaction that swept out the + /// funds to the intended recipient. + pub sweep_txid: BitcoinTxId, + /// The bitcoin transaction fulfilling the withdrawal request. + pub sweep_tx: BitcoinTx, + /// The block id of the stacks block that includes this sweep + /// transaction. + pub sweep_block_hash: BitcoinBlockHash, + /// Request ID of the withdrawal request. These are supposed to be + /// unique, but there can be duplicates if there is a reorg that + /// affects a transaction that calls the `initiate-withdrawal-request` + /// public function. + #[sqlx(try_from = "i64")] + pub request_id: u64, + /// The stacks transaction ID that lead to the creation of the + /// withdrawal request. + pub txid: StacksTxId, + /// Stacks block ID of the block that includes the transaction + /// associated with this withdrawal request. + pub block_hash: StacksBlockHash, + /// The ScriptPubKey that should receive the BTC withdrawal. + pub recipient: ScriptPubKey, + /// The amount of satoshis to withdraw. + #[sqlx(try_from = "i64")] + #[cfg_attr(feature = "testing", dummy(faker = "100..1_000_000_000"))] + pub amount: u64, + /// The maximum amount that may be spent as for the bitcoin miner + /// transaction fee. + #[sqlx(try_from = "i64")] + #[cfg_attr(feature = "testing", dummy(faker = "100..10000"))] + pub max_fee: u64, + /// The stacks address that initiated the request. This is populated + /// using `tx-sender`. + pub sender_address: StacksPrincipal, +} + /// Persisted DKG shares #[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord, sqlx::FromRow)] #[cfg_attr(feature = "testing", derive(fake::Dummy))] @@ -520,6 +591,12 @@ impl From for StacksBlockHash { } } +impl From for StacksBlockId { + fn from(value: StacksBlockHash) -> Self { + value.0 + } +} + impl From<[u8; 32]> for StacksBlockHash { fn from(bytes: [u8; 32]) -> Self { Self(StacksBlockId(bytes)) @@ -544,6 +621,12 @@ impl From for StacksTxId { } } +impl From for blockstack_lib::burnchains::Txid { + fn from(value: StacksTxId) -> Self { + value.0 + } +} + impl From<[u8; 32]> for StacksTxId { fn from(bytes: [u8; 32]) -> Self { Self(blockstack_lib::burnchains::Txid(bytes)) diff --git a/signer/src/storage/postgres.rs b/signer/src/storage/postgres.rs index 83fa36266..99500bde5 100644 --- a/signer/src/storage/postgres.rs +++ b/signer/src/storage/postgres.rs @@ -1143,6 +1143,78 @@ impl super::DbRead for PgStore { .await .map_err(Error::SqlxQuery) } + + async fn get_swept_deposit_requests( + &self, + chain_tip: &model::BitcoinBlockHash, + context_window: u16, + ) -> Result, Error> { + // TODO: This query is definitely incorrect. Doing it correctly + // will be much easier once + // https://github.com/stacks-network/sbtc/issues/585 is completed. + sqlx::query_as::<_, model::SweptDepositRequest>( + r#" + WITH RECURSIVE canonical_bitcoin_blockchain AS ( + SELECT + block_hash + , parent_hash + , block_height + , confirms + , 1 AS depth + FROM sbtc_signer.bitcoin_blocks + WHERE block_hash = $1 + + UNION ALL + + SELECT + parent.block_hash + , parent.parent_hash + , parent.block_height + , parent.confirms + , last.depth + 1 + FROM sbtc_signer.bitcoin_blocks AS parent + JOIN canonical_bitcoin_blockchain AS last + ON parent.block_hash = last.parent_hash + WHERE last.depth <= $2 + ) + SELECT + t.txid AS sweep_txid + , t.tx AS sweep_tx + , bt.block_hash AS sweep_block_hash + , cbb.block_height AS sweep_block_height + , dr.txid + , dr.output_index + , dr.recipient + , dr.amount + FROM sbtc_signer.transactions AS t + JOIN sbtc_signer.bitcoin_transactions AS bt + ON t.txid = bt.txid + JOIN canonical_bitcoin_blockchain AS cbb + ON bt.block_hash = cbb.block_hash + CROSS JOIN sbtc_signer.deposit_requests AS dr + LEFT JOIN sbtc_signer.completed_deposit_events AS cde + ON cde.bitcoin_txid = dr.txid + AND cde.output_index = dr.output_index + WHERE cde.bitcoin_txid IS NULL + AND t.tx_type = 'sbtc_transaction' + ORDER BY t.created_at DESC + LIMIT 1 + "#, + ) + .bind(chain_tip) + .bind(i32::from(context_window)) + .fetch_all(&self.0) + .await + .map_err(Error::SqlxQuery) + } + + async fn get_swept_withdrawal_requests( + &self, + _chain_tip: &model::BitcoinBlockHash, + _context_window: u16, + ) -> Result, Error> { + unimplemented!() + } } impl super::DbWrite for PgStore { diff --git a/signer/src/testing/dummy.rs b/signer/src/testing/dummy.rs index 547dabcda..c5a4de892 100644 --- a/signer/src/testing/dummy.rs +++ b/signer/src/testing/dummy.rs @@ -423,7 +423,7 @@ impl fake::Dummy for BitcoinTx { let deposit = DepositScriptInputs { signers_public_key: config.aggregate_key.into(), recipient: fake::Faker.fake_with_rng::(rng).into(), - max_fee: config.max_fee, + max_fee: config.max_fee.min(config.amount), }; let deposit_script = deposit.deposit_script(); // This is the part of the reclaim script that the user controls. @@ -454,6 +454,13 @@ impl fake::Dummy for BitcoinTx { } } +impl fake::Dummy for BitcoinTx { + fn dummy_with_rng(config: &fake::Faker, rng: &mut R) -> Self { + let deposit_config: DepositTxConfig = config.fake_with_rng(rng); + deposit_config.fake_with_rng(rng) + } +} + impl fake::Dummy for model::Transaction { fn dummy_with_rng(config: &DepositTxConfig, rng: &mut R) -> Self { let mut tx = Vec::new(); diff --git a/signer/src/testing/transaction_coordinator.rs b/signer/src/testing/transaction_coordinator.rs index 8d1974b34..c7a41a29c 100644 --- a/signer/src/testing/transaction_coordinator.rs +++ b/signer/src/testing/transaction_coordinator.rs @@ -52,7 +52,7 @@ where fn create( context: C, network: network::in_memory::MpmcBroadcaster, - context_window: usize, + context_window: u16, private_key: PrivateKey, threshold: u16, ) -> Self { @@ -63,7 +63,6 @@ where private_key, context_window, threshold, - bitcoin_network: bitcoin::Network::Testnet, signing_round_max_duration: Duration::from_secs(10), }, context, @@ -102,9 +101,9 @@ pub struct TestEnvironment { /// Signer context pub context: Context, /// Bitcoin context window - pub context_window: usize, + pub context_window: u16, /// Num signers - pub num_signers: usize, + pub num_signers: u16, /// Signing threshold pub signing_threshold: u16, /// Test model parameters @@ -129,7 +128,7 @@ where pub async fn assert_should_be_able_to_coordinate_signing_rounds(mut self) { let mut rng = rand::rngs::StdRng::seed_from_u64(46); let network = network::in_memory::Network::new(); - let signer_info = testing::wsts::generate_signer_info(&mut rng, self.num_signers); + let signer_info = testing::wsts::generate_signer_info(&mut rng, self.num_signers as usize); let mut testing_signer_set = testing::wsts::SignerSet::new(&signer_info, self.signing_threshold as u32, || { @@ -231,10 +230,12 @@ where .expect("failed to signal"); // Await the `wait_for_tx_task` to receive the first transaction broadcasted. - let broadcasted_tx = wait_for_transaction_task - .await - .expect("failed to receive message") - .expect("no message received"); + let broadcasted_tx = + tokio::time::timeout(Duration::from_secs(10), wait_for_transaction_task) + .await + .unwrap() + .expect("failed to receive message") + .expect("no message received"); // Extract the first script pubkey from the broadcasted transaction. let first_script_pubkey = broadcasted_tx @@ -259,7 +260,7 @@ where pub async fn assert_get_signer_utxo_simple(mut self) { let mut rng = rand::rngs::StdRng::seed_from_u64(46); let network = network::in_memory::Network::new(); - let signer_info = testing::wsts::generate_signer_info(&mut rng, self.num_signers); + let signer_info = testing::wsts::generate_signer_info(&mut rng, self.num_signers as usize); let mut signer_set = testing::wsts::SignerSet::new(&signer_info, self.signing_threshold as u32, || { @@ -314,7 +315,7 @@ where assert_eq!(chain_tip, block_ref.block_hash); let signer_utxo = storage - .get_signer_utxo(&chain_tip, &aggregate_key, self.context_window as u16) + .get_signer_utxo(&chain_tip, &aggregate_key, self.context_window) .await .unwrap() .expect("no signer utxo"); @@ -326,7 +327,7 @@ where pub async fn assert_get_signer_utxo_fork(mut self) { let mut rng = rand::rngs::StdRng::seed_from_u64(46); let network = network::in_memory::Network::new(); - let signer_info = testing::wsts::generate_signer_info(&mut rng, self.num_signers); + let signer_info = testing::wsts::generate_signer_info(&mut rng, self.num_signers as usize); let mut signer_set = testing::wsts::SignerSet::new(&signer_info, self.signing_threshold as u32, || { @@ -412,11 +413,7 @@ where public_key: bitcoin::XOnlyPublicKey::from(aggregate_key), }; let signer_utxo = storage - .get_signer_utxo( - &chain_tip.block_hash, - &aggregate_key, - self.context_window as u16, - ) + .get_signer_utxo(&chain_tip.block_hash, &aggregate_key, self.context_window) .await .unwrap() .expect("no signer utxo"); @@ -440,7 +437,7 @@ where pub async fn assert_get_signer_utxo_unspent(mut self) { let mut rng = rand::rngs::StdRng::seed_from_u64(46); let network = network::in_memory::Network::new(); - let signer_info = testing::wsts::generate_signer_info(&mut rng, self.num_signers); + let signer_info = testing::wsts::generate_signer_info(&mut rng, self.num_signers as usize); let mut signer_set = testing::wsts::SignerSet::new(&signer_info, self.signing_threshold as u32, || { @@ -518,7 +515,7 @@ where assert_eq!(chain_tip, block_ref.block_hash); let signer_utxo = storage - .get_signer_utxo(&chain_tip, &aggregate_key, self.context_window as u16) + .get_signer_utxo(&chain_tip, &aggregate_key, self.context_window) .await .unwrap() .expect("no signer utxo"); @@ -561,7 +558,7 @@ where .write_as_rotate_keys_tx( &self.context.get_storage_mut(), &bitcoin_chain_tip, - aggregate_key, + all_dkg_shares.first().unwrap(), rng, ) .await; diff --git a/signer/src/testing/transaction_signer.rs b/signer/src/testing/transaction_signer.rs index 65e8b36f0..791395854 100644 --- a/signer/src/testing/transaction_signer.rs +++ b/signer/src/testing/transaction_signer.rs @@ -63,7 +63,6 @@ where context_window, wsts_state_machines: HashMap::new(), threshold, - network_kind: bitcoin::Network::Regtest, rng, }, context, @@ -878,13 +877,13 @@ async fn run_dkg_and_store_results_for_signers<'s: 'r, 'r, S, Rng>( testing::wsts::SignerSet::new(signer_info, threshold, || network.connect()); let dkg_txid = testing::dummy::txid(&fake::Faker, rng); let bitcoin_chain_tip = *chain_tip; - let (aggregate_key, all_dkg_shares) = testing_signer_set + let (_, all_dkg_shares) = testing_signer_set .run_dkg(bitcoin_chain_tip, dkg_txid, rng) .await; for (storage, encrypted_dkg_shares) in stores.into_iter().zip(all_dkg_shares) { testing_signer_set - .write_as_rotate_keys_tx(&storage, chain_tip, aggregate_key, rng) + .write_as_rotate_keys_tx(&storage, chain_tip, &encrypted_dkg_shares, rng) .await; storage diff --git a/signer/src/testing/wallet.rs b/signer/src/testing/wallet.rs index 8cbc0dfa8..28aceaf4f 100644 --- a/signer/src/testing/wallet.rs +++ b/signer/src/testing/wallet.rs @@ -30,10 +30,10 @@ use crate::stacks::wallet::SignerWallet; /// A static for a test 2-3 multi-sig wallet. This wallet is loaded with /// funds in the local devnet environment. It matches the signer.deployer /// address in the default config file. -pub static WALLET: LazyLock<(SignerWallet, [Keypair; 3], u16)> = LazyLock::new(generate_wallet); +pub static WALLET: LazyLock<(SignerWallet, [Keypair; 3])> = LazyLock::new(generate_wallet); /// Helper function for generating a test 2-3 multi-sig wallet -pub fn generate_wallet() -> (SignerWallet, [Keypair; 3], u16) { +pub fn generate_wallet() -> (SignerWallet, [Keypair; 3]) { let mut rng = StdRng::seed_from_u64(100); let signatures_required = 2; @@ -47,7 +47,7 @@ pub fn generate_wallet() -> (SignerWallet, [Keypair; 3], u16) { let wallet = SignerWallet::new(&public_keys, signatures_required, NetworkKind::Testnet, 0).unwrap(); - (wallet, key_pairs, signatures_required) + (wallet, key_pairs) } /// This function creates a signing set where the aggregate key is the diff --git a/signer/src/testing/wsts.rs b/signer/src/testing/wsts.rs index 56a6f265e..b3f3f8426 100644 --- a/signer/src/testing/wsts.rs +++ b/signer/src/testing/wsts.rs @@ -9,6 +9,7 @@ use crate::message; use crate::network; use crate::storage; use crate::storage::model; +use crate::storage::model::EncryptedDkgShares; use crate::wsts_state_machine; use fake::Fake; @@ -517,7 +518,7 @@ impl SignerSet { &self, storage: &S, chain_tip: &model::BitcoinBlockHash, - aggregate_key: PublicKey, + shares: &EncryptedDkgShares, rng: &mut Rng, ) where S: storage::DbWrite + storage::DbRead, @@ -543,7 +544,7 @@ impl SignerSet { }; let rotate_keys_tx = model::RotateKeysTransaction { - aggregate_key, + aggregate_key: shares.aggregate_key, txid, signer_set: self.signer_keys(), signatures_required: self.signers.len() as u16, @@ -598,6 +599,6 @@ mod tests { let (_, dkg_shares) = signer_set.run_dkg(bitcoin_chain_tip, txid, &mut rng).await; - assert_eq!(dkg_shares.len(), num_signers); + assert_eq!(dkg_shares.len(), num_signers as usize); } } diff --git a/signer/src/transaction_coordinator.rs b/signer/src/transaction_coordinator.rs index 5cd2ae4bd..3529d8ff0 100644 --- a/signer/src/transaction_coordinator.rs +++ b/signer/src/transaction_coordinator.rs @@ -7,6 +7,8 @@ use std::collections::BTreeSet; +use futures::StreamExt; +use futures::TryStreamExt; use sha2::Digest; use crate::bitcoin::utxo; @@ -17,7 +19,15 @@ use crate::error::Error; use crate::keys::PrivateKey; use crate::keys::PublicKey; use crate::message; +use crate::message::StacksTransactionSignRequest; use crate::network; +use crate::signature::SighashDigest; +use crate::stacks::api::FeePriority; +use crate::stacks::api::StacksInteract; +use crate::stacks::contracts::CompleteDepositV1; +use crate::stacks::contracts::ContractCall; +use crate::stacks::wallet::MultisigTx; +use crate::stacks::wallet::SignerWallet; use crate::storage::model; use crate::storage::DbRead as _; use crate::wsts_state_machine; @@ -71,9 +81,9 @@ use wsts::state_machine::coordinator::Coordinator as _; /// /// [^1]: A deposit or withdraw request is considered pending if it is confirmed /// on chain but hasn't been fulfilled in an sBTC transaction yet. -/// [^2]: A deposit or withdraw request is considered active if has been +/// [^2]: A deposit or withdraw request is considered active if has been /// fulfilled in an sBTC transaction, -/// but the result hasn't been acknowledged on Stacks as a +/// but the result hasn't been acknowledged on Stacks as a /// `deposit-accept`, `withdraw-accept` or `withdraw-reject` transaction. /// /// The whole flow is illustrated in the following flowchart. @@ -105,12 +115,10 @@ pub struct TxCoordinatorEventLoop { pub network: Network, /// Private key of the coordinator for network communication. pub private_key: PrivateKey, - /// The threshold for the signer + /// the number of signatures required. pub threshold: u16, /// How many bitcoin blocks back from the chain tip the signer will look for requests. - pub context_window: usize, - /// The bitcoin network we're targeting - pub bitcoin_network: bitcoin::Network, + pub context_window: u16, /// The maximum duration of a signing round before the coordinator will time out and return an error. pub signing_round_max_duration: std::time::Duration, } @@ -176,15 +184,14 @@ where if self.is_coordinator(&bitcoin_chain_tip, &signer_public_keys)? { self.construct_and_sign_bitcoin_sbtc_transactions( &bitcoin_chain_tip, - aggregate_key, + &aggregate_key, &signer_public_keys, ) .await?; self.construct_and_sign_stacks_sbtc_response_transactions( &bitcoin_chain_tip, - aggregate_key, - &signer_public_keys, + &aggregate_key, ) .await?; } @@ -198,10 +205,10 @@ where async fn construct_and_sign_bitcoin_sbtc_transactions( &mut self, bitcoin_chain_tip: &model::BitcoinBlockHash, - aggregate_key: PublicKey, + aggregate_key: &PublicKey, signer_public_keys: &BTreeSet, ) -> Result<(), Error> { - let signer_btc_state = self.get_btc_state(&aggregate_key).await?; + let signer_btc_state = self.get_btc_state(aggregate_key).await?; let pending_requests = self .get_pending_requests( @@ -227,32 +234,148 @@ where Ok(()) } - /// Construct and coordinate signing rounds for - /// `deposit-accept`, `withdraw-accept` and `withdraw-reject` transactions. - #[tracing::instrument(skip(self))] + /// Construct and coordinate signing rounds for `deposit-accept`, + /// `withdraw-accept` and `withdraw-reject` transactions. + /// + /// # Notes + /// + /// This function does the following. + /// 1. Load the stacks wallet from the database. This wallet is + /// determined by the public keys and threshold stored in the last + /// [`RotateKeysTransaction`] object that is returned from the + /// database. + /// 2. Fetch all "finalizable" requests from the database. These are + /// requests that where we have a response transactions on bitcoin + /// fulfilling the deposit or withdrawal request. + /// 3. Construct a sign-request object for each finalizable request. + /// 4. Broadcast this sign-request to the network and wait for + /// responses. + /// 5. If there are enough signatures then broadcast the transaction. + #[tracing::instrument(skip_all)] async fn construct_and_sign_stacks_sbtc_response_transactions( &mut self, - bitcoin_chain_tip: &model::BitcoinBlockHash, - aggregate_key: PublicKey, - signer_public_keys: &BTreeSet, + chain_tip: &model::BitcoinBlockHash, + bitcoin_aggregate_key: &PublicKey, ) -> Result<(), Error> { - // TODO(320): Implement + let wallet = SignerWallet::load(&self.context, chain_tip).await?; + let db = self.context.get_storage(); + let stacks = self.context.get_stacks_client(); + + // Fetch deposit and withdrawal requests from the database where + // there has been a confirmed bitcoin transaction associated with + // the request. + // + // For deposits, there will be at most one such bitcoin transaction + // on the blockchain identified by the chain tip, where an input is + // the deposit UTXO. + // + // For withdrawals, we need to have a record of the `request_id` + // associated with the bitcoin transaction's outputs. + + let deposit_requests = db + .get_swept_deposit_requests(chain_tip, self.context_window) + .await?; + + if deposit_requests.is_empty() { + return Ok(()); + } + + // We need to know the nonce to use, so we reach out to our stacks + // node for the account information for our multi-sig address. + // + // Note that the wallet object will automatically increment the + // nonce for each transaction that it creates. + let account = stacks.get_account(wallet.address()).await?; + wallet.set_nonce(account.nonce); + + // Generate + let _sign_requests = futures::stream::iter(deposit_requests) + .then(|req| self.construct_deposit_stacks_sign_request(req, bitcoin_aggregate_key, &wallet)) + .try_collect::>() + .await?; + + // TODO: + // 1. Broadcast the sign requests + // 2. Gather the signatures into the transaction. + // 3. Broadcast the transaction to the stacks network. Then go home + // and relax. Ok(()) } + /// Transform the swept deposit request into a Stacks sign request + /// object. + /// + /// This function uses bitcoin-core to help with the fee assessment of + /// the deposit request, and stacks-core for fee estimation of the + /// transaction. + #[tracing::instrument(skip_all)] + async fn construct_deposit_stacks_sign_request( + &self, + req: model::SweptDepositRequest, + bitcoin_aggregate_key: &PublicKey, + wallet: &SignerWallet, + ) -> Result { + let tx_info = self + .context + .get_bitcoin_client() + .get_tx_info(&req.sweep_txid, &req.sweep_block_hash) + .await? + .ok_or_else(|| { + Error::BitcoinTxMissing(req.sweep_txid.into(), Some(req.sweep_block_hash.into())) + })?; + + let outpoint = bitcoin::OutPoint { + txid: req.txid.into(), + vout: req.output_index, + }; + let assessed_bitcoin_fee = tx_info + .assess_input_fee(&outpoint) + .ok_or_else(|| Error::OutPointMissing(outpoint))?; + + // TODO: we should validate the contract call before asking others + // to sign it. + let contract_call = ContractCall::CompleteDepositV1(CompleteDepositV1 { + amount: req.amount - assessed_bitcoin_fee.to_sat(), + outpoint, + recipient: req.recipient.into(), + deployer: self.context.config().signer.deployer, + sweep_txid: req.sweep_txid, + sweep_block_hash: req.sweep_block_hash, + sweep_block_height: req.sweep_block_height, + }); + + // Complete deposit requests should be done as soon as possible, so + // we set the fee rate to the high priority fee. + let tx_fee = self + .context + .get_stacks_client() + .estimate_fees(&contract_call, FeePriority::High) + .await?; + + let multi_tx = MultisigTx::new_tx(&contract_call, wallet, tx_fee); + + Ok(StacksTransactionSignRequest { + aggregate_key: *bitcoin_aggregate_key, + contract_call, + nonce: multi_tx.tx().get_origin_nonce(), + tx_fee: multi_tx.tx().get_tx_fee(), + digest: multi_tx.tx().digest(), + }) + } + /// Coordinate a signing round for the given request /// and broadcast it once it's signed. #[tracing::instrument(skip(self))] async fn sign_and_broadcast( &mut self, bitcoin_chain_tip: &model::BitcoinBlockHash, - aggregate_key: PublicKey, + aggregate_key: &PublicKey, signer_public_keys: &BTreeSet, mut transaction: utxo::UnsignedTransaction<'_>, ) -> Result<(), Error> { let mut coordinator_state_machine = wsts_state_machine::CoordinatorStateMachine::load( &mut self.context.get_storage_mut(), - aggregate_key, + *aggregate_key, signer_public_keys.clone(), self.threshold, self.private_key, @@ -437,14 +560,10 @@ where return Err(Error::NoChainTip); }; - let context_window = self - .context_window - .try_into() - .map_err(|_| Error::TypeConversion)?; let utxo = self .context .get_storage() - .get_signer_utxo(&chain_tip, aggregate_key, context_window) + .get_signer_utxo(&chain_tip, aggregate_key, self.context_window) .await? .ok_or(Error::MissingSignerUtxo)?; let last_fees = bitcoin_client.get_last_fee(utxo.outpoint).await?; @@ -467,14 +586,10 @@ where &mut self, bitcoin_chain_tip: &model::BitcoinBlockHash, signer_btc_state: utxo::SignerBtcState, - aggregate_key: PublicKey, + aggregate_key: &PublicKey, signer_public_keys: &BTreeSet, ) -> Result { - let context_window = self - .context_window - .try_into() - .map_err(|_| Error::TypeConversion)?; - + let context_window = self.context_window; let threshold = self.threshold; let pending_deposit_requests = self @@ -489,7 +604,7 @@ where .get_pending_accepted_withdrawal_requests(bitcoin_chain_tip, context_window, threshold) .await?; - let signers_public_key = bitcoin::XOnlyPublicKey::from(&aggregate_key); + let signers_public_key = bitcoin::XOnlyPublicKey::from(aggregate_key); let mut deposits: Vec = Vec::new(); @@ -497,7 +612,7 @@ where let votes = self .context .get_storage() - .get_deposit_request_signer_votes(&req.txid, req.output_index, &aggregate_key) + .get_deposit_request_signer_votes(&req.txid, req.output_index, aggregate_key) .await?; let deposit = utxo::DepositRequest::from_model(req, signers_public_key, votes); @@ -510,7 +625,7 @@ where let votes = self .context .get_storage() - .get_withdrawal_request_signer_votes(&req.qualified_id(), &aggregate_key) + .get_withdrawal_request_signer_votes(&req.qualified_id(), aggregate_key) .await?; let withdrawal = utxo::WithdrawalRequest::from_model(req, votes); @@ -630,7 +745,7 @@ mod tests { .with_mocked_clients() .build(); - testing::transaction_coordinator::TestEnvironment { + TestEnvironment { context, context_window: 5, num_signers: 7, diff --git a/signer/src/transaction_signer.rs b/signer/src/transaction_signer.rs index 256c935f1..698dc097a 100644 --- a/signer/src/transaction_signer.rs +++ b/signer/src/transaction_signer.rs @@ -10,7 +10,6 @@ use std::collections::HashMap; use std::time::Duration; use crate::blocklist_client; -use crate::config::NetworkKind; use crate::context::Context; use crate::context::SignerEvent; use crate::context::SignerSignal; @@ -127,8 +126,6 @@ pub struct TxSignerEventLoop { pub threshold: u32, /// How many bitcoin blocks back from the chain tip the signer will look for requests. pub context_window: u16, - /// The network we are working in. - pub network_kind: bitcoin::Network, /// Random number generator used for encryption pub rng: Rng, } @@ -414,13 +411,15 @@ where async fn handle_stacks_transaction_sign_request( &mut self, ctx: &impl Context, - request: &message::StacksTransactionSignRequest, + request: &StacksTransactionSignRequest, bitcoin_chain_tip: &model::BitcoinBlockHash, ) -> Result<(), Error> { self.assert_valid_stackstransaction_sign_request(ctx, request, bitcoin_chain_tip) .await?; - let wallet = self.load_wallet(request, bitcoin_chain_tip).await?; + let wallet = SignerWallet::load(ctx, bitcoin_chain_tip).await?; + wallet.set_nonce(request.nonce); + let multi_sig = MultisigTx::new_tx(&request.contract_call, &wallet, request.tx_fee); let txid = multi_sig.tx().txid(); @@ -433,35 +432,6 @@ where Ok(()) } - /// Load the multi-sig wallet corresponding to the signer set defined - /// in the last key rotation. - /// TODO(255): Add a tests - async fn load_wallet( - &self, - request: &StacksTransactionSignRequest, - bitcoin_chain_tip: &model::BitcoinBlockHash, - ) -> Result { - let last_key_rotation = self - .context - .get_storage() - .get_last_key_rotation(bitcoin_chain_tip) - .await? - .ok_or(Error::MissingKeyRotation)?; - - let public_keys = last_key_rotation.signer_set.as_slice(); - let signatures_required = last_key_rotation.signatures_required; - let network_kind = match self.network_kind { - bitcoin::Network::Bitcoin => NetworkKind::Mainnet, - _ => NetworkKind::Testnet, - }; - SignerWallet::new( - public_keys, - signatures_required, - network_kind, - request.nonce, - ) - } - async fn assert_valid_stackstransaction_sign_request( &mut self, ctx: &impl Context, @@ -616,7 +586,8 @@ where request: model::DepositRequest, bitcoin_chain_tip: &model::BitcoinBlockHash, ) -> Result<(), Error> { - let params = self.network_kind.params(); + let bitcoin_network = bitcoin::Network::from(self.context.config().signer.network); + let params = bitcoin_network.params(); let addresses = request .sender_script_pub_keys .iter() diff --git a/signer/src/wsts_state_machine.rs b/signer/src/wsts_state_machine.rs index 6bdff3da1..6ed08f4ab 100644 --- a/signer/src/wsts_state_machine.rs +++ b/signer/src/wsts_state_machine.rs @@ -128,6 +128,7 @@ impl SignerStateMachine { .encode_to_vec() .map_err(error::Error::Codec)?; + // After DKG, each of the signers will have "new public keys". let encrypted_private_shares = wsts::util::encrypt(&self.0.network_private_key.to_bytes(), &encoded, rng) .map_err(|_| error::Error::Encryption)?; @@ -179,7 +180,7 @@ impl CoordinatorStateMachine { let num_signers: u32 = signer_public_keys .len() .try_into() - .expect("The number of signers is creater than u32::MAX?"); + .expect("The number of signers is greater than u32::MAX?"); let signer_key_ids = (0..num_signers) .map(|signer_id| (signer_id, std::iter::once(signer_id).collect())) .collect(); diff --git a/signer/tests/integration/contracts.rs b/signer/tests/integration/contracts.rs index 1ebffccec..9c1b84f86 100644 --- a/signer/tests/integration/contracts.rs +++ b/signer/tests/integration/contracts.rs @@ -23,10 +23,8 @@ use tokio::sync::OnceCell; use signer::stacks; use signer::stacks::api::FeePriority; -use signer::stacks::api::RejectionReason; use signer::stacks::api::StacksClient; use signer::stacks::api::SubmitTxResponse; -use signer::stacks::api::TxRejection; use signer::stacks::contracts::CompleteDepositV1; use signer::stacks::wallet::MultisigTx; use signer::storage::in_memory::Store; @@ -116,10 +114,6 @@ impl SignerStxState { match self.stacks_client.submit_tx(&tx).await.unwrap() { SubmitTxResponse::Acceptance(_) => (), - SubmitTxResponse::Rejection(TxRejection { - reason: RejectionReason::ContractAlreadyExists, - .. - }) => (), SubmitTxResponse::Rejection(err) => panic!("{}", serde_json::to_string(&err).unwrap()), } } @@ -171,7 +165,7 @@ pub async fn deploy_smart_contracts() -> &'static SignerStxState { static SBTC_DEPLOYMENT: OnceCell<()> = OnceCell::const_new(); static SIGNER_STATE: OnceCell = OnceCell::const_new(); - let (signer_wallet, key_pairs, _) = testing::wallet::generate_wallet(); + let (signer_wallet, key_pairs) = testing::wallet::generate_wallet(); let client = stacks_client(); @@ -208,7 +202,7 @@ pub async fn deploy_smart_contracts() -> &'static SignerStxState { outpoint: bitcoin::OutPoint::null(), amount: 123654789, recipient: PrincipalData::parse("SN2V7WTJ7BHR03MPHZ1C9A9ZR6NZGR4WM8HT4V67Y").unwrap(), - deployer: testing::wallet::WALLET.0.address(), + deployer: *testing::wallet::WALLET.0.address(), sweep_txid: BitcoinTxId::from([0; 32]), sweep_block_hash: BitcoinBlockHash::from([0; 32]), sweep_block_height: 7, @@ -217,7 +211,7 @@ pub async fn deploy_smart_contracts() -> &'static SignerStxState { outpoint: bitcoin::OutPoint::null(), amount: 123654, recipient: PrincipalData::parse("ST1RQHF4VE5CZ6EK3MZPZVQBA0JVSMM9H5PMHMS1Y.my-contract-name").unwrap(), - deployer: testing::wallet::WALLET.0.address(), + deployer: *testing::wallet::WALLET.0.address(), sweep_txid: BitcoinTxId::from([0; 32]), sweep_block_hash: BitcoinBlockHash::from([0; 32]), sweep_block_height: 7, @@ -227,7 +221,7 @@ pub async fn deploy_smart_contracts() -> &'static SignerStxState { outpoint: bitcoin::OutPoint::null(), tx_fee: 2500, signer_bitmap: BitArray::ZERO, - deployer: testing::wallet::WALLET.0.address(), + deployer: *testing::wallet::WALLET.0.address(), sweep_block_hash: BitcoinBlockHash::from([0; 32]), sweep_block_height: 7, }); "accept-withdrawal")] @@ -235,17 +229,16 @@ pub async fn deploy_smart_contracts() -> &'static SignerStxState { amount: 22500, recipient: (0x00, vec![0; 20]), max_fee: 3000, - deployer: testing::wallet::WALLET.0.address(), + deployer: *testing::wallet::WALLET.0.address(), }); "create-withdrawal")] #[test_case(ContractCallWrapper(RejectWithdrawalV1 { request_id: 2, signer_bitmap: BitArray::ZERO, - deployer: testing::wallet::WALLET.0.address(), + deployer: *testing::wallet::WALLET.0.address(), }); "reject-withdrawal")] #[test_case(ContractCallWrapper(RotateKeysV1::new( &testing::wallet::WALLET.0, - testing::wallet::WALLET.0.address(), - testing::wallet::WALLET.2, + *testing::wallet::WALLET.0.address(), )); "rotate-keys")] #[tokio::test] async fn complete_deposit_wrapper_tx_accepted(contract: ContractCallWrapper) { diff --git a/signer/tests/integration/postgres.rs b/signer/tests/integration/postgres.rs index 7918b9462..40f1c5553 100644 --- a/signer/tests/integration/postgres.rs +++ b/signer/tests/integration/postgres.rs @@ -44,6 +44,7 @@ use signer::storage::model::StacksBlock; use signer::storage::model::StacksBlockHash; use signer::storage::model::StacksTxId; use signer::storage::model::WithdrawalSigner; +use signer::storage::postgres::PgStore; use signer::storage::DbRead; use signer::storage::DbWrite; use signer::testing; @@ -56,6 +57,7 @@ use rand::SeedableRng; use signer::testing::context::*; use test_case::test_case; +use crate::setup::TestSweepSetup; use crate::DATABASE_NUM; #[cfg_attr(not(feature = "integration-tests"), ignore)] @@ -131,13 +133,13 @@ impl AsContractCall for InitiateWithdrawalRequest { /// care about, which, naturally, are sBTC related transactions. #[cfg_attr(not(feature = "integration-tests"), ignore)] #[test_case(ContractCallWrapper(InitiateWithdrawalRequest { - deployer: testing::wallet::WALLET.0.address(), + deployer: *testing::wallet::WALLET.0.address(), }); "initiate-withdrawal")] #[test_case(ContractCallWrapper(CompleteDepositV1 { outpoint: bitcoin::OutPoint::null(), amount: 123654, recipient: PrincipalData::parse("ST1RQHF4VE5CZ6EK3MZPZVQBA0JVSMM9H5PMHMS1Y").unwrap(), - deployer: testing::wallet::WALLET.0.address(), + deployer: *testing::wallet::WALLET.0.address(), sweep_txid: BitcoinTxId::from([0; 32]), sweep_block_hash: BitcoinBlockHash::from([0; 32]), sweep_block_height: 7, @@ -146,7 +148,7 @@ impl AsContractCall for InitiateWithdrawalRequest { outpoint: bitcoin::OutPoint::null(), amount: 123654, recipient: PrincipalData::parse("ST1RQHF4VE5CZ6EK3MZPZVQBA0JVSMM9H5PMHMS1Y.my-contract-name").unwrap(), - deployer: testing::wallet::WALLET.0.address(), + deployer: *testing::wallet::WALLET.0.address(), sweep_txid: BitcoinTxId::from([0; 32]), sweep_block_hash: BitcoinBlockHash::from([0; 32]), sweep_block_height: 7, @@ -156,19 +158,18 @@ impl AsContractCall for InitiateWithdrawalRequest { outpoint: bitcoin::OutPoint::null(), tx_fee: 3500, signer_bitmap: BitArray::ZERO, - deployer: testing::wallet::WALLET.0.address(), + deployer: *testing::wallet::WALLET.0.address(), sweep_block_hash: BitcoinBlockHash::from([0; 32]), sweep_block_height: 7, }); "accept-withdrawal")] #[test_case(ContractCallWrapper(RejectWithdrawalV1 { request_id: 0, signer_bitmap: BitArray::ZERO, - deployer: testing::wallet::WALLET.0.address(), + deployer: *testing::wallet::WALLET.0.address(), }); "reject-withdrawal")] #[test_case(ContractCallWrapper(RotateKeysV1::new( &testing::wallet::WALLET.0, - testing::wallet::WALLET.0.address(), - testing::wallet::WALLET.2, + *testing::wallet::WALLET.0.address(), )); "rotate-keys")] #[tokio::test] async fn writing_stacks_blocks_works(contract: ContractCallWrapper) { @@ -599,16 +600,17 @@ async fn should_return_the_same_last_key_rotation_as_in_memory_store() { let mut testing_signer_set = testing::wsts::SignerSet::new(&signer_info, threshold, || dummy_wsts_network.connect()); let dkg_txid = testing::dummy::txid(&fake::Faker, &mut rng); - let (aggregate_key, _) = testing_signer_set + let (_, all_shares) = testing_signer_set .run_dkg(chain_tip, dkg_txid, &mut rng) .await; + let shares = all_shares.first().unwrap(); testing_signer_set - .write_as_rotate_keys_tx(&mut in_memory_store, &chain_tip, aggregate_key, &mut rng) + .write_as_rotate_keys_tx(&mut in_memory_store, &chain_tip, shares, &mut rng) .await; testing_signer_set - .write_as_rotate_keys_tx(&mut pg_store, &chain_tip, aggregate_key, &mut rng) + .write_as_rotate_keys_tx(&mut pg_store, &chain_tip, shares, &mut rng) .await; let last_key_rotation_in_memory = in_memory_store @@ -1362,7 +1364,197 @@ async fn get_signers_script_pubkeys_returns_non_empty_vec_old_rows() { signer::testing::storage::drop_db(db).await; } +/// This tests that deposit requests where there is an associated sweep +/// transaction will show up in the query results from +/// [`DbRead::get_swept_deposit_requests`]. +#[cfg_attr(not(feature = "integration-tests"), ignore)] +#[tokio::test] +async fn get_swept_deposit_requests_returns_swept_deposit_requests() { + let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst); + let db = testing::storage::new_test_database(db_num, true).await; + let mut rng = rand::rngs::StdRng::seed_from_u64(51); + + // This query doesn't *need* bitcoind (it's just a query), we just need + // the transaction data in the database. We use the [`TestSweepSetup`] + // structure because it has helper functions for generating and storing + // sweep transactions, and the [`TestSweepSetup`] structure correctly + // sets up the database. + let (rpc, faucet) = sbtc::testing::regtest::initialize_blockchain(); + let setup = TestSweepSetup::new_setup(&rpc, &faucet, 1_000_000, &mut rng); + + // We need to manually update the database with new bitcoin block + // headers. + crate::setup::backfill_bitcoin_blocks(&db, rpc, &setup.sweep_block_hash).await; + + // This isn't technically required right now, but the deposit + // transaction is supposed to be there, so future versions of our query + // can rely on that fact. + setup.store_deposit_tx(&db).await; + + // We take the sweep transaction as is from the test setup and + // store it in the database. + setup.store_sweep_tx(&db).await; + + // Lastly, the request needs to be added to the database. This stores + // `setup.deposit_request` into the database. + setup.store_deposit_request(&db).await; + + let chain_tip = setup.sweep_block_hash.into(); + let context_window = 20; + + let mut requests = db + .get_swept_deposit_requests(&chain_tip, context_window) + .await + .unwrap(); + + // There should only be one request in the database and it has a sweep + // trasnaction so the length should be 1. + assert_eq!(requests.len(), 1); + + // Its details should match that of the deposit request. + let req = requests.pop().unwrap(); + assert_eq!(req.amount, setup.deposit_request.amount); + assert_eq!(req.txid, setup.deposit_request.outpoint.txid.into()); + assert_eq!(req.output_index, setup.deposit_request.outpoint.vout); + assert_eq!(req.recipient, setup.deposit_recipient.into()); + assert_eq!(req.sweep_block_hash, setup.sweep_block_hash.into()); + assert_eq!(req.sweep_block_height, setup.sweep_block_height); + assert_eq!(req.sweep_txid, setup.sweep_tx_info.txid.into()); + assert_eq!(req.sweep_tx, setup.sweep_tx_info.tx.into()); + + signer::testing::storage::drop_db(db).await; +} + +/// This function tests that deposit requests that do not have a confirmed +/// response bitcoin transaction are not returned from +/// [`DbRead::get_swept_deposit_requests`]. +/// +/// We need to update the query before we can activate this test. Right now +/// we do not associate deposit transactions with their sweep transaction, +/// so the query is very dumb. We should fix this once +/// https://github.com/stacks-network/sbtc/issues/585 gets completed. +#[ignore = "Underlying query has not been completed"] +#[tokio::test] +async fn get_swept_deposit_requests_does_not_return_unswept_deposit_requests() { + let db_num = testing::storage::DATABASE_NUM.fetch_add(1, Ordering::SeqCst); + let db = testing::storage::new_test_database(db_num, true).await; + let mut rng = rand::rngs::StdRng::seed_from_u64(51); + + // This query doesn't *need* bitcoind (it's just a query), we just need + // the transaction data in the database. We use the [`TestSweepSetup`] + // structure because it has helper functions for generating and storing + // sweep transactions, and the [`TestSweepSetup`] structure correctly + // sets up the database. + let (rpc, faucet) = sbtc::testing::regtest::initialize_blockchain(); + let setup = TestSweepSetup::new_setup(&rpc, &faucet, 1_000_000, &mut rng); + + // We need to manually update the database with new bitcoin block + // headers. + crate::setup::backfill_bitcoin_blocks(&db, rpc, &setup.sweep_block_hash).await; + + // This isn't technically required right now, but the deposit + // transaction is supposed to be there, so future versions of our query + // can rely on that fact. + setup.store_deposit_tx(&db).await; + + // The request needs to be added to the database. This stores + // `setup.deposit_request` into the database. + setup.store_deposit_request(&db).await; + + // We are supposed to store a sweep transaction, but we haven't, so the + // deposit request is not considered swept. + let chain_tip = setup.sweep_block_hash.into(); + let context_window = 20; + + let requests = db + .get_swept_deposit_requests(&chain_tip, context_window) + .await + .unwrap(); + + // Womp, the request is not considered swept. + assert!(requests.is_empty()); + + signer::testing::storage::drop_db(db).await; +} + +/// This function tests that [`DbRead::get_swept_deposit_requests`] +/// function does not return requests where we have already confirmed a +/// `complete-deposit` contract call transaction on the canonical Stacks +/// blockchain. +/// +/// Right now the query in [`DbRead::get_swept_deposit_requests`] does not +/// satisfy that criteria, because it does not check that the +/// `complete-deposit` contract call is on the Stacks blockchain that is +/// associated with the canonical bitcoin blockchain. +#[cfg_attr(not(feature = "integration-tests"), ignore)] +#[tokio::test] +async fn get_swept_deposit_requests_does_not_return_deposit_requests_with_responses() { + let db_num = testing::storage::DATABASE_NUM.fetch_add(1, Ordering::SeqCst); + let db = testing::storage::new_test_database(db_num, true).await; + let mut rng = rand::rngs::StdRng::seed_from_u64(51); + + // This query doesn't *need* bitcoind (it's just a query), we just need + // the transaction data in the database. We use the [`TestSweepSetup`] + // structure because it has helper functions for generating and storing + // sweep transactions, and the [`TestSweepSetup`] structure correctly + // sets up the database. + let (rpc, faucet) = sbtc::testing::regtest::initialize_blockchain(); + let setup = TestSweepSetup::new_setup(&rpc, &faucet, 1_000_000, &mut rng); + + // We need to manually update the database with new bitcoin block + // headers. + crate::setup::backfill_bitcoin_blocks(&db, rpc, &setup.sweep_block_hash).await; + + // This isn't technically required right now, but the deposit + // transaction is supposed to be there, so future versions of our query + // can rely on that fact. + setup.store_deposit_tx(&db).await; + + // We take the sweep transaction as is from the test setup and + // store it in the database. + setup.store_sweep_tx(&db).await; + + // The request needs to be added to the database. This stores + // `setup.deposit_request` into the database. + setup.store_deposit_request(&db).await; + + // Here we store an event that signals that the deposit request has been confirmed. + let event = CompletedDepositEvent { + txid: fake::Faker.fake_with_rng::(&mut rng).into(), + block_id: fake::Faker + .fake_with_rng::(&mut rng) + .into(), + amount: setup.deposit_request.amount, + outpoint: setup.deposit_request.outpoint, + }; + + db.write_completed_deposit_event(&event).await.unwrap(); + + let chain_tip = setup.sweep_block_hash.into(); + let context_window = 20; + + let requests = db + .get_swept_deposit_requests(&chain_tip, context_window) + .await + .unwrap(); + + // The only deposit request has a confirmed complete-deposit + // transaction on the canonical stacks blockchain. + assert!(requests.is_empty()); + + signer::testing::storage::drop_db(db).await; +} + +/// This function tests that [`DbRead::get_swept_deposit_requests`] +/// function return requests where we have already confirmed a +/// `complete-deposit` contract call transaction on the Stacks blockchain +/// but that transaction has been reorged while the sweep transaction has not. +#[ignore = "Query does not check for transactions on canonical Stacks blockchain"] +#[tokio::test] +async fn get_swept_deposit_requests_response_tx_reorged() {} + async fn transaction_coordinator_test_environment( + store: PgStore, ) -> testing::transaction_coordinator::TestEnvironment< TestContext< storage::postgres::PgStore, @@ -1371,8 +1563,6 @@ async fn transaction_coordinator_test_environment( WrappedMock, >, > { - use std::sync::atomic::Ordering; - let test_model_parameters = testing::storage::model::Params { num_bitcoin_blocks: 20, num_stacks_blocks_per_bitcoin_block: 3, @@ -1381,9 +1571,6 @@ async fn transaction_coordinator_test_environment( num_signers_per_request: 7, }; - let db_num = testing::storage::DATABASE_NUM.fetch_add(1, Ordering::SeqCst); - let store = testing::storage::new_test_database(db_num, true).await; - let context = TestContext::builder() .with_storage(store) .with_mocked_clients() @@ -1401,26 +1588,41 @@ async fn transaction_coordinator_test_environment( #[cfg_attr(not(feature = "integration-tests"), ignore)] #[tokio::test] async fn should_get_signer_utxo_simple() { - transaction_coordinator_test_environment() + let db_num = testing::storage::DATABASE_NUM.fetch_add(1, Ordering::SeqCst); + let store = testing::storage::new_test_database(db_num, true).await; + + transaction_coordinator_test_environment(store.clone()) .await .assert_get_signer_utxo_simple() .await; + + signer::testing::storage::drop_db(store).await; } #[cfg_attr(not(feature = "integration-tests"), ignore)] #[tokio::test] async fn should_get_signer_utxo_fork() { - transaction_coordinator_test_environment() + let db_num = testing::storage::DATABASE_NUM.fetch_add(1, Ordering::SeqCst); + let store = testing::storage::new_test_database(db_num, true).await; + + transaction_coordinator_test_environment(store.clone()) .await .assert_get_signer_utxo_fork() .await; + + signer::testing::storage::drop_db(store).await; } #[cfg_attr(not(feature = "integration-tests"), ignore)] #[tokio::test] async fn should_get_signer_utxo_unspent() { - transaction_coordinator_test_environment() + let db_num = testing::storage::DATABASE_NUM.fetch_add(1, Ordering::SeqCst); + let store = testing::storage::new_test_database(db_num, true).await; + + transaction_coordinator_test_environment(store.clone()) .await .assert_get_signer_utxo_unspent() .await; + + signer::testing::storage::drop_db(store).await; }