Skip to content

backport: [easy] bitcoin#27895, bitcoin#28028, bitcoin#28557, bitcoin#26780 - #7801

Merged
PastaPastaPasta merged 4 commits into
dashpay:developfrom
thepastaclaw:backport-0.26-b077-easy
Oct 4, 2026
Merged

PastaPastaPasta merged 4 commits into
dashpay:developfrom
thepastaclaw:backport-0.26-b077-easy

Conversation

@DCG-Claude

Copy link
Copy Markdown

⚠️ Easy batch, but not every commit is faithful any more (a later rebase or fix changed one). Read the review column before merging.

Issue being fixed or feature implemented

Backports 4 Bitcoin Core v0.26 pull request(s) that the Dash queue selected, including any discovered prerequisites: bitcoin#27895, bitcoin#28028, bitcoin#28557, bitcoin#26780.

What was done?

upstream commit review gates notes
bitcoin#28028 5e002d178c 🟡 light build:pass mech:warn pick:pass tests:pass tree:pass verify:pass This faithfully backports bitcoin#28028: the expected_stderr check moves from stop_node() into is_node_stopped
bitcoin#27895 4eda3493f9 🟢 faithful build:pass mech:pass pick:pass tests:warn tree:pass verify:pass clean cherry-pick
bitcoin#28557 058184e0a0 🟢 faithful build:pass mech:pass pick:pass tests:warn tree:pass verify:pass clean cherry-pick
bitcoin#26780 072d502f21 🟢 faithful build:pass ci_fork:pass mech:pass pick:pass tests:pass tree:pass verify:pass clean cherry-pick

