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

pool: Handle reorgs #87

Closed
dancoombs opened this issue Mar 28, 2023 · 12 comments
Closed

pool: Handle reorgs #87

dancoombs opened this issue Mar 28, 2023 · 12 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@dancoombs
Copy link
Collaborator

No description provided.

@dancoombs dancoombs added the enhancement New feature or request label Mar 28, 2023
@dancoombs dancoombs added this to the 0.1.0-beta milestone Mar 28, 2023
@dancoombs dancoombs self-assigned this Mar 28, 2023
@dancoombs dancoombs modified the milestones: 0.1.0-beta, 0.1.0-alpha Mar 29, 2023
@moldy530
Copy link
Contributor

Since the builder does validation again before submitting it on chain, the impact here should be minimal right? Though I'm not super familiar with the effects of re-orgs more generally

@dancoombs
Copy link
Collaborator Author

Since the builder does validation again before submitting it on chain, the impact here should be minimal right? Though I'm not super familiar with the effects of re-orgs more generally

The problem is that a UO might get mined in a block, removed from the mempool, and then the chain re-orgs to and the UO is no longer mined. Since we removed the UO from the pool, its lost forever.

@moldy530
Copy link
Contributor

oh I see, that makes sense. Is this something that's mitigated by waiting for some N number of confirmations or is there some other approach that needs to be taken to handle this?

@dancoombs
Copy link
Collaborator Author

I'm planning on looking at Geth/Reth for inspiration. Its possible they don't try to handle this at all and just rely on the wallet layer to resubmit if they notice their transaction has been dropped. If that's the case I'll likely push this out to a later release (I actually think this is going to be the case).

To mitigate this we would have to wait for some notion of "finality" in the L1 blocks (applies to both L1 and L2s that post data there). L1 ethereum has safe (6 minutes) and finalized (12 minutes) commitment levels we can listen for. Finalized is likely overkill. The idea would be that we listen on latest and safe. Mined transactions are set aside, but not discarded, until the block they were included in becomes safe. During that time we track reorgs by looking at block hashes and add transactions back to the pool that were re-orged out.

@dancoombs
Copy link
Collaborator Author

@dancoombs
Copy link
Collaborator Author

To summarize Geth's approach:

  • Geth maintains a history of all blocks that it's received in its database and you can look them up by hash.
  • Each time the txpool learns of a new block it checks for a reorg. It looks at the new head of the chain and checks if its parent is the last block that was processed. If not, it starts reorg processing.
  • Reorg processing essentially rewinds the chain to a common point, then diffs each block's included transactions.
  • Transactions that are no longer included are added back to the pool.

@dancoombs
Copy link
Collaborator Author

Geth doesn't hold any extra state besides this block database and can removes included transactions from its pool when it gets a new block header. If a reorg is detected it can recover the transaction from the reorg-ed block's transactions.

We can take a similar approach. The user operations that are no longer included will be held in the block's transaction data and can be recovered there.

@dancoombs
Copy link
Collaborator Author

General idea that we should pursue:

  • Modify events.rs to add any data to the NewBlockEvent needed for reorg tracking
  • Keep an in-memory map of all blocks up to some limit
    • This limit can either be hardcoded by chain, or can be dynamically determined by listening to finalized blocks.
  • In uo_pool::on_new_block detect reorgs, extract UOs that are no longer included, and inject them back into the pool.
    • It also should remove UOs from the mempool that were included.
    • Reputation system should also be updated.

From Dror in the 4337 Bundlers TG:

I don't think re-orgs are something special of 4337. The same is also true for processing of tx mempool by normal block-builder.
I think this handling is bundler (or block-builder) specific, and doesn't have to be standardized.

The side-effects of a bundler NOT handling reorgs:

  • Such UserOps (which were included, and then dropped) will not be re-handled by this bundler. either will be handled by another bundler, or that the calling user will notice the TX "disappeared", and will re-submit.
  • The bundler's reputation table will be wrong (since it reflects userops that are no longer on-chain). This is again a local change, and since reputation it built over time (and decays over time), the effect is minimal

At a minimum, after noticing a re-org, the bundler must make sure to scan the UserOps that were included, to evict from the mempool already-included UserOps. This can simply be done by scanning for UserOpEvents not from the last block, but from the last-finality-block (e.g. 10 blocks into the past)
But again, this is for its own benefit only: if it doesn't it might create a bundle with already-included UserOps, which will get reverted.

@dancoombs
Copy link
Collaborator Author

From what I can tell Reth doesn't handle reorgs. Some discussion here paradigmxyz/reth#662 but more related to the database. Asking in their TG and my start a discussion post.

@dancoombs dancoombs modified the milestones: 0.1.0-alpha, 0.1.0-beta Mar 30, 2023
@dancoombs dancoombs changed the title op_pool: Consider impact of re-orgs on mempool op_pool: Handle reorgs in pool Mar 30, 2023
@dancoombs
Copy link
Collaborator Author

Added an issue to Reth here: paradigmxyz/reth#2040

@dancoombs dancoombs removed their assignment Mar 31, 2023
@dancoombs
Copy link
Collaborator Author

Reth's implementation: paradigmxyz/reth#2298

@dancoombs dancoombs changed the title op_pool: Handle reorgs in pool pool: Handle reorgs in pool Apr 29, 2023
@dancoombs dancoombs changed the title pool: Handle reorgs in pool pool: Handle reorgs Apr 29, 2023
@dancoombs dancoombs modified the milestones: 0.1.0-beta, 0.1.0 May 1, 2023
@dancoombs
Copy link
Collaborator Author

Closed by #276

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants