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

feat(anvil): 7368 reorg #8489

Merged
merged 7 commits into from
Sep 5, 2024
Merged

feat(anvil): 7368 reorg #8489

merged 7 commits into from
Sep 5, 2024

Conversation

EdwardJES
Copy link
Contributor

@EdwardJES EdwardJES commented Jul 21, 2024

closes #7368

Motivation

Chain reorgs are an unavoidable part of the Ethereum PoS network, and applications should be tolerant to them. L2's also often rely on Ethereum for their finality, such as OP and the derivation pipeline. Having functionality to test reorgs is an important part of building fault-tolerant and resilient systems. What happens when your indexer reorgs? How will you relayer service handle duplicate nonces being consumed? Should the reorged relay transactions be resubmitted? Any service that has side effects from chain state needs to be tested against reorg scenarios.

Solution

  • Find the common ancestor between current chain and reorg chain
  • Revert state back to common ancestor
  • Mine new blocks to the desired len, optionally passing in a set of tx block pairs

@EdwardJES EdwardJES changed the title [WIP] feat(anvil): 7368 reorg feat(anvil): 7368 reorg Jul 29, 2024
@EdwardJES EdwardJES marked this pull request as ready for review July 29, 2024 02:03
@EdwardJES
Copy link
Contributor Author

Hey @zerosnacks, @mattsse

I've had a go at trying to implement this, from what I can see the logic seems to be sound. However, there are likely some edge cases that were missed, or more efficient code paths could have been taken.

I would also appreciate any feedback on the ergonomics of this API. What are your thoughts on supplying a JSON file where each object is Vec<(TransactionRequest, u64).

Thank you 🙏

@mattsse
Copy link
Member

mattsse commented Aug 4, 2024

hey @EdwardJES sorry for missing the pings...
taking a look shortly

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.

ty, I think we can get this supported in some form, but I have some suggestions questions re implementation.

This is supposed to delete a given number of blocks and re-execute transactions to form a new chain, right?

so is this effectively multiple sequential calls of

async fn do_mine_block(
&self,
pool_transactions: Vec<Arc<PoolTransaction>>,
) -> MinedBlockOutcome {

after blocks and their state have been removed?
I feel like we can simplify a few things here

crates/anvil/src/anvil.rs Outdated Show resolved Hide resolved
crates/anvil/src/eth/backend/db.rs Outdated Show resolved Hide resolved
crates/anvil/core/src/eth/mod.rs Outdated Show resolved Hide resolved
crates/anvil/src/eth/api.rs Outdated Show resolved Hide resolved
@EdwardJES
Copy link
Contributor Author

Hey @mattsse, this is ready for another review. I've simplified the logic that touches storage, but struggled to simplify the logic of transforming the TransactionRequests into a mineable state.

Here is an example payload if you'd like to try it out tyvm 🙏

curl -X POST http://127.0.0.1:8545/ \
-H "Content-Type: application/json" \
-d '{
    "jsonrpc": "2.0",
    "method": "anvil_reorg",
    "params": [
        5,  # Example depth
        [
            [
                {
                    "from": "0x976EA74026E726554dB657fA54763abd0C3a0aa9",
                    "to": "0x1199bc69f16FDD6690DC40339EC445FaE1b6DD11",
                    "value": 100
                },
                1
            ],
            [
                {
                    "from": "0x976EA74026E726554dB657fA54763abd0C3a0aa9",
                    "to": "0x1199bc69f16FDD6690DC40339EC445FaE1b6DD11",
                    "value": 200
                },
                2
            ]
        ]
    ],
    "id": 1
}'

@EdwardJES EdwardJES requested a review from mattsse August 16, 2024 05:54
@EdwardJES
Copy link
Contributor Author

Hey @zerosnacks, @mattsse, could I please get another round of reviews at your earliest convenience? 🙏 Tyvm.

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.

last few nits, otherwise I think this should work.

sorry for the delay

crates/anvil/core/src/types.rs Outdated Show resolved Hide resolved
crates/anvil/src/eth/backend/mem/storage.rs Outdated Show resolved Hide resolved
crates/anvil/src/eth/backend/mem/mod.rs Show resolved Hide resolved
crates/anvil/src/eth/backend/mem/mod.rs Outdated Show resolved Hide resolved
@EdwardJES EdwardJES requested a review from klkvr as a code owner August 28, 2024 22:54
@EdwardJES EdwardJES force-pushed the 7368/reorg branch 3 times, most recently from d9c0ffc to cddc619 Compare August 29, 2024 07:33
@EdwardJES EdwardJES requested a review from mattsse August 31, 2024 10:30
@EdwardJES
Copy link
Contributor Author

Thank you @mattsse, I think this is ready for a final review now.

@EdwardJES
Copy link
Contributor Author

Sorry @mattsse, last bump on this one 🙏

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.

last nits

sorry for taking so long...

crates/anvil/src/eth/api.rs Outdated Show resolved Hide resolved
crates/anvil/src/eth/api.rs Outdated Show resolved Hide resolved
crates/anvil/src/eth/backend/mem/storage.rs Outdated Show resolved Hide resolved
crates/anvil/src/eth/backend/mem/storage.rs Outdated Show resolved Hide resolved
crates/anvil/src/eth/backend/mem/storage.rs Outdated Show resolved Hide resolved
@EdwardJES
Copy link
Contributor Author

No problems @mattsse, thanks for your help. That should be the last of the nits 🙏

@mattsse mattsse merged commit 3f15f9a into foundry-rs:master Sep 5, 2024
20 checks passed
@EdwardJES EdwardJES deleted the 7368/reorg branch September 5, 2024 21:30
@s7v7nislands
Copy link
Contributor

Is this should add doc to book?

@zerosnacks
Copy link
Member

Is this should add doc to book?

That would be great, feel free to open a PR documenting it

@amankakar
Copy link

Hi guys ,
Thanks for adding this feature , Is there any guide on how to use it , in forge tests.

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.

feat(anvil): support for multi-block depth reorgs in Anvil
5 participants