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.

Sign up for GitHub

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

refactor: make sender recovery explicit in provider #5776

Merged
merged 2 commits into from
Dec 15, 2023
Merged

refactor: make sender recovery explicit in provider #5776

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
05f29f3
refactor: explicit sender recovery in provider
onbjerg Dec 15, 2023
26d5a5c
fix: import
onbjerg Dec 15, 2023
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: 9 additions & 2 deletions bin/reth/src/debug_cmd/in_memory_merkle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use backon::{ConstantBuilder, Retryable};
use clap::Parser;
use reth_config::Config;
use reth_db::{init_db, DatabaseEnv};
use reth_interfaces::executor::BlockValidationError;
use reth_network::NetworkHandle;
use reth_network_api::NetworkInfo;
use reth_primitives::{fs, stage::StageId, BlockHashOrNumber, ChainSpec};
Expand Down Expand Up @@ -185,13 +186,19 @@ impl Command {

if in_memory_state_root == block.state_root {
info!(target: "reth::cli", state_root = ?in_memory_state_root, "Computed in-memory state root matches");
return Ok(())
return Ok(());
}

let provider_rw = factory.provider_rw()?;

// Insert block, state and hashes
provider_rw.insert_block(block.clone(), None, None)?;
provider_rw.insert_block(
block
.clone()
.try_seal_with_senders()
.map_err(|_| BlockValidationError::SenderRecoveryError)?,
None,
)?;
block_state.write_to_db(provider_rw.tx_ref(), OriginalValuesKnown::No)?;
let storage_lists = provider_rw.changed_storages_with_range(block.number..=block.number)?;
let storages = provider_rw.plain_state_storages(storage_lists)?;
Expand Down
8 changes: 4 additions & 4 deletions bin/reth/src/debug_cmd/merkle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,10 @@ impl Command {
Ok(senders) => senders,
Err(err) => {
warn!(target: "reth::cli", "Error sealing block with senders: {err:?}. Skipping...");
continue
continue;
}
};
provider_rw.insert_block(sealed_block.block, Some(sealed_block.senders), None)?;
provider_rw.insert_block(sealed_block, None)?;
}

// Check if any of hashing or merkle stages aren't on the same block number as
Expand Down Expand Up @@ -277,7 +277,7 @@ impl Command {
let clean_result = merkle_stage.execute(&provider_rw, clean_input);
assert!(clean_result.is_ok(), "Clean state root calculation failed");
if clean_result.unwrap().done {
break
break;
}
}

Expand Down Expand Up @@ -343,7 +343,7 @@ impl Command {
clean.1.nibbles.len() > self.skip_node_depth.unwrap_or_default()
{
first_mismatched_storage = Some((incremental, clean));
break
break;
}
}
(Some(incremental), None) => {
Expand Down
68 changes: 38 additions & 30 deletions crates/blockchain-tree/src/blockchain_tree.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,33 +161,35 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
if block.number <= last_finalized_block {
// check if block is canonical
if self.is_block_hash_canonical(&block.hash)? {
return Ok(Some(BlockStatus::Valid))
return Ok(Some(BlockStatus::Valid));
}

// check if block is inside database
if self.externals.provider_factory.provider()?.block_number(block.hash)?.is_some() {
return Ok(Some(BlockStatus::Valid))
return Ok(Some(BlockStatus::Valid));
}

return Err(BlockchainTreeError::PendingBlockIsFinalized {
last_finalized: last_finalized_block,
}
.into())
.into());
}

// check if block is part of canonical chain
if self.is_block_hash_canonical(&block.hash)? {
return Ok(Some(BlockStatus::Valid))
return Ok(Some(BlockStatus::Valid));
}

// is block inside chain
if let Some(status) = self.is_block_inside_chain(&block) {
return Ok(Some(status))
return Ok(Some(status));
}

// check if block is disconnected
if let Some(block) = self.state.buffered_blocks.block(block) {
return Ok(Some(BlockStatus::Disconnected { missing_ancestor: block.parent_num_hash() }))
return Ok(Some(BlockStatus::Disconnected {
missing_ancestor: block.parent_num_hash(),
}));
}

Ok(None)
Expand Down Expand Up @@ -278,7 +280,7 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {

// get canonical fork.
let canonical_fork = self.canonical_fork(chain_id)?;
return Some(BundleStateData { state, parent_block_hashed, canonical_fork })
return Some(BundleStateData { state, parent_block_hashed, canonical_fork });
}

// check if there is canonical block
Expand All @@ -288,7 +290,7 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
canonical_fork: ForkBlock { number: canonical_number, hash: block_hash },
state: BundleStateWithReceipts::default(),
parent_block_hashed: self.canonical_chain().inner().clone(),
})
});
}

None
Expand All @@ -311,15 +313,15 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
// check if block parent can be found in any side chain.
if let Some(chain_id) = self.block_indices().get_blocks_chain_id(&parent.hash) {
// found parent in side tree, try to insert there
return self.try_insert_block_into_side_chain(block, chain_id, block_validation_kind)
return self.try_insert_block_into_side_chain(block, chain_id, block_validation_kind);
}

