Merge branch 'next' into 4388-nakamoto-stacks-signer-should-store-its-party-shares-on-the-side-to-enable-restart

Author: netrome

Cargo.lock

@@ -31,6 +31,7 @@ stacks_common = { package = "stacks-common", path = "../stacks-common" }
 rstest = "0.17.0"
 rstest_reuse = "0.5.0"
 hashbrown = { workspace = true }
+mutants = "0.0.3"
 
 [dependencies.serde_json]
 version = "1.0"

clarity/Cargo.toml

@@ -45,6 +45,7 @@ impl TraitsResolver {
         TraitsResolver {}
     }
 
+    #[cfg_attr(test, mutants::skip)]
     pub fn run(&mut self, contract_ast: &mut ContractAST) -> ParseResult<()> {
         let mut referenced_traits = HashMap::new();
 

clarity/src/vm/ast/traits_resolver/mod.rs

@@ -786,6 +786,9 @@ impl TrackerData {
     /// `apply_updates` - tells this function to look for any changes in the cost voting contract
     ///   which would need to be applied. if `false`, just load the last computed cost state in this
     ///   fork.
+    /// TODO: #4587 add test for Err cases
+    /// Or keep the skip and remove the comment
+    #[cfg_attr(test, mutants::skip)]
     fn load_costs(&mut self, clarity_db: &mut ClarityDatabase, apply_updates: bool) -> Result<()> {
         clarity_db.begin();
         let epoch_id = clarity_db

clarity/src/vm/costs/mod.rs

@@ -1529,6 +1529,10 @@ impl TupleData {
         self.data_map.is_empty()
     }
 
+    ///TODO: #4587 create default for TupleData, then check if the mutation tests are caught for the case:
+    /// Ok((Default::default()))    
+    /// Or keep the skip and remove the comment
+    #[cfg_attr(test, mutants::skip)]
     pub fn from_data(data: Vec<(ClarityName, Value)>) -> Result<TupleData> {
         let mut type_map = BTreeMap::new();
         let mut data_map = BTreeMap::new();
@@ -1545,6 +1549,10 @@ impl TupleData {
         Self::new(TupleTypeSignature::try_from(type_map)?, data_map)
     }
 
+    ///TODO: #4587 create default for TupleData, then check if the mutation tests are caught for the case:
+    /// Ok((Default::default()))
+    /// Or keep the skip and remove the comment
+    #[cfg_attr(test, mutants::skip)]
     pub fn from_data_typed(
         epoch: &StacksEpochId,
         data: Vec<(ClarityName, Value)>,

clarity/src/vm/types/mod.rs

@@ -21,7 +21,11 @@ use blockstack_lib::chainstate::stacks::address::PoxAddress;
 use blockstack_lib::util_lib::signed_structured_data::pox4::Pox4SignatureTopic;
 use clap::{Parser, ValueEnum};
 use clarity::vm::types::QualifiedContractIdentifier;
-use stacks_common::address::b58;
+use stacks_common::address::{
+    b58, AddressHashMode, C32_ADDRESS_VERSION_MAINNET_MULTISIG,
+    C32_ADDRESS_VERSION_MAINNET_SINGLESIG, C32_ADDRESS_VERSION_TESTNET_MULTISIG,
+    C32_ADDRESS_VERSION_TESTNET_SINGLESIG,
+};
 use stacks_common::types::chainstate::StacksPrivateKey;
 
 use crate::config::Network;
@@ -260,12 +264,26 @@ fn parse_contract(contract: &str) -> Result<QualifiedContractIdentifier, String>
     QualifiedContractIdentifier::parse(contract).map_err(|e| format!("Invalid contract: {}", e))
 }
 
-/// Parse a BTC address argument and return a `PoxAddress`
+/// Parse a BTC address argument and return a `PoxAddress`.
+/// This function behaves similarly to `PoxAddress::from_b58`, but also handles
+/// addresses where the parsed AddressHashMode is None.
 pub fn parse_pox_addr(pox_address_literal: &str) -> Result<PoxAddress, String> {
-    PoxAddress::from_b58(pox_address_literal).map_or_else(
+    let parsed_addr = PoxAddress::from_b58(pox_address_literal).map_or_else(
         || Err(format!("Invalid pox address: {pox_address_literal}")),
-        |pox_address| Ok(pox_address),
-    )
+        Ok,
+    );
+    match parsed_addr {
+        Ok(PoxAddress::Standard(addr, None)) => match addr.version {
+            C32_ADDRESS_VERSION_MAINNET_MULTISIG | C32_ADDRESS_VERSION_TESTNET_MULTISIG => Ok(
+                PoxAddress::Standard(addr, Some(AddressHashMode::SerializeP2SH)),
+            ),
+            C32_ADDRESS_VERSION_MAINNET_SINGLESIG | C32_ADDRESS_VERSION_TESTNET_SINGLESIG => Ok(
+                PoxAddress::Standard(addr, Some(AddressHashMode::SerializeP2PKH)),
+            ),
+            _ => Err(format!("Invalid address version: {}", addr.version)),
+        },
+        _ => parsed_addr,
+    }
 }
 
 /// Parse the hexadecimal Stacks private key
@@ -306,13 +324,50 @@ fn parse_network(network: &str) -> Result<Network, String> {
 #[cfg(test)]
 mod tests {
     use blockstack_lib::chainstate::stacks::address::{PoxAddressType20, PoxAddressType32};
+    use blockstack_lib::util_lib::signed_structured_data::pox4::make_pox_4_signer_key_message_hash;
+    use clarity::consts::CHAIN_ID_TESTNET;
+    use clarity::util::hash::Sha256Sum;
 
     use super::*;
 
+    /// Helper just to ensure that a the pox address
+    /// can be turned into a clarity tuple
+    fn make_message_hash(pox_addr: &PoxAddress) -> Sha256Sum {
+        make_pox_4_signer_key_message_hash(
+            pox_addr,
+            0,
+            &Pox4SignatureTopic::StackStx,
+            CHAIN_ID_TESTNET,
+            0,
+            0,
+            0,
+        )
+    }
+
+    fn clarity_tuple_version(pox_addr: &PoxAddress) -> u8 {
+        pox_addr
+            .as_clarity_tuple()
+            .expect("Failed to generate clarity tuple for pox address")
+            .get("version")
+            .expect("Expected version in clarity tuple")
+            .clone()
+            .expect_buff(1)
+            .expect("Expected version to be a u128")
+            .get(0)
+            .expect("Expected version to be a uint")
+            .clone()
+    }
+
     #[test]
     fn test_parse_pox_addr() {
         let tr = "bc1p8vg588hldsnv4a558apet4e9ff3pr4awhqj2hy8gy6x2yxzjpmqsvvpta4";
         let pox_addr = parse_pox_addr(tr).expect("Failed to parse segwit address");
+        assert_eq!(tr, pox_addr.clone().to_b58());
+        make_message_hash(&pox_addr);
+        assert_eq!(
+            clarity_tuple_version(&pox_addr),
+            PoxAddressType32::P2TR.to_u8()
+        );
         match pox_addr {
             PoxAddress::Addr32(_, addr_type, _) => {
                 assert_eq!(addr_type, PoxAddressType32::P2TR);
@@ -322,35 +377,105 @@ mod tests {
 
         let legacy = "1N8GMS991YDY1E696e9SB9EsYY5ckSU7hZ";
         let pox_addr = parse_pox_addr(legacy).expect("Failed to parse legacy address");
+        assert_eq!(legacy, pox_addr.clone().to_b58());
+        make_message_hash(&pox_addr);
+        assert_eq!(
+            clarity_tuple_version(&pox_addr),
+            AddressHashMode::SerializeP2PKH as u8
+        );
         match pox_addr {
             PoxAddress::Standard(stacks_addr, hash_mode) => {
                 assert_eq!(stacks_addr.version, 22);
-                assert!(hash_mode.is_none());
+                assert_eq!(hash_mode, Some(AddressHashMode::SerializeP2PKH));
             }
             _ => panic!("Invalid parsed address"),
         }
 
         let p2sh = "33JNgVMNMC9Xm6mJG9oTVf5zWbmt5xi1Mv";
         let pox_addr = parse_pox_addr(p2sh).expect("Failed to parse legacy address");
+        assert_eq!(p2sh, pox_addr.clone().to_b58());
+        assert_eq!(
+            clarity_tuple_version(&pox_addr),
+            AddressHashMode::SerializeP2SH as u8
+        );
+        make_message_hash(&pox_addr);
         match pox_addr {
             PoxAddress::Standard(stacks_addr, hash_mode) => {
                 assert_eq!(stacks_addr.version, 20);
-                assert!(hash_mode.is_none());
+                assert_eq!(hash_mode, Some(AddressHashMode::SerializeP2SH));
+            }
+            _ => panic!("Invalid parsed address"),
+        }
+
+        let testnet_p2pkh = "mnr5asd1MLSutHLL514WZXNpUNN3L98zBc";
+        let pox_addr = parse_pox_addr(testnet_p2pkh).expect("Failed to parse testnet address");
+        assert_eq!(
+            clarity_tuple_version(&pox_addr),
+            AddressHashMode::SerializeP2PKH as u8
+        );
+        assert_eq!(testnet_p2pkh, pox_addr.clone().to_b58());
+        make_message_hash(&pox_addr);
+        match pox_addr {
+            PoxAddress::Standard(stacks_addr, hash_mode) => {
+                assert_eq!(stacks_addr.version, C32_ADDRESS_VERSION_TESTNET_SINGLESIG);
+                assert_eq!(hash_mode, Some(AddressHashMode::SerializeP2PKH));
             }
             _ => panic!("Invalid parsed address"),
         }
 
         let wsh = "bc1qvnpcphdctvmql5gdw6chtwvvsl6ra9gwa2nehc99np7f24juc4vqrx29cs";
         let pox_addr = parse_pox_addr(wsh).expect("Failed to parse segwit address");
+        assert_eq!(
+            clarity_tuple_version(&pox_addr),
+            PoxAddressType32::P2WSH.to_u8()
+        );
+        assert_eq!(wsh, pox_addr.clone().to_b58());
+        make_message_hash(&pox_addr);
         match pox_addr {
             PoxAddress::Addr32(_, addr_type, _) => {
                 assert_eq!(addr_type, PoxAddressType32::P2WSH);
             }
             _ => panic!("Invalid parsed address"),
         }
 
-        let wpkh = "BC1QW508D6QEJXTDG4Y5R3ZARVARY0C5XW7KV8F3T4";
+        let wpkh = "bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t4";
         let pox_addr = parse_pox_addr(wpkh).expect("Failed to parse segwit address");
+        assert_eq!(
+            clarity_tuple_version(&pox_addr),
+            PoxAddressType20::P2WPKH.to_u8()
+        );
+        assert_eq!(wpkh, pox_addr.clone().to_b58());
+        make_message_hash(&pox_addr);
+        match pox_addr {
+            PoxAddress::Addr20(_, addr_type, _) => {
+                assert_eq!(addr_type, PoxAddressType20::P2WPKH);
+            }
+            _ => panic!("Invalid parsed address"),
+        }
+
+        let testnet_tr = "tb1p46cgptxsfwkqpnnj552rkae3nf6l52wxn4snp4vm6mcrz2585hwq6cdwf2";
+        let pox_addr = parse_pox_addr(testnet_tr).expect("Failed to parse testnet address");
+        assert_eq!(testnet_tr, pox_addr.clone().to_b58());
+        make_message_hash(&pox_addr);
+        assert_eq!(
+            clarity_tuple_version(&pox_addr),
+            PoxAddressType32::P2TR.to_u8()
+        );
+        match pox_addr {
+            PoxAddress::Addr32(_, addr_type, _) => {
+                assert_eq!(addr_type, PoxAddressType32::P2TR);
+            }
+            _ => panic!("Invalid parsed address"),
+        }
+
+        let testnet_segwit = "tb1q38eleudmqyg4jrm39dnudj23pv6jcjrksa437s";
+        let pox_addr = parse_pox_addr(testnet_segwit).expect("Failed to parse testnet address");
+        assert_eq!(testnet_segwit, pox_addr.clone().to_b58());
+        make_message_hash(&pox_addr);
+        assert_eq!(
+            clarity_tuple_version(&pox_addr),
+            PoxAddressType20::P2WPKH.to_u8()
+        );
         match pox_addr {
             PoxAddress::Addr20(_, addr_type, _) => {
                 assert_eq!(addr_type, PoxAddressType20::P2WPKH);

stacks-signer/src/cli.rs

@@ -1773,6 +1773,7 @@ impl NakamotoChainState {
         chainstate: &mut StacksChainState,
         for_burn_block_height: u64,
         at_block_id: &StacksBlockId,
+        warn_if_not_found: bool,
     ) -> Result<Point, ChainstateError> {
         // Get the current reward cycle
         let Some(rc) = sort_handle.pox_constants().block_height_to_reward_cycle(
@@ -1788,18 +1789,22 @@ impl NakamotoChainState {
             return Err(ChainstateError::InvalidStacksBlock(msg));
         };
 
-        debug!(
+        test_debug!(
             "get-approved-aggregate-key at block {}, cycle {}",
-            at_block_id, rc
+            at_block_id,
+            rc
         );
         match chainstate.get_aggregate_public_key_pox_4(sortdb, at_block_id, rc)? {
             Some(key) => Ok(key),
             None => {
-                warn!(
-                    "Failed to get aggregate public key";
-                    "block_id" => %at_block_id,
-                    "reward_cycle" => rc,
-                );
+                // this can happen for a whole host of reasons
+                if warn_if_not_found {
+                    warn!(
+                        "Failed to get aggregate public key";
+                        "block_id" => %at_block_id,
+                        "reward_cycle" => rc,
+                    );
+                }
                 Err(ChainstateError::InvalidStacksBlock(
                     "Failed to get aggregate public key".into(),
                 ))
@@ -1828,6 +1833,7 @@ impl NakamotoChainState {
             chainstate,
             block_sn.block_height,
             &aggregate_key_block_header.index_block_hash(),
+            true,
         )?;
         Ok(aggregate_public_key)
     }

stackslib/src/chainstate/nakamoto/mod.rs

@@ -5388,7 +5388,7 @@ impl PeerNetwork {
             .last_key_value()
             .map(|(rc, _)| rc.saturating_add(1))
             .unwrap_or(0);
-        let new_agg_pubkeys = (next_agg_pubkey_rc..=sort_tip_rc)
+        let mut new_agg_pubkeys: Vec<_> = (next_agg_pubkey_rc..=sort_tip_rc)
             .filter_map(|key_rc| {
                 let ih = sortdb.index_handle(&tip_sn.sortition_id);
                 let agg_pubkey_opt = if self.get_current_epoch().epoch_id < StacksEpochId::Epoch25 {
@@ -5404,6 +5404,7 @@ impl PeerNetwork {
                         chainstate,
                         self.burnchain.reward_cycle_to_block_height(key_rc),
                         &stacks_tip_block_id,
+                        false,
                     )
                     .ok()
                 };
@@ -5414,6 +5415,10 @@ impl PeerNetwork {
             })
             .collect();
 
+        if new_agg_pubkeys.len() == 0 && self.aggregate_public_keys.len() == 0 {
+            // special case -- we're before epoch 3.0, so don't waste time doing this again
+            new_agg_pubkeys.push((sort_tip_rc, None));
+        }
         Ok(new_agg_pubkeys)
     }
 

stackslib/src/net/p2p.rs

@@ -106,7 +106,11 @@ pub mod pox4 {
             TupleData::from_data(vec![
                 (
                     "pox-addr".into(),
-                    pox_addr.clone().as_clarity_tuple().unwrap().into(),
+                    pox_addr
+                        .clone()
+                        .as_clarity_tuple()
+                        .expect("Error creating signature hash - invalid PoX Address")
+                        .into(),
                 ),
                 ("reward-cycle".into(), Value::UInt(reward_cycle)),
                 ("period".into(), Value::UInt(period)),
@@ -117,7 +121,7 @@ pub mod pox4 {
                 ("auth-id".into(), Value::UInt(auth_id)),
                 ("max-amount".into(), Value::UInt(max_amount)),
             ])
-            .unwrap(),
+            .expect("Error creating signature hash"),
         );
         structured_data_message_hash(data_tuple, domain_tuple)
     }