mempool: expose optimality of mempool to log / rpc

instagibbs commented at 3:44 pm on February 18, 2026: member

Post-SFL #34023 I don’t think we expect the mempool to be unordered for long periods of time. If we consider it likely to be a serious regression in production, it would be useful to expose the fact that the mempool is not known to be optimal.

do a MEMPOOL log after any DoWork() returns false, meaning non-optimal
expose it via getmempoolinfo, by calling DoWork(0), which does nothing but return known-optimality

I’m not wedded to either approach, I just think something is better than nothing for the next release.

DrahtBot added the label Mempool on Feb 18, 2026

instagibbs commented at 3:44 pm on February 18, 2026: member

cc @sipa

DrahtBot commented at 3:44 pm on February 18, 2026: contributor

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	ajtowns, ismaelsadeeq, sedited, sipa

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34616 (Cluster mempool: SFL cost model (take 2) by sipa)

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.

sipa commented at 3:55 pm on February 18, 2026: member

ACK da6da77818aed7634f50b3638ec6965f2946091b if CI is happy with this.

instagibbs commented at 3:57 pm on February 18, 2026: member

key error in a test I probably am not running locally yet, diagnosing

edit: oh it’s running old releases without the key, will adapt

fanquake added this to the milestone 31.0 on Feb 18, 2026

instagibbs force-pushed on Feb 18, 2026

DrahtBot added the label CI failed on Feb 18, 2026

fanquake commented at 4:16 pm on February 18, 2026: member

cc @ajtowns

in test/functional/test_framework/test_framework.py:738 in c89b93b958

730@@ -731,6 +731,12 @@ def sync_mempools(self, nodes=None, wait=1, timeout=60, flush_scheduler=True):
731                 if flush_scheduler:
732                     for r in rpc_connections:
733                         r.syncwithvalidationinterfacequeue()
734+                # We also should never see a non-optimal mempool in functional tests
735+                for r in rpc_connections:
736+                    info = r.getmempoolinfo()
737+                    # Note: This gate can be removed once compatibility tests run versions exposing this key
738+                    if "optimal" in info:

sipa commented at 4:19 pm on February 18, 2026:

Can you assert that the key is present in non-compatibility-tests? Otherwise, if the key is somehow missing, this wouldn’t be testing anything.

instagibbs commented at 4:30 pm on February 18, 2026:

https://github.com/bitcoin/bitcoin/pull/34615/changes/c89b93b9584f09401f9740061fdb6e4036f6dbbf#diff-3c23fc59380814b4762d212931213e9a39dd5ab134a5f34a1a626ee5390c34beR310

does this suffice?

sipa commented at 4:46 pm on February 18, 2026:

Great.

DrahtBot removed the label CI failed on Feb 18, 2026

ismaelsadeeq commented at 4:07 pm on February 19, 2026: member

ACK c89b93b9584f09401f9740061fdb6e4036f6dbbf

DrahtBot requested review from sipa on Feb 19, 2026

sipa commented at 7:40 pm on February 19, 2026: member

crACK c89b93b9584f09401f9740061fdb6e4036f6dbbf

in src/rpc/mempool.cpp:877 in c89b93b958

873@@ -874,6 +874,7 @@ UniValue MempoolInfoToJSON(const CTxMemPool& pool)
874     ret.pushKV("maxdatacarriersize", pool.m_opts.max_datacarrier_bytes.value_or(0));
875     ret.pushKV("limitclustercount", pool.m_opts.limits.cluster_count);
876     ret.pushKV("limitclustersize", pool.m_opts.limits.cluster_size_vbytes);
877+    ret.pushKV("optimal", pool.m_txgraph->DoWork(0)); // 0 work is a quick check for knwon optimality

ajtowns commented at 7:36 am on February 20, 2026:

Would slightly prefer an inline bool IsOptimal() { return DoWork(0); } in txgraph.h, but whatever.

Also “known”.

instagibbs commented at 2:28 pm on February 20, 2026:

fixed misspelling, left as is otherwise

in src/txmempool.cpp:225 in c89b93b958 outdated

220@@ -221,7 +221,9 @@ void CTxMemPool::Apply(ChangeSet* changeset)
221 
222         addNewTransaction(it);
223     }
224-    m_txgraph->DoWork(POST_CHANGE_WORK);
225+    if (!m_txgraph->DoWork(POST_CHANGE_WORK)) {
226+        LogDebug(BCLog::MEMPOOL, "Mempool in non-optimal ordering after addition(s).");

ajtowns commented at 7:41 am on February 20, 2026:

I don’t follow why this is only there for additions and not also the remove for reorg/block cases?

instagibbs commented at 2:27 pm on February 20, 2026:

added for all 3 invocations

in test/functional/test_framework/test_framework.py:739 in c89b93b958

730@@ -731,6 +731,12 @@ def sync_mempools(self, nodes=None, wait=1, timeout=60, flush_scheduler=True):
731                 if flush_scheduler:
732                     for r in rpc_connections:
733                         r.syncwithvalidationinterfacequeue()
734+                # We also should never see a non-optimal mempool in functional tests
735+                for r in rpc_connections:
736+                    info = r.getmempoolinfo()
737+                    # Note: This gate can be removed once compatibility tests run versions exposing this key
738+                    if "optimal" in info:
739+                        assert info['optimal'], f"Mempool not optimal on node after sync! {info}"

ajtowns commented at 7:43 am on February 20, 2026:

I get "optimal": false immediately after invoking prioritisetransaction, fwiw. (Expected as there is no immediate call to DoWork in that path) Is this assumption really valid?

instagibbs commented at 2:08 pm on February 20, 2026:

oops, sync_mempool is definitely wrong place for this due to lazy linearization, it can be added after say a new block, or reorg, or added tx. Will fix.

instagibbs commented at 2:27 pm on February 20, 2026:

removed, and just left sanity check instances in functional suite

ajtowns commented at 7:44 am on February 20, 2026: contributor

Concept ACK. Not sure about the implementation.

mempool: log if we detect a non-optimal mempool

We expect this to be rare in practice, and to not be the
usual state of the mempool. If we we detect non-optimal
ordering after a DoWork() invocation, allow this to be
observed in MEMPOOL logs.

a3fb3dd55c

rpc: add optimal result to getmempoolinfo

Expose this value to allow rpc based tooling to track this
value for network health diagnostics.

a9e59f7d95

instagibbs force-pushed on Feb 20, 2026

DrahtBot added the label CI failed on Feb 20, 2026

ajtowns commented at 5:52 am on February 21, 2026: contributor

ACK a9e59f7d955f995078b3e0bf3b527c03c74fef8d

Seems fine to me. Might be good to have optimality reported on a per-cluster basis via getmempoolcluster as well? CI failure looks to be #34637.

DrahtBot requested review from ismaelsadeeq on Feb 21, 2026

DrahtBot requested review from sipa on Feb 21, 2026

instagibbs commented at 1:18 pm on February 21, 2026: member

@ajtowns yes I reported the failure as it seemed unrelated

Might be good to have optimality reported on a per-cluster basis"

Agreed (future work TM)

ismaelsadeeq commented at 1:40 pm on February 23, 2026: member

reACK a9e59f7d955f995078b3e0bf3b527c03c74fef8d c89b93b95..a9e59f7d95 fixed typo, added more logging for block/reorg additions to mempool, and fixed brittle test case.

sedited approved

sedited commented at 3:59 pm on February 23, 2026: contributor

ACK a9e59f7d955f995078b3e0bf3b527c03c74fef8d

sipa commented at 4:00 pm on February 23, 2026: member

ACK a9e59f7d955f995078b3e0bf3b527c03c74fef8d

sedited merged this on Feb 23, 2026

sedited closed this on Feb 23, 2026

mempool: expose optimality of mempool to log / rpc #34615

Reviews

Conflicts