-
Notifications
You must be signed in to change notification settings - Fork 2.3k
fix(op-reth/consensus): fixes header validation for jovian. decouple excess blob gas and blob gas used #19338
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,9 +18,9 @@ use core::fmt::Debug; | |
| use reth_chainspec::EthChainSpec; | ||
| use reth_consensus::{Consensus, ConsensusError, FullConsensus, HeaderValidator}; | ||
| use reth_consensus_common::validation::{ | ||
| validate_against_parent_4844, validate_against_parent_eip1559_base_fee, | ||
| validate_against_parent_hash_number, validate_against_parent_timestamp, validate_cancun_gas, | ||
| validate_header_base_fee, validate_header_extra_data, validate_header_gas, | ||
| validate_against_parent_eip1559_base_fee, validate_against_parent_hash_number, | ||
| validate_against_parent_timestamp, validate_cancun_gas, validate_header_base_fee, | ||
| validate_header_extra_data, validate_header_gas, | ||
| }; | ||
| use reth_execution_types::BlockExecutionResult; | ||
| use reth_optimism_forks::OpHardforks; | ||
|
|
@@ -188,9 +188,32 @@ where | |
| &self.chain_spec, | ||
| )?; | ||
|
|
||
| // ensure that the blob gas fields for this block | ||
| if let Some(blob_params) = self.chain_spec.blob_params_at_timestamp(header.timestamp()) { | ||
| validate_against_parent_4844(header.header(), parent.header(), blob_params)?; | ||
| // Ensure that the blob gas fields for this block are correctly set. | ||
| // In the op-stack, the excess blob gas is always 0 for all blocks after ecotone. | ||
| // The blob gas used and the excess blob gas should both be set after ecotone. | ||
| // After Jovian, the blob gas used contains the current DA footprint. | ||
| if self.chain_spec.is_ecotone_active_at_timestamp(header.timestamp()) { | ||
| let blob_gas_used = header.blob_gas_used().ok_or(ConsensusError::BlobGasUsedMissing)?; | ||
|
|
||
| // Before Jovian and after ecotone, the blob gas used should be 0. | ||
| if !self.chain_spec.is_jovian_active_at_timestamp(header.timestamp()) && | ||
| blob_gas_used != 0 | ||
|
Comment on lines
+199
to
+200
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see, after jovian the blob gas used value isnt validated, right?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blob gas used value is validated in the |
||
| { | ||
| return Err(ConsensusError::BlobGasUsedDiff(GotExpected { | ||
| got: blob_gas_used, | ||
| expected: 0, | ||
| })); | ||
| } | ||
|
|
||
| let excess_blob_gas = | ||
| header.excess_blob_gas().ok_or(ConsensusError::ExcessBlobGasMissing)?; | ||
|
Comment on lines
+208
to
+209
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see, excess_blob_gas must always be 0 after ecotone |
||
| if excess_blob_gas != 0 { | ||
| return Err(ConsensusError::ExcessBlobGasDiff { | ||
| diff: GotExpected { got: excess_blob_gas, expected: 0 }, | ||
| parent_excess_blob_gas: parent.excess_blob_gas().unwrap_or(0), | ||
| parent_blob_gas_used: parent.blob_gas_used().unwrap_or(0), | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| Ok(()) | ||
|
|
@@ -204,11 +227,14 @@ mod tests { | |
| use alloy_consensus::{BlockBody, Eip658Value, Header, Receipt, TxEip7702, TxReceipt}; | ||
| use alloy_eips::{eip4895::Withdrawals, eip7685::Requests}; | ||
| use alloy_primitives::{Address, Bytes, Signature, U256}; | ||
| use op_alloy_consensus::OpTypedTransaction; | ||
| use reth_consensus::{Consensus, ConsensusError, FullConsensus}; | ||
| use op_alloy_consensus::{ | ||
| encode_holocene_extra_data, encode_jovian_extra_data, OpTypedTransaction, | ||
| }; | ||
| use reth_chainspec::BaseFeeParams; | ||
| use reth_consensus::{Consensus, ConsensusError, FullConsensus, HeaderValidator}; | ||
| use reth_optimism_chainspec::{OpChainSpec, OpChainSpecBuilder, OP_MAINNET}; | ||
| use reth_optimism_primitives::{OpPrimitives, OpReceipt, OpTransactionSigned}; | ||
| use reth_primitives_traits::{proofs, GotExpected, RecoveredBlock, SealedBlock}; | ||
| use reth_primitives_traits::{proofs, GotExpected, RecoveredBlock, SealedBlock, SealedHeader}; | ||
| use reth_provider::BlockExecutionResult; | ||
|
|
||
| use crate::OpBeaconConsensus; | ||
|
|
@@ -452,4 +478,285 @@ mod tests { | |
| }) | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_header_min_base_fee_validation() { | ||
| const MIN_BASE_FEE: u64 = 1000; | ||
|
|
||
| let chain_spec = OpChainSpecBuilder::default() | ||
| .jovian_activated() | ||
| .genesis(OP_MAINNET.genesis.clone()) | ||
| .chain(OP_MAINNET.chain) | ||
| .build(); | ||
|
|
||
| // create a tx | ||
| let transaction = mock_tx(0); | ||
|
|
||
| let beacon_consensus = OpBeaconConsensus::new(Arc::new(chain_spec)); | ||
|
|
||
| let receipt = OpReceipt::Eip7702(Receipt { | ||
| status: Eip658Value::success(), | ||
| cumulative_gas_used: 0, | ||
| logs: vec![], | ||
| }); | ||
|
|
||
| let parent = Header { | ||
| number: 0, | ||
| base_fee_per_gas: Some(MIN_BASE_FEE / 10), | ||
| withdrawals_root: Some(proofs::calculate_withdrawals_root(&[])), | ||
| blob_gas_used: Some(0), | ||
| excess_blob_gas: Some(0), | ||
| transactions_root: proofs::calculate_transaction_root(std::slice::from_ref( | ||
| &transaction, | ||
| )), | ||
| gas_used: 0, | ||
| timestamp: u64::MAX - 1, | ||
| receipts_root: proofs::calculate_receipt_root(std::slice::from_ref(&receipt)), | ||
| logs_bloom: receipt.bloom(), | ||
| extra_data: encode_jovian_extra_data( | ||
| Default::default(), | ||
| BaseFeeParams::optimism(), | ||
| MIN_BASE_FEE, | ||
| ) | ||
| .unwrap(), | ||
| ..Default::default() | ||
| }; | ||
| let parent = SealedHeader::seal_slow(parent); | ||
|
|
||
| let header = Header { | ||
| number: 1, | ||
| base_fee_per_gas: Some(MIN_BASE_FEE), | ||
| withdrawals_root: Some(proofs::calculate_withdrawals_root(&[])), | ||
| blob_gas_used: Some(0), | ||
| excess_blob_gas: Some(0), | ||
| transactions_root: proofs::calculate_transaction_root(std::slice::from_ref( | ||
| &transaction, | ||
| )), | ||
| gas_used: 0, | ||
| timestamp: u64::MAX, | ||
| receipts_root: proofs::calculate_receipt_root(std::slice::from_ref(&receipt)), | ||
| logs_bloom: receipt.bloom(), | ||
| parent_hash: parent.hash(), | ||
| ..Default::default() | ||
| }; | ||
| let header = SealedHeader::seal_slow(header); | ||
|
|
||
| let result = beacon_consensus.validate_header_against_parent(&header, &parent); | ||
|
|
||
| assert!(result.is_ok()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_header_min_base_fee_validation_failure() { | ||
| const MIN_BASE_FEE: u64 = 1000; | ||
|
|
||
| let chain_spec = OpChainSpecBuilder::default() | ||
| .jovian_activated() | ||
| .genesis(OP_MAINNET.genesis.clone()) | ||
| .chain(OP_MAINNET.chain) | ||
| .build(); | ||
|
|
||
| // create a tx | ||
| let transaction = mock_tx(0); | ||
|
|
||
| let beacon_consensus = OpBeaconConsensus::new(Arc::new(chain_spec)); | ||
|
|
||
| let receipt = OpReceipt::Eip7702(Receipt { | ||
| status: Eip658Value::success(), | ||
| cumulative_gas_used: 0, | ||
| logs: vec![], | ||
| }); | ||
|
|
||
| let parent = Header { | ||
| number: 0, | ||
| base_fee_per_gas: Some(MIN_BASE_FEE / 10), | ||
| withdrawals_root: Some(proofs::calculate_withdrawals_root(&[])), | ||
| blob_gas_used: Some(0), | ||
| excess_blob_gas: Some(0), | ||
| transactions_root: proofs::calculate_transaction_root(std::slice::from_ref( | ||
| &transaction, | ||
| )), | ||
| gas_used: 0, | ||
| timestamp: u64::MAX - 1, | ||
| receipts_root: proofs::calculate_receipt_root(std::slice::from_ref(&receipt)), | ||
| logs_bloom: receipt.bloom(), | ||
| extra_data: encode_jovian_extra_data( | ||
| Default::default(), | ||
| BaseFeeParams::optimism(), | ||
| MIN_BASE_FEE, | ||
| ) | ||
| .unwrap(), | ||
| ..Default::default() | ||
| }; | ||
| let parent = SealedHeader::seal_slow(parent); | ||
|
|
||
| let header = Header { | ||
| number: 1, | ||
| base_fee_per_gas: Some(MIN_BASE_FEE - 1), | ||
| withdrawals_root: Some(proofs::calculate_withdrawals_root(&[])), | ||
| blob_gas_used: Some(0), | ||
| excess_blob_gas: Some(0), | ||
| transactions_root: proofs::calculate_transaction_root(std::slice::from_ref( | ||
| &transaction, | ||
| )), | ||
| gas_used: 0, | ||
| timestamp: u64::MAX, | ||
| receipts_root: proofs::calculate_receipt_root(std::slice::from_ref(&receipt)), | ||
| logs_bloom: receipt.bloom(), | ||
| parent_hash: parent.hash(), | ||
| ..Default::default() | ||
| }; | ||
| let header = SealedHeader::seal_slow(header); | ||
|
|
||
| let result = beacon_consensus.validate_header_against_parent(&header, &parent); | ||
|
|
||
| assert!(result.is_err()); | ||
| assert_eq!( | ||
| result.unwrap_err(), | ||
| ConsensusError::BaseFeeDiff(GotExpected { | ||
| got: MIN_BASE_FEE - 1, | ||
| expected: MIN_BASE_FEE, | ||
| }) | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_header_da_footprint_validation() { | ||
| const MIN_BASE_FEE: u64 = 100_000; | ||
| const DA_FOOTPRINT: u64 = GAS_LIMIT - 1; | ||
| const GAS_LIMIT: u64 = 100_000_000; | ||
|
|
||
| let chain_spec = OpChainSpecBuilder::default() | ||
| .jovian_activated() | ||
| .genesis(OP_MAINNET.genesis.clone()) | ||
| .chain(OP_MAINNET.chain) | ||
| .build(); | ||
|
|
||
| // create a tx | ||
| let transaction = mock_tx(0); | ||
|
|
||
| let beacon_consensus = OpBeaconConsensus::new(Arc::new(chain_spec)); | ||
|
|
||
| let receipt = OpReceipt::Eip7702(Receipt { | ||
| status: Eip658Value::success(), | ||
| cumulative_gas_used: 0, | ||
| logs: vec![], | ||
| }); | ||
|
|
||
| let parent = Header { | ||
| number: 0, | ||
| base_fee_per_gas: Some(MIN_BASE_FEE), | ||
| withdrawals_root: Some(proofs::calculate_withdrawals_root(&[])), | ||
| blob_gas_used: Some(DA_FOOTPRINT), | ||
| excess_blob_gas: Some(0), | ||
| transactions_root: proofs::calculate_transaction_root(std::slice::from_ref( | ||
| &transaction, | ||
| )), | ||
| gas_used: 0, | ||
| timestamp: u64::MAX - 1, | ||
| receipts_root: proofs::calculate_receipt_root(std::slice::from_ref(&receipt)), | ||
| logs_bloom: receipt.bloom(), | ||
| extra_data: encode_jovian_extra_data( | ||
| Default::default(), | ||
| BaseFeeParams::optimism(), | ||
| MIN_BASE_FEE, | ||
| ) | ||
| .unwrap(), | ||
| gas_limit: GAS_LIMIT, | ||
| ..Default::default() | ||
| }; | ||
| let parent = SealedHeader::seal_slow(parent); | ||
|
|
||
| let header = Header { | ||
| number: 1, | ||
| base_fee_per_gas: Some(MIN_BASE_FEE + MIN_BASE_FEE / 10), | ||
| withdrawals_root: Some(proofs::calculate_withdrawals_root(&[])), | ||
| blob_gas_used: Some(DA_FOOTPRINT), | ||
| excess_blob_gas: Some(0), | ||
| transactions_root: proofs::calculate_transaction_root(std::slice::from_ref( | ||
| &transaction, | ||
| )), | ||
| gas_used: 0, | ||
| timestamp: u64::MAX, | ||
| receipts_root: proofs::calculate_receipt_root(std::slice::from_ref(&receipt)), | ||
| logs_bloom: receipt.bloom(), | ||
| parent_hash: parent.hash(), | ||
| ..Default::default() | ||
| }; | ||
| let header = SealedHeader::seal_slow(header); | ||
|
|
||
| let result = beacon_consensus.validate_header_against_parent(&header, &parent); | ||
|
|
||
| assert!(result.is_ok()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_header_isthmus_validation() { | ||
| const MIN_BASE_FEE: u64 = 100_000; | ||
| const DA_FOOTPRINT: u64 = GAS_LIMIT - 1; | ||
| const GAS_LIMIT: u64 = 100_000_000; | ||
|
|
||
| let chain_spec = OpChainSpecBuilder::default() | ||
| .isthmus_activated() | ||
| .genesis(OP_MAINNET.genesis.clone()) | ||
| .chain(OP_MAINNET.chain) | ||
| .build(); | ||
|
|
||
| // create a tx | ||
| let transaction = mock_tx(0); | ||
|
|
||
| let beacon_consensus = OpBeaconConsensus::new(Arc::new(chain_spec)); | ||
|
|
||
| let receipt = OpReceipt::Eip7702(Receipt { | ||
| status: Eip658Value::success(), | ||
| cumulative_gas_used: 0, | ||
| logs: vec![], | ||
| }); | ||
|
|
||
| let parent = Header { | ||
| number: 0, | ||
| base_fee_per_gas: Some(MIN_BASE_FEE), | ||
| withdrawals_root: Some(proofs::calculate_withdrawals_root(&[])), | ||
| blob_gas_used: Some(DA_FOOTPRINT), | ||
| excess_blob_gas: Some(0), | ||
| transactions_root: proofs::calculate_transaction_root(std::slice::from_ref( | ||
| &transaction, | ||
| )), | ||
| gas_used: 0, | ||
| timestamp: u64::MAX - 1, | ||
| receipts_root: proofs::calculate_receipt_root(std::slice::from_ref(&receipt)), | ||
| logs_bloom: receipt.bloom(), | ||
| extra_data: encode_holocene_extra_data(Default::default(), BaseFeeParams::optimism()) | ||
| .unwrap(), | ||
| gas_limit: GAS_LIMIT, | ||
| ..Default::default() | ||
| }; | ||
| let parent = SealedHeader::seal_slow(parent); | ||
|
|
||
| let header = Header { | ||
| number: 1, | ||
| base_fee_per_gas: Some(MIN_BASE_FEE - 2 * MIN_BASE_FEE / 100), | ||
| withdrawals_root: Some(proofs::calculate_withdrawals_root(&[])), | ||
| blob_gas_used: Some(DA_FOOTPRINT), | ||
| excess_blob_gas: Some(0), | ||
| transactions_root: proofs::calculate_transaction_root(std::slice::from_ref( | ||
| &transaction, | ||
| )), | ||
| gas_used: 0, | ||
| timestamp: u64::MAX, | ||
| receipts_root: proofs::calculate_receipt_root(std::slice::from_ref(&receipt)), | ||
| logs_bloom: receipt.bloom(), | ||
| parent_hash: parent.hash(), | ||
| ..Default::default() | ||
| }; | ||
| let header = SealedHeader::seal_slow(header); | ||
|
|
||
| let result = beacon_consensus.validate_header_against_parent(&header, &parent); | ||
|
|
||
| assert!(result.is_err()); | ||
| assert_eq!( | ||
| result.unwrap_err(), | ||
| ConsensusError::BlobGasUsedDiff(GotExpected { got: DA_FOOTPRINT, expected: 0 }) | ||
| ); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see we don't really need the parent validation