// if not found, check if the parent can be found inside canonical chain.
if self
.is_block_hash_canonical(&parent.hash)
.map_err(|err| InsertBlockError::new(block.block.clone(), err.into()))?
{
return self.try_append_canonical_chain(block, block_validation_kind)
return self.try_append_canonical_chain(block, block_validation_kind);
}

// this is another check to ensure that if the block points to a canonical block its block
Expand All @@ -335,7 +337,7 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
block_number: block.number,
},
block.block,
))
));
}
}

Expand Down Expand Up @@ -412,7 +414,7 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
return Err(InsertBlockError::execution_error(
BlockValidationError::BlockPreMerge { hash: block.hash }.into(),
block.block,
))
));
}

let parent_header = provider
Expand Down Expand Up @@ -575,7 +577,7 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
} else {
// if there is no fork block that point to other chains, break the loop.
// it means that this fork joins to canonical block.
break
break;
}
}
hashes
Expand All @@ -596,9 +598,9 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
// get fork block chain
if let Some(fork_chain_id) = self.block_indices().get_blocks_chain_id(&fork.hash) {
chain_id = fork_chain_id;
continue
continue;
}
break
break;
}
(self.block_indices().canonical_hash(&fork.number) == Some(fork.hash)).then_some(fork)
}
Expand Down Expand Up @@ -705,7 +707,7 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
pub fn buffer_block(&mut self, block: SealedBlockWithSenders) -> Result<(), InsertBlockError> {
// validate block consensus rules
if let Err(err) = self.validate_block(&block) {
return Err(InsertBlockError::consensus_error(err, block.block))
return Err(InsertBlockError::consensus_error(err, block.block));
}

self.state.buffered_blocks.insert_block(block);
Expand All @@ -722,17 +724,17 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
?block,
"Failed to validate total difficulty for block {}: {e:?}", block.header.hash
);
return Err(e)
return Err(e);
}

if let Err(e) = self.externals.consensus.validate_header(block) {
error!(?block, "Failed to validate header {}: {e:?}", block.header.hash);
return Err(e)
return Err(e);
}

if let Err(e) = self.externals.consensus.validate_block(block) {
error!(?block, "Failed to validate block {}: {e:?}", block.header.hash);
return Err(e)
return Err(e);
}

Ok(())
Expand All @@ -753,7 +755,7 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
Some(BlockStatus::Valid)
} else {
Some(BlockStatus::Accepted)
}
};
}
None
}
Expand Down Expand Up @@ -794,7 +796,7 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {

// validate block consensus rules
if let Err(err) = self.validate_block(&block) {
return Err(InsertBlockError::consensus_error(err, block.block))
return Err(InsertBlockError::consensus_error(err, block.block));
}

Ok(InsertPayloadOk::Inserted(
Expand Down Expand Up @@ -963,7 +965,7 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
}

if header.is_none() && self.is_block_hash_inside_chain(*hash) {
return Ok(None)
return Ok(None);
}

if header.is_none() {
Expand Down Expand Up @@ -1018,17 +1020,17 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
return Err(CanonicalError::from(BlockValidationError::BlockPreMerge {
hash: *block_hash,
})
.into())
.into());
}
return Ok(CanonicalOutcome::AlreadyCanonical { header })
return Ok(CanonicalOutcome::AlreadyCanonical { header });
}

let Some(chain_id) = self.block_indices().get_blocks_chain_id(block_hash) else {
debug!(target: "blockchain_tree", ?block_hash, "Block hash not found in block indices");
return Err(CanonicalError::from(BlockchainTreeError::BlockHashNotFoundInChain {
block_hash: *block_hash,
})
.into())
.into());
};
let chain = self.state.chains.remove(&chain_id).expect("To be present");

Expand Down Expand Up @@ -1190,7 +1192,7 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
block_number: tip.number,
block_hash: tip.hash,
},
))))
))));
}

let (blocks, state) = chain.into_inner();
Expand All @@ -1214,7 +1216,7 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
pub fn unwind(&mut self, unwind_to: BlockNumber) -> RethResult<()> {
// nothing to be done if unwind_to is higher then the tip
if self.block_indices().canonical_tip().number <= unwind_to {
return Ok(())
return Ok(());
}
// revert `N` blocks from current canonical chain and put them inside BlockchanTree
let old_canon_chain = self.revert_canonical_from_database(unwind_to)?;
Expand Down Expand Up @@ -1343,7 +1345,12 @@ mod tests {
genesis.header.header.state_root = EMPTY_ROOT_HASH;
let provider = factory.provider_rw().unwrap();

provider.insert_block(genesis, None, None).unwrap();
provider
.insert_block(
genesis.try_seal_with_senders().expect("invalid tx signature in genesis"),
None,
)
.unwrap();

// insert first 10 blocks
for i in 0..10 {
Expand Down Expand Up @@ -1454,8 +1461,9 @@ mod tests {
let provider_rw = provider_factory.provider_rw().unwrap();
provider_rw
.insert_block(
SealedBlock::new(chain_spec.sealed_genesis_header(), Default::default()),
Some(Vec::new()),
SealedBlock::new(chain_spec.sealed_genesis_header(), Default::default())
.try_seal_with_senders()
.unwrap(),
None,
)
.unwrap();
Expand Down
Loading