From 60b3747da0fd13a74a0ee26da8874648cced2958 Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Mon, 15 Feb 2021 12:58:23 -0500 Subject: [PATCH 1/5] fix 2451 -- make it so we don't load a descendant microblock stream if we're missing the stream's prefix --- src/chainstate/stacks/db/blocks.rs | 140 +++++++++++++++++++++++++++++ 1 file changed, 140 insertions(+) diff --git a/src/chainstate/stacks/db/blocks.rs b/src/chainstate/stacks/db/blocks.rs index 9b58baca0..7043bc638 100644 --- a/src/chainstate/stacks/db/blocks.rs +++ b/src/chainstate/stacks/db/blocks.rs @@ -1356,6 +1356,7 @@ impl StacksChainState { let mut ret = vec![]; let mut tip: Option = None; let mut fork_poison = None; + let mut expected_sequence = 0; // load associated staging microblock data, but best-effort. // Stop loading once we find a fork juncture. @@ -1384,6 +1385,18 @@ impl StacksChainState { } }; + if mblock.header.sequence > expected_sequence { + warn!( + "Discontinuous microblock stream: expected seq {}, got {}", + expected_sequence, mblock.header.sequence + ); + break; + } + + // expect forks, so expected_sequence may not always increase + expected_sequence = + cmp::min(mblock.header.sequence, expected_sequence).saturating_add(1); + if let Some(tip_mblock) = tip { if mblock.header.sequence == tip_mblock.header.sequence { debug!( @@ -6912,6 +6925,133 @@ pub mod test { ); } + #[test] + fn stacks_db_staging_microblock_stream_load_continuous_streams() { + let mut chainstate = instantiate_chainstate( + false, + 0x80000000, + "stacks_db_staging_microblock_stream_load_continuous_streams", + ); + let privk = StacksPrivateKey::from_hex( + "eb05c83546fdd2c79f10f5ad5434a90dd28f7e3acb7c092157aa1bc3656b012c01", + ) + .unwrap(); + + let block = make_empty_coinbase_block(&privk); + let microblocks = make_sample_microblock_stream(&privk, &block.block_hash()); + let mut child_block = make_empty_coinbase_block(&privk); + + child_block.header.parent_block = block.block_hash(); + child_block.header.parent_microblock = microblocks.first().as_ref().unwrap().block_hash(); + child_block.header.parent_microblock_sequence = + microblocks.first().as_ref().unwrap().header.sequence; + + assert!(StacksChainState::load_staging_microblock( + &chainstate.db(), + &ConsensusHash([2u8; 20]), + &block.block_hash(), + µblocks[0].block_hash() + ) + .unwrap() + .is_none()); + assert!(StacksChainState::load_descendant_staging_microblock_stream( + &chainstate.db(), + &StacksBlockHeader::make_index_block_hash( + &ConsensusHash([2u8; 20]), + &block.block_hash() + ), + 0, + u16::max_value() + ) + .unwrap() + .is_none()); + + store_staging_block( + &mut chainstate, + &ConsensusHash([2u8; 20]), + &block, + &ConsensusHash([1u8; 20]), + 1, + 2, + ); + + // don't store the first microblock, but store the rest + for (i, mb) in microblocks.iter().enumerate() { + if i > 0 { + store_staging_microblock( + &mut chainstate, + &ConsensusHash([2u8; 20]), + &block.block_hash(), + mb, + ); + } + } + store_staging_block( + &mut chainstate, + &ConsensusHash([3u8; 20]), + &child_block, + &ConsensusHash([2u8; 20]), + 1, + 2, + ); + + // block should be stored to staging + assert_block_staging_not_processed(&mut chainstate, &ConsensusHash([2u8; 20]), &block); + assert_block_staging_not_processed( + &mut chainstate, + &ConsensusHash([3u8; 20]), + &child_block, + ); + assert_block_not_stored(&mut chainstate, &ConsensusHash([2u8; 20]), &block); + assert_block_not_stored(&mut chainstate, &ConsensusHash([3u8; 20]), &child_block); + + // missing head + assert!(StacksChainState::load_staging_microblock( + &chainstate.db(), + &ConsensusHash([2u8; 20]), + &block.block_hash(), + µblocks[0].block_hash() + ) + .unwrap() + .is_none()); + + // subsequent microblock stream should be stored to staging + assert!(StacksChainState::load_staging_microblock( + &chainstate.db(), + &ConsensusHash([2u8; 20]), + &block.block_hash(), + µblocks[1].block_hash() + ) + .unwrap() + .is_some()); + assert_eq!( + StacksChainState::load_staging_microblock( + &chainstate.db(), + &ConsensusHash([2u8; 20]), + &block.block_hash(), + µblocks[1].block_hash() + ) + .unwrap() + .unwrap() + .try_into_microblock() + .unwrap(), + microblocks[1] + ); + + // can't load descendent stream because missing head + assert!(StacksChainState::load_descendant_staging_microblock_stream( + &chainstate.db(), + &StacksBlockHeader::make_index_block_hash( + &ConsensusHash([2u8; 20]), + &block.block_hash() + ), + 0, + u16::max_value() + ) + .unwrap() + .is_none()); + } + #[test] fn stacks_db_validate_parent_microblock_stream() { let privk = StacksPrivateKey::from_hex( From 1a7252d8b55b1d51d85738a067752fc99b58839c Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Mon, 15 Feb 2021 12:58:53 -0500 Subject: [PATCH 2/5] don't consider the stream length when considering whether to mine; consider the maximum microblock sequence --- testnet/stacks-node/src/neon_node.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/testnet/stacks-node/src/neon_node.rs b/testnet/stacks-node/src/neon_node.rs index 2bd717d7f..e3dc4dda8 100644 --- a/testnet/stacks-node/src/neon_node.rs +++ b/testnet/stacks-node/src/neon_node.rs @@ -1392,21 +1392,28 @@ impl InitializedNeonNode { u16::MAX, ) { + let max_seq = stream.iter().fold(0, |max_seq, mblock| { + if mblock.header.sequence > max_seq { + mblock.header.sequence + } else { + max_seq + } + }); if (prev_block.anchored_block.header.parent_microblock == BlockHeaderHash([0u8; 32]) - && stream.len() == 0) + && max_seq == 0) || (prev_block.anchored_block.header.parent_microblock != BlockHeaderHash([0u8; 32]) - && stream.len() + && (max_seq as u32) <= (prev_block.anchored_block.header.parent_microblock_sequence - as usize) + as u32) + 1) { // the chain tip hasn't changed since we attempted to build a block. Use what we // already have. debug!("Stacks tip is unchanged since we last tried to mine a block ({}/{} at height {} with {} txs, in {} at burn height {}), and no new microblocks ({} <= {})", &prev_block.parent_consensus_hash, &prev_block.anchored_block.block_hash(), prev_block.anchored_block.header.total_work.work, - prev_block.anchored_block.txs.len(), prev_block.my_burn_hash, parent_block_burn_height, stream.len(), prev_block.anchored_block.header.parent_microblock_sequence); + prev_block.anchored_block.txs.len(), prev_block.my_burn_hash, parent_block_burn_height, max_seq, prev_block.anchored_block.header.parent_microblock_sequence); return None; } else { @@ -1416,7 +1423,7 @@ impl InitializedNeonNode { // fee minus the old BTC fee debug!("Stacks tip is unchanged since we last tried to mine a block ({}/{} at height {} with {} txs, in {} at burn height {}), but there are new microblocks ({} > {})", &prev_block.parent_consensus_hash, &prev_block.anchored_block.block_hash(), prev_block.anchored_block.header.total_work.work, - prev_block.anchored_block.txs.len(), prev_block.my_burn_hash, parent_block_burn_height, stream.len(), prev_block.anchored_block.header.parent_microblock_sequence); + prev_block.anchored_block.txs.len(), prev_block.my_burn_hash, parent_block_burn_height, max_seq, prev_block.anchored_block.header.parent_microblock_sequence); best_attempt = cmp::max(best_attempt, prev_block.attempt); } From fcf9f44809e7d46ceb22e5f50a32ad9c8744e5a8 Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Mon, 15 Feb 2021 14:23:01 -0500 Subject: [PATCH 3/5] fix failing unit test --- src/chainstate/stacks/db/blocks.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/chainstate/stacks/db/blocks.rs b/src/chainstate/stacks/db/blocks.rs index 7043bc638..02521d025 100644 --- a/src/chainstate/stacks/db/blocks.rs +++ b/src/chainstate/stacks/db/blocks.rs @@ -1356,7 +1356,7 @@ impl StacksChainState { let mut ret = vec![]; let mut tip: Option = None; let mut fork_poison = None; - let mut expected_sequence = 0; + let mut expected_sequence = start_seq; // load associated staging microblock data, but best-effort. // Stop loading once we find a fork juncture. From bacd337ce742e80138c7f422e7bd8bb088c5169c Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Mon, 15 Feb 2021 15:26:48 -0500 Subject: [PATCH 4/5] revert microblock-mining check (it was correct) --- testnet/stacks-node/src/neon_node.rs | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/testnet/stacks-node/src/neon_node.rs b/testnet/stacks-node/src/neon_node.rs index e3dc4dda8..2bd717d7f 100644 --- a/testnet/stacks-node/src/neon_node.rs +++ b/testnet/stacks-node/src/neon_node.rs @@ -1392,28 +1392,21 @@ impl InitializedNeonNode { u16::MAX, ) { - let max_seq = stream.iter().fold(0, |max_seq, mblock| { - if mblock.header.sequence > max_seq { - mblock.header.sequence - } else { - max_seq - } - }); if (prev_block.anchored_block.header.parent_microblock == BlockHeaderHash([0u8; 32]) - && max_seq == 0) + && stream.len() == 0) || (prev_block.anchored_block.header.parent_microblock != BlockHeaderHash([0u8; 32]) - && (max_seq as u32) + && stream.len() <= (prev_block.anchored_block.header.parent_microblock_sequence - as u32) + as usize) + 1) { // the chain tip hasn't changed since we attempted to build a block. Use what we // already have. debug!("Stacks tip is unchanged since we last tried to mine a block ({}/{} at height {} with {} txs, in {} at burn height {}), and no new microblocks ({} <= {})", &prev_block.parent_consensus_hash, &prev_block.anchored_block.block_hash(), prev_block.anchored_block.header.total_work.work, - prev_block.anchored_block.txs.len(), prev_block.my_burn_hash, parent_block_burn_height, max_seq, prev_block.anchored_block.header.parent_microblock_sequence); + prev_block.anchored_block.txs.len(), prev_block.my_burn_hash, parent_block_burn_height, stream.len(), prev_block.anchored_block.header.parent_microblock_sequence); return None; } else { @@ -1423,7 +1416,7 @@ impl InitializedNeonNode { // fee minus the old BTC fee debug!("Stacks tip is unchanged since we last tried to mine a block ({}/{} at height {} with {} txs, in {} at burn height {}), but there are new microblocks ({} > {})", &prev_block.parent_consensus_hash, &prev_block.anchored_block.block_hash(), prev_block.anchored_block.header.total_work.work, - prev_block.anchored_block.txs.len(), prev_block.my_burn_hash, parent_block_burn_height, max_seq, prev_block.anchored_block.header.parent_microblock_sequence); + prev_block.anchored_block.txs.len(), prev_block.my_burn_hash, parent_block_burn_height, stream.len(), prev_block.anchored_block.header.parent_microblock_sequence); best_attempt = cmp::max(best_attempt, prev_block.attempt); } From 1b4cb41563d5be10fc6b6e7a5db55d1691f6267e Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Mon, 15 Feb 2021 15:27:19 -0500 Subject: [PATCH 5/5] add changelog entry for the pending 2.0.6 release --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index eba41c6c5..7139266ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.0.6] + +### Fixed + +- The miner will no longer attempt to mine a new Stacks block if it receives a + microblock in a discontinuous microblock stream. + ## [2.0.5] - 2021-02-12 ### Added