Skip to content

Commit

Permalink
refactor: make sender recovery explicit in provider (paradigmxyz#5776)
Browse files Browse the repository at this point in the history
  • Loading branch information
onbjerg authored Dec 15, 2023
1 parent faa9a22 commit 3f7760d
Show file tree
Hide file tree
Showing 12 changed files with 157 additions and 142 deletions.
8 changes: 7 additions & 1 deletion bin/reth/src/debug_cmd/in_memory_merkle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,13 @@ impl Command {
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
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
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

0 comments on commit 3f7760d

Please sign in to comment.