rotate key - wait for all signers, but sign only required

signer/src/stacks/api.rs

@@ -194,7 +194,6 @@ pub trait StacksInteract: Send + Sync {
         wallet: &SignerWallet,
         payload: &T,
         priority: FeePriority,
-        require_all_signatures: bool,
     ) -> impl Future<Output = Result<u64, Error>> + Send
     where
         T: AsTxPayload + Send + Sync;
@@ -1180,13 +1179,11 @@ impl StacksInteract for StacksClient {
         wallet: &SignerWallet,
         payload: &T,
         priority: FeePriority,
-        require_all_signatures: bool,
     ) -> Result<u64, Error>
     where
         T: AsTxPayload + Send + Sync,
     {
-        let transaction_size =
-            super::wallet::get_full_tx_size(payload, wallet, require_all_signatures)?;
+        let transaction_size = super::wallet::get_full_tx_size(payload, wallet)?;
 
         // In Stacks core, the minimum fee is 1 mSTX per byte, so take the
         // transaction size and multiply it by the TX_FEE_TX_SIZE_MULTIPLIER
@@ -1369,15 +1366,12 @@ impl StacksInteract for ApiFallbackClient<StacksClient> {
         wallet: &SignerWallet,
         payload: &T,
         priority: FeePriority,
-        require_all_signatures: bool,
     ) -> Result<u64, Error>
     where
         T: AsTxPayload + Send + Sync,
     {
-        self.exec(|client, _| {
-            StacksClient::estimate_fees(client, wallet, payload, priority, require_all_signatures)
-        })
-        .await
+        self.exec(|client, _| StacksClient::estimate_fees(client, wallet, payload, priority))
+            .await
     }
 
     async fn get_pox_info(&self) -> Result<RPCPoxInfoData, Error> {
@@ -1927,16 +1921,11 @@ mod tests {
         let client =
             StacksClient::new(url::Url::parse(stacks_node_server.url().as_str()).unwrap()).unwrap();
 
-        let expected_fee = get_full_tx_size(&DUMMY_STX_TRANSFER_PAYLOAD, &wallet, false).unwrap()
+        let expected_fee = get_full_tx_size(&DUMMY_STX_TRANSFER_PAYLOAD, &wallet).unwrap()
             * TX_FEE_TX_SIZE_MULTIPLIER;
 
         let resp = client
-            .estimate_fees(
-                &wallet,
-                &DUMMY_STX_TRANSFER_PAYLOAD,
-                FeePriority::High,
-                false,
-            )
+            .estimate_fees(&wallet, &DUMMY_STX_TRANSFER_PAYLOAD, FeePriority::High)
             .await
             .unwrap();
 
@@ -1992,34 +1981,19 @@ mod tests {
         // Now lets check that the interface function returns the requested
         // priority fees.
         let fee = client
-            .estimate_fees(
-                &wallet,
-                &DUMMY_STX_TRANSFER_PAYLOAD,
-                FeePriority::Low,
-                false,
-            )
+            .estimate_fees(&wallet, &DUMMY_STX_TRANSFER_PAYLOAD, FeePriority::Low)
             .await
             .unwrap();
         assert_eq!(fee, 7679);
 
         let fee = client
-            .estimate_fees(
-                &wallet,
-                &DUMMY_STX_TRANSFER_PAYLOAD,
-                FeePriority::Medium,
-                false,
-            )
+            .estimate_fees(&wallet, &DUMMY_STX_TRANSFER_PAYLOAD, FeePriority::Medium)
             .await
             .unwrap();
         assert_eq!(fee, 7680);
 
         let fee = client
-            .estimate_fees(
-                &wallet,
-                &DUMMY_STX_TRANSFER_PAYLOAD,
-                FeePriority::High,
-                false,
-            )
+            .estimate_fees(&wallet, &DUMMY_STX_TRANSFER_PAYLOAD, FeePriority::High)
             .await
             .unwrap();
         assert_eq!(fee, 25505);

signer/src/stacks/wallet.rs

@@ -345,6 +345,11 @@ impl MultisigTx {
         &self.tx
     }
 
+    /// Return the message digest associated with the transaction
+    pub fn digest(&self) -> &Message {
+        &self.digest
+    }
+
     /// Return the total number of signatures that have been received so
     /// far for this transaction.
     pub fn num_signatures(&self) -> u16 {
@@ -402,11 +407,7 @@ impl MultisigTx {
 /// given payload.
 ///
 /// This function is very unlikely to fail in practice.
-pub fn get_full_tx_size<T>(
-    payload: &T,
-    wallet: &SignerWallet,
-    require_all_signatures: bool,
-) -> Result<u64, Error>
+pub fn get_full_tx_size<T>(payload: &T, wallet: &SignerWallet) -> Result<u64, Error>
 where
     T: AsTxPayload,
 {
@@ -436,14 +437,11 @@ where
         0,
     )?;
 
-    let sigantures_required = if require_all_signatures {
-        wallet.num_signers()
-    } else {
-        wallet.signatures_required
-    } as usize;
-
     let mut multisig_tx = MultisigTx::new_tx(payload, &wallet, 0);
-    for private_key in private_keys.iter().take(sigantures_required) {
+    for private_key in private_keys
+        .iter()
+        .take(wallet.signatures_required as usize)
+    {
         let signature = crate::signature::sign_stacks_tx(multisig_tx.tx(), private_key);
         // This won't fail, since this is a proper signature
         multisig_tx.add_signature(signature)?;
@@ -759,13 +757,10 @@ mod tests {
         SignerWallet::load_boostrap_wallet(&ctx.config().signer).unwrap();
     }
 
-    #[test_case(1, 1, true)]
-    #[test_case(1, 1, false)]
-    #[test_case(2, 3, true)]
-    #[test_case(2, 3, false)]
-    #[test_case(11, 15, true)]
-    #[test_case(11, 15, false)]
-    fn can_get_full_tx_size(signatures_required: u16, num_keys: u16, require_all_signatures: bool) {
+    #[test_case(1, 1)]
+    #[test_case(2, 3)]
+    #[test_case(11, 15)]
+    fn can_get_full_tx_size(signatures_required: u16, num_keys: u16) {
         const BASE_TX_SIZE: u64 = 55;
         const SIGNATURE_SIZE: u64 = 66;
         const PUBKEY_SIZE: u64 = 34;
@@ -783,18 +778,12 @@ mod tests {
 
         let payload_size = payload.tx_payload().serialize_to_vec().len() as u64;
 
-        let signatures_used = if require_all_signatures {
-            num_keys
-        } else {
-            signatures_required
-        };
-
         let expected_size = BASE_TX_SIZE
             + payload_size
-            + (signatures_used as u64 * SIGNATURE_SIZE)
-            + ((num_keys - signatures_used) as u64 * PUBKEY_SIZE);
+            + (signatures_required as u64 * SIGNATURE_SIZE)
+            + ((num_keys - signatures_required) as u64 * PUBKEY_SIZE);
 
-        let size = get_full_tx_size(&payload, &wallet, require_all_signatures).unwrap();
+        let size = get_full_tx_size(&payload, &wallet).unwrap();
 
         assert_eq!(size, expected_size);
     }

signer/src/testing/block_observer.rs

@@ -407,13 +407,7 @@ impl StacksInteract for TestHarness {
         })
     }
 
-    async fn estimate_fees<T>(
-        &self,
-        _: &SignerWallet,
-        _: &T,
-        _: FeePriority,
-        _: bool,
-    ) -> Result<u64, Error>
+    async fn estimate_fees<T>(&self, _: &SignerWallet, _: &T, _: FeePriority) -> Result<u64, Error>
     where
         T: crate::stacks::contracts::AsTxPayload,
     {

signer/src/testing/context.rs

@@ -438,15 +438,14 @@ impl StacksInteract for WrappedMock<MockStacksInteract> {
         wallet: &SignerWallet,
         payload: &T,
         priority: FeePriority,
-        require_all_signatures: bool,
     ) -> Result<u64, Error>
     where
         T: AsTxPayload + Send + Sync,
     {
         self.inner
             .lock()
             .await
-            .estimate_fees(wallet, payload, priority, require_all_signatures)
+            .estimate_fees(wallet, payload, priority)
             .await
     }
 

signer/src/testing/transaction_coordinator.rs

@@ -513,7 +513,7 @@ where
                 client
                     .expect_estimate_fees()
                     .times(5)
-                    .returning(|_, _, _, _| Box::pin(async { Ok(123000) }));
+                    .returning(|_, _, _| Box::pin(async { Ok(123000) }));
 
                 client.expect_get_account().times(5).returning(|_| {
                     Box::pin(async {

signer/src/transaction_coordinator.rs

@@ -43,6 +43,7 @@ use crate::metrics::Metrics;
 use crate::metrics::BITCOIN_BLOCKCHAIN;
 use crate::metrics::STACKS_BLOCKCHAIN;
 use crate::network;
+use crate::signature::RecoverableEcdsaSignature as _;
 use crate::signature::TaprootSignature;
 use crate::stacks::api::FeePriority;
 use crate::stacks::api::GetNakamotoStartHeight;
@@ -719,7 +720,7 @@ where
         let tx_fee = self
             .context
             .get_stacks_client()
-            .estimate_fees(wallet, &contract_call, FeePriority::High, true)
+            .estimate_fees(wallet, &contract_call, FeePriority::High)
             .await?;
 
         let multi_tx = MultisigTx::new_tx(&contract_call, wallet, tx_fee);
@@ -872,7 +873,7 @@ where
         let tx_fee = self
             .context
             .get_stacks_client()
-            .estimate_fees(wallet, &contract_call, FeePriority::High, false)
+            .estimate_fees(wallet, &contract_call, FeePriority::High)
             .await?;
 
         let multi_tx = MultisigTx::new_tx(&contract_call, wallet, tx_fee);
@@ -914,7 +915,7 @@ where
             wallet.signatures_required()
         } else {
             wallet.num_signers()
-        };
+        } as usize;
 
         // We ask for the signers to sign our transaction (including
         // ourselves, via our tx signer event loop)
@@ -929,7 +930,9 @@ where
         tokio::pin!(signal_stream);
 
         let future = async {
-            while multi_tx.num_signatures() < signatures_required {
+            let mut pending_signer = wallet.public_keys().clone();
+
+            while pending_signer.len() < signatures_required {
                 // If signal_stream.next() returns None then one of the
                 // underlying streams has closed. That means either the
                 // network stream, the internal message stream, or the
@@ -954,6 +957,19 @@ where
                     _ => continue,
                 };
 
+                let enough_signatures = multi_tx.num_signatures() >= wallet.signatures_required();
+                let recovered_key = sig.signature.recover_ecdsa(multi_tx.digest());
+
+                if let Ok(key) = recovered_key {
+                    pending_signer.remove(&key);
+                }
+
+                // Stop collecting signatures once we have enough, but keep tracking responses
+                // from remaining signers for key rotation transactions
+                if enough_signatures {
+                    continue;
+                }
+
                 if let Err(error) = multi_tx.add_signature(sig.signature) {
                     tracing::warn!(
                         %txid,
@@ -1552,12 +1568,7 @@ where
         let tx_fee = self
             .context
             .get_stacks_client()
-            .estimate_fees(
-                wallet,
-                &contract_deploy.tx_payload(),
-                FeePriority::High,
-                false,
-            )
+            .estimate_fees(wallet, &contract_deploy.tx_payload(), FeePriority::High)
             .await?;
         let multi_tx = MultisigTx::new_tx(&contract_deploy, wallet, tx_fee);
         let tx = multi_tx.tx();

signer/tests/integration/contracts.rs

@@ -245,12 +245,7 @@ async fn estimate_tx_fees() {
     // transaction.
 
     let fee = client
-        .estimate_fees(
-            &testing::wallet::WALLET.0,
-            &payload,
-            FeePriority::Medium,
-            false,
-        )
+        .estimate_fees(&testing::wallet::WALLET.0, &payload, FeePriority::Medium)
         .await
         .unwrap();
     more_asserts::assert_gt!(fee, 0);