feat: DKG verification pre-key-rotation (#1301)

* 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

* pop stash

* wip

* working verifications

* saving

* fixing tests

* seems to work

* remove const generic, use 0 amount and add test for random keypair

* vet bitcoinconsensus

* working on cleaning up

* fix merge artifacts

* tracing fields

* migration needs to update block hash/height

* 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)

* import some changes manually from parent branch to help a confused merge tool

* more diff-reducing imports

* confused merge tool

* reduce diff

* add validation for nonceresponse + signatureshareresponse

* newline

* pr comments

* pr comments utxo

* utxo comments

* remove error conversion method

* Squashed commit of the following:

commit 2168f58
Author: Cyle Witruk <[email protected]>
Date:   Thu Feb 6 16:08:22 2025 +0100

    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 <[email protected]>

* Change the DKG shares status type.
Lots of small follow-up modifications

* Update the migration query

* Fix up remaining queries

* Oops forgot this one

* Forgot this rename

* Rename the new status field to
match the new column

* Fix up the tests and add a new one

* Change the return value of some of the
queries and simplify the validation check. Also add a test.

* Change it back

* Update the tests

* Clean up the comments in the shares enum

* rename the rotate keys error variant

* Use a better error variant when extracting
the started_at from the state machine Id

* Remove some of our unused error variants

* Match the behavior in the in memory store
with the postgres implementation

* We do not need these errors anymore either

* address nits

---------

Co-authored-by: Joey Yandle <[email protected]>
Co-authored-by: djordon <[email protected]>
Co-authored-by: Francesco Leacche <[email protected]>

Cargo.toml

@@ -39,6 +39,7 @@ backoff = { version = "0.4.0", default-features = false, features = ["tokio"] }
 base64 = { version = "0.22.1", default-features = false, features = ["alloc"] }
 bincode = { version = "1.3.3", default-features = false }
 bitcoin = { version = "0.32.5", default-features = false, features = ["serde", "rand-std"] }
+bitcoinconsensus = { version = "0.106.0", default-features = false }
 bitcoincore-rpc = { version = "0.19.0", default-features = false }
 bitcoincore-rpc-json = { version = "0.19.0", default-features = false }
 bitcoincore-zmq = { version = "1.5.2", default-features = false, features = ["async"] }

signer/Cargo.toml

@@ -14,6 +14,7 @@ testing = ["fake", "mockall", "sbtc/testing"]
 aquamarine.workspace = true
 axum.workspace = true
 bitcoin.workspace = true
+bitcoinconsensus.workspace = true
 bitcoincore-rpc.workspace = true
 bitcoincore-rpc-json.workspace = true
 bitcoincore-zmq.workspace = true

signer/migrations/0012__dkg_verification_extensions.sql

@@ -0,0 +1,52 @@
+
+CREATE TYPE sbtc_signer.dkg_shares_status AS ENUM (
+    'unverified',
+    'verified',
+    'failed'
+);
+
+-- Add the new columns to the `dkg_shares` table. We're not adding indexes for
+-- now because the table is so small that the overhead likely outweighs the
+-- benefits.
+ALTER TABLE sbtc_signer.dkg_shares
+    ADD COLUMN dkg_shares_status sbtc_signer.dkg_shares_status,
+    ADD COLUMN started_at_bitcoin_block_hash BYTEA,
+    ADD COLUMN started_at_bitcoin_block_height BIGINT;
+
+
+UPDATE sbtc_signer.dkg_shares
+SET dkg_shares_status = 'unverified';
+
+-- These are all DKG shares associated scriptPubKeys that have been successfully spent
+UPDATE sbtc_signer.dkg_shares
+SET dkg_shares_status = 'verified'
+FROM sbtc_signer.bitcoin_tx_inputs
+WHERE sbtc_signer.dkg_shares.script_pubkey = sbtc_signer.bitcoin_tx_inputs.script_pubkey;
+
+
+-- Fill in the started at bitcoin blockhash and block height. The timestamp
+-- of when we write the DKG shares row to the database should correspond
+-- with the tenure of the block that started the DKG round.
+WITH block_times AS (
+    SELECT 
+        bb1.block_hash
+      , bb1.block_height
+      , bb1.created_at
+      , bb2.created_at AS ended_at
+    FROM sbtc_signer.bitcoin_blocks AS bb2
+    JOIN sbtc_signer.bitcoin_blocks AS bb1
+      ON bb2.parent_hash = bb1.block_hash
+)
+UPDATE sbtc_signer.dkg_shares
+SET 
+    started_at_bitcoin_block_hash = block_times.block_hash
+  , started_at_bitcoin_block_height = block_times.block_height
+FROM block_times
+WHERE sbtc_signer.dkg_shares.created_at >= block_times.created_at
+  AND sbtc_signer.dkg_shares.created_at < block_times.ended_at;
+
+-- Make the new column `NOT NULL` now that they should all have a value.
+ALTER TABLE sbtc_signer.dkg_shares
+    ALTER COLUMN dkg_shares_status SET NOT NULL,
+    ALTER COLUMN started_at_bitcoin_block_hash SET NOT NULL,
+    ALTER COLUMN started_at_bitcoin_block_height SET NOT NULL;

