backport: bitcoin#18628, #19393, #25204, #25689, #25785, bitcoin-core/gui#613 - #7551
Conversation
… mempool entries 885694d doc: add release note about removal of `deprecatedrpc=fees` flag (Sebastian Falbesoner) 387ae8b rpc: remove deprecated fee fields from mempool entries (Sebastian Falbesoner) Pull request description: Deprecating the top-level fee fields (`fee`, `modifiedfee`, `ancestorfees` and `descendantfees`) from the mempool entries and introducing `-deprecatedrpc=fees` was done in PR bitcoin#22689 (released in v23.0). For the next release v24.0, this configuration option can be removed. ACKs for top commit: fanquake: ACK 885694d Tree-SHA512: fec6b5be5c3f0cd55738a888b390ef9271e70b2dba913a14ce82427dac002e999f93df298bb3b494f3d1b850a23d2b5b3e010e901543b0d18db9be133579e1ec Co-authored-by: MacroFake <falke.marco@gmail.com>
…s from rpcconsole.cpp BACKPORT NOTE: RPCConsole::onWalletChanged still uses walletutil.h (GetWalletDir) - can't be fully removed 0994273 qt: Remove unnecessary wallet includes from rpcconsole.cpp (laanwj) Pull request description: Fixes bitcoin#25266 ACKs for top commit: MarcoFalke: cr ACK 0994273 hebasto: ACK 0994273. Tree-SHA512: 02cef4a1f3522c4cd662853eb930dfdf0866a5bd959a00f4c42d8c741b0751df4cf2e14c304b93ca3ce699c0e9730caf3d6fa2053009af312c60e861b0f79179 Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
fa57c44 fuzz: Remove no-op SetMempoolConstraints (MacroFake) Pull request description: Now that the mempool no longer uses the args manager (after commit e4e201d), there is no point setting the mempool limits after it is constructed. Fix that by setting them once right before the mempool is constructed. ACKs for top commit: dongcarl: utACK fa57c44 glozow: utACK fa57c44 Tree-SHA512: d236f9cdcee8c2076272b82c97f8a5942f1ecf119ab36edafd42088ef97554592348a61e1fbe504fd52b30301ef0177813042599ad12e8cb95b4a20586c85bb0 Co-authored-by: glozow <gloriajzhao@gmail.com>
BACKPORT NOTE: maximum amount of keys in dash core is 15 per descriptor 4786959 bench: Add a benchmark for descriptor expansion (Ben Woosley) Pull request description: Taken from bitcoin#16116 , as requested here: bitcoin#25748 (comment) ACKs for top commit: achow101: ACK 4786959 Tree-SHA512: f2efdf8f84e1783c7c298abe65123191d25cab0a9da2d0ff5957a60acc2d10e356151d7ecec0d98d28c456f42ddef50efd70c7edc0c9012df2a977e080515b9d fixup for bench descriptors Co-authored-by: MacroFake <falke.marco@gmail.com>
… first thing in the connected block
BACKPORT NOTE for Dash Core This backport has been done in the past except one failing check. That check had been failing due to double run of all checks in this functional test: once for mempool, once for block. This fix complete bitcoin#18628 backporting ---- fa4c29b test: Add various low-level p2p tests (MarcoFalke) Pull request description: ACKs for top commit: jonatack: ACK fa4c29b Tree-SHA512: 842821b97359d4747c763398f7013415858c18a300cd882887bc812d039b5cbb67b9aa6f68434575dbc3c52f7eb8c43d1b293a59555a7242c0ca615cf44dc0aa Co-authored-by: MarcoFalke <falke.marco@gmail.com>
c0a5fce test: Add test for erase orphan tx conflicted by block (Hennadii Stepanov) fa45bb2 test: Add test for erase orphan tx included by block (Hennadii Stepanov) 5c04978 test: Add test for erase orphan tx from peer (Hennadii Stepanov) Pull request description: This PR adds test coverage for the following cases: - erase orphan transactions when a peer is disconnected - erase an orphan transaction when it is included in a new tip block - erase an orphan transaction when it is conflicted with other transactions included in a new tip block Found useful while working on bitcoin#19374. ACKs for top commit: aureleoules: tACK c0a5fce (`make check` and `test/functional/test_runner.py`). kouloumos: ACK c0a5fce with a nit per bitcoin#19393 (comment). pg156: Reviewed to bitcoin@c0a5fce. Concept ACK. Agree due to the lack of RPC calls to inspect orphan pool, using `assert_debug_log` to match strings in log is a reasonable way to test. Tree-SHA512: 98f8deeee2d1c588c7e28a82e513d4a18655084198369db33fe2710458251eeaffed030626940072d7576f57fcbf7d856d761990129e2ca9e372d2ccbd86d07d Co-authored-by: MacroFake <falke.marco@gmail.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
✅ Final review complete — no blockers (commit 452481a) |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
💤 Files with no reviewable changes (4)
WalkthroughThe PR updates block-connected orphan transaction cleanup and expands functional coverage for orphan rejection, peer cleanup, inclusion, and conflicts. It removes deprecated top-level mempool fee fields and their functional test. Transaction-pool fuzz targets now initialize constraints after setup and keep them fixed per iteration. It adds a descriptor expansion benchmark, removes unused wallet database includes, and registers the benchmark source. Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Peer
participant BlockConnected
participant OrphanPool
participant Mempool
Peer->>BlockConnected: submit block
BlockConnected->>OrphanPool: erase included or conflicting orphans
BlockConnected->>OrphanPool: reconsider remaining orphans
OrphanPool->>Mempool: accept newly valid transactions
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Codex + Opus
The backported production and test changes are technically sound, and no build, runtime, consensus, or Dash-subsystem blockers were confirmed. The branch should still document the breaking mempool RPC field removal and exclude the unrelated #7544 LLMQ merge; the standalone orphan-ordering fix also needs a durable commit message.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— dash-core-commit-history (completed),gpt-5.6-sol— backport-reviewer (completed) - Verifier:
gpt-5.6-sol— final-verifier (Opus verifier fallback) - Opus reviewers:
claude-sonnet-5— general (failed),claude-sonnet-5— dash-core-commit-history (failed),claude-sonnet-5— backport-reviewer (completed),claude-sonnet-5— general (completed),claude-sonnet-5— dash-core-commit-history (failed),claude-sonnet-5— dash-core-commit-history (completed)
🟡 2 suggestion(s) | 💬 1 nitpick(s)
3 additional finding(s) omitted (not in diff).
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `src/rpc/mempool.cpp`:
- [SUGGESTION] src/rpc/mempool.cpp:245-287: Breaking RPC change lacks a release-notes entry
Add a `doc/release-notes-7551.md` fragment documenting this removal. The backport eliminates `-deprecatedrpc=fees` and the top-level `fee`, `modifiedfee`, `ancestorfees`, and `descendantfees` fields from all RPCs that expose this shared mempool-entry representation. Upstream bitcoin#25204 included commit `885694d7941`, which explicitly documented the removal, and merge commit `13336e83ec0` lists that commit while omitting its documentation hunk. Dash uses per-PR release-note fragments for equivalent user-facing RPC changes, and the earlier Dash release notes only announce these fields as deprecated, so users currently have no release-note record that the compatibility option and fields are now gone.
In `<commit:dc1d21e>`:
- [SUGGESTION] <commit:dc1d21e>:1: Remove the unrelated #7544 merge from this PR stack
Commit `dc1d21ea81b` is a true two-parent merge of PR #7544 that adds an unrelated LLMQ `IsQuorumActive` change before the advertised Bitcoin Core backports. Nothing in the subsequent RPC, GUI, fuzz, benchmark, or orphan-transaction changes depends on this guard. Landing the branch as-is would attribute #7544's behavior to both PRs and add unrelated merge topology. Rebase this branch after #7544 lands independently, or rebuild it from `43ec910c250` without `dc1d21ea81b` and its side commits.
|
What was done?
Regular backports from Bitcoin Core v24
How Has This Been Tested?
Run unit & functional tests
Breaking Changes
Removed command line option
-deprecatedrpc=feesand relevant RPC output.Checklist:
Go over all the following points, and put an
xin all the boxes that apply.