Repository navigation
backport: v0.26 bitcoin#27501, bitcoin#28542 - #80
Merged
Merged
Conversation
This was referenced Sep 22, 2026
2 of 8 tasks
DCG-Claude
changed the title
DCG-Claude
force-pushed
the
backport-0.26-b060-misc
branch
from
September 28, 2026 04:58
a0ee316 to
bc74110
Compare
…elete a mapDeltas entry when delta==0 67b7fec [mempool] clear mapDeltas entry if prioritisetransaction sets delta to 0 (glozow) c1061ac [functional test] prioritisation is not removed during replacement and expiry (glozow) 0e5874f [functional test] getprioritisedtransactions RPC (glozow) 99f8046 [rpc] add getprioritisedtransactions (glozow) 9e9ca36 [mempool] add GetPrioritisedTransactions (glozow) Pull request description: Add an RPC to get prioritised transactions (also tells you whether the tx is in mempool or not), helping users clean up `mapDeltas` manually. When `CTxMemPool::PrioritiseTransaction` sets a delta to 0, remove the entry from `mapDeltas`. Motivation / Background - `mapDeltas` entries are never removed from mapDeltas except when the tx is mined in a block or conflicted. - Mostly it is a feature to allow `prioritisetransaction` for a tx that isn't in the mempool {yet, anymore}. A user can may resbumit a tx and it retains its priority, or mark a tx as "definitely accept" before it is seen. - Since bitcoin#8448, `mapDeltas` is persisted to mempool.dat and loaded on restart. This is also good, otherwise we lose prioritisation on restart. - Note the removal due to block/conflict is only done when `removeForBlock` is called, i.e. when the block is received. If you load a mempool.dat containing `mapDeltas` with transactions that were mined already (e.g. the file was saved prior to the last few blocks), you don't delete them. - Related: dashpay#4818 and dashpay#6464. - There is no way to query the node for not-in-mempool `mapDeltas`. If you add a priority and forget what the value was, the only way to get that information is to inspect mempool.dat. - Calling `prioritisetransaction` with an inverse value does not remove it from `mapDeltas`, it just sets the value to 0. It disappears on a restart (`LoadMempool` checks if delta is 0), but that might not happen for a while. Added together, if a user calls `prioritisetransaction` very regularly and not all those transactions get mined/conflicted, `mapDeltas` might keep lots of entries of delta=0 around. A user should clean up the not-in-mempool prioritisations, but that's currently difficult without keeping track of what those txids/amounts are. ACKs for top commit: achow101: ACK 67b7fec theStack: Code-review ACK 67b7fec instagibbs: code review ACK 67b7fec ajtowns: ACK 67b7fec code review only, some nits Tree-SHA512: 9df48b622ef27f33db1a2748f682bb3f16abe8172fcb7ac3c1a3e1654121ffb9b31aeaad5570c4162261f7e2ff5b5912ddc61a1b8beac0e9f346a86f5952260a Dash adaptations: - src/txmempool.cpp: the two new PrioritiseTransaction log lines use LogPrint(BCLog::MEMPOOL, ...) instead of upstream's unconditional LogPrintf, because Dash deliberately gated this message behind the mempool log category in commit a8bef50 ("Silence/tweak some log output", dash#4102); the old single LogPrint line it replaces was already BCLog::MEMPOOL - src/rpc/mining.cpp: fee_delta RPCResult description says "in duffs" instead of "in satoshis", matching the neighbouring prioritisetransaction help in Dash - src/test/fuzz/rpc.cpp: "getprioritisedtransactions" inserted before Dash's "getrawaddrman" entry to keep the list alphabetical (Dash has getrawaddrman here, upstream at this commit did not) - test/functional/mempool_expiry.py: node.prioritisetransaction(parent_txid, COIN) — Dash's prioritisetransaction RPC has no legacy `dummy` second argument, so upstream's 3-arg call form is dropped to 2 args - test/functional/mining_prioritisetransaction.py: clear_prioritisation() calls node.prioritisetransaction(txid, -delta) (2 args) for the same reason - test/functional/mining_prioritisetransaction.py: kept Dash's self.mocktime / plain getblocktemplate() at the end of run_test instead of upstream's mock_time = int(time.time()) and getblocktemplate({'rules': ['segwit']}) — Dash has no segwit and this file has no `time` import; only the new `assert tx_id not in getprioritisedtransactions()` lines were taken from that hunk - test/functional/mining_prioritisetransaction.py: pre-existing positional prioritisetransaction() calls left as-is where upstream's hunk only used them as context Not applicable to Dash (intentionally omitted): - test/functional/mining_prioritisetransaction.py: test_replacement() and its run_test() call site: the test relies on replace-by-fee (sending a higher-feerate tx spending the same input and expecting the first to leave the mempool). Dash has no RBF — src/validation.cpp rejects any mempool conflict outright with "txn-mempool-conflict" and the comment "RBF doesn't exist in Dash" — so the scenario cannot be reproduced in any Dash counterpart. The helper clear_prioritisation() introduced by the same hunk IS kept, since test_diamond uses it. The additive-delta behaviour test_replacement also covered is still exercised by test_diamond (two deltas applied to txid_c).
…nd conflicting heights in MarkConflicted 782701c test: Test loading wallets with conflicts without a chain (Andrew Chow) 4660fc8 wallet: Check last block and conflict height are valid in MarkConflicted (Andrew Chow) Pull request description: `MarkConflicted` assumes that `m_last_block_processed_height` is always valid. However it may not be valid when a chain is not attached, as happens in the wallet tool and during migration. In such situations, when the conflicting height is also negative (which occurs on loading when no chain is available), the calculation of the number of conflict confirms results in a non-negative value which passes the existing check for valid values. This will subsequently hit an assertion in `GetTxDepthInMainChain`. Furthermore, `MarkConflicted` is also only called on loading a transaction whose parent has a stored state of `TxStateConflicted` and was loaded before the child transaction. This depends on the loading order, which for both sqlite and bdb depends on the txids. We can avoid this by explicitly checking that both `m_last_block_processed_height` and `conflicting_height` are non-negative. Both `tool_wallet.py` and `wallet_migration.py` are updated to create wallets with a state that triggers the assertion. Fixes bitcoin#28510 ACKs for top commit: ryanofsky: Code review ACK 782701c. Nice catch, and clever test (grinding the txid) furszy: ACK 782701c Tree-SHA512: 1344e0279ec5413a43a2819d101fb571fbf4821de2d13958a0fdffc99f57082ef3243ec454c8343f97dc02ed1fce8c8b0fd89388420ab2e55618af42ad5630a9 Dash adaptations: - test/functional/wallet_migration.py: conflict was only in run_test() — kept Dash's self.test_hybrid_pubkey() and appended self.test_conflict_txs() (upstream's neighbouring self.test_addressbook() does not exist in Dash) - test/functional/tool_wallet.py + wallet_migration.py: wallet.send(..., add_to_wallet=False, locktime=...) rewritten as wallet.send(..., options={"add_to_wallet": False, "locktime": locktime}) because Dash's send RPC still takes a plain "options" OBJ, not upstream's OBJ_NAMED_PARAMS (matches existing Dash tests e.g. wallet_fundrawtransaction.py, rpc_coinjoin.py) - test/functional/tool_wallet.py + wallet_migration.py: Dash has no RBF (validation.cpp rejects with txn-mempool-conflict), so the conflicting tx cannot be broadcast while the parent sits in the mempool; instead its txid is taken from decoderawtransaction and it is mined straight into a block with self.generateblock(node, output=def_wallet.getnewaddress(), transactions=[conflict_signed]) — same end state (parent and child conflicted at -1, conflict tx at 1 confirmation), and mining to the default wallet keeps the 'conflicts' wallet at 4 transactions / 4 address-book entries - test/functional/tool_wallet.py: expected 'Keypool Size' for the descriptor case is 2 instead of upstream's 8 — upstream's 8 is 1 key per active descriptor over Bitcoin's 8 active descriptors (4 output types x internal/external); Dash has only p2pkh, so GetActiveScriptPubKeyMans() yields 2 descriptors. The legacy value stays 1 (external pool drained by the last getnewaddress, internal pool holding the returned change key), since that code path is identical in Dash
knst
force-pushed
the
backport-0.26-b060-misc
branch
from
October 2, 2026 15:39
bc74110 to
3f26cb6
Compare
Collaborator
Author
|
merged as dashpay#7762 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated Bitcoin Core v0.26 backports, batch
backport-0.26-b060-misc.c24de1bc09bc74110be2Skipped in this batch:
Provenance
Each commit passed: cherry-pick (adapted by an Opus lane only where conflicts existed), build, touched tests, a mechanical diff-of-diffs check (every upstream hunk landed; no added line without an upstream counterpart), and an independent Opus verification lane where anything was adapted. Gate rows and lane artifacts are in the backportsys DB.