#36540 mempool: remove time-based transaction expiry
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #36540 mempool: remove time-based transaction expiry | Ready | 0 | Blocked | 15 + 31 tests | |||
|
|
|
|
| |||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | |||||||
Goal
Removes time-based transaction expiry (-mempoolexpiry) and deletes the entry_time multi-index from the mempool. Transactions will now only leave the mempool due to size limiting (-maxmempool), block inclusion, replacement, or conflict. The -mempoolexpiry startup argument is retained as a hidden no-op that logs an initialization warning if set.
Problem: Evicting transactions based purely on age (previously defaulting to two weeks) is incentive-incompatible: it can drop transactions that could still be mined, such as a low-feerate parent awaiting a fee-bumping child. Maintaining the entry_time index also incurs an ordered-tree node of memory overhead per mempool entry.
Evidence of importance:
- Fixes issue #33510: 'Mempool Expiry eviction might remove txs that could be mined in the next block'.
- PR description and commit d40e511d9d note that age-based eviction 'can drop still-mineable transactions, such as a low-feerate parent waiting on a fee-bumping child.'
- PR description notes that dropping the index removes one ordered-tree node per entry, lowering per-entry DynamicMemoryUsage() overhead so the mempool fits more transactions within the same -maxmempool budget.
Scope: Deletes test/functional/mempool_expiry.py and updates existing tests (interface_usdt_mempool.py, wallet_resendwallettransactions.py) and mempool fuzz targets to remove dependence on expiry.
Risk of a mistake: medium (mempool_policy). Mistakes in mempool eviction rules can alter mempool consistency across nodes or lead to unintended transaction retention and memory pressure.
Reviewability
Ready
Nothing known that would invalidate a review now.
Waiting on (from the thread read):
- author: response to review feedback on unminable transactions, deprecation, and test coverage loss 2026-10-27
Agreement
Blocked
objection that needs a change, nobody has replied (fametrano)
Thread notes: Added musaHaruna to participants with an objection and suggestions from r:5667388831 regarding the retention of unminable high-feerate transactions and deprecating expiry instead of removing it.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| ajtowns (member) | objection, approach | removing expiry entirely prevents node operators and miners from reverting to previous default behavior if the change turns out harmful | answered · stickies-v argued immediate code cleanup is preferable unless someone provides good reasons to keep the setting | changed | by others | 2026-10-16: 'I don't think it's a good idea to change the node's behaviour in a way that doesn't allow users/miners to revert to the previous default in case the change turns out to be undesirable or harmful for s' Status from: 2026-10-16: stickies-v: 'I think it's nice that we can clean up the code right away. If people can come up with good reasons to use this setting,' |
| ajtowns (member) | suggestion, approach | phase in the change by setting DEFAULT_MEMPOOL_EXPIRY_HOURS to a large value (e.g. 4 years) rather than removing it entirely | answered · stickies-v answered that cleaning up the code right away is preferable unless concrete use cases exist | answered | by others | 2026-10-16: 'Is there any reason why this change shouldn't be phased in via setting `DEFAULT_MEMPOOL_EXPIRY_HOURS` to a large value, such as 35040 (4 years), instead?' Status from: 2026-10-16: stickies-v: 'If people can come up with good reasons to use this setting, it might make sense to keep it, or deprecate rather than re' |
| tumutech (none) | question, maintenance | whether MallocUsage() should be updated to account for 6 pointers now that entry_time index is removed | open | answered | no | 2026-10-16: 'would it be fine to use 6 in MallocUsage() so that the same -maxmempool budget can hold extra txn you talked about in the nice side-effect?' |
| fametrano (none) | objection, correctness | wallet_resendwallettransactions.py no longer verifies that a parent transaction is resubmitted before its child | open | changed | no | 2026-10-25: 'wallet_resendwallettransactions.py no longer checks that a parent is resubmitted before its child.' |
| fametrano (none) | suggestion, maintenance | update DynamicMemoryUsage() to use 6 pointers per entry or drop the claim from the PR description | open | answered | no | 2026-10-25: 'Could this PR use 6 pointers, as tumutech suggests, or drop the "fits slightly more transactions" sentence?' |
| fametrano (none) | suggestion, maintenance | document in the release notes that abandontransaction now fails for transactions older than two weeks | open | answered | no | 2026-10-25: 'One change may deserve a line in the note: `abandontransaction` on a wallet transaction older than two weeks now fails with "Transaction not eligible for abandonment".' |
| musaHaruna (contributor) | objection, approach | Removing expiry entirely could cause outdated nodes to retain high-feerate transactions rejected by miners, displacing minable transactions under memory pressure and raising the minimum mempool feerate. | open | answered | no | 2026-10-27: 'Under memory pressure, they could displace lower-feerate transactions that miners would actually include, potentially causing more `SIZELIMIT` removals and raising the minimum mempool feerate.' |
| musaHaruna (contributor) | suggestion, approach | Deprecate `-mempoolexpiry` with age expiry disabled by default rather than removing its functionality immediately. | open | answered | no | 2026-10-27: 'I'm leaning toward deprecating `-mempoolexpiry` rather than removing its functionality immediately, with age expiry disabled by default but still available explicitly during the deprecation period.' |
| musaHaruna (contributor) | question, approach | Whether there is existing evidence that the protection expiry provides against persistent policy disagreement is negligible enough to justify removing the mechanism entirely. | open | answered | no | 2026-10-27: 'Is there already evidence that the protection expiry provides against persistent policy disagreement is negligible enough to justify removing the mechanism entirely, rather than just changing the default?' |
Support:
- stickies-v (member) (Concept ACK, 2026-10-16) [read]: operators should assume mempool usage up to -maxmempool, historical reasons for expiry no longer hold, and immediate cleanup is beneficial
- maflcko (member) (Concept ACK, 2026-10-16): supports per historical PR #16409
- tumutech (none) (Concept ACK, 2026-10-16): (no reason given) [not substantive]
Participants:
- ajtowns (member, objection, 1 statements 2026-10-16 to 2026-10-16): argues against removing the ability to revert to expiry and suggests phasing it in by increasing the default hours instead
- maflcko (member, support, 1 statements 2026-10-16 to 2026-10-16): Concept ACK citing previous PR #16409
- stickies-v (member, support, 1 statements 2026-10-16 to 2026-10-16): Concept ACK; argues historical justifications for expiry no longer hold and immediate cleanup is sensible
- tumutech (none, support, 2 statements 2026-10-16 to 2026-10-16): Concept ACK; asked whether MallocUsage pointer calculation should be updated
- fametrano (none, objection, 1 statements 2026-10-25 to 2026-10-25): objects to test coverage loss in wallet_resendwallettransactions.py and suggests updating memory calculation and release notes
- musaHaruna (contributor, objection, 1 statements 2026-10-27 to 2026-10-27): concerned that removing expiry could retain unminable high-feerate transactions that displace minable ones under memory pressure; suggests deprecating instead
Files
198 lines under test/bench/ci.
- test/functional/mempool_expiry.py +0/-117
- test/functional/wallet_resendwallettransactions.py +23/-26
- src/txmempool.h +4/-24
- test/functional/interface_usdt_mempool.py +8/-12
- src/txmempool.cpp +0/-18
- src/node/mempool_persist.cpp +2/-5
- src/test/fuzz/tx_pool.cpp +0/-7
- doc/release-notes-xxxxx.md +5/-0
- src/init.cpp +4/-1
- src/node/mempool_args.cpp +0/-5
- src/validation.cpp +0/-5
- src/kernel/mempool_options.h +0/-4
- test/functional/test_framework/messages.py +0/-2
- src/kernel/mempool_removal_reason.cpp +0/-1
- src/kernel/mempool_removal_reason.h +0/-1
- src/test/fuzz/package_eval.cpp +0/-1
- src/validation.h +0/-1
- src/validationinterface.h +0/-1
- test/functional/test_runner.py +0/-1
- test/lint/lint-includes.py +0/-1
Card
The short text the ranking pass compares across PRs.
Removes time-based transaction expiry (-mempoolexpiry) and drops the mempool's entry_time index. Age-based eviction is incentive-incompatible because it discards transactions that are still mineable, such as low-feerate parents waiting for CPFP bumps. Dropping the index also saves memory overhead per transaction entry, fitting slightly more transactions into the configured -maxmempool budget. The -mempoolexpiry option is kept as a hidden no-op that logs a warning.
Data
thread read through 2026-10-27 (7 statements), head 1556184fbd
labels: Mempool
Categories
Mempool and policy
P2 · cleanup
Removing time-based mempool eviction fixes an incentive-incompatibility where still-mineable transactions are evicted and eliminates per-entry index memory overhead. As noted in issue #33510 and the description, age-based eviction 'can drop still-mineable transactions, such as a low-feerate parent waiting on a fee-bumping child' while deleting the index lowers per-entry memory usage.
Membership: Mempool label; deletes entry_time index from txmempool and removes time-based eviction logic across mempool files.
Factors: security/stability 1, bug 1, performance 1, user value 1, leverage 1