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
wip: feat: op transaction validator #6403
Conversation
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.
beautiful
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.
Great cleanup, this is nice. Small nit on the OpBlockInfo
struct name, also need to add back in the check that disallows deposit transactions to enter the tx pool.
|
||
/// Tracks additional infos for the current block. | ||
#[derive(Debug, Default)] | ||
struct OpBlockInfo { |
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.
Can we rename this to OpL1BlockInfo
? This information is from the L1 origin block, name / doc could confuse readers into thinking it's L2 block information unless they look at the struct definition.
origin: TransactionOrigin, | ||
transaction: Tx, | ||
) -> TransactionValidationOutcome<Tx> { | ||
let outcome = self.inner.validate_one(origin, transaction); |
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.
We still have to disallow deposit transactions in the tx pool. Old deleted check: https://github.com/paradigmxyz/reth/pull/6403/files#diff-e68f26515a6d47a44e471aee555af9e2a01fa9306084aa4ea4b39591a01a4abcL145-L151
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 is not required, because they get rejected by the eth impl anyway, the test ensures that
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.
but I can add it back to make this clear
@mattsse bump on this post-builder merge when you have time |
closes #5892
This adds a standalone tx validator for OP.
BLOCKED by: integration in reth CLI