feat: consensus on successful DKG prior to rotate-keys submission (#1285

)

* update for wsts 11

* use wsts 11.0.0 from crates

* fix rebase

* use wsts-12.0.0 from crates

* add FrostCoordinator to wsts_state_machine so we can run signing rounds where all signers must participate

* wip

* run a dkg signing before deploying contracts

* new wsts commit

* wip

* seems to work

* working version

* remove dead code

* minor wsts logging tweaks

* rename wstsmessageid variant

* it works, now need to clean up

* downgrade wsts

* proto backwards compatability fixes

* some wsts cleanup/refactor

* think that's it

* add message support for a specific dkg wstsmessageid

* add a dkg wstsmessageid variant

* use block hash instead of random data

* fmt

* remove 100% requirement for stacks signing of rotate keys

* bump p256k1 to 7.2.2

* fix merge artifacts

* tracing fields

* storage mut

* lovely cascading changes

* remove wstsmessage.txid infavor of id

* remove save and prefer trait default impls

* remove unused trait methods

* logging stuff

* rename message ids

* use tracing constants

* remove stale comment

* remove box from some types

* missed dkg_begin_pause

* leftover dbg!()

* refactor some validation

* various pr comments

* mut thing

* remove unneeded allow(deprecated)

* confused merge tool

---------

Co-authored-by: Joey Yandle <xoloki@gmail.com>

Cargo.toml

@@ -52,7 +52,7 @@ lru = { version = "0.12.5", default-features = false }
 metrics = { version = "0.24.1", default-features = false }
 metrics-exporter-prometheus = { version = "0.16.1", default-features = false, features = ["http-listener"] }
 openssl = { version = "0.10.68", default-features = false, features = ["vendored"] }
-p256k1 = { version = "7.2.0", default-features = false }
+p256k1 = { version = "7.2.2", default-features = false }
 prost = { version = "0.13.4", default-features = false, features = ["derive"] }
 rand = { version = "0.8.5", default-features = false }
 reqwest = { version = "0.11.27", default-features = false, features = ["json"] }

docker/docker-compose.yml

@@ -478,7 +478,7 @@ services:
       context: stacks-api
       args:
         GIT_URI: "https://github.com/hirosystems/stacks-blockchain-api.git"
-        GIT_BRANCH: "v8.0.3"
+        GIT_BRANCH: "v8.5.0"
     ports:
       - 3999:3999
       - 3700:3700
@@ -513,7 +513,7 @@ services:
       context: stacks-explorer
       args:
         GIT_URI: "https://github.com/hirosystems/explorer.git"
-        GIT_BRANCH: "v1.211.2"
+        GIT_BRANCH: "v1.245.0"
     ports:
       - 3020:3000
     depends_on:

signer/src/error.rs

@@ -7,6 +7,7 @@ use crate::blocklist_client::BlocklistClientError;
 use crate::codec;
 use crate::emily_client::EmilyClientError;
 use crate::keys::PublicKey;
+use crate::keys::PublicKeyXOnly;
 use crate::stacks::contracts::DepositValidationError;
 use crate::stacks::contracts::RotateKeysValidationError;
 use crate::stacks::contracts::WithdrawalAcceptValidationError;
@@ -15,6 +16,26 @@ use crate::storage::model::SigHash;
 /// Top-level signer error
 #[derive(Debug, thiserror::Error)]
 pub enum Error {
+    /// We have received a request/response which has been deemed invalid in
+    /// the current context.
+    #[error("invalid signing request")]
+    InvalidSigningOperation,
+
+    /// The pre-rotate-key frost verification signing round was not reported as
+    /// successful.
+    #[error("rotate-key frost verification signing round not reported as successful")]
+    FrostVerificationNotSuccessful,
+
+    /// No WSTS FROST state machine was found for the given aggregate key. This
+    /// state machine is used during the DKG verification signing round
+    /// following DKG.
+    #[error("no state machine found for frost signing round for the given aggregate key: {0}")]
+    MissingFrostStateMachine(PublicKeyXOnly),
+
+    /// Expected two aggregate keys to match, but they did not.
+    #[error("two aggregate keys were expected to match but did not: {0:?}, {1:?}")]
+    AggregateKeyMismatch(Box<PublicKeyXOnly>, Box<PublicKeyXOnly>),
+
     /// The aggregate key for the given block hash could not be determined.
     #[error("the signer set aggregate key could not be determined for bitcoin block {0}")]
     MissingAggregateKey(bitcoin::BlockHash),

signer/src/main.rs

@@ -1,4 +1,3 @@
-use std::num::NonZeroUsize;
 use std::path::PathBuf;
 use std::sync::atomic::AtomicU64;
 use std::sync::atomic::Ordering;
@@ -10,7 +9,6 @@ use axum::http::Response;
 use cfg_if::cfg_if;
 use clap::Parser;
 use clap::ValueEnum;
-use lru::LruCache;
 use signer::api;
 use signer::api::ApiState;
 use signer::bitcoin::rpc::BitcoinCoreClient;
@@ -315,25 +313,9 @@ async fn run_block_observer(ctx: impl Context) -> Result<(), Error> {
 
 /// Run the transaction signer event-loop.
 async fn run_transaction_signer(ctx: impl Context) -> Result<(), Error> {
-    let config = ctx.config().clone();
     let network = P2PNetwork::new(&ctx);
 
-    // The _ as usize cast is fine, since we know that
-    // MAX_SIGNER_STATE_MACHINES is less than u32::MAX, and we only support
-    // running this binary on 32 or 64-bit CPUs.
-    let max_state_machines = NonZeroUsize::new(signer::MAX_SIGNER_STATE_MACHINES as usize)
-        .ok_or(Error::TypeConversion)?;
-
-    let signer = transaction_signer::TxSignerEventLoop {
-        network,
-        context: ctx.clone(),
-        context_window: config.signer.context_window,
-        threshold: config.signer.bootstrap_signatures_required.into(),
-        rng: rand::thread_rng(),
-        signer_private_key: config.signer.private_key,
-        wsts_state_machines: LruCache::new(max_state_machines),
-        dkg_begin_pause: config.signer.dkg_begin_pause.map(Duration::from_secs),
-    };
+    let signer = transaction_signer::TxSignerEventLoop::new(ctx, network, rand::thread_rng())?;
 
     signer.run().await
 }

signer/src/stacks/wallet.rs

@@ -422,7 +422,9 @@ where
         .collect();
 
     // This will only fail if we get very unlucky with private keys that we
-    // generate.
+    // generate. We create a new wallet so that we don't alter the state of the
+    // wallet that was passed in, which will increment nonces for new
+    // transactions.
     let wallet = SignerWallet::new(
         &public_keys,
         wallet.signatures_required,

signer/src/testing/transaction_signer.rs

@@ -60,6 +60,8 @@ where
                 threshold,
                 rng,
                 dkg_begin_pause: None,
+                dkg_verification_state_machines: LruCache::new(NonZeroUsize::new(5).unwrap()),
+                dkg_verification_results: LruCache::new(NonZeroUsize::new(5).unwrap()),
             },
             context,
         }

signer/src/transaction_coordinator.rs

@@ -60,6 +60,7 @@ use crate::storage::model;
 use crate::storage::model::StacksTxId;
 use crate::storage::DbRead as _;
 use crate::wsts_state_machine::FireCoordinator;
+use crate::wsts_state_machine::FrostCoordinator;
 use crate::wsts_state_machine::WstsCoordinator;
 
 use bitcoin::hashes::Hash as _;
@@ -376,19 +377,27 @@ where
         }
 
         let wallet = self.get_signer_wallet(bitcoin_chain_tip).await?;
+
         // current_aggregate_key define which wallet can sign stacks tx interacting
         // with the registry smart contract; fallbacks to `aggregate_key` if it's
         // the first rotate key tx.
         let signing_key = &current_aggregate_key.unwrap_or(*aggregate_key);
 
-        self.construct_and_sign_rotate_key_transaction(
-            bitcoin_chain_tip,
-            signing_key,
-            &last_dkg.aggregate_key,
-            &wallet,
-        )
-        .await
-        .map(|_| ())
+        tracing::info!("preparing to submit a rotate-key transaction");
+        let txid = self
+            .construct_and_sign_rotate_key_transaction(
+                bitcoin_chain_tip,
+                signing_key,
+                &last_dkg.aggregate_key,
+                &wallet,
+            )
+            .await
+            .inspect_err(
+                |error| tracing::error!(%error, "failed to sign or submit rotate-key transaction"),
+            )?;
+
+        tracing::info!(%txid, "rotate-key transaction submitted successfully");
+        Ok(())
     }
 
     /// Constructs a BitcoinPreSignRequest from the given transaction package and
@@ -703,7 +712,10 @@ where
         ));
 
         // Rotate key transactions should be done as soon as possible, so
-        // we set the fee rate to the high priority fee.
+        // we set the fee rate to the high priority fee. We also require
+        // signatures from all signers, so we specify the total signer count
+        // as the number of signatures to include in the estimation transaction
+        // as each signature increases the transaction size.
         let tx_fee = self
             .context
             .get_stacks_client()
@@ -713,6 +725,51 @@ where
         let multi_tx = MultisigTx::new_tx(&contract_call, wallet, tx_fee);
         let tx = multi_tx.tx();
 
+        // We run a DKG signing round on the current bitcoin chain tip block
+        // hash using the new aggregate key using the FROST coordinator, which
+        // requires 100% signing participation vs. FIRE which only uses
+        // {threshold} signers.
+        //
+        // The idea behind this is that since the rotate-keys contract call is a
+        // Stacks transaction and thus only signed using the signers' private
+        // keys, we have no guarantees at this point that there wasn't a fault
+        // in the DKG process. By running a FROST signing round, we can
+        // cryptographically assert that all signers have signed with the new
+        // aggregate key, and thus have valid private shares before we proceed
+        // with the actual rotate keys transaction.
+        //
+        // Note that while we specify the threshold as `signatures_required` in
+        // the coordinator below, the FROST coordinator implicitly requires all
+        // signers to participate.
+        tracing::info!("running a FROST signing round on random data to assert that all signers have signed with the new aggregate key");
+        let mut coordinator_state_machine = FrostCoordinator::load(
+            &self.context.get_storage(),
+            rotate_key_aggregate_key.into(),
+            wallet.public_keys().iter().cloned(),
+            wallet.signatures_required(),
+            self.private_key,
+        )
+        .await?;
+
+        // We use the current bitcoin chain tip block hash as the data to sign
+        // as it is benign and can be validated by the signers. This may need
+        // to change in the future if we de-couple DKG from blocks.
+        let to_sign = bitcoin_chain_tip.as_byte_array().as_slice();
+        self.coordinate_signing_round(
+            bitcoin_chain_tip,
+            &mut coordinator_state_machine,
+            WstsMessageId::DkgVerification(*rotate_key_aggregate_key),
+            to_sign,
+            SignatureType::Schnorr
+        ).await
+        .inspect_err(|error| {
+            tracing::error!(%error, "failed to assert that all signers have signed random data with the new aggregate key; aborting");
+        })?;
+        tracing::info!(
+            "all signers have signed random data with the new aggregate key; proceeding"
+        );
+
+        // We can now proceed with the actual rotate key transaction.
         let sign_request = StacksTransactionSignRequest {
             aggregate_key: *aggregate_key,
             contract_tx: contract_call.into(),
@@ -758,7 +815,10 @@ where
         )
         .increment(1);
 
-        match self.context.get_stacks_client().submit_tx(&tx?).await {
+        // Submit the transaction to the Stacks node
+        let submit_tx_result = self.context.get_stacks_client().submit_tx(&tx?).await;
+
+        match submit_tx_result {
             Ok(SubmitTxResponse::Acceptance(txid)) => Ok(txid.into()),
             Ok(SubmitTxResponse::Rejection(err)) => Err(err.into()),
             Err(err) => Err(err),
@@ -917,12 +977,13 @@ where
         let msg = sighashes.signers.to_raw_hash().to_byte_array();
 
         let txid = transaction.tx.compute_txid();
+        let message_id = txid.into();
         let instant = std::time::Instant::now();
         let signature = self
             .coordinate_signing_round(
                 bitcoin_chain_tip,
                 &mut coordinator_state_machine,
-                txid.into(),
+                message_id,
                 &msg,
                 SignatureType::Taproot(None),
             )
@@ -963,7 +1024,7 @@ where
                 .coordinate_signing_round(
                     bitcoin_chain_tip,
                     &mut coordinator_state_machine,
-                    txid.into(),
+                    message_id,
                     &msg,
                     SignatureType::Schnorr,
                 )
@@ -1195,7 +1256,7 @@ where
                 match coordinator_state_machine.process_message(&msg) {
                     Ok(val) => val,
                     Err(err) => {
-                        tracing::warn!(?msg, reason = %err, "ignoring packet");
+                        tracing::warn!(?msg, reason = %err, "ignoring message");
                         continue;
                     }
                 };