mempool: remove time-based transaction expiry #36540

ismaelsadeeq commented at 9:14 AM on October 15, 2026: member

This is a simple mempool PR that fixes #33510.

Evicting mempool transactions by age (-mempoolexpiry, default two weeks) is incentive-incompatible: it may drop transactions that could still be mined As discussed IRL and in #33510, this PR hence removes age-based eviction entirely.

Transactions now leave the mempool only via size limiting (-maxmempool), replacement, conflicts, or inclusion in a block. The EXPIRY removal reason is dropped. -mempoolexpiry is retained as a hidden no-op that warns when set, so existing configurations keep starting.

Nice side effects:

With expiry gone, the entry_time index has no readers. Removing it saves an ordered-tree node per mempool entry and drops a now unused Boost include.
The DynamicMemoryUsage() per-entry estimate drops from three indexes to two, so within the same -maxmempool budget the mempool fits slightly more transactions.

test: stop relying on mempool expiry in tests

Rework the tests that exercise time-based mempool expiry so they no
longer depend on it, before the feature is removed: trigger
mempool:removed via RBF in the USDT test, check chain rebroadcast via
peer invs in the wallet resend test, and drop the expiry knobs from the
mempool fuzz targets.

8d4b1abbec

mempool: remove time-based transaction expiry

Evicting transactions purely by age (-mempoolexpiry, default two weeks)
is incentive-incompatible: it can drop still-mineable transactions, such
as a low-feerate parent waiting on a fee-bumping child. Remove it;
-mempoolexpiry is kept as a no-op that warns when set.

d40e511d9d

mempool: remove now-unused entry_time index and includes

Expire() was the entry_time index's only user; drop the index, its tag
and comparator, and the boost headers only it needed. Also tidy includes:
move util/time.h into txmempool.h, drop <chrono>/<memory> from
mempool_args.cpp, and remove the now-unused identity.hpp from
EXPECTED_BOOST_INCLUDES.

1556184fbd

DrahtBot added the label Mempool on Oct 15, 2026

DrahtBot commented at 9:14 AM on October 15, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

External sites

Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36540
Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36540.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36540#author-ralph

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Concept ACK	stickies-v, maflcko, tumutech

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#36627 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36627.svg"> (tracing: run USDT tests without BPF privileges by willcl-ark)
#36482 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36482.svg"> (wallet: Only broadcast resubmitted transactions that were added to the mempool by achow101)
#36257 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36257.svg"> (qa: assert_equals -> assert_true/assert_false by hodlinator)
#10102 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/10102.svg"> (Multiprocess bitcoin by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

ajtowns commented at 1:41 AM on October 16, 2026: contributor

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 some or many users.

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?

stickies-v commented at 1:31 PM on October 16, 2026: contributor

Concept ACK

