-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feat(op-isthmus): l1tol2-msg-passer
storage root validation
#13311
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
//! Optimism consensus errors | ||
|
||
use alloy_primitives::B256; | ||
use derive_more::{Display, Error, From}; | ||
use reth_storage_errors::provider::ProviderError; | ||
|
||
/// Optimism consensus error. | ||
#[derive(Debug, PartialEq, Eq, Clone, Display, Error, From)] | ||
pub enum OpConsensusError { | ||
/// Block body has non-empty withdrawals list. | ||
#[display("non-empty withdrawals list")] | ||
Comment on lines
+8
to
+11
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. we can now use thiserror again and should stick to that for new error types |
||
WithdrawalsNonEmpty, | ||
/// Failed to load storage root of | ||
/// [`L2toL1MessagePasser`](reth_optimism_primitives::ADDRESS_L2_TO_L1_MESSAGE_PASSER). | ||
#[display("failed to load storage root of L2toL1MessagePasser pre-deploy: {_0}")] | ||
#[from] | ||
LoadStorageRootFailed(ProviderError), | ||
/// Storage root of | ||
/// [`L2toL1MessagePasser`](reth_optimism_primitives::ADDRESS_L2_TO_L1_MESSAGE_PASSER) missing | ||
/// in block (withdrawals root field). | ||
#[display("storage root of L2toL1MessagePasser missing (withdrawals root field empty)")] | ||
StorageRootMissing, | ||
/// Storage root of | ||
/// [`L2toL1MessagePasser`](reth_optimism_primitives::ADDRESS_L2_TO_L1_MESSAGE_PASSER) | ||
/// in block (withdrawals field), doesn't match local storage root. | ||
#[display("L2toL1MessagePasser storage root mismatch, got: {}, expected {expected}", got.map(|hash| hash.to_string()).unwrap_or_else(|| "null".to_string()))] | ||
StorageRootMismatch { | ||
/// Storage root of pre-deploy in block. | ||
got: Option<B256>, | ||
/// Storage root of pre-deploy loaded from local state. | ||
expected: B256, | ||
}, | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
//! Block validation w.r.t. consensus rules new in isthmus hardfork. | ||
|
||
use reth_consensus::ConsensusError; | ||
use reth_optimism_primitives::predeploys::ADDRESS_L2_TO_L1_MESSAGE_PASSER; | ||
use reth_primitives::SealedHeader; | ||
use reth_storage_api::{StateProviderFactory, StorageRootProvider}; | ||
use tracing::{debug, trace}; | ||
|
||
use crate::OpConsensusError; | ||
|
||
/// Validates block header field `withdrawals_root` against storage root of | ||
/// `L2-to-L1-message-passer` predeploy. | ||
pub fn validate_l2_to_l1_msg_passer( | ||
provider: impl StateProviderFactory, | ||
header: &SealedHeader, | ||
) -> Result<(), ConsensusError> { | ||
Comment on lines
+13
to
+16
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. can we split this into two, one fn that validates given the storage root, and one that computes the storage root and then calls this other fn (which we could move to op-alloy as well) because I assume kona could maybe also use this |
||
let state = provider.latest().map_err(|err| { | ||
debug!(target: "op::consensus", | ||
block_number=header.number, | ||
err=%OpConsensusError::LoadStorageRootFailed(err), | ||
"failed to load latest state", | ||
); | ||
|
||
ConsensusError::Other | ||
})?; | ||
|
||
let storage_root_msg_passer = | ||
state.storage_root(ADDRESS_L2_TO_L1_MESSAGE_PASSER, Default::default()).map_err(|err| { | ||
debug!(target: "op::consensus", | ||
block_number=header.number, | ||
err=%OpConsensusError::LoadStorageRootFailed(err), | ||
"failed to load storage root for l2tol1-msg-passer predeploy", | ||
); | ||
|
||
ConsensusError::Other | ||
})?; | ||
|
||
if header.withdrawals_root.is_none_or(|root| root != storage_root_msg_passer) { | ||
trace!(target: "op::consensus", | ||
block_number=header.number, | ||
err=%OpConsensusError::StorageRootMismatch { | ||
got: header.withdrawals_root, | ||
expected: storage_root_msg_passer | ||
}, | ||
"block failed validation", | ||
); | ||
|
||
return Err(ConsensusError::Other) | ||
} | ||
|
||
Ok(()) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
//! Addresses of OP pre-deploys. | ||
// todo: move to op-alloy | ||
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. yeah let's do that |
||
|
||
use alloy_primitives::{address, Address}; | ||
|
||
/// The L2 contract `L2ToL1MessagePasser`, stores commitments to withdrawal transactions. | ||
pub const ADDRESS_L2_TO_L1_MESSAGE_PASSER: Address = | ||
address!("4200000000000000000000000000000000000016"); |
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.
hmm, can we use box core::Error and remove the clone from this error type?
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 tried to remove the clone from the error type but it required too many changes and grew the scope of the pr significantly
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, I believe this is due to some other error type Clone derives
we can keep this but perhaps add a string value?
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.
this failed for another reason I recall, think it was requirement to be
Send + Sync
, in that case wrapped in anArc