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(oppool): Return reorged ops to the mempool #276

Merged
merged 1 commit into from
Aug 3, 2023
Merged

Conversation

dphilipson
Copy link
Collaborator

@dphilipson dphilipson commented Jul 31, 2023

Previously, ops could be lost forever if they were mined into a block,
but then that block got reorged away. Now, we detect reorgs and return
any ops contained in them to our mempool.

This requires some involved changes:

  • We replace the entire events mod and all its listeners. Instead, we
    introduce a new type, Chain, which represents our current knowledge
    of what blocks make up the blockchain.
  • We watch for the latest block hash to change. When it does, we read
    backwards from the latest block until we connect back to our known
    blocks, which lets us see any blocks that were replaced as well.
  • This process produces a ChainUpdate event, which is sent to the op
    pool (replacing NewBlockEvent). This may cause the op pool to not
    only remove mined ops but also restore unmined ops.
  • In order for the op pool to do so, it remembers mined ops for a time
    rather than deleting them fully. We add new methods to the op pool for
    restoring unmined blocks to make this possible.

[Closes/Fixes] # #87

@dphilipson
Copy link
Collaborator Author

I seriously need to write some tests for this, but wanted to get a version up first for discussion and see if anything needs changing (especially for the upcoming op pool server refactor) before I go ham on the mocks.

src/cli/pool.rs Outdated Show resolved Hide resolved
Comment on lines 236 to 237
self.mined_at_block_number_by_hash.remove(&hash);
self.mined_hashes_with_block_numbers.remove(&(bn, hash));
Copy link
Collaborator

Choose a reason for hiding this comment

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

The goal of the op_pool_size_bytes metric is to track memory usage of the pool. We should probably take a look at adding new metrics/updating the old ones to handle these mined operations

src/op_pool/chain.rs Outdated Show resolved Hide resolved
src/op_pool/chain.rs Outdated Show resolved Hide resolved
src/op_pool/chain.rs Outdated Show resolved Hide resolved
@dphilipson dphilipson force-pushed the dp/reorgs branch 2 times, most recently from d3c8808 to 6620301 Compare August 1, 2023 23:26
Copy link
Collaborator

@dancoombs dancoombs left a comment

Choose a reason for hiding this comment

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

One last comment + some testing and this looks good to me! 🚀

@dphilipson dphilipson force-pushed the dp/reorgs branch 2 times, most recently from fef2b62 to 0654565 Compare August 3, 2023 15:30
Previously, ops could be lost forever if they were mined into a block,
but then that block got reorged away. Now, we detect reorgs and return
any ops contained in them to our mempool.

This requires some involved changes:

* We replace the entire `events` mod and all its listeners. Instead, we
  introduce a new type, `Chain`, which represents our current knowledge
  of what blocks make up the blockchain.
* We watch for the latest block hash to change. When it does, we read
  backwards from the latest block until we connect back to our known
  blocks, which lets us see any blocks that were replaced as well.
* This process produces a `ChainUpdate` event, which is sent to the op
  pool (replacing `NewBlockEvent`). This may cause the op pool to not
  only remove mined ops but also restore unmined ops.
* In order for the op pool to do so, it remembers mined ops for a time
  rather than deleting them fully. We add new methods to the op pool for
  restoring unmined blocks to make this possible.
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.

2 participants