Each commit keeps the upstream subject (partial Merge … only where a hunk is deferred to a prerequisite still to be backported, named in the commit body; a hunk Dash intentionally never wants is recorded in the commit body or the reviewer's note above and does not make the backport partial). Conflicts were resolved commit by commit; commits that needed no resolution were cherry-picked unchanged.

How Has This Been Tested?

Recorded per commit, at that commit's own sha, not once for the branch:

  • built at each commit, reusing the worktree build cache
  • the test suites touched by each diff run at that commit, where the diff selected one
  • a mechanical diff-of-diffs against the upstream patch (every upstream hunk present, no added line without an upstream counterpart, no Dash-specific line dropped)
  • an independent verification pass on 4 commit(s) that needed adaptation
  • CI green on the fork gate PR backport: v0.26 bitcoin#27895, bitcoin#28028, bitcoin#28557, bitcoin#26780 thepastaclaw/dash#89 at 072d502f21, which is this branch's head

Gates that did not come back clean — please weigh these:

Breaking Changes

None beyond the upstream changes themselves.

Checklist:

Left for the reviewer; backportsys does not tick boxes on its own behalf.

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone

Maintainer controls

Tick a box and backportsys acts on it within a few minutes, then clears the box. For anything else — a hunk to drop, a resolution to redo, a question — just leave a review comment; nothing here needs a box.

  • 🔒 Hands off — stop every automated update to this branch
  • 🔄 Rebase onto current develop
  • ❌ Close — abandon this batch and release its items

@thepastaclaw

thepastaclaw commented Oct 4, 2026 •

Copy link
Copy Markdown
Collaborator

🕓 Review not started yet because the new head is waiting for the 30-minute push debounce.

  • Request normal review — click when the PR is ready for review.
  • Request priority review — click to move this review to the front of the queue.

Commit 971341b. Normal review starts when eligible; priority review starts as soon as a slot is available.

@coderabbitai

coderabbitai Bot commented Oct 4, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (2)
AGENTS.md — auto-discovered
doc/developer-notes.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 9ab3cb2e-1335-481d-a345-10ac61a5bc94
📥 Commits

Reviewing files that changed from the base of the PR and between 8819507 and 072d502.

📒 Files selected for processing (6)
  • src/bench/disconnected_transactions.cpp
  • src/rpc/blockchain.cpp
  • test/functional/feature_abortnode.py
  • test/functional/rpc_blockchain.py
  • test/functional/rpc_scanblocks.py
  • test/functional/test_framework/test_node.py

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.


Walkthrough

The scanblocks RPC now scans inclusive chunks of up to 10,000 blocks and returns a completed field with its result. Shutdown checks in the functional test framework now validate expected exit codes and optional stderr. Related functional tests check the expected shutdown messages and scan completion. The disconnected-transaction benchmark callbacks no longer use NO_THREAD_SAFETY_ANALYSIS.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant scanblocks RPC
  participant Active chain
  participant Block filter
  Client->>scanblocks RPC: Start scan with height range
  scanblocks RPC->>Active chain: Resolve start and stop heights
  Active chain-->>scanblocks RPC: Return block indices
  loop Inclusive ranges of up to 10,000 blocks
    scanblocks RPC->>Block filter: Check filters in current range
    Block filter-->>scanblocks RPC: Return matching results
  end
  scanblocks RPC-->>Client: Return results, last endpoint, and completed status
Loading

Merge Risk: ⚪ Minimal · up to 072d5

The change chunks scanblocks ranges, tightens shutdown checks in the functional tests, and removes thread-safety annotations from the benchmark callbacks. No concrete merge-blocking risk was identified.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 072d5

Chunked scans can lose a consistent chain view during a reorganization. Results may combine different branches, and a sufficiently shortened chain can cause node-wide failure. Existing access paths are unchanged; production exposure and downstream consequences remain uncertain.

Retained concerns

  • Medium · reliability · inferred: Chunk endpoints are not anchored to one chain ancestry. A concurrent reorganization can mix branch results, while sufficient chain shortening can produce a null endpoint and fail the node process. The former membership-checked traversal contained these transitions more safely. Security impact beyond node availability and scan-result integrity is not established.
Security review details

Security Blast Radius

  • inferred — The supported exposure is scan-output integrity and availability of the node process executing the scan. A null endpoint can escape scan-local failure containment. Cross-service, tenant, financial, or persistent-data consequences are not established.

Security Findings and Attack Paths

  • inferred — The conditional failure path requires an admitted scan request overlapping a chain transition. Changing ancestry can mix results; shortening below a selected intermediate height can fail the process. Ordinary request parameters alone do not establish an exploit, and practical attacker control over that chain transition remains unverified.

Trust Boundaries and Controls

  • observed — Filter-type validation, index availability checks, active-chain height validation, and exclusive reservation constrain scanning. These controls do not preserve endpoint ancestry throughout the operation. Production RPC authentication and deployment ACLs were not established by the inspected evidence.

Hardening Proposals

  • proposed — Anchor chunk endpoints to the captured stop block's ancestry, or explicitly detect chain invalidation and terminate safely. Keep abort reporting distinct from successful range-read coverage so callers can distinguish cancellation, invalidation, and incomplete reads.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 54.55% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 6 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title identifies this as a backport and lists the four upstream pull requests covered by the changes.
Description check ✅ Passed The description explains the four backports, summarizes their changes, and reports testing and review notes relevant to the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final validation — Phase 1 + Phase 2

Verified all six changed files and the four backport commits at head 072d502 against their upstream diffs and relevant Dash callers; no actionable in-scope defects or missing upstream hunks were found. The shutdown changes preserve caller compatibility and correctly account for Dash’s governance-pruning warning, while the scanblocks refactor retains special-transaction matching. Static whitespace validation passed; no builds or tests were run locally, and the supplied CI snapshot shows successful lint and completed platform builds, with three Linux test jobs, the ASan build, and multiprocess dependency build still pending.

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: backport-reviewer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

  • Triage: normal by gpt-6.1-sol (effort low) — The batch changes scanblocks RPC iteration, progress and completion reporting alongside shared test-node shutdown checks, making it an ordinary logic change rather than a trivial cleanup or a change to a critical surface.
  • Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer
  • Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
  • Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
  • Phase 2 reviewers: gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — backport-reviewer (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort high); agent phase2-reviewer

@thepastaclaw thepastaclaw added the pastaclaw:approved thepastaclaw's latest review approved this PR label Oct 4, 2026

@PastaPastaPasta PastaPastaPasta left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 072d502

6779e6e test: clean up is node stopped (dimitaracev)

Pull request description:

  Fixes bitcoin#27893

  Use f'strings for the message when asserting `expected_ret_code` and `return_code`. Change the `expected_ret_code` from an optional to have a default value of `0`.

  cc MarcoFalke

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 6779e6e
  stickies-v:
    ACK 6779e6e
  brunoerg:
    ACK 6779e6e

Tree-SHA512: af84e7ffe467ced29236dee9206687786a2efb89ab8b039c3ebfb93ea23fc273206cd51f20c9fb6bee4135770e9a649538942571d9c0be83ba9535fa8e59cb28
faf9028 test: Check expected_stderr after stop (MarcoFalke)

Pull request description:

  This fixes a bug where stderr wasn't checked for the shutdown sequence.

  Fix that by waiting for the shutdown to finish and then check stderr.

ACKs for top commit:
  theStack:
    ACK faf9028

Tree-SHA512: a70cd1e6cda84d542782e41e8b59741dbcd472c0d0575bcef5cbfd1418473ce94efe921481d557bae3fbbdd78f1c49c09c48872883c052d87c5c9a9a51492692
d67aa25 bench: drop NO_THREAD_SAFETY_ANALYSIS from disconnected_txs (fanquake)

Pull request description:

  Followup to bitcoin#28385 (comment).

ACKs for top commit:
  MarcoFalke:
    lgtm ACK d67aa25
  hebasto:
    ACK d67aa25, tested on Ubuntu 22.04 with clang 18.0.
  glozow:
    utACK d67aa25

Tree-SHA512: a9a9a8cc70a50d4fbd51779a3203bbc2a29d354b557e8a99cfd649d5998b71ff1087f5bae7170471bed9a917a93c8f3351ae90c9a6e87d88928c35912d007b64
b922f6b rpc: scanblocks, add "completed" flag to the result obj (furszy)
ce50acc rpc: scanblocks, do not traverse the whole chain block by block (furszy)

Pull request description:

  Coming from bitcoin#23549 (review)

  The current `scanblocks` flow walks-through every block in the active chain
  until hits the chain tip or processes 10k blocks, then calls `lookupFilterRange`
  function to obtain all filters from that particular range.

  This is only done to obtain the heights range to look up the block
  filters. Which is unneeded.

  As `scanblocks` only lookup block filters in the active chain, we can
  directly calculate the lookup range heights, by using the chain tip,
  without requiring to traverse the chain block by block.

ACKs for top commit:
  achow101:
    ACK b922f6b
  TheCharlatan:
    ACK b922f6b

Tree-SHA512: 0587e6d9cf87a59184adb2dbc26a4e2bce3a16233594c6c330f69feb49bf7dc63fdacf44fc20308e93441159ebc604c63eb7de19204d3e745a2ff16004892b45
@PastaPastaPasta
PastaPastaPasta force-pushed the backport-0.26-b077-easy branch from 072d502 to 971341b Compare October 4, 2026 15:42
@thepastaclaw thepastaclaw removed the pastaclaw:approved thepastaclaw's latest review approved this PR label Oct 4, 2026
@PastaPastaPasta
PastaPastaPasta merged commit fd269cf into dashpay:develop Oct 4, 2026
11 checks passed
@UdjinM6 UdjinM6 added this to the 24.1 milestone Oct 6, 2026
PastaPastaPasta added a commit that referenced this pull request Oct 7, 2026
fbe6526 Merge #7807: feat(consensus): hold EvoNode shares and multiple payouts behind a new evo_shares deployment (pasta)
57fd2d9 Merge #7776: fix: keep a pending ProUpServTx across a registrar update that keeps the operator key (pasta)
ca69801 Merge #7803: fix: return non-zero amount of accounts after sethdseed (pasta)
57a8f07 Merge #7796: backport: bitcoin#26720 (remaining), partial bitcoin#29523: weight-check the knapsack exact-match selection (pasta)
4eb82f6 Merge #7786: fix(net): keep a dedicated onion listener when -bind is given and correct the bind release note (pasta)
70626a0 Merge #7793: backport: partial bitcoin#28868 (wallet: reload wallet if migration exited early, keep mixed watchonly txs) (pasta)
250a3cd Merge #7790: fix: guard cached governance object flags with the object lock (pasta)
700c195 Merge #7787: fix: only sign EHF signals inside the deployment's start/timeout window (pasta)
a3b0f3c Merge #7785: fix: restore the txindex requirement when governance validation is enabled (pasta)
2d1735d Merge #7794: fix(wallet): floor the denomination gap threshold so rebalancing cannot oscillate (pasta)
d9f6592 Merge #7783: fix: stop fish completion from re-running the typed dash-cli command (pasta)
89d44c8 Merge #7795: backport: bitcoin#34272, partial bitcoin#31650 (pass PSBT by const reference in PSBTInputSignedAndVerified) (pasta)
a1d7411 Merge #7779: fix!: report legacy evonode Platform addresses when listdiff changes its address (pasta)
73f6ea2 Merge #7789: fix(qt): report malformed lang/font settings instead of aborting at startup (pasta)
40de493 Merge #7781: fix(rpc): use a consistent tip height in quorum dkgstatus (pasta)
7d1a90d Merge #7791: fix(net): request object votes from nPeersPerHashMax peers (pasta)
51b145c Merge #7778: test: use single-member in functional tests: feature_mnehf, feature_notifications, p2p_instantsend (pasta)

Pull request description:

  ## Issue being fixed or feature implemented

  Backports for v24.0.0-rc.3. `v24.0.x` was cut at the v24.0.0-rc.2 tag (1e239d4), and develop has since moved to 24.1.0 (#7775), so rc.3 is built from this branch instead of being tagged on develop like rc.1 and rc.2.

  ## What was done?

  Cherry-picked (`git cherry-pick -m1 -x`) every develop merge since rc.2 that carries `backport-candidate-24.0.x`, plus #7778, which #7807's test needs. They are in develop merge order:

  - #7778 test: use single-member in functional tests: feature_mnehf, feature_notifications, p2p_instantsend (test-only. Needed because #7807's `feature_evo_shares_activation.py` depends on its `mine_quorum_single_member()` fix and fails without it.)
  - #7791 fix(net): request object votes from nPeersPerHashMax peers
  - #7781 fix(rpc): use a consistent tip height in quorum dkgstatus
  - #7789 fix(qt): report malformed lang/font settings instead of aborting at startup
  - #7779 fix!: report legacy evonode Platform addresses when listdiff changes its address
  - #7795 backport: bitcoin#34272, partial bitcoin#31650 (pass PSBT by const reference in PSBTInputSignedAndVerified)
  - #7783 fix: stop fish completion from re-running the typed dash-cli command
  - #7794 fix(wallet): floor the denomination gap threshold so rebalancing cannot oscillate
  - #7785 fix: restore the txindex requirement when governance validation is enabled
  - #7787 fix: only sign EHF signals inside the deployment's start/timeout window
  - #7790 fix: guard cached governance object flags with the object lock
  - #7793 backport: partial bitcoin#28868 (wallet: reload wallet if migration exited early, keep mixed watchonly txs)
  - #7786 fix(net): keep a dedicated onion listener when -bind is given and correct the bind release note. Includes partial bitcoin#36170 (5f40d56). Upstream's dedicated-onion-bind changes to `feature_torcontrol.py` and `p2p_private_broadcast.py` are not included, because those tests depend on upstream commits (5693833, e74d54e) that Dash does not have.
  - #7796 backport: bitcoin#26720 (remaining), partial bitcoin#29523: weight-check the knapsack exact-match selection
  - #7803 fix: return non-zero amount of accounts after sethdseed
  - #7776 fix: keep a pending ProUpServTx across a registrar update that keeps the operator key
  - #7807 feat(consensus): hold EvoNode shares and multiple payouts behind a new evo_shares deployment

  One pick needed conflict resolution, in a test only, described in its commit message: #7793, `test/functional/wallet_migration.py`. `test_conflict_txs()` comes from bitcoin#28542 (#7762), which is not on this branch, so it is left out. The C++ part of #7793 is identical to develop.

  Merged since rc.2 but intentionally not included: #7775 (24.1 version bump), #7260 and #7804 (new Qt feature and its CI fix), #7584 (refactor), routine upstream backports #7257, #7759, #7762, #7768, #7788, #7801, and test/CI-only changes #7777 and #7797.

  No version change is needed. `configure.ac` is already 24.0.0 with `CLIENT_VERSION_IS_RELEASE` false, so the build takes its version string from the `v24.0.0-rc.3` tag, as it did for rc.2.

  ## How Has This Been Tested?

  Built on macOS arm64 against depends with `--enable-debug --enable-werror` (dashd, dash-qt, tests).

  - `make check`: all unit test suites pass, including `test_dash-qt`.
  - Functional tests, all passing: `feature_evo_shares_activation.py`, `feature_masternode_shares.py`, `feature_mnehf.py`, `feature_governance.py --descriptors`, `feature_governance_objects.py`, `feature_governance_txindex_devnet.py`, `feature_proxy.py`, `feature_notifications.py`, `feature_llmq_connections.py`, `feature_llmq_chainlocks_automatic.py`, `feature_llmq_singlenode.py`, `p2p_instantsend.py`, `rpc_blockchain.py` (v1 and v2 transport), `rpc_netinfo.py`, `rpc_quorum.py`, `rpc_verifychainlock.py`, `wallet_hd.py` (legacy and descriptors), `wallet_migration.py`.
  - `feature_bind_extra.py` (from #7786) only runs on Linux and was skipped locally, so CI covers it.

  Every touched file was diffed against develop. The remaining differences come only from develop PRs that are not part of this backport.

  ## Breaking Changes

  Carried over from the picked PRs:

  - #7807: consensus change behind the new `evo_shares` deployment (bit 14). Mainnet and testnet are unaffected until it activates. Devnets with `v24` already active and an EvoNode carrying several owner payouts need to check or reset. New RPC `protx shared_register_prepare_evo`.
  - #7785: outside regtest, a node with `-txindex=0` and without `-disablegovernance` refuses to start again, as before v23.1.0.
  - #7786: carries a partial bitcoin#36170. A node started with `-bind` but no specific `-bind=<addr:port>=onion` now refuses to start while `-listenonion` is enabled. That is the default when listening, even without Tor configured. A wildcard onion bind is also refused. Such nodes need `-bind=127.0.0.1:<port>=onion` or `-listenonion=0`. Nodes without `-bind`, including those using only `-whitebind`, keep the default `127.0.0.1:9996` (`19996` testnet) onion target. See `doc/release-notes-7300.md`.
  - #7779: `protx listdiff` now includes `platform_p2p` and `platform_https` for a legacy-address evonode whose core P2P address changes.

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [ ] I have made corresponding changes to the documentation
  - [ ] I have assigned this pull request to a milestone

  🤖 Generated with [Claude Code](https://claude.com/claude-code)

Top commit has no ACKs.

Tree-SHA512: 8cb01bdb0234f31b5dd2c105b69673de148922991a7523627395c0e0ccfa22b94be5ffbecc842398d443bc90fd9baf2eee6a9cf6fcdbbd1626eb00b93726a55a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants