diff --git a/stacks-node/src/nakamoto_node/miner.rs b/stacks-node/src/nakamoto_node/miner.rs index 9558972887..86b5a4985f 100644 --- a/stacks-node/src/nakamoto_node/miner.rs +++ b/stacks-node/src/nakamoto_node/miner.rs @@ -1029,18 +1029,13 @@ impl BlockMinerThread { let burn_view_ch = NakamotoChainState::get_block_burn_view(sort_db, block, &parent_block_info)?; let mut sortition_handle = sort_db.index_handle_at_ch(&burn_view_ch)?; - let chainstate_config = chain_state.config(); - let (headers_conn, staging_tx) = chain_state.headers_conn_and_staging_tx_begin()?; let accepted = NakamotoChainState::accept_block( - &chainstate_config, + chain_state, block, &mut sortition_handle, - &staging_tx, - headers_conn, reward_set, NakamotoBlockObtainMethod::Mined, )?; - staging_tx.commit()?; if !accepted { // this can happen if the p2p network and relayer manage to receive this block prior to diff --git a/stackslib/src/chainstate/nakamoto/mod.rs b/stackslib/src/chainstate/nakamoto/mod.rs index 0d6bc89ba5..d89e1aba3b 100644 --- a/stackslib/src/chainstate/nakamoto/mod.rs +++ b/stackslib/src/chainstate/nakamoto/mod.rs @@ -2391,76 +2391,6 @@ impl NakamotoChainState { Ok(()) } - /// Insert a Nakamoto block into the staging blocks DB. - /// We only store a block in the following cases: - /// - /// * No block with this block's sighash exists in the DB - /// * A block with this block's sighash exists, AND - /// * this block represents more signing power - /// - /// If neither of the above is true, then this is a no-op. - pub(crate) fn store_block_if_better( - staging_db_tx: &NakamotoStagingBlocksTx, - block: &NakamotoBlock, - burn_attachable: bool, - signing_weight: u32, - obtain_method: NakamotoBlockObtainMethod, - ) -> Result { - let block_id = block.block_id(); - let block_hash = block.header.block_hash(); - - // case 1 -- no block with this sighash exists. - if staging_db_tx.try_store_block_with_new_signer_sighash( - block, - burn_attachable, - signing_weight, - obtain_method, - )? { - debug!("Stored block with new sighash"; - "block_id" => %block_id, - "block_hash" => %block_hash); - return Ok(true); - } - - // case 2 -- the block exists. Consider replacing it, but only if its - // signing weight is higher. - let (existing_block_id, _processed, orphaned, existing_signing_weight) = staging_db_tx.conn().get_block_processed_and_signed_weight(&block.header.consensus_hash, &block_hash)? - .ok_or_else(|| { - // this should be unreachable -- there's no record of this block - error!("Could not store block {} ({}) with block hash {} -- no record of its processed status or signing weight!", &block_id, &block.header.consensus_hash, &block_hash); - ChainstateError::NoSuchBlockError - })?; - - if orphaned { - // nothing to do - debug!("Will not store alternative copy of block {} ({}) with block hash {}, since a block with the same block hash was orphaned", &block_id, &block.header.consensus_hash, &block_hash); - return Ok(false); - } - - let ret = if existing_signing_weight < signing_weight { - staging_db_tx.replace_block(block, signing_weight, obtain_method)?; - debug!("Replaced block"; - "existing_block_id" => %existing_block_id, - "block_id" => %block_id, - "block_hash" => %block_hash, - "existing_signing_weight" => existing_signing_weight, - "signing_weight" => signing_weight); - true - } else { - if existing_signing_weight > signing_weight { - debug!("Will not store alternative copy of block {} ({}) with block hash {}, since it has less signing power", &block_id, &block.header.consensus_hash, &block_hash); - } else { - debug!( - "Will not store duplicate copy of block {} ({}) with block hash {}", - &block_id, &block.header.consensus_hash, &block_hash - ); - } - false - }; - - return Ok(ret); - } - /// Accept a Nakamoto block into the staging blocks DB. /// Fails if: /// * the public key cannot be recovered from the miner's signature @@ -2470,17 +2400,17 @@ impl NakamotoChainState { /// * we already have the block /// Returns true if we stored the block; false if not. pub fn accept_block( - config: &ChainstateConfig, + chainstate: &mut StacksChainState, block: &NakamotoBlock, db_handle: &mut SortitionHandleConn, - staging_db_tx: &NakamotoStagingBlocksTx, - headers_conn: &Connection, reward_set: &RewardSet, obtain_method: NakamotoBlockObtainMethod, ) -> Result { let block_id = block.block_id(); test_debug!("Consider Nakamoto block {block_id}"); + let config = chainstate.config(); // do nothing if we already have this block + let (headers_conn, staging_db_tx) = chainstate.headers_conn_and_staging_tx_begin()?; if Self::get_block_header(headers_conn, &block_id)?.is_some() { debug!("Already have block {block_id}"); return Ok(false); @@ -2555,13 +2485,13 @@ impl NakamotoChainState { // same sortition history as `db_handle` (and thus it must be burn_attachable) let burn_attachable = true; - let ret = Self::store_block_if_better( - staging_db_tx, + let ret = staging_db_tx.store_block_if_better( block, burn_attachable, signing_weight, obtain_method, )?; + staging_db_tx.commit()?; if ret { test_debug!("Stored Nakamoto block {block_id}"); } else { diff --git a/stackslib/src/chainstate/nakamoto/shadow.rs b/stackslib/src/chainstate/nakamoto/shadow.rs index 52973fb113..8a5a8d03f3 100644 --- a/stackslib/src/chainstate/nakamoto/shadow.rs +++ b/stackslib/src/chainstate/nakamoto/shadow.rs @@ -865,7 +865,7 @@ impl NakamotoStagingBlocksTx<'_> { // shadow blocks cannot be replaced let signing_weight = u32::MAX; - self.store_block( + self.store_block_if_better( shadow_block, burn_attachable, signing_weight, diff --git a/stackslib/src/chainstate/nakamoto/staging_blocks.rs b/stackslib/src/chainstate/nakamoto/staging_blocks.rs index a378a68ec7..035159b736 100644 --- a/stackslib/src/chainstate/nakamoto/staging_blocks.rs +++ b/stackslib/src/chainstate/nakamoto/staging_blocks.rs @@ -590,8 +590,81 @@ impl NakamotoStagingBlocksTx<'_> { Ok(()) } + /// Insert a Nakamoto block into the staging blocks DB. + /// We only store a block in the following cases: + /// + /// * No block with this block's sighash exists in the DB + /// * A block with this block's sighash exists, AND + /// * this block represents more signing power + /// + /// If neither of the above is true, then this is a no-op. + /// NOTE: This is intentionally the only public access into + /// storing additional blocks inside the staging blocks DB + pub fn store_block_if_better( + &self, + block: &NakamotoBlock, + burn_attachable: bool, + signing_weight: u32, + obtain_method: NakamotoBlockObtainMethod, + ) -> Result { + let block_id = block.block_id(); + let block_hash = block.header.block_hash(); + let consensus_hash = block.header.consensus_hash.clone(); + + // case 1 -- no block with this sighash exists. + if self.try_store_block_with_new_signer_sighash( + block, + burn_attachable, + signing_weight, + obtain_method, + )? { + debug!("Stored block with new sighash"; + "block_id" => %block_id, + "block_hash" => %block_hash); + return Ok(true); + } + + // case 2 -- the block exists. Consider replacing it, but only if its + // signing weight is higher. + let (existing_block_id, _processed, orphaned, existing_signing_weight) = self.conn().get_block_processed_and_signed_weight(&consensus_hash, &block_hash)? + .ok_or_else(|| { + // this should be unreachable -- there's no record of this block + error!("Could not store block {block_id} ({consensus_hash}) with block hash {block_hash} -- no record of its processed status or signing weight!"); + ChainstateError::NoSuchBlockError + })?; + + if orphaned { + // nothing to do + debug!("Will not store alternative copy of block {block_id} ({consensus_hash}) with block hash {block_hash}, since a block with the same block hash was orphaned"); + return Ok(false); + } + + let ret = if existing_signing_weight < signing_weight { + self.replace_block(block, signing_weight, obtain_method)?; + debug!("Replaced block"; + "existing_block_id" => %existing_block_id, + "block_id" => %block_id, + "block_hash" => %block_hash, + "existing_signing_weight" => existing_signing_weight, + "signing_weight" => signing_weight); + true + } else { + if existing_signing_weight > signing_weight { + debug!("Will not store alternative copy of block {block_id} ({consensus_hash}) with block hash {block_hash}, since it has less signing power"); + } else { + debug!( + "Will not store duplicate copy of block {block_id} ({consensus_hash}) with block hash {block_hash}" + ); + } + false + }; + + Ok(ret) + } + /// Store a block into the staging DB. - pub(crate) fn store_block( + /// NOTE: This should not be made public + fn store_block( &self, block: &NakamotoBlock, burn_attachable: bool, @@ -665,7 +738,7 @@ impl NakamotoStagingBlocksTx<'_> { /// Do we have a block with the given signer sighash? /// NOTE: the block hash and sighash are the same for Nakamoto blocks - pub(crate) fn has_nakamoto_block_with_block_hash( + fn has_nakamoto_block_with_block_hash( &self, consensus_hash: &ConsensusHash, block_hash: &BlockHeaderHash, @@ -681,7 +754,8 @@ impl NakamotoStagingBlocksTx<'_> { /// NOTE: the block hash and sighash are the same for Nakamoto blocks, so this is equivalent to /// storing a new block. /// Return true if stored; false if not. - pub(crate) fn try_store_block_with_new_signer_sighash( + /// NOTE: This should not be made public + fn try_store_block_with_new_signer_sighash( &self, block: &NakamotoBlock, burn_attachable: bool, @@ -698,7 +772,8 @@ impl NakamotoStagingBlocksTx<'_> { /// Replace an already-stored block with a newer copy with more signing /// power. Arguments will not be validated; the caller must do this. - pub(crate) fn replace_block( + /// NOTE: This should not be made public + fn replace_block( &self, block: &NakamotoBlock, signing_weight: u32, diff --git a/stackslib/src/chainstate/nakamoto/tests/mod.rs b/stackslib/src/chainstate/nakamoto/tests/mod.rs index 6e59899a54..022bebaab7 100644 --- a/stackslib/src/chainstate/nakamoto/tests/mod.rs +++ b/stackslib/src/chainstate/nakamoto/tests/mod.rs @@ -977,14 +977,14 @@ pub fn test_load_store_update_nakamoto_blocks() { 300, ) .unwrap(); - NakamotoChainState::store_block_if_better( - &staging_tx, - &nakamoto_block, - false, - 1, - NakamotoBlockObtainMethod::Downloaded, - ) - .unwrap(); + staging_tx + .store_block_if_better( + &nakamoto_block, + false, + 1, + NakamotoBlockObtainMethod::Downloaded, + ) + .unwrap(); // tenure has one block assert_eq!( @@ -1007,14 +1007,14 @@ pub fn test_load_store_update_nakamoto_blocks() { ) .unwrap(); - NakamotoChainState::store_block_if_better( - &staging_tx, - &nakamoto_block_2, - false, - 1, - NakamotoBlockObtainMethod::Downloaded, - ) - .unwrap(); + staging_tx + .store_block_if_better( + &nakamoto_block_2, + false, + 1, + NakamotoBlockObtainMethod::Downloaded, + ) + .unwrap(); // tenure has two blocks assert_eq!( @@ -1032,14 +1032,14 @@ pub fn test_load_store_update_nakamoto_blocks() { ); // store, but do not process, a block - NakamotoChainState::store_block_if_better( - &staging_tx, - &nakamoto_block_3, - false, - 1, - NakamotoBlockObtainMethod::Downloaded, - ) - .unwrap(); + staging_tx + .store_block_if_better( + &nakamoto_block_3, + false, + 1, + NakamotoBlockObtainMethod::Downloaded, + ) + .unwrap(); assert_eq!( staging_tx .conn() @@ -1062,14 +1062,14 @@ pub fn test_load_store_update_nakamoto_blocks() { ); // store, but do not process, the same block with a heavier weight - NakamotoChainState::store_block_if_better( - &staging_tx, - &nakamoto_block_3_weight_2, - false, - 2, - NakamotoBlockObtainMethod::Downloaded, - ) - .unwrap(); + staging_tx + .store_block_if_better( + &nakamoto_block_3_weight_2, + false, + 2, + NakamotoBlockObtainMethod::Downloaded, + ) + .unwrap(); assert_eq!( staging_tx .conn() @@ -1244,14 +1244,14 @@ pub fn test_load_store_update_nakamoto_blocks() { // store a sibling with more weight, even though this block has been processed. // This is allowed because we don't commit to signatures. - NakamotoChainState::store_block_if_better( - &staging_tx, - &nakamoto_block_3, - false, - 3, - NakamotoBlockObtainMethod::Downloaded, - ) - .unwrap(); + staging_tx + .store_block_if_better( + &nakamoto_block_3, + false, + 3, + NakamotoBlockObtainMethod::Downloaded, + ) + .unwrap(); assert_eq!( staging_tx .conn() @@ -1307,14 +1307,14 @@ pub fn test_load_store_update_nakamoto_blocks() { ); // store block 4, which descends from block 3 weight 2 - NakamotoChainState::store_block_if_better( - &staging_tx, - &nakamoto_block_4, - false, - 1, - NakamotoBlockObtainMethod::Downloaded, - ) - .unwrap(); + staging_tx + .store_block_if_better( + &nakamoto_block_4, + false, + 1, + NakamotoBlockObtainMethod::Downloaded, + ) + .unwrap(); assert_eq!( staging_tx .conn() @@ -1341,14 +1341,14 @@ pub fn test_load_store_update_nakamoto_blocks() { (true, false) ); - NakamotoChainState::store_block_if_better( - &staging_tx, - &nakamoto_block_3, - false, - 3, - NakamotoBlockObtainMethod::Downloaded, - ) - .unwrap(); + staging_tx + .store_block_if_better( + &nakamoto_block_3, + false, + 3, + NakamotoBlockObtainMethod::Downloaded, + ) + .unwrap(); assert_eq!( staging_tx .conn() @@ -1389,14 +1389,14 @@ pub fn test_load_store_update_nakamoto_blocks() { ); // can't re-store it, even if its signing power is better - assert!(!NakamotoChainState::store_block_if_better( - &staging_tx, - &nakamoto_block_3_weight_2, - false, - 3, - NakamotoBlockObtainMethod::Downloaded - ) - .unwrap()); + assert!(!staging_tx + .store_block_if_better( + &nakamoto_block_3_weight_2, + false, + 3, + NakamotoBlockObtainMethod::Downloaded + ) + .unwrap()); assert_eq!( NakamotoChainState::get_nakamoto_block_status( staging_tx.conn(), @@ -1411,14 +1411,14 @@ pub fn test_load_store_update_nakamoto_blocks() { // can't store a sibling with the same sighash either, since if a block with the given sighash is orphaned, then // it doesn't matter how many signers it has - assert!(!NakamotoChainState::store_block_if_better( - &staging_tx, - &nakamoto_block_3, - false, - 3, - NakamotoBlockObtainMethod::Downloaded - ) - .unwrap()); + assert!(!staging_tx + .store_block_if_better( + &nakamoto_block_3, + false, + 3, + NakamotoBlockObtainMethod::Downloaded + ) + .unwrap()); assert_eq!( staging_tx .conn() diff --git a/stackslib/src/net/relay.rs b/stackslib/src/net/relay.rs index 882dd000f8..025f2815ac 100644 --- a/stackslib/src/net/relay.rs +++ b/stackslib/src/net/relay.rs @@ -997,7 +997,6 @@ impl Relayer { &block.header.block_hash() ); - let config = chainstate.config(); let tip = block_sn.sortition_id; let reward_info = match load_nakamoto_reward_set( @@ -1039,17 +1038,13 @@ impl Relayer { return Err(chainstate_error::NoRegisteredSigners(reward_cycle)); }; - let (headers_conn, staging_db_tx) = chainstate.headers_conn_and_staging_tx_begin()?; let accepted = NakamotoChainState::accept_block( - &config, + chainstate, block, sort_handle, - &staging_db_tx, - headers_conn, &reward_set, obtained_method, )?; - staging_db_tx.commit()?; if accepted { info!("{}", &accept_msg); @@ -1058,10 +1053,10 @@ impl Relayer { return Err(chainstate_error::NetError(net_error::CoordinatorClosed)); } } - return Ok(BlockAcceptResponse::Accepted); + Ok(BlockAcceptResponse::Accepted) } else { - info!("{}", &reject_msg); - return Ok(BlockAcceptResponse::AlreadyStored); + info!("{reject_msg}"); + Ok(BlockAcceptResponse::AlreadyStored) } }