Merge of #9260

mergify[bot] · web-flow · commit 00fa3b4e3dd1 · 2025-02-26T12:19:57.000Z
diff --git a/zebra-state/src/constants.rs b/zebra-state/src/constants.rs
@@ -117,6 +117,12 @@ pub const MAX_FIND_BLOCK_HEADERS_RESULTS: u32 = 160;
 /// These database versions can be recreated from their directly preceding versions.
 pub const RESTORABLE_DB_VERSIONS: [u64; 1] = [26];
 
+/// The maximum number of invalidated block records.
+///
+/// This limits the memory use to around:
+/// `100 entries * up to 99 blocks * 2 MB per block = 20 GB`
+pub const MAX_INVALIDATED_BLOCKS: usize = 100;
+
 lazy_static! {
     /// Regex that matches the RocksDB error when its lock file is already open.
     pub static ref LOCK_FILE_ERROR: Regex = Regex::new("(lock file).*(temporarily unavailable)|(in use)|(being used by another process)").expect("regex is valid");
diff --git a/zebra-state/src/error.rs b/zebra-state/src/error.rs
@@ -46,11 +46,32 @@ pub type BoxError = Box<dyn std::error::Error + Send + Sync + 'static>;
 #[error("block is not contextually valid: {}", .0)]
 pub struct CommitSemanticallyVerifiedError(#[from] ValidateContextError);
 
+/// An error describing the reason a block or its descendants could not be reconsidered after
+/// potentially being invalidated from the chain_set.
+#[derive(Debug, Error)]
+pub enum ReconsiderError {
+    #[error("Block with hash {0} was not previously invalidated")]
+    MissingInvalidatedBlock(block::Hash),
+
+    #[error("Parent chain not found for block {0}")]
+    ParentChainNotFound(block::Hash),
+
+    #[error("Invalidated blocks list is empty when it should contain at least one block")]
+    InvalidatedBlocksEmpty,
+
+    #[error("{0}")]
+    ValidationError(#[from] ValidateContextError),
+}
+
 /// An error describing why a block failed contextual validation.
 #[derive(Debug, Error, Clone, PartialEq, Eq)]
 #[non_exhaustive]
 #[allow(missing_docs)]
 pub enum ValidateContextError {
+    #[error("block hash {block_hash} was previously invalidated")]
+    #[non_exhaustive]
+    BlockPreviouslyInvalidated { block_hash: block::Hash },
+
     #[error("block parent not found in any chain, or not enough blocks in chain")]
     #[non_exhaustive]
     NotReadyToBeCommitted,
diff --git a/zebra-state/src/service/non_finalized_state.rs b/zebra-state/src/service/non_finalized_state.rs
@@ -8,14 +8,17 @@ use std::{
     sync::Arc,
 };
 
+use indexmap::IndexMap;
 use zebra_chain::{
-    block::{self, Block, Hash},
+    block::{self, Block, Hash, Height},
     parameters::Network,
-    sprout, transparent,
+    sprout::{self},
+    transparent,
 };
 
 use crate::{
-    constants::MAX_NON_FINALIZED_CHAIN_FORKS,
+    constants::{MAX_INVALIDATED_BLOCKS, MAX_NON_FINALIZED_CHAIN_FORKS},
+    error::ReconsiderError,
     request::{ContextuallyVerifiedBlock, FinalizableBlock},
     service::{check, finalized_state::ZebraDb},
     SemanticallyVerifiedBlock, ValidateContextError,
@@ -47,7 +50,7 @@ pub struct NonFinalizedState {
 
     /// Blocks that have been invalidated in, and removed from, the non finalized
     /// state.
-    invalidated_blocks: HashMap<Hash, Arc<Vec<ContextuallyVerifiedBlock>>>,
+    invalidated_blocks: IndexMap<Height, Arc<Vec<ContextuallyVerifiedBlock>>>,
 
     // Configuration
     //
@@ -233,6 +236,10 @@ impl NonFinalizedState {
             self.insert(side_chain);
         }
 
+        // Remove all invalidated_blocks at or below the finalized height
+        self.invalidated_blocks
+            .retain(|height, _blocks| *height >= best_chain_root.height);
+
         self.update_metrics_for_chains();
 
         // Add the treestate to the finalized block.
@@ -294,13 +301,100 @@ impl NonFinalizedState {
             invalidated_blocks
         };
 
-        self.invalidated_blocks
-            .insert(block_hash, Arc::new(invalidated_blocks));
+        self.invalidated_blocks.insert(
+            invalidated_blocks.first().unwrap().clone().height,
+            Arc::new(invalidated_blocks),
+        );
+
+        while self.invalidated_blocks.len() > MAX_INVALIDATED_BLOCKS {
+            self.invalidated_blocks.shift_remove_index(0);
+        }
 
         self.update_metrics_for_chains();
         self.update_metrics_bars();
     }
 
+    /// Reconsiders a previously invalidated block and its descendants into the non-finalized state
+    /// based on a block_hash. Reconsidered blocks are inserted into the previous chain and re-inserted
+    /// into the chain_set.
+    pub fn reconsider_block(
+        &mut self,
+        block_hash: block::Hash,
+        finalized_state: &ZebraDb,
+    ) -> Result<(), ReconsiderError> {
+        // Get the invalidated blocks that were invalidated by the given block_hash
+        let height = self
+            .invalidated_blocks
+            .iter()
+            .find_map(|(height, blocks)| {
+                if blocks.first()?.hash == block_hash {
+                    Some(height)
+                } else {
+                    None
+                }
+            })
+            .ok_or(ReconsiderError::MissingInvalidatedBlock(block_hash))?;
+
+        let mut invalidated_blocks = self
+            .invalidated_blocks
+            .clone()
+            .shift_remove(height)
+            .ok_or(ReconsiderError::MissingInvalidatedBlock(block_hash))?;
+        let mut_blocks = Arc::make_mut(&mut invalidated_blocks);
+
+        // Find and fork the parent chain of the invalidated_root. Update the parent chain
+        // with the invalidated_descendants
+        let invalidated_root = mut_blocks
+            .first()
+            .ok_or(ReconsiderError::InvalidatedBlocksEmpty)?;
+
+        let root_parent_hash = invalidated_root.block.header.previous_block_hash;
+
+        // If the parent is the tip of the finalized_state we create a new chain and insert it
+        // into the non finalized state
+        let chain_result = if root_parent_hash == finalized_state.finalized_tip_hash() {
+            let chain = Chain::new(
+                &self.network,
+                finalized_state
+                    .finalized_tip_height()
+                    .ok_or(ReconsiderError::ParentChainNotFound(block_hash))?,
+                finalized_state.sprout_tree_for_tip(),
+                finalized_state.sapling_tree_for_tip(),
+                finalized_state.orchard_tree_for_tip(),
+                finalized_state.history_tree(),
+                finalized_state.finalized_value_pool(),
+            );
+            Arc::new(chain)
+        } else {
+            // The parent is not the finalized_tip and still exist in the NonFinalizedState
+            // or else we return an error due to the parent not existing in the NonFinalizedState
+            self.parent_chain(root_parent_hash)
+                .map_err(|_| ReconsiderError::ParentChainNotFound(block_hash))?
+        };
+
+        let mut modified_chain = Arc::unwrap_or_clone(chain_result);
+        for block in Arc::unwrap_or_clone(invalidated_blocks) {
+            modified_chain = modified_chain.push(block)?;
+        }
+
+        let (height, hash) = modified_chain.non_finalized_tip();
+
+        // Only track invalidated_blocks that are not yet finalized. Once blocks are finalized (below the best_chain_root_height)
+        // we can discard the block.
+        if let Some(best_chain_root_height) = finalized_state.finalized_tip_height() {
+            self.invalidated_blocks
+                .retain(|height, _blocks| *height >= best_chain_root_height);
+        }
+
+        self.insert_with(Arc::new(modified_chain), |chain_set| {
+            chain_set.retain(|chain| chain.non_finalized_tip_hash() != root_parent_hash)
+        });
+
+        self.update_metrics_for_committed_block(height, hash);
+
+        Ok(())
+    }
+
     /// Commit block to the non-finalized state as a new chain where its parent
     /// is the finalized tip.
     #[tracing::instrument(level = "debug", skip(self, finalized_state, prepared))]
@@ -352,6 +446,12 @@ impl NonFinalizedState {
         prepared: SemanticallyVerifiedBlock,
         finalized_state: &ZebraDb,
     ) -> Result<Arc<Chain>, ValidateContextError> {
+        if self.invalidated_blocks.contains_key(&prepared.height) {
+            return Err(ValidateContextError::BlockPreviouslyInvalidated {
+                block_hash: prepared.hash,
+            });
+        }
+
         // Reads from disk
         //
         // TODO: if these disk reads show up in profiles, run them in parallel, using std::thread::spawn()
@@ -624,7 +724,7 @@ impl NonFinalizedState {
     }
 
     /// Return the invalidated blocks.
-    pub fn invalidated_blocks(&self) -> HashMap<block::Hash, Arc<Vec<ContextuallyVerifiedBlock>>> {
+    pub fn invalidated_blocks(&self) -> IndexMap<Height, Arc<Vec<ContextuallyVerifiedBlock>>> {
         self.invalidated_blocks.clone()
     }
 
diff --git a/zebra-state/src/service/non_finalized_state/chain.rs b/zebra-state/src/service/non_finalized_state/chain.rs
@@ -359,15 +359,15 @@ impl Chain {
         (block, treestate)
     }
 
-    // Returns the block at the provided height and all of its descendant blocks.
+    /// Returns the block at the provided height and all of its descendant blocks.
     pub fn child_blocks(&self, block_height: &block::Height) -> Vec<ContextuallyVerifiedBlock> {
         self.blocks
             .range(block_height..)
             .map(|(_h, b)| b.clone())
             .collect()
     }
 
-    // Returns a new chain without the invalidated block or its descendants.
+    /// Returns a new chain without the invalidated block or its descendants.
     pub fn invalidate_block(
         &self,
         block_hash: block::Hash,
diff --git a/zebra-state/src/service/non_finalized_state/tests/vectors.rs b/zebra-state/src/service/non_finalized_state/tests/vectors.rs
@@ -216,6 +216,17 @@ fn finalize_pops_from_best_chain_for_network(network: Network) -> Result<()> {
     Ok(())
 }
 
+#[test]
+fn invalidate_block_removes_block_and_descendants_from_chain() -> Result<()> {
+    let _init_guard = zebra_test::init();
+
+    for network in Network::iter() {
+        invalidate_block_removes_block_and_descendants_from_chain_for_network(network)?;
+    }
+
+    Ok(())
+}
+
 fn invalidate_block_removes_block_and_descendants_from_chain_for_network(
     network: Network,
 ) -> Result<()> {
@@ -267,43 +278,125 @@ fn invalidate_block_removes_block_and_descendants_from_chain_for_network(
     );
 
     let invalidated_blocks_state = &state.invalidated_blocks;
-    assert!(
-        invalidated_blocks_state.contains_key(&block2.hash()),
-        "invalidated blocks map should reference the hash of block2"
-    );
 
-    let invalidated_blocks_state_descendants =
-        invalidated_blocks_state.get(&block2.hash()).unwrap();
+    // Find an entry in the IndexMap that contains block2 hash
+    let (_, invalidated_blocks_state_descendants) = invalidated_blocks_state
+        .iter()
+        .find_map(|(height, blocks)| {
+            assert!(
+                blocks.iter().any(|block| block.hash == block2.hash()),
+                "invalidated_blocks should reference the hash of block2"
+            );
+
+            if blocks.iter().any(|block| block.hash == block2.hash()) {
+                Some((height, blocks))
+            } else {
+                None
+            }
+        })
+        .unwrap();
 
     match network {
         Network::Mainnet => assert!(
             invalidated_blocks_state_descendants
                 .iter()
                 .any(|block| block.height == block::Height(653601)),
-            "invalidated descendants vec should contain block3"
+            "invalidated descendants should contain block3"
         ),
         Network::Testnet(_parameters) => assert!(
             invalidated_blocks_state_descendants
                 .iter()
                 .any(|block| block.height == block::Height(584001)),
-            "invalidated descendants vec should contain block3"
+            "invalidated descendants should contain block3"
         ),
     }
 
     Ok(())
 }
 
 #[test]
-fn invalidate_block_removes_block_and_descendants_from_chain() -> Result<()> {
+fn reconsider_block_and_reconsider_chain_correctly_reconsiders_blocks_and_descendants() -> Result<()>
+{
     let _init_guard = zebra_test::init();
 
     for network in Network::iter() {
-        invalidate_block_removes_block_and_descendants_from_chain_for_network(network)?;
+        reconsider_block_inserts_block_and_descendants_into_chain_for_network(network.clone())?;
     }
 
     Ok(())
 }
 
+fn reconsider_block_inserts_block_and_descendants_into_chain_for_network(
+    network: Network,
+) -> Result<()> {
+    let block1: Arc<Block> = Arc::new(network.test_block(653599, 583999).unwrap());
+    let block2 = block1.make_fake_child().set_work(10);
+    let block3 = block2.make_fake_child().set_work(1);
+
+    let mut state = NonFinalizedState::new(&network);
+    let finalized_state = FinalizedState::new(
+        &Config::ephemeral(),
+        &network,
+        #[cfg(feature = "elasticsearch")]
+        false,
+    );
+
+    let fake_value_pool = ValueBalance::<NonNegative>::fake_populated_pool();
+    finalized_state.set_finalized_value_pool(fake_value_pool);
+
+    state.commit_new_chain(block1.clone().prepare(), &finalized_state)?;
+    state.commit_block(block2.clone().prepare(), &finalized_state)?;
+    state.commit_block(block3.clone().prepare(), &finalized_state)?;
+
+    assert_eq!(
+        state
+            .best_chain()
+            .unwrap_or(&Arc::new(Chain::default()))
+            .blocks
+            .len(),
+        3
+    );
+
+    // Invalidate block2 to update the invalidated_blocks NonFinalizedState
+    state.invalidate_block(block2.hash());
+
+    // Perform checks to ensure the invalidated_block and descendants were added to the invalidated_block
+    // state
+    let post_invalidated_chain = state.best_chain().unwrap();
+
+    assert_eq!(post_invalidated_chain.blocks.len(), 1);
+    assert!(
+        post_invalidated_chain.contains_block_hash(block1.hash()),
+        "the new modified chain should contain block1"
+    );
+
+    assert!(
+        !post_invalidated_chain.contains_block_hash(block2.hash()),
+        "the new modified chain should not contain block2"
+    );
+    assert!(
+        !post_invalidated_chain.contains_block_hash(block3.hash()),
+        "the new modified chain should not contain block3"
+    );
+
+    // Reconsider block2 and check that both block2 and block3 were `reconsidered` into the
+    // best chain
+    state.reconsider_block(block2.hash(), &finalized_state.db)?;
+
+    let best_chain = state.best_chain().unwrap();
+
+    assert!(
+        best_chain.contains_block_hash(block2.hash()),
+        "the best chain should again contain block2"
+    );
+    assert!(
+        best_chain.contains_block_hash(block3.hash()),
+        "the best chain should again contain block3"
+    );
+
+    Ok(())
+}
+
 #[test]
 // This test gives full coverage for `take_chain_if`
 fn commit_block_extending_best_chain_doesnt_drop_worst_chains() -> Result<()> {
