Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(tree): retain max(additional, max_reorg_depth) block hashes #4612

Merged
merged 7 commits into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions crates/blockchain-tree/src/blockchain_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
.tx()?
.cursor_read::<tables::CanonicalHeaders>()?
.walk_back(None)?
.take((max_reorg_depth + config.num_of_additional_canonical_block_hashes()) as usize)
.take(config.num_of_canonical_hashes() as usize)
.collect::<Result<Vec<(BlockNumber, BlockHash)>, _>>()?;

// TODO(rakita) save last finalized block inside database but for now just take
Expand Down Expand Up @@ -746,7 +746,7 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
/// Reads the last `N` canonical hashes from the database and updates the block indices of the
/// tree.
///
/// `N` is the `max_reorg_depth` plus the number of block hashes needed to satisfy the
/// `N` is the maximum of `max_reorg_depth` and the number of block hashes needed to satisfy the
/// `BLOCKHASH` opcode in the EVM.
///
/// # Note
Expand All @@ -765,19 +765,16 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
/// Reads the last `N` canonical hashes from the database and updates the block indices of the
/// tree.
///
/// `N` is the `max_reorg_depth` plus the number of block hashes needed to satisfy the
/// `N` is the maximum of `max_reorg_depth` and the number of block hashes needed to satisfy the
/// `BLOCKHASH` opcode in the EVM.
pub fn restore_canonical_hashes(&mut self) -> Result<(), Error> {
let num_of_canonical_hashes =
self.config.max_reorg_depth() + self.config.num_of_additional_canonical_block_hashes();

let last_canonical_hashes = self
.externals
.db
.tx()?
.cursor_read::<tables::CanonicalHeaders>()?
.walk_back(None)?
.take(num_of_canonical_hashes as usize)
.take(self.config.num_of_canonical_hashes() as usize)
.collect::<Result<BTreeMap<BlockNumber, BlockHash>, _>>()?;

let (mut remove_chains, _) =
Expand Down
22 changes: 19 additions & 3 deletions crates/blockchain-tree/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,13 @@ pub struct BlockchainTreeConfig {
max_reorg_depth: u64,
/// The number of unconnected blocks that we are buffering
max_unconnected_blocks: usize,
/// For EVM's "BLOCKHASH" opcode we require last 256 block hashes. So we need to specify
/// at least `additional_canonical_block_hashes`+`max_reorg_depth`, for eth that would be
/// 256+64.
/// Number of additional block hashes to save in blockchain tree. For `BLOCKHASH` EVM opcode we
/// need last 256 block hashes.
///
/// The total number of block hashes retained in-memory will be
/// `max(additional_canonical_block_hashes, max_reorg_depth)`, and for Ethereum that would
/// be 256. It covers both number of blocks required for reorg, and number of blocks
/// required for `BLOCKHASH` EVM opcode.
num_of_additional_canonical_block_hashes: u64,
}

Expand Down Expand Up @@ -68,6 +72,18 @@ impl BlockchainTreeConfig {
self.num_of_additional_canonical_block_hashes
}

/// Return total number of canonical hashes that we need to retain in order to have enough
/// information for reorg and EVM execution.
///
/// It is calculated as the maximum of `max_reorg_depth` (which is the number of blocks required
/// for the deepest reorg possible according to the consensus protocol) and
/// `num_of_additional_canonical_block_hashes` (which is the number of block hashes needed to
/// satisfy the `BLOCKHASH` opcode in the EVM. See [`crate::BundleStateDataRef`] and
/// [`crate::AppendableChain::new_canonical_head_fork`] where it's used).
pub fn num_of_canonical_hashes(&self) -> u64 {
self.max_reorg_depth.max(self.num_of_additional_canonical_block_hashes)
}

/// Return max number of unconnected blocks that we are buffering
pub fn max_unconnected_blocks(&self) -> usize {
self.max_unconnected_blocks
Expand Down
4 changes: 2 additions & 2 deletions crates/interfaces/src/blockchain_tree/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ pub trait BlockchainTreeEngine: BlockchainTreeViewer + Send + Sync {
/// Reads the last `N` canonical hashes from the database and updates the block indices of the
/// tree.
///
/// `N` is the `max_reorg_depth` plus the number of block hashes needed to satisfy the
/// `N` is the maximum of `max_reorg_depth` and the number of block hashes needed to satisfy the
/// `BLOCKHASH` opcode in the EVM.
///
/// # Note
Expand All @@ -70,7 +70,7 @@ pub trait BlockchainTreeEngine: BlockchainTreeViewer + Send + Sync {
/// Reads the last `N` canonical hashes from the database and updates the block indices of the
/// tree.
///
/// `N` is the `max_reorg_depth` plus the number of block hashes needed to satisfy the
/// `N` is the maximum of `max_reorg_depth` and the number of block hashes needed to satisfy the
/// `BLOCKHASH` opcode in the EVM.
fn restore_canonical_hashes(&self) -> Result<(), Error>;

Expand Down
Loading