From 44ac06775bf6bb77dea06d53a6fc5e04a79baf31 Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 2 Sep 2021 12:25:42 +1000 Subject: [PATCH] Reset `ChainTipChange`s on chain fork and network upgrade activation (#2721) * Use `TipAction::Reset` for initialization and out-of-order blocks Needs tests for: - cloning a `ChainTipChange` resets the cloned instance - skipped updates reset the cloned instance - changing forks resets the cloned instance * Use `TipAction::Reset` for network upgrade activation blocks * Use an `if` expression Co-authored-by: Janito Vaqueiro Ferreira Filho * Another if expression Co-authored-by: Janito Vaqueiro Ferreira Filho --- zebra-chain/src/parameters/network_upgrade.rs | 8 ++ zebra-chain/src/parameters/tests.rs | 35 ++++- zebra-state/src/arbitrary.rs | 3 +- zebra-state/src/service.rs | 2 +- zebra-state/src/service/chain_tip.rs | 120 +++++++++++++++--- .../src/service/chain_tip/tests/prop.rs | 27 ++-- .../src/service/chain_tip/tests/vectors.rs | 11 +- zebra-state/src/service/tests.rs | 21 ++- 8 files changed, 186 insertions(+), 41 deletions(-) diff --git a/zebra-chain/src/parameters/network_upgrade.rs b/zebra-chain/src/parameters/network_upgrade.rs index 297dc0d1..0f2c5cba 100644 --- a/zebra-chain/src/parameters/network_upgrade.rs +++ b/zebra-chain/src/parameters/network_upgrade.rs @@ -208,6 +208,14 @@ impl NetworkUpgrade { .next() } + /// Returns `true` if `height` is the activation height of any network upgrade + /// on `network`. + /// + /// Use [`activation_height`] to get the specific network upgrade. + pub fn is_activation_height(network: Network, height: block::Height) -> bool { + NetworkUpgrade::activation_list(network).contains_key(&height) + } + /// Returns a BTreeMap of NetworkUpgrades and their ConsensusBranchIds. /// /// Branch ids are the same for mainnet and testnet. diff --git a/zebra-chain/src/parameters/tests.rs b/zebra-chain/src/parameters/tests.rs index 33e5b683..166ab31a 100644 --- a/zebra-chain/src/parameters/tests.rs +++ b/zebra-chain/src/parameters/tests.rs @@ -50,6 +50,11 @@ fn activation_extremes(network: Network) { Some(&Genesis) ); assert_eq!(Genesis.activation_height(network), Some(block::Height(0))); + assert!(NetworkUpgrade::is_activation_height( + network, + block::Height(0) + )); + assert_eq!(NetworkUpgrade::current(network, block::Height(0)), Genesis); assert_eq!( NetworkUpgrade::next(network, block::Height(0)), @@ -64,6 +69,11 @@ fn activation_extremes(network: Network) { BeforeOverwinter.activation_height(network), Some(block::Height(1)) ); + assert!(NetworkUpgrade::is_activation_height( + network, + block::Height(1) + )); + assert_eq!( NetworkUpgrade::current(network, block::Height(1)), BeforeOverwinter @@ -73,12 +83,22 @@ fn activation_extremes(network: Network) { Some(Overwinter) ); + assert!(!NetworkUpgrade::is_activation_height( + network, + block::Height(2) + )); + // We assume that the last upgrade we know about continues forever // (even if we suspect that won't be true) assert_ne!( NetworkUpgrade::activation_list(network).get(&block::Height::MAX), Some(&Genesis) ); + assert!(!NetworkUpgrade::is_activation_height( + network, + block::Height::MAX + )); + assert_ne!( NetworkUpgrade::current(network, block::Height::MAX), Genesis @@ -98,8 +118,8 @@ fn activation_consistent_testnet() { activation_consistent(Testnet) } -/// Check that the activation_height, current, and next functions are consistent -/// for `network`. +/// Check that the `activation_height`, `is_activation_height`, +/// `current`, and `next` functions are consistent for `network`. fn activation_consistent(network: Network) { let activation_list = NetworkUpgrade::activation_list(network); let network_upgrades: HashSet<&NetworkUpgrade> = activation_list.values().collect(); @@ -108,6 +128,17 @@ fn activation_consistent(network: Network) { let height = network_upgrade .activation_height(network) .expect("activations must have a height"); + assert!(NetworkUpgrade::is_activation_height(network, height)); + + if height > block::Height(0) { + // Genesis is immediately followed by BeforeOverwinter, + // but the other network upgrades have multiple blocks between them + assert!(!NetworkUpgrade::is_activation_height( + network, + (height + 1).unwrap() + )); + } + assert_eq!(NetworkUpgrade::current(network, height), network_upgrade); // Network upgrades don't repeat assert_ne!(NetworkUpgrade::next(network, height), Some(network_upgrade)); diff --git a/zebra-state/src/arbitrary.rs b/zebra-state/src/arbitrary.rs index d43e76f1..968c572e 100644 --- a/zebra-state/src/arbitrary.rs +++ b/zebra-state/src/arbitrary.rs @@ -45,7 +45,7 @@ where impl From for ChainTipBlock { fn from(prepared: PreparedBlock) -> Self { let PreparedBlock { - block: _, + block, hash, height, new_outputs: _, @@ -55,6 +55,7 @@ impl From for ChainTipBlock { hash, height, transaction_hashes, + previous_block_hash: block.header.previous_block_hash, } } } diff --git a/zebra-state/src/service.rs b/zebra-state/src/service.rs index 23419214..3153f852 100644 --- a/zebra-state/src/service.rs +++ b/zebra-state/src/service.rs @@ -83,7 +83,7 @@ impl StateService { .map(FinalizedBlock::from) .map(ChainTipBlock::from); let (chain_tip_sender, latest_chain_tip, chain_tip_change) = - ChainTipSender::new(initial_tip); + ChainTipSender::new(initial_tip, network); let mem = NonFinalizedState::new(network); let queued_blocks = QueuedBlocks::default(); diff --git a/zebra-state/src/service/chain_tip.rs b/zebra-state/src/service/chain_tip.rs index f86f63a8..7c581902 100644 --- a/zebra-state/src/service/chain_tip.rs +++ b/zebra-state/src/service/chain_tip.rs @@ -9,7 +9,12 @@ use std::sync::Arc; use tokio::sync::watch; -use zebra_chain::{block, chain_tip::ChainTip, transaction}; +use zebra_chain::{ + block, + chain_tip::ChainTip, + parameters::{Network, NetworkUpgrade}, + transaction, +}; use crate::{request::ContextuallyValidBlock, FinalizedBlock}; @@ -37,12 +42,21 @@ pub struct ChainTipBlock { /// The mined transaction IDs of the transactions in `block`, /// in the same order as `block.transactions`. pub transaction_hashes: Arc<[transaction::Hash]>, + + /// The hash of the previous block in the best chain. + /// This block is immediately behind the best chain tip. + /// + /// ## Note + /// + /// If the best chain fork has changed, or some blocks have been skipped, + /// this hash will be different to the last returned `ChainTipBlock.hash`. + pub(crate) previous_block_hash: block::Hash, } impl From for ChainTipBlock { fn from(contextually_valid: ContextuallyValidBlock) -> Self { let ContextuallyValidBlock { - block: _, + block, hash, height, new_outputs: _, @@ -53,6 +67,7 @@ impl From for ChainTipBlock { hash, height, transaction_hashes, + previous_block_hash: block.header.previous_block_hash, } } } @@ -60,7 +75,7 @@ impl From for ChainTipBlock { impl From for ChainTipBlock { fn from(finalized: FinalizedBlock) -> Self { let FinalizedBlock { - block: _, + block, hash, height, new_outputs: _, @@ -70,6 +85,7 @@ impl From for ChainTipBlock { hash, height, transaction_hashes, + previous_block_hash: block.header.previous_block_hash, } } } @@ -93,9 +109,10 @@ pub struct ChainTipSender { impl ChainTipSender { /// Create new linked instances of [`ChainTipSender`], [`LatestChainTip`], and [`ChainTipChange`], - /// using `initial_tip` as the tip. + /// using an `initial_tip` and a [`Network`]. pub fn new( initial_tip: impl Into>, + network: Network, ) -> (Self, LatestChainTip, ChainTipChange) { let (sender, receiver) = watch::channel(None); @@ -106,7 +123,7 @@ impl ChainTipSender { }; let current = LatestChainTip::new(receiver.clone()); - let change = ChainTipChange::new(receiver); + let change = ChainTipChange::new(receiver, network); sender.update(initial_tip); @@ -227,8 +244,16 @@ pub struct ChainTipChange { /// The receiver for the current chain tip's data. receiver: watch::Receiver, - /// The most recent hash provided by this instance. - previous_change_hash: Option, + /// The most recent [`block::Hash`] provided by this instance. + /// + /// ## Note + /// + /// If the best chain fork has changed, or some blocks have been skipped, + /// this hash will be different to the last returned `ChainTipBlock.hash`. + last_change_hash: Option, + + /// The network for the chain tip. + network: Network, } /// Actions that we can take in response to a [`ChainTipChange`]. @@ -288,18 +313,64 @@ impl ChainTipChange { pub async fn tip_change(&mut self) -> Result { let block = self.tip_block_change().await?; - // TODO: handle resets here + let action = self.action(block.clone()); - self.previous_change_hash = Some(block.hash); + self.last_change_hash = Some(block.hash); - Ok(Grow { block }) + Ok(action) } - /// Create a new [`ChainTipChange`] from a watch channel receiver. - fn new(receiver: watch::Receiver) -> Self { + /// Return an action based on `block` and the last change we returned. + fn action(&self, block: ChainTipBlock) -> TipAction { + // check for an edge case that's dealt with by other code + assert!( + Some(block.hash) != self.last_change_hash, + "ChainTipSender ignores unchanged tips" + ); + + // If the previous block hash doesn't match, reset. + // We've either: + // - just initialized this instance, + // - changed the best chain to another fork (a rollback), or + // - skipped some blocks in the best chain. + // + // Consensus rules: + // + // > It is possible for a reorganization to occur + // > that rolls back from after the activation height, to before that height. + // > This can handled in the same way as any regular chain orphaning or reorganization, + // > as long as the new chain is valid. + // + // https://zips.z.cash/zip-0200#chain-reorganization + + // If we're at a network upgrade activation block, reset. + // + // Consensus rules: + // + // > When the current chain tip height reaches ACTIVATION_HEIGHT, + // > the node's local transaction memory pool SHOULD be cleared of transactions + // > that will never be valid on the post-upgrade consensus branch. + // + // https://zips.z.cash/zip-0200#memory-pool + // + // Skipped blocks can include network upgrade activation blocks. + // Fork changes can activate or deactivate a network upgrade. + // So we must perform the same actions for network upgrades and skipped blocks. + if Some(block.previous_block_hash) != self.last_change_hash + || NetworkUpgrade::is_activation_height(self.network, block.height) + { + TipAction::reset_with(block) + } else { + TipAction::grow_with(block) + } + } + + /// Create a new [`ChainTipChange`] from a watch channel receiver and [`Network`]. + fn new(receiver: watch::Receiver, network: Network) -> Self { Self { receiver, - previous_change_hash: None, + last_change_hash: None, + network, } } @@ -318,11 +389,6 @@ impl ChainTipChange { // Wait until there is actually Some block, // so we don't have `Option`s inside `TipAction`s. if let Some(block) = self.best_tip_block() { - assert!( - Some(block.hash) != self.previous_change_hash, - "ChainTipSender must ignore unchanged tips" - ); - return Ok(block); } } @@ -339,8 +405,11 @@ impl Clone for ChainTipChange { fn clone(&self) -> Self { Self { receiver: self.receiver.clone(), + // clear the previous change hash, so the first action is a reset - previous_change_hash: None, + last_change_hash: None, + + network: self.network, } } } @@ -359,4 +428,17 @@ impl TipAction { Reset { hash, .. } => *hash, } } + + /// Returns a [`Grow`] based on `block`. + pub(crate) fn grow_with(block: ChainTipBlock) -> Self { + Grow { block } + } + + /// Returns a [`Reset`] based on `block`. + pub(crate) fn reset_with(block: ChainTipBlock) -> Self { + Reset { + height: block.height, + hash: block.hash, + } + } } diff --git a/zebra-state/src/service/chain_tip/tests/prop.rs b/zebra-state/src/service/chain_tip/tests/prop.rs index f94f8e73..0ffd70cd 100644 --- a/zebra-state/src/service/chain_tip/tests/prop.rs +++ b/zebra-state/src/service/chain_tip/tests/prop.rs @@ -4,13 +4,15 @@ use futures::FutureExt; use proptest::prelude::*; use proptest_derive::Arbitrary; -use zebra_chain::{block::Block, chain_tip::ChainTip}; - -use crate::{ - service::chain_tip::{ChainTipBlock, ChainTipSender, TipAction::*}, - FinalizedBlock, +use zebra_chain::{ + block::Block, + chain_tip::ChainTip, + fmt::{DisplayToDebug, SummaryDebug}, + parameters::Network, }; +use crate::service::chain_tip::{ChainTipBlock, ChainTipSender, TipAction}; + const DEFAULT_BLOCK_VEC_PROPTEST_CASES: u32 = 4; proptest! { @@ -25,9 +27,10 @@ proptest! { /// or otherwise the finalized tip. #[test] fn best_tip_is_latest_non_finalized_then_latest_finalized( - tip_updates in any::>(), + tip_updates in any::>>(), + network in any::(), ) { - let (mut chain_tip_sender, latest_chain_tip, mut chain_tip_change) = ChainTipSender::new(None); + let (mut chain_tip_sender, latest_chain_tip, mut chain_tip_change) = ChainTipSender::new(None, network); let mut latest_finalized_tip = None; let mut latest_non_finalized_tip = None; @@ -36,14 +39,14 @@ proptest! { for update in tip_updates { match update { BlockUpdate::Finalized(block) => { - let chain_tip = block.clone().map(FinalizedBlock::from).map(ChainTipBlock::from); + let chain_tip = block.clone().map(|block| ChainTipBlock::from(block.0)); chain_tip_sender.set_finalized_tip(chain_tip.clone()); if let Some(block) = block { latest_finalized_tip = Some((chain_tip, block)); } } BlockUpdate::NonFinalized(block) => { - let chain_tip = block.clone().map(FinalizedBlock::from).map(ChainTipBlock::from); + let chain_tip = block.clone().map(|block| ChainTipBlock::from(block.0)); chain_tip_sender.set_best_non_finalized_tip(chain_tip.clone()); if let Some(block) = block { latest_non_finalized_tip = Some((chain_tip, block)); @@ -101,13 +104,13 @@ proptest! { .now_or_never() .transpose() .expect("watch sender is not dropped"), - expected_tip.map(|(_chain_tip, block)| Grow { block: block.into() }) + expected_tip.map(|(_chain_tip, block)| TipAction::reset_with(block.0.into())) ); } } #[derive(Arbitrary, Clone, Debug)] enum BlockUpdate { - Finalized(Option>), - NonFinalized(Option>), + Finalized(Option>>), + NonFinalized(Option>>), } diff --git a/zebra-state/src/service/chain_tip/tests/vectors.rs b/zebra-state/src/service/chain_tip/tests/vectors.rs index 1efcfc43..30597c6b 100644 --- a/zebra-state/src/service/chain_tip/tests/vectors.rs +++ b/zebra-state/src/service/chain_tip/tests/vectors.rs @@ -2,13 +2,17 @@ use std::iter; use futures::FutureExt; -use zebra_chain::chain_tip::{ChainTip, NoChainTip}; +use zebra_chain::{ + chain_tip::{ChainTip, NoChainTip}, + parameters::Network::*, +}; use super::super::ChainTipSender; #[test] fn current_best_tip_is_initially_empty() { - let (_chain_tip_sender, latest_chain_tip, _chain_tip_change) = ChainTipSender::new(None); + let (_chain_tip_sender, latest_chain_tip, _chain_tip_change) = + ChainTipSender::new(None, Mainnet); assert_eq!(latest_chain_tip.best_tip_height(), None); assert_eq!(latest_chain_tip.best_tip_hash(), None); @@ -32,7 +36,8 @@ fn empty_latest_chain_tip_is_empty() { #[test] fn chain_tip_change_is_initially_not_ready() { - let (_chain_tip_sender, _latest_chain_tip, mut chain_tip_change) = ChainTipSender::new(None); + let (_chain_tip_sender, _latest_chain_tip, mut chain_tip_change) = + ChainTipSender::new(None, Mainnet); let first = chain_tip_change .tip_change() diff --git a/zebra-state/src/service/tests.rs b/zebra-state/src/service/tests.rs index 814d65e3..22913990 100644 --- a/zebra-state/src/service/tests.rs +++ b/zebra-state/src/service/tests.rs @@ -17,7 +17,7 @@ use zebra_test::{prelude::*, transcript::Transcript}; use crate::{ arbitrary::Prepare, constants, init_test, - service::{chain_tip::TipAction::*, StateService}, + service::{chain_tip::TipAction, StateService}, tests::setup::{partial_nu5_chain_strategy, transaction_v4_from_coinbase}, BoxError, Config, FinalizedBlock, PreparedBlock, Request, Response, }; @@ -312,6 +312,14 @@ proptest! { for block in finalized_blocks { let expected_block = block.clone(); + let expected_action = if expected_block.height <= block::Height(1) { + // 0: reset by both initialization and the Genesis network upgrade + // 1: reset by the BeforeOverwinter network upgrade + TipAction::reset_with(expected_block.clone().into()) + } else { + TipAction::grow_with(expected_block.clone().into()) + }; + state_service.queue_and_commit_finalized(block); prop_assert_eq!(latest_chain_tip.best_tip_height(), Some(expected_block.height)); @@ -321,13 +329,20 @@ proptest! { .now_or_never() .transpose() .expect("watch sender is not dropped"), - Some(Grow { block: expected_block.into() }) + Some(expected_action) ); } for block in non_finalized_blocks { let expected_block = block.clone(); + let expected_action = if expected_block.height == block::Height(1) { + // 1: reset by the BeforeOverwinter network upgrade + TipAction::reset_with(expected_block.clone().into()) + } else { + TipAction::grow_with(expected_block.clone().into()) + }; + state_service.queue_and_commit_non_finalized(block); prop_assert_eq!(latest_chain_tip.best_tip_height(), Some(expected_block.height)); @@ -337,7 +352,7 @@ proptest! { .now_or_never() .transpose() .expect("watch sender is not dropped"), - Some(Grow { block: expected_block.into() }) + Some(expected_action) ); } }