chore(consensus): Clear up the naming and intention behind checks (#2415)

Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
This commit is contained in:
rakita
2023-04-26 18:04:36 +02:00
committed by GitHub
parent 57a19f91f0
commit 837555e296
10 changed files with 59 additions and 28 deletions

View File

@@ -149,9 +149,10 @@ impl AppendableChain {
C: Consensus,
EF: ExecutorFactory,
{
externals.consensus.validate_header(&block, U256::MAX)?;
externals.consensus.pre_validate_header(&block, parent_block)?;
externals.consensus.pre_validate_block(&block)?;
externals.consensus.validate_header_with_total_difficulty(&block, U256::MAX)?;
externals.consensus.validate_header(&block)?;
externals.consensus.validate_header_agains_parent(&block, parent_block)?;
externals.consensus.validate_block(&block)?;
let (unseal, senders) = block.into_components();
let unseal = unseal.unseal();

View File

@@ -50,7 +50,11 @@ impl AutoSealConsensus {
}
impl Consensus for AutoSealConsensus {
fn pre_validate_header(
fn validate_header(&self, _header: &SealedHeader) -> Result<(), ConsensusError> {
Ok(())
}
fn validate_header_agains_parent(
&self,
_header: &SealedHeader,
_parent: &SealedHeader,
@@ -58,7 +62,7 @@ impl Consensus for AutoSealConsensus {
Ok(())
}
fn validate_header(
fn validate_header_with_total_difficulty(
&self,
_header: &Header,
_total_difficulty: U256,
@@ -66,7 +70,7 @@ impl Consensus for AutoSealConsensus {
Ok(())
}
fn pre_validate_block(&self, _block: &SealedBlock) -> Result<(), ConsensusError> {
fn validate_block(&self, _block: &SealedBlock) -> Result<(), ConsensusError> {
Ok(())
}
}

View File

@@ -23,18 +23,21 @@ impl BeaconConsensus {
}
impl Consensus for BeaconConsensus {
fn pre_validate_header(
fn validate_header(&self, header: &SealedHeader) -> Result<(), ConsensusError> {
validation::validate_header_standalone(header, &self.chain_spec)?;
Ok(())
}
fn validate_header_agains_parent(
&self,
header: &SealedHeader,
parent: &SealedHeader,
) -> Result<(), ConsensusError> {
validation::validate_header_standalone(header, &self.chain_spec)?;
validation::validate_header_regarding_parent(parent, header, &self.chain_spec)?;
Ok(())
}
fn validate_header(
fn validate_header_with_total_difficulty(
&self,
header: &Header,
total_difficulty: U256,
@@ -76,7 +79,7 @@ impl Consensus for BeaconConsensus {
Ok(())
}
fn pre_validate_block(&self, block: &SealedBlock) -> Result<(), ConsensusError> {
fn validate_block(&self, block: &SealedBlock) -> Result<(), ConsensusError> {
validation::validate_block_standalone(block, &self.chain_spec)
}
}

View File

@@ -256,11 +256,8 @@ pub fn validate_header_regarding_parent(
})
}
// difficulty check is done by consensus.
// TODO(onbjerg): Unsure what the check here is supposed to be, but it should be moved to
// [BeaconConsensus]. if chain_spec.paris_status().block_number() > Some(child.number) {
// // TODO how this needs to be checked? As ice age did increment it by some formula
//}
// TODO Check difficulty increment between parent and child
// Ace age did increment it by some formula that we need to follow.
let mut parent_gas_limit = parent.gas_limit;

View File

@@ -11,13 +11,21 @@ pub use reth_rpc_types::engine::ForkchoiceState;
#[async_trait]
#[auto_impl::auto_impl(&, Arc)]
pub trait Consensus: Debug + Send + Sync {
/// Validate if the header is correct and follows consensus specification.
/// Validate if header is correct and follows consensus specification.
///
/// This is called on standalone header to check if all hashe
fn validate_header(&self, header: &SealedHeader) -> Result<(), ConsensusError>;
/// Validate that the header information regarding parent are correct.
/// This checks the block number, timestamp, basefee and gas limit increment.
///
/// This is called before properties that are not in the header itself (like total difficulty)
/// have been computed.
///
/// **This should not be called for the genesis block**.
fn pre_validate_header(
///
/// Note: Validating header against its parent does not include other Consensus validations.
fn validate_header_agains_parent(
&self,
header: &SealedHeader,
parent: &SealedHeader,
@@ -27,7 +35,9 @@ pub trait Consensus: Debug + Send + Sync {
/// computed properties (like total difficulty).
///
/// Some consensus engines may want to do additional checks here.
fn validate_header(
///
/// Note: validating headers with TD does not include other Consensus validation.
fn validate_header_with_total_difficulty(
&self,
header: &Header,
total_difficulty: U256,
@@ -40,7 +50,9 @@ pub trait Consensus: Debug + Send + Sync {
/// 11.1 "Ommer Validation".
///
/// **This should not be called for the genesis block**.
fn pre_validate_block(&self, block: &SealedBlock) -> Result<(), ConsensusError>;
///
/// Note: validating blocks does not include other validations of the Consensus
fn validate_block(&self, block: &SealedBlock) -> Result<(), ConsensusError>;
}
/// Consensus Errors

View File

@@ -77,8 +77,13 @@ pub fn validate_header_download(
parent: &SealedHeader,
) -> DownloadResult<()> {
ensure_parent(header, parent)?;
// validate header against parent
consensus
.pre_validate_header(header, parent)
.validate_header_agains_parent(header, parent)
.map_err(|error| DownloadError::HeaderValidation { hash: parent.hash(), error })?;
// validate header standalone
consensus
.validate_header(header)
.map_err(|error| DownloadError::HeaderValidation { hash: parent.hash(), error })?;
Ok(())
}

View File

@@ -6,6 +6,7 @@ use reth_primitives::{
};
use secp256k1::{KeyPair, Message as SecpMessage, Secp256k1, SecretKey, SECP256K1};
use std::{
cmp::{max, min},
collections::BTreeMap,
ops::{Range, RangeInclusive, Sub},
};
@@ -192,7 +193,7 @@ where
let (prev_from, _) = state.get_mut(&from).unwrap();
transition.push((from, *prev_from, Vec::new()));
transfer = transfer.min(prev_from.balance).max(U256::from(1));
transfer = max(min(transfer, prev_from.balance), U256::from(1));
prev_from.balance = prev_from.balance.wrapping_sub(transfer);
// deposit in receiving account and update storage

View File

@@ -156,7 +156,7 @@ impl Stream for TestDownload {
}
let empty = SealedHeader::default();
if let Err(error) = this.consensus.pre_validate_header(&empty, &empty) {
if let Err(error) = this.consensus.validate_header_agains_parent(&empty, &empty) {
this.done = true;
return Poll::Ready(Some(Err(DownloadError::HeaderValidation {
hash: empty.hash(),
@@ -306,7 +306,15 @@ impl StatusUpdater for TestStatusUpdater {
#[async_trait::async_trait]
impl Consensus for TestConsensus {
fn pre_validate_header(
fn validate_header(&self, _header: &SealedHeader) -> Result<(), ConsensusError> {
if self.fail_validation() {
Err(consensus::ConsensusError::BaseFeeMissing)
} else {
Ok(())
}
}
fn validate_header_agains_parent(
&self,
header: &SealedHeader,
parent: &SealedHeader,
@@ -318,7 +326,7 @@ impl Consensus for TestConsensus {
}
}
fn validate_header(
fn validate_header_with_total_difficulty(
&self,
header: &Header,
total_difficulty: U256,
@@ -330,7 +338,7 @@ impl Consensus for TestConsensus {
}
}
fn pre_validate_block(&self, _block: &SealedBlock) -> Result<(), consensus::ConsensusError> {
fn validate_block(&self, _block: &SealedBlock) -> Result<(), consensus::ConsensusError> {
if self.fail_validation() {
Err(consensus::ConsensusError::BaseFeeMissing)
} else {

View File

@@ -169,7 +169,7 @@ where
withdrawals: next_body.withdrawals,
};
if let Err(error) = self.consensus.pre_validate_block(&block) {
if let Err(error) = self.consensus.validate_block(&block) {
// Put the header back and return an error
let hash = block.hash();
self.headers.push_front(block.header);

View File

@@ -77,7 +77,7 @@ impl<DB: Database> Stage<DB> for TotalDifficultyStage {
td += header.difficulty;
self.consensus
.validate_header(&header, td)
.validate_header_with_total_difficulty(&header, td)
.map_err(|error| StageError::Validation { block: header.number, error })?;
cursor_td.append(block_number, td.into())?;
}