Skip to content

Conversation

@mdehoog
Copy link
Contributor

@mdehoog mdehoog commented Oct 20, 2023

Motivation

This PR adds anvil support for op-stack's deposit transaction. This is a requirement for mocktimism (see ethereum-optimism/mocktimism#4), which is an anvil wrapper for testing and forking op-stack chains.

Fortunately, deposit tx support has already been added to ethers-rs (gakonst/ethers-rs#2434) and revm (bluealloy/revm#682), so adding to anvil is mostly just plumbing the tx types through.

Solution

This PR adds all the required fields and types for deposit transactions. Support for optimism can be enabled by passing the --optimism flag to anvil.

TODO

@Evalir
Copy link
Member

Evalir commented Nov 3, 2023

gm! what's the status here? this is gonna need a rebase. sorry about that!

@mdehoog
Copy link
Contributor Author

mdehoog commented Nov 6, 2023

@Evalir thanks for the ping, I just rebased. Hoping to get to the tests this week sometime.

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

this looks pretty good already, a few nits

impersonated-tx = []
fastrlp = ["dep:open-fastrlp"]
serde = ["dep:serde"]
optimism = ["dep:ethers-core", "dep:ethers-contract", "dep:ethers-providers", "dep:ethers-middleware"]
Copy link
Member

Choose a reason for hiding this comment

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

this should just be optimism = ["ethers-core/optimism"]

}

/// Returns true if op-stack deposits are active
pub fn is_op_deposits(&self) -> bool {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pub fn is_op_deposits(&self) -> bool {
pub fn is_optimism(&self) -> bool {

Comment on lines 1929 to 1932
let mut deposit_nonce: Option<u64> = None;
if transaction_type.unwrap_or_default() == 0x7E {
deposit_nonce = Some(info.nonce);
}
Copy link
Member

Choose a reason for hiding this comment

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

deposit_nonce = transaction_type.and_then(|ty| (x == 0x7E).then_some(info.nonce))

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

this is great!

love the tests.

I bumped lock file so that it compiles, only have a few nitpicks.

there's a failing test that can be attributed to the activated feature can_read_cache which I will fix

@mattsse mattsse marked this pull request as ready for review November 21, 2023 15:29
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

lgtm, ptal @Evalir

Copy link
Member

@Evalir Evalir left a comment

Choose a reason for hiding this comment

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

thanks! this looks quite good!

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

fantastic!

@mattsse mattsse merged commit 388e181 into foundry-rs:master Nov 23, 2023
@mdehoog mdehoog deleted the op-deposit branch December 11, 2023 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants