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 <janito.vff@gmail.com>

* Another if expression

Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>

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.

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
+/// Check that the `activation_height`, `is_activation_height`,
-/// for `network`.
+/// `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));

zebra-state/src/arbitrary.rs

@@ -45,7 +45,7 @@ where
 impl From<PreparedBlock> for ChainTipBlock {
     fn from(prepared: PreparedBlock) -> Self {
         let PreparedBlock {
-            block: _,
+            block,
             hash,
             height,
             new_outputs: _,
@@ -55,6 +55,7 @@ impl From<PreparedBlock> for ChainTipBlock {
             hash,
             height,
             transaction_hashes,
+            previous_block_hash: block.header.previous_block_hash,
         }
     }
 }

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();

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<ContextuallyValidBlock> for ChainTipBlock {
     fn from(contextually_valid: ContextuallyValidBlock) -> Self {
         let ContextuallyValidBlock {
-            block: _,
+            block,
             hash,
             height,
             new_outputs: _,
@@ -53,6 +67,7 @@ impl From<ContextuallyValidBlock> for ChainTipBlock {
             hash,
             height,
             transaction_hashes,
+            previous_block_hash: block.header.previous_block_hash,
         }
     }
 }
@@ -60,7 +75,7 @@ impl From<ContextuallyValidBlock> for ChainTipBlock {
 impl From<FinalizedBlock> for ChainTipBlock {
     fn from(finalized: FinalizedBlock) -> Self {
         let FinalizedBlock {
-            block: _,
+            block,
             hash,
             height,
             new_outputs: _,
@@ -70,6 +85,7 @@ impl From<FinalizedBlock> 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<Option<ChainTipBlock>>,
+        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<ChainTipData>,
 
-    /// The most recent hash provided by this instance.
+    /// The most recent [`block::Hash`] provided by this instance.
-    previous_change_hash: Option<block::Hash>,
+    ///
+    /// ## 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<block::Hash>,
+
+    /// 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<TipAction, watch::error::RecvError> {
         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.
+    /// Return an action based on `block` and the last change we returned.
-    fn new(receiver: watch::Receiver<ChainTipData>) -> Self {
+    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<ChainTipData>, 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,
+        }
+    }
 }

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 zebra_chain::{
-
+    block::Block,
-use crate::{
+    chain_tip::ChainTip,
-    service::chain_tip::{ChainTipBlock, ChainTipSender, TipAction::*},
+    fmt::{DisplayToDebug, SummaryDebug},
-    FinalizedBlock,
+    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::<Vec<BlockUpdate>>(),
+        tip_updates in any::<SummaryDebug<Vec<BlockUpdate>>>(),
+        network in any::<Network>(),
     ) {
-        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<Arc<Block>>),
+    Finalized(Option<DisplayToDebug<Arc<Block>>>),
-    NonFinalized(Option<Arc<Block>>),
+    NonFinalized(Option<DisplayToDebug<Arc<Block>>>),
 }

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()

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)
             );
         }
     }