signer/src/bitcoin/utxo.rs

@@ -5,6 +5,7 @@ use std::ops::Deref as _;
 use std::sync::LazyLock;
 
 use bitcoin::absolute::LockTime;
+use bitcoin::consensus::Encodable;
 use bitcoin::hashes::Hash as _;
 use bitcoin::sighash::Prevouts;
 use bitcoin::sighash::SighashCache;
@@ -712,6 +713,20 @@ impl SignerUtxo {
     }
 }
 
+/// A struct for constructing a mock transaction that can be signed. This is
+/// used as part of the verification process after a new DKG round has been
+/// completed.
+///
+/// The Bitcoin transaction has the following layout:
+/// 1. The first input is spending the signers' UTXO.
+/// 2. There is only one output which is an OP_RETURN and an amount equal to 0.
+pub struct UnsignedMockTransaction {
+    /// The Bitcoin transaction that needs to be signed.
+    tx: Transaction,
+    /// The signers' UTXO used as an input to this transaction.
+    utxo: SignerUtxo,
+}
+
 /// Given a set of requests, create a BTC transaction that can be signed.
 ///
 /// This BTC transaction in this struct has correct amounts but no witness
@@ -802,6 +817,94 @@ impl SignatureHashes<'_> {
     }
 }
 
+impl UnsignedMockTransaction {
+    const AMOUNT: u64 = 0;
+
+    /// Construct an unsigned mock transaction.
+    ///
+    /// This will use the provided `aggregate_key` to construct
+    /// a [`Transaction`] with a single input and output with value 0.
+    pub fn new(signer_public_key: XOnlyPublicKey) -> Self {
+        let utxo = SignerUtxo {
+            outpoint: OutPoint::null(),
+            amount: Self::AMOUNT,
+            public_key: signer_public_key,
+        };
+
+        let tx = Transaction {
+            version: Version::TWO,
+            lock_time: LockTime::ZERO,
+            input: vec![utxo.as_tx_input(&DUMMY_SIGNATURE)],
+            output: vec![TxOut {
+                value: Amount::from_sat(Self::AMOUNT),
+                script_pubkey: ScriptBuf::new_op_return([]),
+            }],
+        };
+
+        Self { tx, utxo }
+    }
+
+    /// Gets the sighash for the signers' input UTXO which needs to be signed
+    /// before the transaction can be broadcast.
+    pub fn compute_sighash(&self) -> Result<TapSighash, Error> {
+        let prevouts = [self.utxo.as_tx_output()];
+        let mut sighasher = SighashCache::new(&self.tx);
+
+        sighasher
+            .taproot_key_spend_signature_hash(0, &Prevouts::All(&prevouts), TapSighashType::All)
+            .map_err(Into::into)
+    }
+
+    /// Tests if the provided taproot [`Signature`] is valid for spending the
+    /// signers' UTXO. This function will return  [`Error::BitcoinConsensus`]
+    /// error if the signature fails verification, passing the underlying error
+    /// from [`bitcoinconsensus`].
+    pub fn verify_signature(&self, signature: &Signature) -> Result<(), Error> {
+        // Create a copy of the transaction so that we don't modify the
+        // transaction stored in the struct.
+        let mut tx = self.tx.clone();
+
+        // Set the witness data on the input from the provided signature.
+        tx.input[0].witness = Witness::p2tr_key_spend(signature);
+
+        // Encode the transaction to bytes (needed by the bitcoinconsensus
+        // library).
+        let mut tx_bytes: Vec<u8> = Vec::new();
+        tx.consensus_encode(&mut tx_bytes)
+            .map_err(Error::BitcoinIo)?;
+
+        // Get the prevout for the signers' UTXO.
+        let prevout = self.utxo.as_tx_output();
+        let prevout_script_bytes = prevout.script_pubkey.as_script().as_bytes();
+
+        // Create the bitcoinconsensus UTXO object.
+        let prevout_utxo = bitcoinconsensus::Utxo {
+            script_pubkey: prevout_script_bytes.as_ptr(),
+            script_pubkey_len: prevout_script_bytes.len() as u32,
+            value: Self::AMOUNT as i64,
+        };
+
+        // We specify the flags to include all pre-taproot and taproot
+        // verifications explicitly.
+        // https://github.com/rust-bitcoin/rust-bitcoinconsensus/blob/master/src/lib.rs
+        let flags = bitcoinconsensus::VERIFY_ALL_PRE_TAPROOT | bitcoinconsensus::VERIFY_TAPROOT;
+
+        // Verify that the transaction updated with the provided signature can
+        // successfully spend the signers' UTXO. Note that the amount is not
+        // used in the verification process for taproot spends, only the
+        // signature.
+        bitcoinconsensus::verify_with_flags(
+            prevout_script_bytes,
+            Self::AMOUNT,
+            &tx_bytes,
+            Some(&[prevout_utxo]),
+            0,
+            flags,
+        )
+        .map_err(Error::BitcoinConsensus)
+    }
+}
+
 impl<'a> UnsignedTransaction<'a> {
     /// Construct an unsigned transaction.
     ///
@@ -1457,6 +1560,7 @@ mod tests {
     use std::str::FromStr;
 
     use super::*;
+    use bitcoin::key::TapTweak;
     use bitcoin::BlockHash;
     use bitcoin::CompressedPublicKey;
     use bitcoin::Txid;
@@ -1652,6 +1756,88 @@ mod tests {
         }
     }
 
+    /// This test verifies that our implementation of Bitcoin script
+    /// verification using [`bitcoinconsensus`] works as expected. This
+    /// functionality is used in the verification of WSTS signing after a new
+    /// DKG round has completed.
+    #[test]
+    fn mock_signer_utxo_signing_and_spending_verification() {
+        let secp = secp256k1::Secp256k1::new();
+
+        // Generate a key pair which will serve as the signers' aggregate key.
+        let secret_key = SecretKey::new(&mut OsRng);
+        let keypair = secp256k1::Keypair::from_secret_key(&secp, &secret_key);
+        let tweaked = keypair.tap_tweak(&secp, None);
+        let (aggregate_key, _) = keypair.x_only_public_key();
+
+        // Create a new transaction using the aggregate key.
+        let unsigned = UnsignedMockTransaction::new(aggregate_key);
+
+        let tapsig = unsigned
+            .compute_sighash()
+            .expect("failed to compute taproot sighash");
+
+        // Sign the taproot sighash.
+        let message = secp256k1::Message::from_digest_slice(tapsig.as_byte_array())
+            .expect("Failed to create message");
+
+        // [1] Verify the correct signature, which should succeed.
+        let schnorr_sig = secp.sign_schnorr(&message, &tweaked.to_inner());
+        let taproot_sig = bitcoin::taproot::Signature {
+            signature: schnorr_sig,
+            sighash_type: TapSighashType::All,
+        };
+        unsigned
+            .verify_signature(&taproot_sig)
+            .expect("signature verification failed");
+
+        // [2] Verify the correct signature, but with a different sighash type,
+        // which should fail.
+        let taproot_sig = bitcoin::taproot::Signature {
+            signature: schnorr_sig,
+            sighash_type: TapSighashType::None,
+        };
+        unsigned
+            .verify_signature(&taproot_sig)
+            .expect_err("signature verification should have failed");
+
+        // [3] Verify an incorrect signature with the correct sighash type,
+        // which should fail. In this case we've created the signature using
+        // the untweaked keypair.
+        let schnorr_sig = secp.sign_schnorr(&message, &keypair);
+        let taproot_sig = bitcoin::taproot::Signature {
+            signature: schnorr_sig,
+            sighash_type: TapSighashType::All,
+        };
+        unsigned
+            .verify_signature(&taproot_sig)
+            .expect_err("signature verification should have failed");
+
+        // [4] Verify an incorrect signature with the correct sighash type, which
+        // should fail. In this case we use a completely newly generated keypair.
+        let secret_key = SecretKey::new(&mut OsRng);
+        let keypair = secp256k1::Keypair::from_secret_key(&secp, &secret_key);
+        let schnorr_sig = secp.sign_schnorr(&message, &keypair);
+        let taproot_sig = bitcoin::taproot::Signature {
+            signature: schnorr_sig,
+            sighash_type: TapSighashType::All,
+        };
+        unsigned
+            .verify_signature(&taproot_sig)
+            .expect_err("signature verification should have failed");
+
+        // [5] Same as [4], but using its tweaked key.
+        let tweaked = keypair.tap_tweak(&secp, None);
+        let schnorr_sig = secp.sign_schnorr(&message, &tweaked.to_inner());
+        let taproot_sig = bitcoin::taproot::Signature {
+            signature: schnorr_sig,
+            sighash_type: TapSighashType::All,
+        };
+        unsigned
+            .verify_signature(&taproot_sig)
+            .expect_err("signature verification should have failed");
+    }
+
     #[ignore = "For generating the SOLO_(DEPOSIT|WITHDRAWAL)_SIZE constants"]
     #[test]
     fn create_deposit_only_tx() {

signer/src/block_observer.rs

@@ -694,6 +694,7 @@ mod tests {
     use crate::keys::PublicKey;
     use crate::keys::SignerScriptPubKey as _;
     use crate::storage;
+    use crate::storage::model::DkgSharesStatus;
     use crate::testing::block_observer::TestHarness;
     use crate::testing::context::*;
 
@@ -996,6 +997,9 @@ mod tests {
             public_shares: Vec::new(),
             signer_set_public_keys: vec![aggregate_key],
             signature_share_threshold: 1,
+            dkg_shares_status: DkgSharesStatus::Unverified,
+            started_at_bitcoin_block_hash: block_hash.into(),
+            started_at_bitcoin_block_height: 1,
         };
         storage.write_encrypted_dkg_shares(&shares).await.unwrap();
 

signer/src/error.rs

@@ -16,15 +16,33 @@ use crate::storage::model::SigHash;
 /// Top-level signer error
 #[derive(Debug, thiserror::Error)]
 pub enum Error {
+    /// Unexpected [`StateMachineId`] in the given context.
+    #[error("unexpected state machine id in the given context: {0:?}")]
+    UnexpectedStateMachineId(Box<crate::wsts_state_machine::StateMachineId>),
+
+    /// An IO error was returned from the [`bitcoin`] library. This is usually an
+    /// error that occurred during encoding/decoding of bitcoin types.
+    #[error("an io error was returned from the bitcoin library: {0}")]
+    BitcoinIo(#[source] bitcoin::io::Error),
+
+    /// An error was returned from the bitcoinconsensus library.
+    #[error("error returned from libbitcoinconsensus: {0}")]
+    BitcoinConsensus(bitcoinconsensus::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 mock transaction was found after DKG successfully completed. Spending
+    /// a signer UTXO locked by the new aggregate key could not be verified.
+    #[error("no mock transaction found when attempting to sign")]
+    MissingMockTransaction,
+
+    /// The rotate-key frost verification signing round failed for the aggregate
+    /// key.
+    #[error("rotate-key frost verification signing failed for aggregate key: {0}")]
+    DkgVerificationFailed(PublicKeyXOnly),
 
     /// No WSTS FROST state machine was found for the given aggregate key. This
     /// state machine is used during the DKG verification signing round