Skip to content
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

refactor: use op-alloy deposit signature #12016

Merged

Conversation

caglaryucekaya
Copy link
Contributor

Closes #11781

Copy link
Member

@onbjerg onbjerg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

smol nit

@@ -82,7 +82,7 @@ pub fn legacy_parity(signature: &Signature, chain_id: Option<u64>) -> Parity {
// transactions with an empty signature
//
// NOTE: this is very hacky and only relevant for op-mainnet pre bedrock
if *signature == optimism_deposit_tx_signature() {
if *signature == TxDeposit::signature() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we want to move away from the cfg imports, so let's do

Suggested change
if *signature == TxDeposit::signature() {
if *signature == op_alloy_consensus::TxDeposit::signature() {

and remove the top level cfg import

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@onbjerg onbjerg added C-debt Refactor of code section that is hard to understand or maintain A-op-reth Related to Optimism and op-reth labels Oct 24, 2024
@onbjerg onbjerg added this pull request to the merge queue Oct 24, 2024
Merged via the queue into paradigmxyz:main with commit 082f2cd Oct 24, 2024
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-op-reth Related to Optimism and op-reth C-debt Refactor of code section that is hard to understand or maintain
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move optimism_deposit_signature to op-alloy
2 participants