From 78b05a6adc222f86d5e56b5a95d32d2630d9990f Mon Sep 17 00:00:00 2001 From: Jeff Bencin Date: Tue, 26 Mar 2024 15:54:04 -0400 Subject: [PATCH] chore: Address PR comments and fix tests --- libsigner/src/messages.rs | 2 +- .../src/chainstate/nakamoto/staging_blocks.rs | 5 +- .../src/chainstate/nakamoto/tests/mod.rs | 7 ++- stackslib/src/chainstate/stacks/auth.rs | 19 ++---- stackslib/src/chainstate/stacks/block.rs | 44 ++++++------- stackslib/src/chainstate/stacks/db/blocks.rs | 2 +- .../src/chainstate/stacks/transaction.rs | 63 +++++++------------ stackslib/src/main.rs | 6 +- stackslib/src/net/api/getblock_v3.rs | 8 ++- stackslib/src/net/api/gettenure.rs | 6 +- stackslib/src/net/api/tests/getblock_v3.rs | 10 ++- stackslib/src/net/api/tests/gettenure.rs | 7 ++- 12 files changed, 86 insertions(+), 93 deletions(-) diff --git a/libsigner/src/messages.rs b/libsigner/src/messages.rs index 431267af6e..86752135e5 100644 --- a/libsigner/src/messages.rs +++ b/libsigner/src/messages.rs @@ -1315,7 +1315,7 @@ mod test { use blockstack_lib::util_lib::strings::StacksString; use rand::Rng; use rand_core::OsRng; - use stacks_common::consts::CHAIN_ID_TESTNET; + use stacks_common::consts::{CHAIN_ID_TESTNET, SIGNER_SLOTS_PER_USER}; use stacks_common::types::chainstate::StacksPrivateKey; use wsts::common::Signature; diff --git a/stackslib/src/chainstate/nakamoto/staging_blocks.rs b/stackslib/src/chainstate/nakamoto/staging_blocks.rs index a20f870063..6f11d73694 100644 --- a/stackslib/src/chainstate/nakamoto/staging_blocks.rs +++ b/stackslib/src/chainstate/nakamoto/staging_blocks.rs @@ -218,7 +218,10 @@ impl<'a> NakamotoStagingBlocksConnRef<'a> { let Some(block_bytes) = data else { return Ok(None); }; - let block = NakamotoBlock::consensus_deserialize(&mut block_bytes.as_slice())?; + let block = NakamotoBlock::consensus_deserialize_with_epoch( + &mut block_bytes.as_slice(), + StacksEpochId::latest(), + )?; if &block.header.consensus_hash != consensus_hash { error!( "Staging DB corruption: expected {}, got {}", diff --git a/stackslib/src/chainstate/nakamoto/tests/mod.rs b/stackslib/src/chainstate/nakamoto/tests/mod.rs index 9844fa7b74..acc8965c85 100644 --- a/stackslib/src/chainstate/nakamoto/tests/mod.rs +++ b/stackslib/src/chainstate/nakamoto/tests/mod.rs @@ -27,7 +27,7 @@ use rand::{thread_rng, RngCore}; use rusqlite::{Connection, ToSql}; use stacks_common::address::AddressHashMode; use stacks_common::bitvec::BitVec; -use stacks_common::codec::StacksMessageCodec; +use stacks_common::codec::{DeserializeWithEpoch, StacksMessageCodec}; use stacks_common::consts::{ CHAIN_ID_MAINNET, CHAIN_ID_TESTNET, FIRST_BURNCHAIN_CONSENSUS_HASH, FIRST_STACKS_BLOCK_HASH, }; @@ -101,7 +101,10 @@ impl<'a> NakamotoStagingBlocksConnRef<'a> { let block_data: Vec> = query_rows(self, qry, args)?; let mut blocks = Vec::with_capacity(block_data.len()); for data in block_data.into_iter() { - let block = NakamotoBlock::consensus_deserialize(&mut data.as_slice())?; + let block = NakamotoBlock::consensus_deserialize_with_epoch( + &mut data.as_slice(), + StacksEpochId::latest(), + )?; blocks.push(block); } Ok(blocks) diff --git a/stackslib/src/chainstate/stacks/auth.rs b/stackslib/src/chainstate/stacks/auth.rs index 24caa33402..c7d0ec72c6 100644 --- a/stackslib/src/chainstate/stacks/auth.rs +++ b/stackslib/src/chainstate/stacks/auth.rs @@ -449,7 +449,6 @@ impl OrderIndependentMultisigSpendingCondition { cond_code: &TransactionAuthFlags, ) -> Result { let mut pubkeys = vec![]; - let cur_sighash = initial_sighash.clone(); let mut num_sigs: u16 = 0; let mut have_uncompressed = false; for field in self.fields.iter() { @@ -466,7 +465,7 @@ impl OrderIndependentMultisigSpendingCondition { } let (pubkey, _next_sighash) = TransactionSpendingCondition::next_verification( - &cur_sighash, + &initial_sighash, cond_code, self.tx_fee, self.nonce, @@ -515,7 +514,7 @@ impl OrderIndependentMultisigSpendingCondition { ))); } - Ok(cur_sighash) + Ok(initial_sighash.clone()) } } @@ -1209,26 +1208,20 @@ impl TransactionAuth { privks: &[StacksPrivateKey], num_sigs: u16, ) -> Option { - let mut pubks = vec![]; - for privk in privks.iter() { - pubks.push(StacksPublicKey::from_private(privk)); - } + let pubks = privks.iter().map(StacksPublicKey::from_private).collect(); TransactionSpendingCondition::new_multisig_order_independent_p2sh(num_sigs, pubks) - .map(|auth| TransactionAuth::Standard(auth)) + .map(TransactionAuth::Standard) } pub fn from_order_independent_p2wsh( privks: &[StacksPrivateKey], num_sigs: u16, ) -> Option { - let mut pubks = vec![]; - for privk in privks.iter() { - pubks.push(StacksPublicKey::from_private(privk)); - } + let pubks = privks.iter().map(StacksPublicKey::from_private).collect(); TransactionSpendingCondition::new_multisig_order_independent_p2wsh(num_sigs, pubks) - .map(|auth| TransactionAuth::Standard(auth)) + .map(TransactionAuth::Standard) } pub fn from_p2wpkh(privk: &StacksPrivateKey) -> Option { diff --git a/stackslib/src/chainstate/stacks/block.rs b/stackslib/src/chainstate/stacks/block.rs index 8760460ce4..911ab07b6d 100644 --- a/stackslib/src/chainstate/stacks/block.rs +++ b/stackslib/src/chainstate/stacks/block.rs @@ -41,6 +41,15 @@ use crate::chainstate::stacks::{Error, StacksBlockHeader, StacksMicroblockHeader use crate::core::*; use crate::net::Error as net_error; +/// Quietable error +macro_rules! qerror { + ($cond: expr, $($arg:tt)*) => ({ + if ($cond) { + error!($($arg)*); + } + }); +} + impl StacksMessageCodec for StacksBlockHeader { fn consensus_serialize(&self, fd: &mut W) -> Result<(), codec_error> { write_next(fd, &self.version)?; @@ -583,40 +592,33 @@ impl StacksBlock { if let TransactionPayload::Coinbase(_, ref recipient_opt, ref proof_opt) = &tx.payload { if proof_opt.is_some() && epoch_id < StacksEpochId::Epoch30 { // not supported - if !quiet { - error!("Coinbase with VRF proof not supported before Stacks 3.0"; "txid" => %tx.txid()); - } + qerror!(quiet, "Coinbase with VRF proof not supported before Stacks 3.0"; "txid" => %tx.txid()); return false; } if proof_opt.is_none() && epoch_id >= StacksEpochId::Epoch30 { // not supported - if !quiet { - error!("Coinbase with VRF proof is required in Stacks 3.0 and later"; "txid" => %tx.txid()); - } + qerror!(quiet, "Coinbase with VRF proof is required in Stacks 3.0 and later"; "txid" => %tx.txid()); return false; } if recipient_opt.is_some() && epoch_id < StacksEpochId::Epoch21 { // not supported - if !quiet { - error!("Coinbase pay-to-alt-recipient not supported before Stacks 2.1"; "txid" => %tx.txid()); - } + qerror!(quiet, "Coinbase pay-to-alt-recipient not supported before Stacks 2.1"; "txid" => %tx.txid()); return false; } } if let TransactionPayload::SmartContract(_, ref version_opt) = &tx.payload { if version_opt.is_some() && epoch_id < StacksEpochId::Epoch21 { // not supported - if !quiet { - error!("Versioned smart contracts not supported before Stacks 2.1"); - } + qerror!( + quiet, + "Versioned smart contracts not supported before Stacks 2.1" + ); return false; } } if let TransactionPayload::TenureChange(..) = &tx.payload { if epoch_id < StacksEpochId::Epoch30 { - if !quiet { - error!("TenureChange transaction not supported before Stacks 3.0"; "txid" => %tx.txid()); - } + qerror!(quiet, "TenureChange transaction not supported before Stacks 3.0"; "txid" => %tx.txid()); return false; } } @@ -625,9 +627,7 @@ impl StacksBlock { match origin { TransactionSpendingCondition::OrderIndependentMultisig(..) => { if epoch_id < StacksEpochId::Epoch30 { - if !quiet { - error!("Order independent multisig transactions not supported before Stacks 3.0"); - } + qerror!(quiet, "Order independent multisig transactions not supported before Stacks 3.0"); return false; } } @@ -636,9 +636,7 @@ impl StacksBlock { match sponsor { TransactionSpendingCondition::OrderIndependentMultisig(..) => { if epoch_id < StacksEpochId::Epoch30 { - if !quiet { - error!("Order independent multisig transactions not supported before Stacks 3.0"); - } + qerror!(quiet, "Order independent multisig transactions not supported before Stacks 3.0"); return false; } } @@ -648,9 +646,7 @@ impl StacksBlock { TransactionAuth::Standard(ref origin) => match origin { TransactionSpendingCondition::OrderIndependentMultisig(..) => { if epoch_id < StacksEpochId::Epoch30 { - if !quiet { - error!("Order independent multisig transactions not supported before Stacks 3.0"); - } + qerror!(quiet, "Order independent multisig transactions not supported before Stacks 3.0"); return false; } } diff --git a/stackslib/src/chainstate/stacks/db/blocks.rs b/stackslib/src/chainstate/stacks/db/blocks.rs index bdd3be4f30..1f6ac10e69 100644 --- a/stackslib/src/chainstate/stacks/db/blocks.rs +++ b/stackslib/src/chainstate/stacks/db/blocks.rs @@ -6719,7 +6719,7 @@ impl StacksChainState { let epoch = clarity_connection.get_epoch().clone(); let is_transaction_valid_in_epoch = - StacksBlock::validate_transactions_static_epoch(&vec![tx.clone()], epoch, true); + StacksBlock::validate_transactions_static_epoch(&[tx.clone()], epoch, true); if !is_transaction_valid_in_epoch { return Err(MemPoolRejection::Other( diff --git a/stackslib/src/chainstate/stacks/transaction.rs b/stackslib/src/chainstate/stacks/transaction.rs index 47b06e32e4..9d94addf42 100644 --- a/stackslib/src/chainstate/stacks/transaction.rs +++ b/stackslib/src/chainstate/stacks/transaction.rs @@ -699,7 +699,7 @@ impl StacksTransaction { payload, }; - if !StacksBlock::validate_transactions_static_epoch(&vec![tx.clone()], epoch_id, false) { + if !StacksBlock::validate_transactions_static_epoch(&[tx.clone()], epoch_id, false) { warn!("Invalid tx: target epoch is not activated"); return Err(codec_error::DeserializeError( "Failed to parse transaction: target epoch is not activated".to_string(), @@ -973,7 +973,8 @@ impl StacksTransaction { Ok(next_sighash) } - pub fn sign_no_append_origin( + #[cfg(any(test, feature = "testing"))] + fn sign_no_append_origin( &self, cur_sighash: &Txid, privk: &StacksPrivateKey, @@ -994,7 +995,8 @@ impl StacksTransaction { Ok(next_sig) } - pub fn append_origin_signature( + #[cfg(any(test, feature = "testing"))] + fn append_origin_signature( &mut self, signature: MessageSignature, key_encoding: TransactionPublicKeyEncoding, @@ -1016,7 +1018,8 @@ impl StacksTransaction { Ok(()) } - pub fn sign_no_append_sponsor( + #[cfg(any(test, feature = "testing"))] + fn sign_no_append_sponsor( &mut self, cur_sighash: &Txid, privk: &StacksPrivateKey, @@ -1496,30 +1499,20 @@ mod test { TransactionSpendingCondition::Multisig(ref data) => { let mut j = 0; for f in 0..data.fields.len() { - match data.fields[f] { - TransactionAuthField::Signature(_, _) => { - j = f; - break; - } - _ => { - continue; - } - } + if matches!(data.fields[f], TransactionAuthField::Signature(..)) { + j = f; + break; + }; } j } TransactionSpendingCondition::OrderIndependentMultisig(ref data) => { let mut j = 0; for f in 0..data.fields.len() { - match data.fields[f] { - TransactionAuthField::Signature(_, _) => { - j = f; - break; - } - _ => { - continue; - } - } + if matches!(data.fields[f], TransactionAuthField::Signature(..)) { + j = f; + break; + }; } j } @@ -1532,30 +1525,20 @@ mod test { TransactionSpendingCondition::Multisig(ref data) => { let mut j = 0; for f in 0..data.fields.len() { - match data.fields[f] { - TransactionAuthField::PublicKey(_) => { - j = f; - break; - } - _ => { - continue; - } - } + if matches!(data.fields[f], TransactionAuthField::PublicKey(_)) { + j = f; + break; + }; } j } TransactionSpendingCondition::OrderIndependentMultisig(ref data) => { let mut j = 0; for f in 0..data.fields.len() { - match data.fields[f] { - TransactionAuthField::PublicKey(_) => { - j = f; - break; - } - _ => { - continue; - } - } + if matches!(data.fields[f], TransactionAuthField::PublicKey(_)) { + j = f; + break; + }; } j } diff --git a/stackslib/src/main.rs b/stackslib/src/main.rs index e7c4787cc1..7aebe72ea0 100644 --- a/stackslib/src/main.rs +++ b/stackslib/src/main.rs @@ -1354,7 +1354,6 @@ simulating a miner. let events_file = &argv[3]; let mine_tip_height: u64 = argv[4].parse().expect("Could not parse mine_tip_height"); let mine_max_txns: u64 = argv[5].parse().expect("Could not parse mine-num-txns"); - let epoch_id = parse_input_epoch(6); let sort_db = SortitionDB::open(&sort_db_path, false, PoxConstants::mainnet_default()) .unwrap_or_else(|_| panic!("Failed to open {sort_db_path}")); @@ -1368,6 +1367,11 @@ simulating a miner. let estimator = Box::new(UnitEstimator); let metric = Box::new(UnitMetric); + let epoch_id = SortitionDB::get_stacks_epoch(&sort_db.conn(), chain_tip.block_height) + .expect("Error getting Epoch") + .expect("No Epoch found") + .epoch_id; + let mut mempool_db = MemPoolDB::open(true, chain_id, &chain_state_path, estimator, metric) .expect("Failed to open mempool db"); diff --git a/stackslib/src/net/api/getblock_v3.rs b/stackslib/src/net/api/getblock_v3.rs index 090afec04c..ab359f8ecb 100644 --- a/stackslib/src/net/api/getblock_v3.rs +++ b/stackslib/src/net/api/getblock_v3.rs @@ -20,9 +20,10 @@ use std::{fs, io}; use regex::{Captures, Regex}; use rusqlite::Connection; use serde::de::Error as de_Error; -use stacks_common::codec::{StacksMessageCodec, MAX_MESSAGE_LEN}; +use stacks_common::codec::{DeserializeWithEpoch, StacksMessageCodec, MAX_MESSAGE_LEN}; use stacks_common::types::chainstate::{ConsensusHash, StacksBlockId}; use stacks_common::types::net::PeerHost; +use stacks_common::types::StacksEpochId; use stacks_common::util::hash::to_hex; use {serde, serde_json}; @@ -317,7 +318,10 @@ impl StacksHttpResponse { // contents will be raw bytes let block_bytes: Vec = contents.try_into()?; - let block = NakamotoBlock::consensus_deserialize(&mut &block_bytes[..])?; + let block = NakamotoBlock::consensus_deserialize_with_epoch( + &mut &block_bytes[..], + StacksEpochId::latest(), + )?; Ok(block) } diff --git a/stackslib/src/net/api/gettenure.rs b/stackslib/src/net/api/gettenure.rs index 58ff2f96f0..17bd3b8a73 100644 --- a/stackslib/src/net/api/gettenure.rs +++ b/stackslib/src/net/api/gettenure.rs @@ -19,9 +19,10 @@ use std::{fs, io}; use regex::{Captures, Regex}; use serde::de::Error as de_Error; -use stacks_common::codec::{StacksMessageCodec, MAX_MESSAGE_LEN}; +use stacks_common::codec::{DeserializeWithEpoch, StacksMessageCodec, MAX_MESSAGE_LEN}; use stacks_common::types::chainstate::{ConsensusHash, StacksBlockId}; use stacks_common::types::net::PeerHost; +use stacks_common::types::StacksEpochId; use stacks_common::util::hash::to_hex; use {serde, serde_json}; @@ -355,7 +356,8 @@ impl StacksHttpResponse { let mut blocks = vec![]; while ptr.len() > 0 { - let block = NakamotoBlock::consensus_deserialize(ptr)?; + let block = + NakamotoBlock::consensus_deserialize_with_epoch(ptr, StacksEpochId::latest())?; blocks.push(block); } diff --git a/stackslib/src/net/api/tests/getblock_v3.rs b/stackslib/src/net/api/tests/getblock_v3.rs index de1a76f748..b24a81f815 100644 --- a/stackslib/src/net/api/tests/getblock_v3.rs +++ b/stackslib/src/net/api/tests/getblock_v3.rs @@ -18,12 +18,12 @@ use std::net::{IpAddr, Ipv4Addr, SocketAddr}; use clarity::vm::types::{QualifiedContractIdentifier, StacksAddressExtensions}; use clarity::vm::{ClarityName, ContractName}; -use stacks_common::codec::StacksMessageCodec; +use stacks_common::codec::{DeserializeWithEpoch, StacksMessageCodec}; use stacks_common::types::chainstate::{ ConsensusHash, StacksAddress, StacksBlockId, StacksPrivateKey, }; use stacks_common::types::net::PeerHost; -use stacks_common::types::Address; +use stacks_common::types::{Address, StacksEpochId}; use super::TestRPC; use crate::chainstate::burn::db::sortdb::{SortitionDB, SortitionHandle}; @@ -183,6 +183,10 @@ fn test_stream_nakamoto_blocks() { all_block_bytes.append(&mut next_bytes); } - let staging_block = NakamotoBlock::consensus_deserialize(&mut &all_block_bytes[..]).unwrap(); + let staging_block = NakamotoBlock::consensus_deserialize_with_epoch( + &mut &all_block_bytes[..], + StacksEpochId::latest(), + ) + .unwrap(); assert_eq!(staging_block.header.block_id(), nakamoto_tip_block_id); } diff --git a/stackslib/src/net/api/tests/gettenure.rs b/stackslib/src/net/api/tests/gettenure.rs index c4f179acc9..1b3a280510 100644 --- a/stackslib/src/net/api/tests/gettenure.rs +++ b/stackslib/src/net/api/tests/gettenure.rs @@ -18,12 +18,12 @@ use std::net::{IpAddr, Ipv4Addr, SocketAddr}; use clarity::vm::types::{QualifiedContractIdentifier, StacksAddressExtensions}; use clarity::vm::{ClarityName, ContractName}; -use stacks_common::codec::StacksMessageCodec; +use stacks_common::codec::{DeserializeWithEpoch, StacksMessageCodec}; use stacks_common::types::chainstate::{ ConsensusHash, StacksAddress, StacksBlockId, StacksPrivateKey, }; use stacks_common::types::net::PeerHost; -use stacks_common::types::Address; +use stacks_common::types::{Address, StacksEpochId}; use super::TestRPC; use crate::chainstate::burn::db::sortdb::{SortitionDB, SortitionHandle}; @@ -192,7 +192,8 @@ fn test_stream_nakamoto_tenure() { let ptr = &mut all_block_bytes.as_slice(); let mut blocks = vec![]; while ptr.len() > 0 { - let block = NakamotoBlock::consensus_deserialize(ptr).unwrap(); + let block = + NakamotoBlock::consensus_deserialize_with_epoch(ptr, StacksEpochId::latest()).unwrap(); blocks.push(block); }