Skip to content

Commit 321dc6f

Browse files
authored
Merge of #9260
2 parents 0b8bef3 + 512721e commit 321dc6f

File tree

4 files changed

+153
-5
lines changed

4 files changed

+153
-5
lines changed

zebra-state/src/error.rs

+12
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,18 @@ pub type BoxError = Box<dyn std::error::Error + Send + Sync + 'static>;
4646
#[error("block is not contextually valid: {}", .0)]
4747
pub struct CommitSemanticallyVerifiedError(#[from] ValidateContextError);
4848

49+
/// An error describing the reason a block or its descendants could not be reconsidered after
50+
/// potentially being invalidated from the chain_set.
51+
#[derive(Debug, Error)]
52+
pub enum ReconsiderError {
53+
#[error("Block with hash {0} was not previously invalidated")]
54+
NonPreviouslyInvalidatedBlock(block::Hash),
55+
#[error("Parent chain not found for block {0}")]
56+
ParentChainNotFound(block::Hash),
57+
#[error("{0}")]
58+
ValidationError(#[from] ValidateContextError),
59+
}
60+
4961
/// An error describing why a block failed contextual validation.
5062
#[derive(Debug, Error, Clone, PartialEq, Eq)]
5163
#[non_exhaustive]

zebra-state/src/service/non_finalized_state.rs

+54-1
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,13 @@ use std::{
1111
use zebra_chain::{
1212
block::{self, Block, Hash},
1313
parameters::Network,
14-
sprout, transparent,
14+
sprout::{self},
15+
transparent,
1516
};
1617

1718
use crate::{
1819
constants::MAX_NON_FINALIZED_CHAIN_FORKS,
20+
error::ReconsiderError,
1921
request::{ContextuallyVerifiedBlock, FinalizableBlock},
2022
service::{check, finalized_state::ZebraDb},
2123
SemanticallyVerifiedBlock, ValidateContextError,
@@ -301,6 +303,57 @@ impl NonFinalizedState {
301303
self.update_metrics_bars();
302304
}
303305

306+
/// Reconsiders a previously invalidated block and its descendants into the non-finalized state
307+
/// based on a block_hash. Reconsidered blocks are inserted into the previous chain and re-inserted
308+
/// into the chain_set.
309+
pub fn reconsider_block(&mut self, block_hash: block::Hash) -> Result<(), ReconsiderError> {
310+
// Get the invalidated blocks that were invalidated by the given block_hash
311+
let Some(invalidated_blocks) = self.invalidated_blocks.get(&block_hash) else {
312+
return Err(ReconsiderError::NonPreviouslyInvalidatedBlock(block_hash));
313+
};
314+
315+
let mut blocks = invalidated_blocks.clone();
316+
let mut_blocks = Arc::make_mut(&mut blocks);
317+
318+
let Some(invalidated_root) = mut_blocks.first() else {
319+
return Err(ReconsiderError::NonPreviouslyInvalidatedBlock(block_hash));
320+
};
321+
322+
// Find and fork the parent chain of the invalidated_root. Update the parent chain
323+
// with the invalidated_descendants
324+
let prev_block_hash = invalidated_root.block.header.previous_block_hash;
325+
let Ok(parent_chain) = self.parent_chain(prev_block_hash) else {
326+
return Err(ReconsiderError::ParentChainNotFound(block_hash));
327+
};
328+
329+
let Some(new_chain) = parent_chain.fork(parent_chain.non_finalized_tip_hash()) else {
330+
return Err(ReconsiderError::ValidationError(
331+
ValidateContextError::NonSequentialBlock {
332+
candidate_height: invalidated_root.height,
333+
parent_height: parent_chain.non_finalized_tip_height(),
334+
},
335+
));
336+
};
337+
338+
let mut chain = new_chain
339+
.push(mut_blocks.remove(0))
340+
.map_err(ReconsiderError::ValidationError)?;
341+
for block in mut_blocks {
342+
chain = chain
343+
.push(block.clone())
344+
.map_err(ReconsiderError::ValidationError)?;
345+
}
346+
347+
self.insert_with(Arc::new(chain), |chain_set| {
348+
chain_set.retain(|c| c != &parent_chain)
349+
});
350+
351+
self.update_metrics_for_chains();
352+
self.update_metrics_bars();
353+
354+
Ok(())
355+
}
356+
304357
/// Commit block to the non-finalized state as a new chain where its parent
305358
/// is the finalized tip.
306359
#[tracing::instrument(level = "debug", skip(self, finalized_state, prepared))]

zebra-state/src/service/non_finalized_state/chain.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -359,15 +359,15 @@ impl Chain {
359359
(block, treestate)
360360
}
361361

362-
// Returns the block at the provided height and all of its descendant blocks.
362+
/// Returns the block at the provided height and all of its descendant blocks.
363363
pub fn child_blocks(&self, block_height: &block::Height) -> Vec<ContextuallyVerifiedBlock> {
364364
self.blocks
365365
.range(block_height..)
366366
.map(|(_h, b)| b.clone())
367367
.collect()
368368
}
369369

370-
// Returns a new chain without the invalidated block or its descendants.
370+
/// Returns a new chain without the invalidated block or its descendants.
371371
pub fn invalidate_block(
372372
&self,
373373
block_hash: block::Hash,

zebra-state/src/service/non_finalized_state/tests/vectors.rs

+85-2
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,17 @@ fn finalize_pops_from_best_chain_for_network(network: Network) -> Result<()> {
216216
Ok(())
217217
}
218218

219+
#[test]
220+
fn invalidate_block_removes_block_and_descendants_from_chain() -> Result<()> {
221+
let _init_guard = zebra_test::init();
222+
223+
for network in Network::iter() {
224+
invalidate_block_removes_block_and_descendants_from_chain_for_network(network)?;
225+
}
226+
227+
Ok(())
228+
}
229+
219230
fn invalidate_block_removes_block_and_descendants_from_chain_for_network(
220231
network: Network,
221232
) -> Result<()> {
@@ -294,16 +305,88 @@ fn invalidate_block_removes_block_and_descendants_from_chain_for_network(
294305
}
295306

296307
#[test]
297-
fn invalidate_block_removes_block_and_descendants_from_chain() -> Result<()> {
308+
fn reconsider_block_and_reconsider_chain_correctly_reconsiders_blocks_and_descendants() -> Result<()>
309+
{
298310
let _init_guard = zebra_test::init();
299311

300312
for network in Network::iter() {
301-
invalidate_block_removes_block_and_descendants_from_chain_for_network(network)?;
313+
reconsider_block_inserts_block_and_descendants_into_chain_for_network(network.clone())?;
302314
}
303315

304316
Ok(())
305317
}
306318

319+
fn reconsider_block_inserts_block_and_descendants_into_chain_for_network(
320+
network: Network,
321+
) -> Result<()> {
322+
let block1: Arc<Block> = Arc::new(network.test_block(653599, 583999).unwrap());
323+
let block2 = block1.make_fake_child().set_work(10);
324+
let block3 = block2.make_fake_child().set_work(1);
325+
326+
let mut state = NonFinalizedState::new(&network);
327+
let finalized_state = FinalizedState::new(
328+
&Config::ephemeral(),
329+
&network,
330+
#[cfg(feature = "elasticsearch")]
331+
false,
332+
);
333+
334+
let fake_value_pool = ValueBalance::<NonNegative>::fake_populated_pool();
335+
finalized_state.set_finalized_value_pool(fake_value_pool);
336+
337+
state.commit_new_chain(block1.clone().prepare(), &finalized_state)?;
338+
state.commit_block(block2.clone().prepare(), &finalized_state)?;
339+
state.commit_block(block3.clone().prepare(), &finalized_state)?;
340+
341+
assert_eq!(
342+
state
343+
.best_chain()
344+
.unwrap_or(&Arc::new(Chain::default()))
345+
.blocks
346+
.len(),
347+
3
348+
);
349+
350+
// Invalidate block2 to update the invalidated_blocks NonFinalizedState
351+
state.invalidate_block(block2.hash());
352+
353+
// Perform checks to ensure the invalidated_block and descendants were added to the invalidated_block
354+
// state
355+
let post_invalidated_chain = state.best_chain().unwrap();
356+
357+
assert_eq!(post_invalidated_chain.blocks.len(), 1);
358+
assert!(
359+
post_invalidated_chain.contains_block_hash(block1.hash()),
360+
"the new modified chain should contain block1"
361+
);
362+
363+
assert!(
364+
!post_invalidated_chain.contains_block_hash(block2.hash()),
365+
"the new modified chain should not contain block2"
366+
);
367+
assert!(
368+
!post_invalidated_chain.contains_block_hash(block3.hash()),
369+
"the new modified chain should not contain block3"
370+
);
371+
372+
// Reconsider block2 and check that both block2 and block3 were `reconsidered` into the
373+
// best chain
374+
state.reconsider_block(block2.hash())?;
375+
376+
let best_chain = state.best_chain().unwrap();
377+
378+
assert!(
379+
best_chain.contains_block_hash(block2.hash()),
380+
"the best chain should again contain block2"
381+
);
382+
assert!(
383+
best_chain.contains_block_hash(block3.hash()),
384+
"the best chain should again contain block3"
385+
);
386+
387+
Ok(())
388+
}
389+
307390
#[test]
308391
// This test gives full coverage for `take_chain_if`
309392
fn commit_block_extending_best_chain_doesnt_drop_worst_chains() -> Result<()> {

0 commit comments

Comments
 (0)