sequencer, mempool: rewrite mempool benchmark tests to work with new interface #1334
Comments
Lilyjjo
changed the title
github-merge-queue bot
pushed a commit
that referenced
this issue
Aug 20, 2024
…ge and maintenance (#1323) ## Summary This rewrite makes our mempool similar to Geth's/Reth's with per-account transaction storage and maintenance. This rewrite tracks 'pending' transactions that are ready to execute, and 'parked' transactions that could be ready for execution in the future on a per-account basis. This rewrite adds minor new functionality from the previous mempool and changes the mempool interface. Was co-written with @Fraser999. ## Background Prior to this rewrite, the mempool was not in a state to be improved upon. All transactions, ready to execute or not, were being stored together in a giant queue. This structure didn't allow for mempool optimizations such as performing mempool upkeep on a per account basis-- instead the only option was to loop through all transactions on the per-block maintenance upkeep. ### `mempool/mod.rs` Changes This file contains the rewrite of the mempool interface. Instead of returning a modifiable queue of all transactions to the builder in `prepare_proposal()`, now we return a copy of the transactions. Instead of removing transactions as we process them, we only remove transactions during the `finalize_block()` logic, updating accounts to reflect their new nonces. The main app-exposed mempool calls are: - `insert()`, which will only insert a transaction if it meets the mempool's transaction policies. - `remove_tx_invalid()`, to be used by `prepare_proposal()` to remove transactions that fail execution from the mempool. - `builder_queue()`, returns a copy of the ready-to-execute transactions sorted by priority order. - `run_maintenance()`, updates the stored transactions post new block height increases. The mempool implements the following transaction policies: 1. Nonce replacement is not allowed. 2. Accounts cannot have more than `PARKED_SIZE_LIMIT` transactions in their parked queues (currently set to 15). 3. There is no account limit on pending transactions. 4. Transactions will expire and can be removed after `TX_TTL` time (currently set to 4 minutes). 5. If an account has a transaction removed for being invalid or expired, all transactions for that account with a higher nonce can be removed as well. This is due to the fact that we do not execute failing transactions, so a transaction 'failing' will mean that further account nonces will not be able to execute either. ### `mempool/transactions_container.rs` This is a new file containing the data structures for the per-account transaction logic. Several new code level constructs are added: - `TimemarkedTransaction`: a `SignedTransaction` wrapper that also stores the time that the transaction was first seen by the mempool. This is used for implementing the transaction expiry policy. - The trait `TransactionsForAccount` with types `PendingTransactionsForAccount` and `ParkedTransactionForAccount`. This is used to house the per-account logic of storing pending and parked transactions, respectively. - `TransactionsContainer<T>`: a struct with a generic over `TransactionsForAccount`. Types `PendingTransactions` and `ParkedTransactions` are defined over this as the mempool level containers for pending and parked transactions. ### Other notable changes - All transaction eviction reasons are surfaced to the `CheckTx()` CometBFT mempool logic. TBD on how to surface this to the end user. - Fixed hidden bug in the app's `finalize_block()` logic which was updating the mempool with the previous block's state. - Returned transactions for block building are ordered by nonce difference and then time first seen. The previous implementation only did not sort by time first seen. ### Mempool Overview The overall mempool structure as of this PR for Astria is shown in the below diagram:  ## Testing - Unit tests were written for all desired behaviors. - Ran locally with a single node. - The mempool's benchmarks were modified to work with the new interface, showing a 2x speed increase on mempools sized 100 transactions and 10x speed increase on 10,000 transactions. ## TODO still - Rewrite and add new metrics. - Implement pending/parked logic that takes into consideration and account's balance. - Rewrite benchmarks to better represent expected user behavior. ## Related Issues closes #1154, #1150, #1334
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
PR #1323 was written without updating the associated mempool benchmarking tests, instead they were just commented out. We should rewrite the benchmarks to work with the new interface.
┆Issue Number: ENG-681
The text was updated successfully, but these errors were encountered: