Skip to content

fix(rpc): derive active DKG count from membership - #7812

Merged
PastaPastaPasta merged 2 commits into
dashpay:developfrom
PastaPastaPasta:fix/dkginfo-current-membership
Oct 9, 2026
Merged

PastaPastaPasta merged 2 commits into
dashpay:developfrom
PastaPastaPasta:fix/dkginfo-current-membership

Conversation

@PastaPastaPasta

@PastaPastaPasta PastaPastaPasta commented Oct 7, 2026 •

Copy link
Copy Markdown
Member

Issue being fixed or feature implemented

quorum dkginfo is meant to answer a simple question: is this masternode participating in a DKG right now, or can it be restarted safely? active_dkgs answers it wrongly at both ends of a session. At a DKG start block it stays zero until the asynchronous DKG worker publishes its local debug record, and afterwards it keeps counting expired debug records.

What was done?

active_dkgs is now computed from membership at the captured chain tip instead of debug-session tracking:

  • Sessions the given or local masternode is a member of count from their start block, even before the DKG worker initializes them.
  • Each session counts through its Commit phase and stops at quorumHeight + 5 * dkgPhaseBlocks. Rotated indices expire individually, not at the shared cycle mining window.
  • If membership can't be determined (for example before v20), the session is counted, so zero never hides possible participation.
  • Calls without a supplied or local proTxHash report zero.

No RPC fields are added. upcoming_dkgs is unchanged. The membership lookup is shared between active_dkgs and upcoming_dkgs, and CDKGDebugManager::GetSessionCount(), which lost its only caller, is removed.

Together with upcoming_dkgs, this lets dashpay/platform#5231 decide whether a masternode can stop safely from a single quorum dkginfo call.

How Has This Been Tested?

  • test/functional/rpc_quorum.py: with DKG publication disabled, checks the count for a member and a non-member at the start block, inside the window, and after expiry, against the upcoming_dkgs predictions taken one block before the start.
  • test/functional/feature_llmq_rotation.py: checks conservative counting of unknown membership before v20, zero counts despite expired debug sessions, and per-index expiry of rotated sessions at cycle offsets 0, 1, 9, 10, 11, 12 and 13.

Breaking Changes

active_dkgs stays numeric, but its meaning is corrected: it includes sessions not yet initialized locally, excludes expired sessions, counts unknown membership, and follows the supplied proTxHash. Clients that need the local DKG worker state should use quorum dkgstatus.

Checklist:

  • 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

@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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: 1cecaa40-3619-42fd-9d38-7a8de946b1cf
📥 Commits

Reviewing files that changed from the base of the PR and between c2c50fb and 1ed0310.

📒 Files selected for processing (4)
  • doc/release-notes-7812.md
  • src/llmq/debug.cpp
  • src/llmq/debug.h
  • src/rpc/quorums.cpp
💤 Files with no reviewable changes (2)
  • src/llmq/debug.cpp
  • src/llmq/debug.h

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

quorum dkginfo now computes active_dkgs from eligible current and upcoming sessions. It counts current sessions through the Commit phase and counts sessions with unknown membership conservatively. Upcoming entries report membership details when available or a reason when membership cannot be determined. Functional tests cover pre-v20 unknown membership, rotated membership, and upcoming type-100 membership. The release note describes the updated counting behavior.

Priority: ➖ Normal

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

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant DkginfoRPC
  participant QuorumData
  Client->>DkginfoRPC: Request dkginfo
  DkginfoRPC->>QuorumData: Check cycle, work block, and membership
  QuorumData-->>DkginfoRPC: Membership result or unknown reason
  DkginfoRPC-->>Client: active_dkgs and upcoming DKG details
Loading

Merge Risk: ⚪ Minimal · up to 1ed03

The changed DKG participation count is consistent with session timing and membership selection. No issue requiring a fix before merge was established.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 4 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Title check Passed The title clearly and concisely describes the main change: deriving the active DKG count from membership in the RPC.
Description check Passed The description directly explains the active_dkgs behavior change, implementation, testing, compatibility impact, and rationale.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 4 files. (1 skipped: 1 unsupported.)

  • 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 commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