Node operators should assume that at any point in time, and for any duration of time, their mempool can consume up to -maxmempool of resources. Of course, we should make the mempool maximally useful within that constraint, and transactions that should (from the node's perspective) get mined, but don't, can affect its ability to hold transactions that actually will/can get mined.

The main downside from this removal is that in a non-adversarial case (an adversary can just rebroadcast every 2 weeks), transactions can remain in the mempool forever with a high enough fee rate, e.g. in case of diverging policy or consensus rules, and this reduces a node's effective mempool capacity. While that is not ideal, I also don't think we need to have startup options that make it easier to run a node that is out of sync with the policy and consensus rules that the network actually operates on.

I think the other historically mentioned reasons don't hold up anymore either:

it's nice to be able to replace a large, low fee rate transaction without having to pay the RBF "tax", but RBF mitigates free relay for a reason, I don't think we need a carve-out, even a conservatively rate-limited one
pinning attacks also seem largely irrelevant, given the 2 week delay and the fact that we have TRUC now

Is there any reason why this change shouldn't be phased in via setting

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, it might make sense to keep it, or deprecate rather than remove. But if we can't, then I think it's sensible to remove it?

maflcko commented at 4:22 PM on October 16, 2026: member

Concept ACK, per #16409 :sweat_smile:

tumutech commented at 5:33 PM on October 16, 2026: none

Concept ACK

in src/txmempool.cpp:829 in d40e511d9d

tumutech commented at 6:19 PM on October 16, 2026:

@ismaelsadeeq since you removed entry_time index meaning we are left with 2 indexes. 2 x 3 = 6 pointers. 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?

The DynamicMemoryUsage() per-entry estimate drops from three indexes to two, so within the same -maxmempool budget the mempool fits slightly more transactions.

fametrano commented at 7:35 AM on October 25, 2026: none

Two things look unfinished: the memory estimate still counts three indexes, and wallet_resendwallettransactions.py no longer checks that a parent is resubmitted before its child.

tumutech's inline question on the first is still open. getmempoolinfo reports usage 1192 for one transaction on both 649bc0a (base) and 1556184 (head), since DynamicMemoryUsage() still counts 9 pointers per entry. The old resend test expired a parent and child and needed both resubmitted, which works only in insertion order. With that order reversed, base's test fails (resubmit 1) and head's passes.

Could this PR use 6 pointers, as tumutech suggests, or drop the "fits slightly more transactions" sentence? For the test, restarting with -persistmempool=0 and checking that both transactions are back would make the order matter again, since the wallet resubmits them on load.

The rest matches the release note. I measured it with a functional-test script on regtest with mocktime, and the touched functional tests and the unit suite pass on head on macOS (the USDT test skipped). Head keeps transactions past 336 h, loads older mempool.dat entries with their entry time, and warns that -mempoolexpiry has no effect. 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". On base it worked after expiry, until the wallet's next resubmission, within 36 h, put it back. I can share the script as a gist.

musaHaruna commented at 12:58 PM on October 27, 2026: contributor

I've spent some time going through the discussion in[ #33510](https://github.com/bitcoin/bitcoin/issues/33510) and the history of mempool expiry. I agree that removing transactions simply because they've been in the mempool for 14 days isn't ideal, especially when an old parent could become attractive through a new high-fee child.

My main concern with removing expiry entirely is the possibility of retaining high-feerate transactions that miners won't include, for example when an outdated node accepts transactions that violate newer policies adopted by miners, such as[ #32521](https://github.com/bitcoin/bitcoin/pull/32521). TrimToSize() wouldn't necessarily help here, since those transactions are still highly ranked locally. 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.

I agree with stickies-v that this mostly concerns outdated nodes, and expiry isn't particularly effective against an attacker who keeps rebroadcasting. However, it does provide eventual recovery from a one-time attack or persistent policy disagreement when the transactions are no longer being submitted.

The data shared by 0xB10C shows that expiry removes transactions that can later be mined, but I'm not sure it tells us enough about the tradeoff. How often does expiry remove economically attractive transactions or CPFP packages, compared with how often it actually frees capacity occupied by transactions miners won't include? And would removing it lead to more size-based evictions or a higher minimum mempool feerates under pressure?

Looking at the hybrid approach suggested by Ismaelsadeeq, i.e combining transaction age with missed mining opportunities, possibly only under memory pressure. I'm leaning toward exploring this longer term, but it has its own problems. Local template inclusion doesn't establish that miners received or rejected a transaction, and counting missed opportunities could penalize censored transactions. There are also the costs of template construction and the question of what happens to previous observations when a parent receives a new high-fee child.

There's also the interaction with fee estimation following[ #34075](https://github.com/bitcoin/bitcoin/pull/34075). Persistently unmined, high-feerate transactions could affect the mempool-based estimator despite its health checks. But I think that such transactions for estimation don't require removing them from the mempool. I think this is worth investigating separately, rather than using fee estimation as a reason to retain expiry.

Given this, 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. This would preserve a recovery option for operators while giving us a chance to understand the consequences of the changed default.

Finally, 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?

I'd appreciate corrections in case I missed something.