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(mempool): mempool transaction limits, saving to db and continue block prod from pending block #372

Merged
merged 15 commits into from
Dec 7, 2024

Conversation

cchudant
Copy link
Member

@cchudant cchudant commented Oct 31, 2024

Pull Request type

  • Feature

Add limits to the mempool:

  • transaction age
  • max number of transactions in the mempool
  • additional limit of declare transactions in the mempool

fixes #373

@cchudant
Copy link
Member Author

cchudant commented Nov 5, 2024

Added:

@cchudant cchudant changed the title feat(mempool): mempool transaction limits feat(mempool): mempool transaction limits, saving to db and continue block prod from pending block Nov 5, 2024
@shamsasari
Copy link
Contributor

This is quite a large PR. Can it not be broken down, especially if there's two features?

configs/chain_config.example.yaml Outdated Show resolved Hide resolved
crates/client/analytics/src/lib.rs Outdated Show resolved Hide resolved
crates/client/block_import/src/types.rs Outdated Show resolved Hide resolved
crates/client/block_production/src/lib.rs Outdated Show resolved Hide resolved
@cchudant
Copy link
Member Author

This is quite a large PR. Can it not be broken down, especially if there's two features?

i am thinking about it
i think it's just easier at this point to have it as one pull request, all of the changes here are in the mempool.

I believe if i were to split this pr in 2 or 3, one of these pr would necessarily still have >50 files changed.. that would be the one fixing saving the txs to the db, it's kind of a pain to split that because it touches all of the mempool crate, adds a db columns, and refactors all of the to_blockifier stuff

@cchudant
Copy link
Member Author

the mempool limits and continue block prod from pending block should only change a few files really, it's the mempool saving that's making it complex

@cchudant cchudant requested a review from shamsasari November 19, 2024 15:31
@cchudant
Copy link
Member Author

@shamsasari i did the fixes you mentioned, except the splitting the pr one

Copy link
Collaborator

@Trantorian1 Trantorian1 left a comment

Choose a reason for hiding this comment

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

A few minute points might need changing. This is really cool work! (Also please for the love of god and everything dear do not force push, I don't want to have to sift through all these files again next time I review 😅 )

crates/client/block_production/src/lib.rs Show resolved Hide resolved
crates/client/db/src/mempool_db.rs Outdated Show resolved Hide resolved
crates/client/devnet/src/lib.rs Outdated Show resolved Hide resolved
crates/client/exec/src/lib.rs Outdated Show resolved Hide resolved
crates/client/mempool/src/inner/proptest.rs Show resolved Hide resolved
crates/client/mempool/src/lib.rs Show resolved Hide resolved
crates/primitives/block/src/lib.rs Show resolved Hide resolved
crates/tests/src/lib.rs Show resolved Hide resolved
Copy link
Collaborator

@Trantorian1 Trantorian1 left a comment

Choose a reason for hiding this comment

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

All good! Great work 🔥

@antiyro antiyro merged commit 031da9e into main Dec 7, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

bug: pending txs should be stored in graceful shutdown
6 participants