✅ Final review complete — no blockers (commit 1ed0310) · triage: normal

@PastaPastaPasta PastaPastaPasta changed the title fix(rpc): cover DKG membership during session initialization fix(rpc): derive active DKG count from membership Oct 7, 2026

@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 2 only (queue backlog)

Verified the supplied Phase-2 claims against head ca29e24 and the four-commit PR range. No in-scope correctness blocker was identified; the commit-history suggestion is supported by the diffs and remains nonblocking. Validation was static only: the supplied CI snapshot shows successful formatting and merge checks, two queued container-manifest jobs, and cancelled conflict prediction; no local builds or tests were run.

🟡 1 suggestion(s)

1 finding(s) not shown inline (the lines are not part of this PR's diff)

🟡 Suggestion: Fold the active-window correction into the implementation it amends
<commit:ca29e24b74>:1

Commit ca29e24 replaces the dkgMiningWindowStart cutoff introduced by cbd8aa7 and retained by 0881935 with 5 * dkgPhaseBlocks, updates the newly added rotated-boundary assertions, and corrects the new current-session reason text. These changes complete the behavior introduced earlier in this same unmerged stack rather than repair behavior predating the PR. Fold this correction into the membership implementation, regrouping the implementation commits as needed; if the separate release-note commit is retained, consolidate its subsequent edits there. This follows the project's atomic-commit guidance and keeps permanent history focused on the complete intended behavior instead of its intermediate cutoff and immediate repair.

source: gpt-6.1-sol (phase2-reviewer: dash-core-commit-history)

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: 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 change adds nontrivial RPC membership prediction and per-session expiration logic with boundary tests, but does not itself alter consensus, funds movement, cryptography, network deserialization, or storage migrations.
  • Phase 1 reviewers: not run (skipped for throughput: 13 PRs queued, above the 10 limit)
  • 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 — dash-core-commit-history (completed, effort high); agent phase2-reviewer
🤖 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 `<commit:ca29e24b74>`:
- [SUGGESTION] <commit:ca29e24b74>:1: Fold the active-window correction into the implementation it amends
  Commit ca29e24b74 replaces the dkgMiningWindowStart cutoff introduced by cbd8aa7865 and retained by 08819354de with 5 * dkgPhaseBlocks, updates the newly added rotated-boundary assertions, and corrects the new current-session reason text. These changes complete the behavior introduced earlier in this same unmerged stack rather than repair behavior predating the PR. Fold this correction into the membership implementation, regrouping the implementation commits as needed; if the separate release-note commit is retained, consolidate its subsequent edits there. This follows the project's atomic-commit guidance and keeps permanent history focused on the complete intended behavior instead of its intermediate cutoff and immediate repair.

@thepastaclaw thepastaclaw added the pastaclaw:commented thepastaclaw's latest review was comment-only label Oct 7, 2026
@PastaPastaPasta
PastaPastaPasta force-pushed the fix/dkginfo-current-membership branch from ca29e24 to 31c1e47 Compare October 7, 2026 07:30
@thepastaclaw thepastaclaw removed the pastaclaw:commented thepastaclaw's latest review was comment-only label Oct 7, 2026

@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.

Re-review — Final validation — Phase 1 + Phase 2

Independently reviewed the complete diff at 31c1e47 and found no actionable in-scope defects. Membership-derived counts, conservative unknown handling, and per-index Commit-phase expiry match the surrounding selection and scheduling code; the prior commit-hygiene finding is fixed. This was a static review without local build or test execution; the supplied CI snapshot shows successful completed checks and four queued container builds.

🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 nitpick(s)

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: dash-core-commit-history); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 5: 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 change adds nontrivial RPC membership prediction and per-session boundary handling with regression tests, but does not change consensus, funds movement, cryptography, network deserialization, or storage migrations.
  • 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 final gate: an independent Phase-2 review ran after iterative findings were reconciled
  • 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 — dash-core-commit-history (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort high); agent phase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify the current code and confirm that no unresolved issues remain.

No unresolved findings remain from the prior review on this head.

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

