consensus: implement validate_against_parent_4844 for Header
#1572
Conversation
tcoratger
requested review from
DaniPopes,
gakonst,
mattsse,
onbjerg and
emhane
as code owners
| /// 1. The current block header contains the `blob_gas_used` and `excess_blob_gas` fields. | ||
| /// 2. The `excess_blob_gas` field matches the expected value calculated from the parent header | ||
| /// fields. | ||
| pub fn validate_against_parent_4844(&self, parent: &Self) -> Result<(), ConsensusError> { |
There was a problem hiding this comment.
Wanted to create a follow up PR with unit tests to validate all the scenarios, but first I wanted to know if this proposal of migration would be accepted
| /// Expected excess blob gas. | ||
| expected: u64, | ||
| /// Actual excess blob gas. | ||
| got: u64, |
There was a problem hiding this comment.
I've seen several places in alloy where the got/expected error pattern is present, maybe it would be good to move the reth primitive
into alloy-primitives to be able to use it everywhere
| @@ -1 +1 @@ | |||
| msrv = "1.79" | |||
| msrv = "1.81" | |||
There was a problem hiding this comment.
I had to bump Rust due to the derive_more error stuff. I think this goes against
Alloy will keep a rolling MSRV policy of at least two versions behind the latest stable release (so if the latest stable release is 1.58, we would support 1.56).
I think that Rust 1.81 is also needed for #1552 so maybe we should wait depending on the prio level
Right now in Reth, we have a
validate_against_parent_4844that is implemented inside the consensus crate.https://github.com/paradigmxyz/reth/blob/1bdf429af5092a0b737e900977c1418bc070ec59/crates/consensus/common/src/validation.rs#L245-L274
Since it affects consensus and applies to the
Header, it seems to me that it makes sense to move it here (I don't know if this proposal will be accepted). So I also created a dedicatedConsensusErrortype in alloy.