Merge pull request #6249 from fdefelici/refactor/clean-op-signer

refactor: clean op signer

Author: fdefelici

stacks-node/src/burnchains/bitcoin_regtest_controller.rs

@@ -3141,7 +3141,6 @@ mod tests {
                 "9e446f6b0c6a96cf2190e54bcd5a8569c3e386f091605499464389b8d4e0bfc201",
             )
             .unwrap(),
-            false,
         );
         assert!(btc_controller.serialize_tx(
             StacksEpochId::Epoch25,

stacks-node/src/keychain.rs

@@ -228,7 +228,7 @@ impl Keychain {
 
     /// Create a BurnchainOpSigner representation of this keychain
     pub fn generate_op_signer(&self) -> BurnchainOpSigner {
-        BurnchainOpSigner::new(self.get_secret_key(), false)
+        BurnchainOpSigner::new(self.get_secret_key())
     }
 }
 
@@ -451,7 +451,7 @@ mod tests {
         }
 
         pub fn generate_op_signer(&self) -> BurnchainOpSigner {
-            BurnchainOpSigner::new(self.secret_keys[0], false)
+            BurnchainOpSigner::new(self.secret_keys[0])
         }
     }
 

stacks-node/src/main.rs

@@ -366,11 +366,11 @@ fn main() {
             let keychain = Keychain::default(seed);
             println!(
                 "Hex formatted secret key: {}",
-                keychain.generate_op_signer().get_sk_as_hex()
+                keychain.generate_op_signer().get_secret_key_as_hex()
             );
             println!(
                 "WIF formatted secret key: {}",
-                keychain.generate_op_signer().get_sk_as_wif()
+                keychain.generate_op_signer().get_secret_key_as_wif()
             );
             return;
         }

stacks-node/src/operations.rs

@@ -2,38 +2,79 @@ use stacks::burnchains::PrivateKey;
 use stacks_common::util::hash::hex_bytes;
 use stacks_common::util::secp256k1::{MessageSignature, Secp256k1PrivateKey, Secp256k1PublicKey};
 
+/// A signer used for burnchain operations, which manages a private key and provides
+/// functionality to derive public keys, sign messages, and export keys in different formats.
+///
+/// The signer can be "disposed" to prevent further use of the private key (e.g., for security
+/// or lifecycle management).
 pub struct BurnchainOpSigner {
+    /// The Secp256k1 private key used for signing operations.
     secret_key: Secp256k1PrivateKey,
-    is_one_off: bool,
+    /// Indicates whether the signer has been disposed and can no longer be used for signing.
     is_disposed: bool,
-    usages: u8,
 }
 
 impl BurnchainOpSigner {
-    pub fn new(secret_key: Secp256k1PrivateKey, is_one_off: bool) -> BurnchainOpSigner {
+    /// Creates a new `BurnchainOpSigner` from the given private key.
+    ///
+    /// # Arguments
+    ///
+    /// * `secret_key` - A Secp256k1 private key used for signing.
+    ///
+    /// # Returns
+    ///
+    /// A new instance of `BurnchainOpSigner`.
+    pub fn new(secret_key: Secp256k1PrivateKey) -> Self {
         BurnchainOpSigner {
             secret_key,
-            usages: 0,
-            is_one_off,
             is_disposed: false,
         }
     }
 
-    pub fn get_sk_as_wif(&self) -> String {
+    /// Returns the private key encoded as a Wallet Import Format (WIF) string.
+    ///
+    /// This format is commonly used for exporting private keys in Bitcoin-related systems.
+    ///
+    /// # Returns
+    ///
+    /// A WIF-encoded string representation of the private key.
+    pub fn get_secret_key_as_wif(&self) -> String {
         let hex_encoded = self.secret_key.to_hex();
         let mut as_bytes = hex_bytes(&hex_encoded).unwrap();
         as_bytes.insert(0, 0x80);
         stacks_common::address::b58::check_encode_slice(&as_bytes)
     }
 
-    pub fn get_sk_as_hex(&self) -> String {
+    /// Returns the private key encoded as a hexadecimal string.
+    ///
+    /// # Returns
+    ///
+    /// A hex-encoded string representation of the private key.
+    pub fn get_secret_key_as_hex(&self) -> String {
         self.secret_key.to_hex()
     }
 
+    /// Derives and returns the public key associated with the private key.
+    ///
+    /// # Returns
+    ///
+    /// A `Secp256k1PublicKey` corresponding to the private key.
     pub fn get_public_key(&mut self) -> Secp256k1PublicKey {
         Secp256k1PublicKey::from_private(&self.secret_key)
     }
 
+    /// Signs the given message hash using the private key.
+    ///
+    /// If the signer has been disposed, no signature will be produced.
+    ///
+    /// # Arguments
+    ///
+    /// * `hash` - A byte slice representing the hash of the message to sign.
+    ///            This must be exactly **32 bytes** long, as required by the Secp256k1 signing algorithm.
+    /// # Returns
+    ///
+    /// `Some(MessageSignature)` if signing was successful, or `None` if the signer
+    /// is disposed or signing failed.
     pub fn sign_message(&mut self, hash: &[u8]) -> Option<MessageSignature> {
         if self.is_disposed {
             debug!("Signer is disposed");
@@ -47,15 +88,13 @@ impl BurnchainOpSigner {
                 return None;
             }
         };
-        self.usages += 1;
-
-        if self.is_one_off && self.usages == 1 {
-            self.is_disposed = true;
-        }
 
         Some(signature)
     }
 
+    /// Marks the signer as disposed, preventing any further signing operations.
+    ///
+    /// Once disposed, the private key can no longer be used to sign messages.
     pub fn dispose(&mut self) {
         self.is_disposed = true;
     }
@@ -77,26 +116,83 @@ impl BurnchainOpSigner {
     /// This is useful in testing scenarios where you need a fresh, undisposed copy
     /// of a signer without recreating the private key.
     pub fn undisposed(&self) -> Self {
-        Self::new(self.secret_key, false)
+        Self::new(self.secret_key)
     }
 }
 
 #[cfg(test)]
-mod test {
-    use stacks_common::util::secp256k1::Secp256k1PrivateKey;
+mod tests {
+    use super::*;
 
-    use super::BurnchainOpSigner;
+    #[test]
+    fn test_get_secret_key_as_wif() {
+        let priv_key_hex = "0c28fca386c7a227600b2fe50b7cae11ec86d3bf1fbe471be89827e19d72aa1d";
+        let expected_wif = "5HueCGU8rMjxEXxiPuD5BDku4MkFqeZyd4dZ1jvhTVqvbTLvyTJ";
+
+        let secret = Secp256k1PrivateKey::from_hex(priv_key_hex).unwrap();
+        let op_signer = BurnchainOpSigner::new(secret);
+        assert_eq!(expected_wif, &op_signer.get_secret_key_as_wif());
+    }
 
     #[test]
-    fn test_wif() {
-        let examples = [(
-            "0C28FCA386C7A227600B2FE50B7CAE11EC86D3BF1FBE471BE89827E19D72AA1D",
-            "5HueCGU8rMjxEXxiPuD5BDku4MkFqeZyd4dZ1jvhTVqvbTLvyTJ",
-        )];
-        for (secret_key, expected_wif) in examples.iter() {
-            let secp_k = Secp256k1PrivateKey::from_hex(secret_key).unwrap();
-            let op_signer = BurnchainOpSigner::new(secp_k, false);
-            assert_eq!(expected_wif, &op_signer.get_sk_as_wif());
-        }
+    fn test_get_secret_key_as_hex() {
+        let priv_key_hex = "0c28fca386c7a227600b2fe50b7cae11ec86d3bf1fbe471be89827e19d72aa1d";
+        let expected_hex = priv_key_hex;
+
+        let secp_k = Secp256k1PrivateKey::from_hex(priv_key_hex).unwrap();
+        let op_signer = BurnchainOpSigner::new(secp_k);
+        assert_eq!(expected_hex, op_signer.get_secret_key_as_hex());
+    }
+
+    #[test]
+    fn test_get_public_key() {
+        let priv_key_hex = "0c28fca386c7a227600b2fe50b7cae11ec86d3bf1fbe471be89827e19d72aa1d";
+        let expected_hex = "04d0de0aaeaefad02b8bdc8a01a1b8b11c696bd3d66a2c5f10780d95b7df42645cd85228a6fb29940e858e7e55842ae2bd115d1ed7cc0e82d934e929c97648cb0a";
+
+        let secp_k = Secp256k1PrivateKey::from_hex(priv_key_hex).unwrap();
+        let mut op_signer = BurnchainOpSigner::new(secp_k);
+        assert_eq!(expected_hex, op_signer.get_public_key().to_hex());
+    }
+
+    #[test]
+    fn test_sign_message_ok() {
+        let priv_key_hex = "0c28fca386c7a227600b2fe50b7cae11ec86d3bf1fbe471be89827e19d72aa1d";
+        let message = &[0u8; 32];
+        let expected_msg_sig = "00b911e6cf9c49b738c4a0f5e33c003fa5b74a00ddc68e574e9f1c3504f6ba7e84275fd62773978cc8165f345cc3f691cf68be274213d552e79af39998df61273f";
+
+        let secp_k = Secp256k1PrivateKey::from_hex(priv_key_hex).unwrap();
+        let mut op_signer = BurnchainOpSigner::new(secp_k);
+
+        let msg_sig = op_signer
+            .sign_message(message)
+            .expect("Message should be signed!");
+
+        assert_eq!(expected_msg_sig, msg_sig.to_hex());
+    }
+
+    #[test]
+    fn test_sign_message_fails_due_to_hash_length() {
+        let priv_key_hex = "0c28fca386c7a227600b2fe50b7cae11ec86d3bf1fbe471be89827e19d72aa1d";
+        let message = &[0u8; 20];
+
+        let secp_k = Secp256k1PrivateKey::from_hex(priv_key_hex).unwrap();
+        let mut op_signer = BurnchainOpSigner::new(secp_k);
+
+        let result = op_signer.sign_message(message);
+        assert!(result.is_none());
+    }
+
+    #[test]
+    fn test_sign_message_fails_due_to_disposal() {
+        let priv_key_hex = "0c28fca386c7a227600b2fe50b7cae11ec86d3bf1fbe471be89827e19d72aa1d";
+        let message = &[0u8; 32];
+
+        let secp_k = Secp256k1PrivateKey::from_hex(priv_key_hex).unwrap();
+        let mut op_signer = BurnchainOpSigner::new(secp_k);
+
+        op_signer.dispose();
+
+        let result = op_signer.sign_message(message);
+        assert!(result.is_none());
     }
 }

stacks-node/src/tests/epoch_21.rs

@@ -697,7 +697,7 @@ fn transition_fixes_bitcoin_rigidity() {
         burn_header_hash: BurnchainHeaderHash([0u8; 32]),
     };
 
-    let mut spender_signer = BurnchainOpSigner::new(spender_sk, false);
+    let mut spender_signer = BurnchainOpSigner::new(spender_sk);
 
     assert!(
         btc_regtest_controller
@@ -849,7 +849,7 @@ fn transition_fixes_bitcoin_rigidity() {
         burn_header_hash: BurnchainHeaderHash([0u8; 32]),
     };
 
-    let mut spender_signer = BurnchainOpSigner::new(spender_sk, false);
+    let mut spender_signer = BurnchainOpSigner::new(spender_sk);
 
     assert!(
         btc_regtest_controller
@@ -923,7 +923,7 @@ fn transition_fixes_bitcoin_rigidity() {
         burn_header_hash: BurnchainHeaderHash([0u8; 32]),
     };
 
-    let mut spender_signer = BurnchainOpSigner::new(spender_2_sk, false);
+    let mut spender_signer = BurnchainOpSigner::new(spender_2_sk);
 
     btc_regtest_controller
         .submit_manual(
@@ -989,7 +989,7 @@ fn transition_fixes_bitcoin_rigidity() {
         burn_header_hash: BurnchainHeaderHash([0u8; 32]),
     };
 
-    let mut spender_signer = BurnchainOpSigner::new(spender_2_sk, false);
+    let mut spender_signer = BurnchainOpSigner::new(spender_2_sk);
 
     btc_regtest_controller
         .submit_manual(

stacks-node/src/tests/nakamoto_integrations.rs

@@ -3553,7 +3553,7 @@ fn vote_for_aggregate_key_burn_op() {
             burn_header_hash: BurnchainHeaderHash::zero(),
         });
 
-    let mut signer_burnop_signer = BurnchainOpSigner::new(signer_sk, false);
+    let mut signer_burnop_signer = BurnchainOpSigner::new(signer_sk);
     assert!(
         btc_regtest_controller
             .submit_operation(
@@ -4739,10 +4739,10 @@ fn burn_ops_integration_test() {
         "reward_cycle" => reward_cycle,
     );
 
-    let mut signer_burnop_signer_1 = BurnchainOpSigner::new(signer_sk_1, false);
-    let mut signer_burnop_signer_2 = BurnchainOpSigner::new(signer_sk_2, false);
-    let mut stacker_burnop_signer_1 = BurnchainOpSigner::new(stacker_sk_1, false);
-    let mut stacker_burnop_signer_2 = BurnchainOpSigner::new(stacker_sk_2, false);
+    let mut signer_burnop_signer_1 = BurnchainOpSigner::new(signer_sk_1);
+    let mut signer_burnop_signer_2 = BurnchainOpSigner::new(signer_sk_2);
+    let mut stacker_burnop_signer_1 = BurnchainOpSigner::new(stacker_sk_1);
+    let mut stacker_burnop_signer_2 = BurnchainOpSigner::new(stacker_sk_2);
 
     info!(
         "Before stack-stx op, signer 1 total: {}",

stacks-node/src/tests/neon_integrations.rs

@@ -2019,7 +2019,7 @@ fn stx_transfer_btc_integration_test() {
         burn_header_hash: BurnchainHeaderHash([0u8; 32]),
     };
 
-    let mut spender_signer = BurnchainOpSigner::new(spender_sk, false);
+    let mut spender_signer = BurnchainOpSigner::new(spender_sk);
 
     assert!(
         btc_regtest_controller
@@ -2090,7 +2090,7 @@ fn stx_transfer_btc_integration_test() {
         burn_header_hash: BurnchainHeaderHash([0u8; 32]),
     };
 
-    let mut spender_signer = BurnchainOpSigner::new(spender_2_sk, false);
+    let mut spender_signer = BurnchainOpSigner::new(spender_2_sk);
 
     btc_regtest_controller
         .submit_manual(
@@ -2288,7 +2288,7 @@ fn stx_delegate_btc_integration_test() {
         until_burn_height: None,
     };
 
-    let mut spender_signer = BurnchainOpSigner::new(spender_sk, false);
+    let mut spender_signer = BurnchainOpSigner::new(spender_sk);
     assert!(
         btc_regtest_controller
             .submit_operation(
@@ -2656,7 +2656,7 @@ fn stack_stx_burn_op_test() {
         burn_header_hash: BurnchainHeaderHash::zero(),
     });
 
-    let mut spender_signer_1 = BurnchainOpSigner::new(signer_sk_1, false);
+    let mut spender_signer_1 = BurnchainOpSigner::new(signer_sk_1);
     assert!(
         btc_regtest_controller
             .submit_operation(
@@ -2684,7 +2684,7 @@ fn stack_stx_burn_op_test() {
         burn_header_hash: BurnchainHeaderHash::zero(),
     });
 
-    let mut spender_signer_2 = BurnchainOpSigner::new(signer_sk_2, false);
+    let mut spender_signer_2 = BurnchainOpSigner::new(signer_sk_2);
     assert!(
         btc_regtest_controller
             .submit_operation(
@@ -3041,7 +3041,7 @@ fn vote_for_aggregate_key_burn_op_test() {
             burn_header_hash: BurnchainHeaderHash::zero(),
         });
 
-    let mut spender_signer = BurnchainOpSigner::new(signer_sk, false);
+    let mut spender_signer = BurnchainOpSigner::new(signer_sk);
     assert!(
         btc_regtest_controller
             .submit_operation(

stacks-node/src/tests/signer/v0.rs

@@ -3738,7 +3738,7 @@ fn tx_replay_btc_on_stx_invalidation() {
     let num_signers = 5;
     let sender_sk = Secp256k1PrivateKey::from_seed("sender_1".as_bytes());
     let sender_addr = tests::to_addr(&sender_sk);
-    let mut sender_burnop_signer = BurnchainOpSigner::new(sender_sk, false);
+    let mut sender_burnop_signer = BurnchainOpSigner::new(sender_sk);
     let send_amt = 100;
     let send_fee = 180;
     let recipient_sk = Secp256k1PrivateKey::from_seed("recipient_1".as_bytes());