@knst knst 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.

what are all of these rpc fields in response are needed for?

simple rpc that answer to question about current active dkg sessions and upcoming suddenly returns so many things. That's not for debugging of dkg quorum process, that's for answering to simple trivial question "is masternode participating in dkg or could be safely rebooted / restarted"?

haveen't checked further more

@PastaPastaPasta
PastaPastaPasta force-pushed the fix/dkginfo-current-membership branch from 31c1e47 to 091b462 Compare October 7, 2026 18:30
PastaPastaPasta added a commit to dashpay/platform that referenced this pull request Oct 7, 2026
Core no longer reports active_dkg_sessions (dashpay/dash#7812 was
slimmed). When upcoming_dkgs is present, stopping is safe only if
active_dkgs is 0 and no member, unknown-membership or malformed upcoming
DKG starts within MIN_BLOCKS_BEFORE_DKG. The legacy next_dkg guard and
dkgstatus refinement are kept for responses without upcoming_dkgs.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@PastaPastaPasta

Copy link
Copy Markdown
Member Author

@knst Agreed, the extra output wasn't needed. I removed active_dkg_sessions entirely (091b462). The PR now adds no RPC fields.

What's left is the fix to active_dkgs itself, so it answers "is this masternode in a DKG right now?" correctly:

  • It counts sessions the given or local masternode is a member of from the start block, even before the DKG worker initializes them locally. Previously the count was 0 until the worker published its debug record, which is the gap that matters for safe restarts.
  • It stops counting once a session's Commit phase ends, instead of keeping expired debug records.
  • If membership can't be determined, the session is counted, so 0 never hides possible participation.

Together with your existing upcoming_dkgs, that's everything dashmate needs: stop only if active_dkgs == 0 and no member or unknown-membership DKG starts within the restart margin. dashpay/platform#5231 now uses exactly that.


🤖 Posted autonomously by Claude on behalf of pasta.

@PastaPastaPasta
PastaPastaPasta force-pushed the fix/dkginfo-current-membership branch from 091b462 to c2c50fb Compare October 7, 2026 18:38

@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.

Re-review — Final validation — Phase 1 + Phase 2

Independently reviewed the complete PR diff at c2c50fb against the surrounding DKG lifecycle, membership-selection code, and functional-test setup; no actionable in-scope defects were found. The implementation counts membership through Commit, expires rotated indices independently, conservatively counts unknown membership, and preserves upcoming-session reporting. Validation was static only: the supplied CI snapshot shows successful preliminary checks, while container builds and conflict prediction remain queued; completed build and functional-test validation is not established.

🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 nitpick(s)

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: dash-core-commit-history); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 5: 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 change revises RPC participation-counting logic across DKG phase boundaries, rotated indices, and unknown membership with functional test coverage, but does not itself alter 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 final gate: an independent Phase-2 review ran after iterative findings were reconciled
  • 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 — dash-core-commit-history (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort high); agent phase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify the current code and confirm that no unresolved issues remain.

No unresolved findings remain from the prior review on this head.

PastaPastaPasta and others added 2 commits October 9, 2026 00:27
`quorum dkginfo` reported `active_dkgs` as the number of local DKG debug
records. Those appear only once the DKG worker initializes a session and
linger after it ends, so the count was 0 at a session's start block and
non-zero long after the masternode stopped participating.

Compute the count from membership at the captured tip instead: a session
counts from its start block through the end of its Commit phase when the
given or local masternode is a member of it. Rotated indices expire on
their own schedule rather than at the shared mining window. Sessions
whose membership cannot be determined are counted, so 0 never hides
possible participation.

The membership lookup is shared with `upcoming_dkgs`, whose output is
unchanged. CDKGDebugManager::GetSessionCount() has no remaining callers
and is removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@PastaPastaPasta
PastaPastaPasta force-pushed the fix/dkginfo-current-membership branch from c2c50fb to 1ed0310 Compare October 9, 2026 05:28

@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.

Re-review — Final validation — Phase 1 + Phase 2

Independently reviewed the complete PR diff at 1ed0310 and verified the membership calculation, conservative unknown-membership handling, per-index Commit-phase expiry, and preservation of upcoming-session output; no actionable in-scope issues were found. The prior commit-organization finding is fixed: the correction and regression coverage are incorporated into the implementation commit. Validation was static only; the supplied CI snapshot reports successful completed builds, lint, and standard Linux, SQLite, and no-wallet tests, with ASan tests, TSan tests, and the multiprocess build still pending.

🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 nitpick(s)

Review provenance

Source: reviewer 1: gemini-3.8-flash-high (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: dash-core-commit-history); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 5: 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 change revises RPC counting logic across DKG membership, phase boundaries, and rotated-session expiry with functional coverage, but does not itself alter consensus, funds, cryptography, network deserialization, or storage migrations.
  • Phase 1 reviewers: gemini-3.8-flash-high — general (completed, effort high); agent phase1-reviewer
  • Phase 1 model: gemini-3.8-flash-high — antigravity quota: weekly 59% left, 5h 39% left
  • Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
  • 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 — dash-core-commit-history (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort high); agent phase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify the current code and confirm that no unresolved issues remain.

No unresolved findings remain from the prior review on this head.

@PastaPastaPasta PastaPastaPasta left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

approved; this looks reasonable; should fix an edge case bug in dashmate

@PastaPastaPasta
PastaPastaPasta merged commit 5c66004 into dashpay:develop Oct 9, 2026
49 checks passed
knst added a commit to knst/dash that referenced this pull request Oct 9, 2026
…status

 - Rotation test. The existing check after the DKG finishes now asserts 0 for each masternode. It no longer sums the stale debug sessions. The PR's two added scenarios are removed.
 - rpc_quorum.py. It now mines to the next DKG start block while the DKG spork is still off. Then it asserts the masternode's active_dkgs is above 0, before turning the spork on. The PR's separate scenario is removed.
  - test_framework.py. The helper that only the removed scenarios used is gone.
knst added a commit to knst/dash that referenced this pull request Oct 10, 2026
…status

 - Rotation test. The existing check after the DKG finishes now asserts 0 for each masternode. It no longer sums the stale debug sessions. The PR's two added scenarios are removed.
 - rpc_quorum.py. It now mines to the next DKG start block while the DKG spork is still off. Then it asserts the masternode's active_dkgs is above 0, before turning the spork on. The PR's separate scenario is removed.
  - test_framework.py. The helper that only the removed scenarios used is gone.
knst added a commit to knst/dash that referenced this pull request Oct 10, 2026
…status

 - Rotation test. The existing check after the DKG finishes now asserts 0 for each masternode. It no longer sums the stale debug sessions. The PR's two added scenarios are removed.
 - rpc_quorum.py. It now mines to the next DKG start block while the DKG spork is still off. Then it asserts the masternode's active_dkgs is above 0, before turning the spork on. The PR's separate scenario is removed.
  - test_framework.py. The helper that only the removed scenarios used is gone.
@PastaPastaPasta
PastaPastaPasta deleted the fix/dkginfo-current-membership branch October 13, 2026 16:14
PastaPastaPasta added a commit that referenced this pull request Oct 13, 2026
…ollow-up fixes for lately added tests.

99cdb0b test: follow-up for #7812: use less fixture to test quorum dkgstatus (Konstantin Akimov)
0288ca9 docs: be more explicit about "scope", "scenario", "case" when writing tests. (Konstantin Akimov)
8a620db test: follow-up for #7838: replace brand new scenario to extra check (Konstantin Akimov)
4a19742 test: follow-up for #7821: check if wallet is locked after failed upgrade instead creating brand-new scenario (Konstantin Akimov)
1435bef test: follow-up for #7829: replace brand-new scenario to just extra check for mixing-unlock (Konstantin Akimov)
8a67e4d test: RPC importelectrumwallet have functional tests, removed an exception (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  #7829, #7821, #7838, #7812, added brand-new scenarious to functional tests while it could tested by extra check in existing scenarious.

  ## What was done?
  Updated guidance for agents about writing new tests.
  Follow-up fixes for #7829, #7821, #7838 by replacing scenarious to single check.
  Removed an exception for `importelectrumwallet` from list of un-tested RPCs
  For #7812:
   - Rotation test. The existing check after the DKG finishes now asserts 0 for each masternode. It no longer sums the stale debug sessions. The PR's two added scenarios are removed.
   - rpc_quorum.py. It now mines to the next DKG start block while the DKG spork is still off. Then it asserts the masternode's active_dkgs is above 0, before turning the spork on. The PR's separate scenario is removed.
   - test_framework.py. The helper that only the removed scenarios used is gone.

  ## How Has This Been Tested?
  Run functional tests.

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

Top commit has no ACKs.

Tree-SHA512: 625e2792ad4a848fa9a70dc65109846dae822e87fe0a8a116c7ce8bac224c7a35179bde6751915391d2d1b1bcab32eb1e28eaeaa4e54a5ff777d5c3f997924ba
@UdjinM6 UdjinM6 added this to the 24.1 milestone Oct 16, 2026
PastaPastaPasta added a commit that referenced this pull request Oct 17, 2026
6c02b2b ci: let DEPENDS_ARCH override the machine arch for multiprocess and tsan (pasta)
ce5dbf3 Merge #7888: fix(consensus): canonical ExtNetInfo encoding and strict netInfo decoding (pasta)
54cba5c Merge #7537: fix: data races and check-then-act races in the CoinJoin server (pasta)
08d8bca Merge #7909: feat(mining): build the masternode list incrementally during block assembly (pasta)
5633761 Merge #7895: test: cover retroactive provider tx refusal, fix wording (pasta)
cbb9500 Merge #7846: fix: treat pending owner keys and Platform node IDs as one conflict namespace (pasta)
8efdf5e Merge #7857: fix: mine a pending ProUpServTx before a key change of its masternode (pasta)
e56e6ae Merge #7842: fix(wallet): reject mnemonic passphrases that seed derivation truncates (pasta)
79480ff test: follow-up for #7821: check if wallet is locked after failed upgrade instead creating brand-new scenario (Konstantin Akimov)
a56ed47 fix(net): announce sporks to new peers instead of requesting them during mnsync (pasta)
cdaea5f Merge #7894: fix: due to logical conflicts feature_pruning.py had been broken while (pasta)
98d92f6 Merge #7817: backport: partial bitcoin#34156 restore cleanup ownership (pasta)
35e6d18 Merge #7858: fix: do not InstantSend-lock provider transactions (pasta)
904d73d Merge #7881: fix: refuse to sign shared registrar updates that move the funder's coins (pasta)
95982f5 Merge #7782: fix(wallet): persist pending-observation coin locks once the record becomes readable (pasta)
cdf65d0 Merge #7836: backport: partial bitcoin#25812, bitcoin#29855 (pasta)
cc5716f Merge #7824: fix: retain initialized DKG sessions during inventory reads (pasta)
588e458 Merge #7827: backport: partial bitcoin#35984 (pasta)
795c6f1 Merge #7839: fix: release InstantSend tracking after mempool eviction (pasta)
dc1a6b2 Merge #7875: docs(rpc): clarify getchainlockbyheight availability (pasta)
e4798ff Merge #7876: fix(consensus): capture operator key scheme changes with unchanged bytes in state diffs (pasta)
fe68ecb Merge #7871: backport!: Merge bitcoin#27217: wallet: Replace use of purpose strings with an enum (pasta)
8348d22 Merge #7867: fix: refuse masternode list RPCs for blocks outside the active chain (pasta)
861a579 Merge #7866: fix: stop `masternode payments` at the genesis block (pasta)
efcef71 Merge #7521: fix: update masternode rate limit on failed governance trigger path (pasta)
0623182 Merge #7831: fix: bound governance object signatures and reset the governance cache (pasta)
b230657 Merge #7841: fix: authenticate cached InstantSend proof bytes (pasta)
a1f37ea Merge #7820: fix: hold chainstate lock during CoinJoin input validation (pasta)
50a5617 Merge #7829: fix: preserve wallet permissions when changing passphrases (pasta)
2ff0103 Merge #7818: fix: return REST errors for unavailable deployment data (pasta)
cb39369 Merge #7834: fix: validate Electrum import input boundaries (pasta)
e7e3daa Merge #7821: fix: relock encrypted wallets after HD upgrade failures (pasta)
2fea45f Merge #7792: fix: treat missing snapshots during evodb repair as repair errors (pasta)
3c5c039 Merge #7838: fix(wallet): authenticate unlock deadline extensions (pasta)
0bf80f8 Merge #7810: fix(rpc): handle unavailable masternode payment inputs (pasta)
bcefee4 Merge #7822: fix: reject malformed detached quorum share batches (pasta)
86a7737 Merge #7828: fix: omit recovery words from invalid mnemonic errors (pasta)
886d5c4 Merge #7816: fix: check credit pool amount accumulation (pasta)
00fc3b9 Merge #7851: fix(rpc): correct masternode status help for top-level service field (pasta)
0e75c22 Merge #7814: fix(qt): restore HTML escaping of send confirmation recipients (pasta)

Pull request description:

  ## Issue being fixed or feature implemented

  Backports for v24.0.0-rc.4, on top of `v24.0.x` at the v24.0.0-rc.3 tag (0364ed6, #7808).

  ## What was done?

  Cherry-picked (`git cherry-pick -m1 -x`) every develop merge since rc.3 that carries `backport-candidate-24.0.x`, plus eight unlabeled fixes picked after an audit of every other develop merge since rc.3 (marked † below). They are in develop merge order:

  - #7814 fix(qt): restore HTML escaping of send confirmation recipients †
  - #7851 fix(rpc): correct masternode status help for top-level service field
  - #7816 fix: check credit pool amount accumulation
  - #7828 fix: omit recovery words from invalid mnemonic errors †
  - #7822 fix: reject malformed detached quorum share batches †
  - #7810 fix(rpc): handle unavailable masternode payment inputs †
  - #7838 fix(wallet): authenticate unlock deadline extensions
  - #7792 fix: treat missing snapshots during evodb repair as repair errors
  - #7821 fix: relock encrypted wallets after HD upgrade failures
  - #7834 fix: validate Electrum import input boundaries
  - #7818 fix: return REST errors for unavailable deployment data
  - #7829 fix: preserve wallet permissions when changing passphrases
  - #7820 fix: hold chainstate lock during CoinJoin input validation
  - #7841 fix: authenticate cached InstantSend proof bytes
  - #7831 fix: bound governance object signatures and reset the governance cache
  - #7521 fix: update masternode rate limit on failed governance trigger path
  - #7866 fix: stop `masternode payments` at the genesis block
  - #7867 fix: refuse masternode list RPCs for blocks outside the active chain †
  - #7871 backport!: bitcoin#27217: wallet: Replace use of purpose strings with an enum
  - #7876 fix(consensus): capture operator key scheme changes with unchanged bytes in state diffs
  - #7875 docs(rpc): clarify getchainlockbyheight availability †
  - #7839 fix: release InstantSend tracking after mempool eviction
  - #7827 backport: partial bitcoin#35984
  - #7824 fix: retain initialized DKG sessions during inventory reads
  - #7836 backport: partial bitcoin#25812, bitcoin#29855
  - #7782 fix(wallet): persist pending-observation coin locks once the record becomes readable
  - #7881 fix: refuse to sign shared registrar updates that move the funder's coins †
  - #7858 fix: do not InstantSend-lock provider transactions
  - #7817 backport: partial bitcoin#34156 restore cleanup ownership
  - #7894 test: fix feature_pruning.py restart arguments † (needed because #7792 makes the evodb repair fatal, which the old restart arguments trip)
  - #7893 fix(net): announce sporks to new peers instead of requesting them during mnsync
  - 4a19742 (from #7859) test: follow-up for #7821: check if wallet is locked after failed upgrade instead creating brand-new scenario. Test-only, picked as its commit. Without it, #7821's `test_failed_upgrade()` expects RPC error -1 for a 257-byte mnemonic passphrase, which #7842's new precheck rejects with -8 first, so `wallet_upgradetohd.py --legacy-wallet` fails. On develop, #7859 replaced that scenario before #7842 merged. The rest of #7859 is test cleanup and docs and is left out.
  - #7842 fix(wallet): reject mnemonic passphrases that seed derivation truncates
  - #7857 fix: mine a pending ProUpServTx before a key change of its masternode
  - #7846 fix: treat pending owner keys and Platform node IDs as one conflict namespace
  - #7895 test: cover retroactive provider tx refusal, fix wording
  - #7909 feat(mining): build the masternode list incrementally during block assembly
  - #7537 fix: data races and check-then-act races in the CoinJoin server
  - #7888 fix(consensus): canonical ExtNetInfo encoding and strict netInfo decoding

  34 of the 39 picks change exactly the same lines as their develop merges. #7888 and #7782 needed test-only changes (see testing), and three others needed adaptation, described in their commit messages:

  - **#7893** was merged to develop twice: f99212b with an earlier revision of its commit, then 72a5997 with the final one, so develop now announces sporks twice in the VERACK handler (a develop fix is open separately). Here the PR's final commit 45acdf3 is picked once instead of either merge. In `src/node/sync_manager.cpp`, this branch still sent `GETSPORKS` from the mnsync tick on every network, because #7847, which limited it to test chains, is not included. The request is removed entirely, as on develop. Mainnet spork values are hardcoded in `CSporkManager::GetSporkValue`, so the mainnet request had no effect.
  - **#7857** and **#7909**: develop folded `CChainstateHelper` into `Chainstate` (d5c1bd7), which is not on this branch, so the new code goes through `ChainHelper().special_tx` / `m_chain_helper.special_tx` instead of `m_special_tx`. The neighbouring develop-only `CSpecialTxProcessor::GetDeterministicMNListHash` is left out.

  #7888 tightens consensus rules for ExtAddr provider transactions behind V24, so I checked it against testnet. V24 is not active there yet (the tip's CbTx is still version 3), so the chain has no ExtAddr ProTxs the new rules could reject. rc.3's own serializer always writes canonical, sorted purposes, so every ProTx it builds passes the new rules. Mixed rc.3/rc.4 networks could only split on a hand-crafted non-canonical ExtAddr ProTx mined after V24 activates. So testnet miners and masternodes should move to rc.4 before V24 is signalled.

  Unlabeled develop fixes since rc.3 that were audited and left out:
  - Worth considering before the final release: #7869 (startup block replay after an unclean shutdown asserts on a quorum commitment, so recovery needs `-reindex-chainstate`; not a v24 regression), #7833 (gate signing intake before quorum retrieval, DoS hardening), #7682 (intermittent `relic_conf.h` parallel-build failure).
  - Left for 24.1: #7812, #7826, #7844, #7847, #7850, #7868, plus the routine upstream backports (#7780, #7806, #7830, #7855, #7856, #7862, #7863, #7865, #7883, #7884, #7898, #7906), refactors (#7805, #7878, #7880), and CI, docs and test-only changes.

  One CI-only commit comes last. It takes only the `ci/test` hunks of 6f71fa5 (#7901), which make `ci/test/00_setup_env_native_{multiprocess,tsan}.sh` honor `DEPENDS_ARCH`. Pull requests into `v24.0.x` run develop's workflow files (`pull_request_target`), and since #7901 those check the depends cache on x86_64 runners and pass the aarch64 build arch in `DEPENDS_ARCH`. Without the hunks, `linux64_multiprocess / Build depends` fails on any depends cache miss (`Build runner computes HOST=aarch64-linux-gnu but check-cache used x86_64-pc-linux-gnu`). Without `DEPENDS_ARCH` the scripts still use `uname -m`, so this branch's own push CI is unchanged.

  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.4` tag.

  ## How Has This Been Tested?

  Built and tested at 6c02b2b on macOS arm64 against depends, with `--enable-debug --enable-werror --disable-hardening`.

  - `make check`: all unit test suites pass, including `test_dash-qt`.
  - Functional tests, all passing: `feature_evo_shares_activation.py`, `feature_governance_objects.py`, `feature_llmq_signing.py` (plain and `--spork21`), `feature_masternode_shares.py`, `feature_sporks.py`, `interface_rest.py`, `p2p_instantsend.py`, `p2p_quorum_data.py`, `rpc_masternode.py`, `rpc_psbt.py`, `rpc_quorum.py`, `wallet_backup.py`, `wallet_dump.py`, `wallet_encryption.py`, `wallet_labels.py` and `wallet_upgradetohd.py`, each with `--descriptors` and `--legacy-wallet` where it supports them.

  - Each pick's changed lines were compared with its develop merge (or, for #7893, its PR commit). They are identical except for the #7893, #7857 and #7909 adaptations above, and the #7888 test adaptation below.
  - A local macOS arm64 debug build (`--enable-debug --enable-werror`) compiled most of the tree and stopped on a single error in `src/test/evo_netinfo_tests.cpp`. #7888's new test used `CNetAddr::V2()`, the serialization-parameter API from bitcoin#25284, which this branch lacks. The pick now writes that entry through an `OverrideStream` with `ADDRV2_FORMAT`, the way `NetInfoEntry::Serialize` does on this branch (see its commit message).
  - #7782's new unit test reloads the wallet with develop's three-argument `CWallet` constructor. On this branch the constructor still takes the CoinJoin loader and an `ArgsManager`, so the test passes them the way its fixture already does (see the commit message).
  - With both test fixes, a full local `make` (`dashd`, `dash-qt`, `test_dash`, `test_dash-qt`) completes with no errors.
  - `feature_pruning.py` is an extended test that CI does not run. #7894 is included because #7792 makes its restart path fail.

  ## Breaking Changes

  Carried over from the picked PRs:

  - #7876: rc.1 to rc.3 nodes that connected an affected block stored a wrong masternode list diff for it. Those nodes need `-reindex`; `-reindex-chainstate` is not enough for nodes that already marked the following block invalid.
  - #7888: stricter ExtAddr `netInfo` decoding and validation behind V24 (see above). The RFC1918 check now applies even with `-allowprivatenet`.
  - #7858: provider transactions no longer get InstantSend locks. Software that waits for an islock on a ProTx should wait for a ChainLock or confirmations instead.
  - #7857: a `ProUpServTx` spending an output of a pending operator key change, asset lock, asset unlock or MNHF signal is rejected from the mempool until that transaction confirms.
  - #7831: governance object signatures over 96 bytes are rejected, and the governance cache is discarded and re-synced on first start (`doc/release-notes-7831.md`).
  - #7871: address purposes are limited to `send`, `receive` and `refund`. Wallets with other purpose strings load with warnings, and `listlabels` errors on an unknown purpose (`doc/release-notes-27217.md`).
  - #7842: creating an HD wallet with a mnemonic passphrase over 248 bytes fails unless `-allow-deprecated-long-mnemonics` is set, and over 256 bytes always fails (`doc/release-notes-7842.md`).
  - #7792: a node whose evodb is missing a masternode list snapshot now stops at startup and asks for `-reindex`, and `evodb repair` reports it in `repairErrors`.
  - #7867: `protx info <proTxHash> <blockHash>`, `protx listdiff` and `masternode payments <blockHash>` return an error for blocks outside the active chain.
  - #7881: `protx shared_combine` on a `ProUpSharedRegTx` is refused unless its inputs are unlocked coins of a single address of the calling wallet, change returns to that address, and the fee is within `-maxtxfee`.
  - #7836: PSBTs referencing nonexistent previous outputs fail to deserialize (RPC error -22).
  - #7834: negative Electrum-import rescan indices are rejected.
  - #7893: on this branch, mainnet nodes no longer send `GETSPORKS` during mnsync (no effect, mainnet spork values are hardcoded).

  ## 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: 7de9535ca51620f4ddf5a46df2c3341a5e5fdd2263606f83f7db6b576c31ff7b82e1b285bf66988ac4b2729a9060513c45710135f61d7fd5c147b670d5f405dc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pastaclaw:approved thepastaclaw's latest review approved this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants