Skip to content

backport: v24.0.x backports for v24.0.1 - #8128

Open
PastaPastaPasta wants to merge 11 commits into
dashpay:v24.0.xfrom
PastaPastaPasta:backport/v24.0.1
Open

backport: v24.0.x backports for v24.0.1#8128
PastaPastaPasta wants to merge 11 commits into
dashpay:v24.0.xfrom
PastaPastaPasta:backport/v24.0.1

Conversation

@PastaPastaPasta

@PastaPastaPasta PastaPastaPasta commented Nov 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Issue being fixed or feature implemented

Backports for v24.0.1, on top of v24.0.x at 938fba5 (v24.0.0). v24.0.0 is running on mainnet ahead of the V24 hard fork, and a handful of fixes that landed on develop since the release should not wait for 24.1: a test_dash-qt failure that blocks distribution packagers (#8075), a broken dependency download, a peer-triggerable busy loop, a way for a quorum commitment to be lost for a whole DKG cycle, and a node abort after a reorg.

It also carries the two fixes for the stalled v24 activation: #8129, which lets masternodes re-sign a pending EHF signal so a first attempt that fell short of the threshold can still complete, and #8130, which stops DKG members from reloading every verification vector at commit time, the reason slow members have been missing the commit phase of llmq_400_85 rounds.

What was done?

Picked

Cherry-picked with git cherry-pick -m1 -x (merged PRs) or git cherry-pick -x (PR commits not yet merged on develop). Each pick changes the same lines as its develop counterpart except where noted.

Merged on develop:

Depends on these develop PRs merging first. If one changes during review, its pick here will be redone to match:

Adaptations

Not picked

How Has This Been Tested?

On this branch at 32a1553, macOS arm64, depends build with --enable-debug --enable-werror, no GUI:

The GUI picks (#8052, #8080) were not built or run locally; CI covers them. The first push of this branch did not compile (m_special_tx, see Adaptations); review caught it and it was fixed before any of the above was run.

Breaking Changes

None.

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 (if you have permission to set one)

🤖 Generated with Claude Code

knst and others added 4 commits November 11, 2026 17:55
@knst @PastaPastaPasta
Merge dashpay#8088: fix: fetch miniupnpc sources from miniupnp.free.fr
65603d5 
cba106e fix(build): fetch miniupnpc sources from miniupnp.free.fr (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented

      curl: (28) Failed to connect to miniupnp.tuxfamily.org port 443 after 30002 ms: Timeout was reached
      Fetching miniupnpc-2.2.7.tar.gz from http://dash-depends-sources.s3-website-us-west-2.amazonaws.com/
        % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                       Dload  Upload   Total   Spent    Left  Speed

        0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
        0   570    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
      curl: (22) The requested URL returned error: 404
      make[1]: *** [funcs.mk:301: /home/runner/work/dash/dash/depends/sources/download-stamps/.stamp_fetched-miniupnpc-2.2.7-b0c3a27056840fd0ec9328a5a9bac3dc5e0ec6d2e8733349cf577b0aa1e70ac1.hash] Error 22
      make[1]: Leaving directory '/home/runner/work/dash/dash/depends'

  So apparently build with depends without cache can't succeed anymore.

  ## What was done?
  Point the package at the author's other mirror, http://miniupnp.free.fr, which serves the tarball with the expected hash. This is what Bitcoin Core did in bitcoin#30151 for the same outage but it had been reverted later by bitcoin core by mistake and never refixed, instead bitcoin core dropped miniupnpc altogether (bitcoin#31130), so there is no later upstream fix to backport.

  ## How Has This Been Tested?
  Can be tested by CI on dashpay#7939 when rebased on top of merged this PR

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(if you have permission to set one)_

Top commit has no ACKs.

Tree-SHA512: c53eb4eaed3bdeb7d024bba61acf21e79054dbb2356d408397b1c7e4232e0bee3f511201d26f1e87b9b688e996501578253377d355d64f178d1de5ef1bf53426
(cherry picked from commit 3d85a15)
@PastaPastaPasta
Merge dashpay#8052: fix(qt): reserve the same shared masternode contr…
033dae3 
…ibution again after a release

01cda37 fix(qt): reserve the same shared masternode contribution again after a release (pasta)

Pull request description:

  ## Issue being fixed or feature implemented

  In the Qt shared masternode wizard, if the coordinator releases and re-reserves its coins after a participant has replied, that participant can no longer accept the locked terms. The coordinator gets no warning. GUI QA found this on the `v24.0.0-rc.7` tag (`111a2c533e19`), on regtest:

  1. alice coordinates. She reserves her coins on "Your contribution" and saves the invitation, which carries her contribution, change address included.
  2. bob opens the invitation and sends his Details.
  3. alice goes back to "Your contribution", presses **Release**, pastes bob's Details and presses **Next**. Next reserves her coins again.
  4. alice locks the terms and sends the locked terms to bob.

  **Result:** bob gets "The locked terms do not contain the details you sent."
  - alice's board shows every reply in, with no "must reply again" warning.
  - In the same window, bob can't load alice's current invitation either: the copy looks like nothing new to him.
  - His only way forward is to open the locked terms in a fresh dialog.

  No funds move and nothing is signed or broadcast, but the participant is stuck with an error that doesn't say what to do.

  **Why it happens:** the re-reserve in step 3 selects coins afresh, so alice's contribution gets a **new change address**. bob's copy still lists the contribution from the invitation, and `MnShareSession::adoptLockedTerms` compares every contribution it knows.

  **Why rc.6 completes the same steps:** absorbing bob's reply used to put alice's released contribution back and relock it. That was a separate bug, fixed by dashpay#8035, which keeps released contributions out of absorbed replies. With that fixed, the new contribution from the re-reserve is what gets locked.

  ## What was done?

  `SharedMnCreateDialog::reserveCoins()` now puts back the most recently released contribution for this share, unchanged, when all of these hold:
  - Reserve built it in this dialog from this wallet's own coin selection. That's tracked in `m_built_contributions`, compared with `MnShareSession::SameContribution`: inputs in order with their sequences, and the same change;
  - every input is still one Reserve could select, from the same `fundingCandidates()` list `selectCoins()` now uses: unspent, unlocked and spendable, with no unconfirmed coins the wallet doesn't trust;
  - the inputs minus the change still equal what the share needs: the share amount, plus the Network fee for the coordinator. A contribution with no change may also exceed it by a dust amount, which `selectCoins()` leaves to the fee in the same way (the shared `isDust()`).

  Otherwise it selects coins as before. In that case the contribution really did change, for example the fee or the share amount changed, or a coin was spent. Earlier replies are then out of date, as they are today.

  The "built here" rule matters because of what a reply can contain. A reply can list a contribution under this wallet's share name that spends another of this wallet's coins, with any change, sequences or repeated inputs the sender chose. The wallet already refuses to sign such a contribution, but a release would put it into the released list. A contribution absorbed from a reply is therefore never put back. Reserve builds the wallet's own contribution instead.

  `fundingCandidates()` is the coin filter `selectCoins()` already used, moved into its own function so that reuse and fresh selection accept the same coins. `reservableAgain()` makes the reuse decision once for both Reserve and the coins card on "Your contribution". While the coins are released, the card says it uses the coins "as reserved before" and that the change "returns to the same address of yours", instead of describing a fresh selection.

  **Known limits, left as is:** reuse doesn't survive reopening a saved session, because neither the released nor the built list is saved. Also, two changes can cancel out. For example, the coordinator releases, raises the fee and reserves, then releases again and restores the fee. The original contribution was replaced in between, so the second re-reserve can't bring it back and builds a new one. This is rare, and it's the general "the coordinator changed their own funding after replies" case, which needs a reply-again signal on the board rather than reuse.

  ## How Has This Been Tested?

  **New scenario:** `SharedMnWizardTests::reservingAgainKeepsEarlierReplies`, with a confirmed coin in a real wallet.
  - **Main case:** reserve, then take the invitation, then bob's and carol's replies to it. Release, absorb bob's reply and reserve again.
    - The contribution must match the one bob holds.
    - After a real `PrepareRegistration` + `freeze`, bob's `adoptLockedTerms` on the locked terms must succeed.
  - **A reply plants a contribution under alice's name:** the variants are change paid elsewhere, one coin listed twice, 100 duffs of change to this wallet, and an exactly funded input at sequence 1. After Release and Reserve, the wallet must go back to its own contribution each time, and the planted coin must stay unlocked.
  - **No change, dust remainder:** Reserve itself builds a no-change contribution from a coin 100 duffs over the share plus fee, with the other coins locked. After a release it is reserved again unchanged.
  - **Coins card:** a larger coin arrives after the first reservation. After Release, the card must describe the contribution Reserve will put back, not the larger coin.
  - **Fee changed:** after a release and a fee change, reserving again must select afresh.

  **Fails without the fix:**
  - With the reuse removed, the scenario fails at the check that the re-reserved contribution is the one sent (`sharedmnwizardtests.cpp:471`).
  - With reuse allowed for any released contribution, it fails at the first planted case (`:547`). With the card still showing a fresh selection, it fails at `:504` with "Uses 1 coin (600 tDASH); change 199.999 tDASH returns to a new address of yours".

  **Full `test_dash-qt` with the fix**, run headless (`QT_QPA_PLATFORM=minimal`) on Linux: 164 passed, 0 failed, 2 skipped. The skips are the OptionTests font tests, which can't run on the headless `minimal` platform. SharedMnWizardTests: 22 passed.

  **GUI on regtest**, run as a 2-party session with the steps above. Each binary was built from its exact commit:
  - **PR base** `9e5e3d59a1e3` and the **`v24.0.0-rc.7`** tag `111a2c533e19`: the re-reserve picks a new change address, and bob gets "The locked terms do not contain the details you sent."
  - **PR head at the time,** `eb4929384856`: the change address stays the same, bob gets Approve terms, and the registration confirms. Its change pays the address bob's Details listed.
  - **Head `496130ce0637`** (since superseded): the same flow was rerun with the same result: unchanged change address, Approve terms, ProRegTx `2db9621d…7834` confirmed. See [that evidence comment](dashpay#8052 (comment)).
  - **Head `55672100f0b0`:** it adds the coins-card change above. The same flow was rerun on it: unchanged change address, Approve terms, ProRegTx `59ba8be9…5c95` confirmed. After Release, the card reads "as reserved before … the same address of yours", where the base reads "a new address of yours". See [the current-head evidence comment](dashpay#8052 (comment)).
  - **Current head `01cda37af25`:** after `55672100` it adds only the reuse restriction above, through the untested intermediate `2a1bcc63`. The same flow was rerun on binaries built from exact `01cda37a`: unchanged change address, the "as reserved before … same address" coins card, Approve terms, and the ProRegTx `dcea870f…105d` confirmed. See [the current-head evidence comment](dashpay#8052 (comment)).

  Screenshots and provenance are in [the UI evidence comment](dashpay#8052 (comment)) and at [dash-ui-artifacts@`034c3d7` / dash/pr-8052](https://github.com/PastaPastaPasta/dash-ui-artifacts/blob/034c3d7dc83b896aca0182126aa748a72b4b1be9/dash/pr-8052/README.md).

  On the earlier pre-review head `46de5fe1dea`, the same flow also passed. So did a participant releasing and reserving again on the contribution page. A fee change after a reply gives a new contribution, as expected.

  ## Breaking Changes

  None.

  ## 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 _(if you have permission to set one)_

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

Top commit has no ACKs.

Tree-SHA512: 0bb7afb3948fd947a73dd5b16f7e1b75ba3e10e650383609509d1220309b6baddb2244f110623c86badf51d7c8adf84dfca36d06482d92988ec7616903661825
(cherry picked from commit b2172a9)
@PastaPastaPasta
Merge dashpay#8094: fix(net): stop busy-looping while a peer's send b…
ec9e944 
…uffer is full

19da39c fix(net): wait for socket writability after failed sends (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  When a nonblocking `send()` to a peer fails with a would-block error (the peer has stopped reading and its TCP window is full), `CConnman::SocketSendData` leaves `fCanSendData` set. The peer therefore stays in the sendable set, the socket handler keeps polling with a zero timeout, and the net thread busy-loops until the peer starts reading again or times out.

  Before the transport refactor (bitcoin#28165, merged here in 63962ec) a failed send cleared the flag; that reset was lost in the merge.

  ## What was done?
  Clear `fCanSendData` whenever the send returns an error, so the next iteration waits for the socket to become writable again. The empty-send path and partial successful sends are unchanged.

  ## How Has This Been Tested?
  Reproduced with a functional script that connects a peer which never reads and streams pings at the node: before the change the node's net thread used ~100% of a CPU core (9.86 s of CPU vs 9.88 s for a reference busy loop); with the change it used 0.02-0.03 s over three runs. That script measures CPU time, so it is not added as a CI test. `test_dash --run_test=net_tests,sock_tests`, `p2p_ping.py` and `p2p_v2_transport.py` pass.

  ## Breaking Changes
  None.

  ## 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 _(if you have permission to set one)_

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

Top commit has no ACKs.

Tree-SHA512: 0af7f3b48164fae294d81f828cf1d5c25f6cb86a41257dd4c981116a30a8fc5c976be0345f389f03033ec390ecc1c42853bf161350be9f8f14e1f011f63d923d
(cherry picked from commit 34f990e)
@PastaPastaPasta
Merge dashpay#8080: test(qt): stop address book tests from answering …
b051d3a 
…the shared masternode wizard's prompt

a5618a0 test(qt): don't leave address book message answers armed after the edit (pasta)

Pull request description:

  ## Issue being fixed or feature implemented

  Fixes dashpay#8075. With a distribution Qt, `test_dash-qt` fails in `SharedMnWizardTests::signedSessionReservation()`. It either hangs until QtTest's 300 s watchdog fires, or aborts with `free(): invalid size`. Builds against depends' Qt pass.

  The shared masternode wizard is not at fault. `AddressBookTests` leaves two answers to a message box behind. `EditAddressAndSubmit()` calls `ConfirmMessage(&warning_text, 5ms)` before every edit, but a successful edit opens no box. Its timer then outlives the function, holding a pointer to the function's stack `QString`. The two successful edits run last, so both timers are still pending when the suite ends. No later suite runs an event loop until the wizard's "Keep this session for later?" prompt (added in dashpay#8035, which is why the bisect lands there).

  Whether that prompt survives depends on Qt's event dispatcher:
  - **Unix dispatcher** (depends builds Qt with `-no-glib`): the test's own "Cancel" ends the modal loop before timers are checked. The stale timers fire later, when no box is left, and do nothing.
  - **GLib dispatcher** (what Fedora's and other distribution Qt packages use): the timers fire in the same iteration as the test's answer. They write the box's text through the dangling pointer and click its default button, "Save and Close", over the test's "Cancel". The dialog then opens a save-file dialog that nothing answers, which is the hang. When the stray write hits a live frame instead, the result is `free(): invalid size`.

  The hardening flags in the report are incidental: the same Fedora build passes with `QT_NO_GLIB=1`. Bitcoin Core has the same `EditAddressAndSubmit()`, but no modal test runs after it there. Since dashpay#8077, `AddressBookTests` also runs on macOS, so the stale timers exist there too.

  ## What was done?

  `ConfirmMessage()` takes an optional context object, and `EditAddressAndSubmit()` scopes it to the edit, so a box that never opens leaves nothing armed. The existing address book scenario now also opens a message box after the successful edits and checks that its own answer wins.

  No production code changes. The dialog did what the stale click told it to do.

  ## How Has This Been Tested?

  Reproduced in a Fedora 44 container (GCC 16.2.1, system Qt 5.15.18) with the exact `CFLAGS`/`CXXFLAGS`/`LDFLAGS`/`configure` line from the issue. Results are for the full `test_dash-qt`. The "develop" column is develop at 4c5c015, and the "This PR" column is this PR on 8dbdcb4.

  | Build | Dispatcher | develop | This PR |
  |---|---|---|---|
  | Fedora RPM flags | GLib (default) | hangs in `signedSessionReservation` | 163 passed, 0 failed |
  | Fedora RPM flags | Unix (`QT_NO_GLIB=1`) | 163 passed | 163 passed, 0 failed |
  | ASan+UBSan, `detect_stack_use_after_return=1` | GLib | **stack-use-after-return** in the `ConfirmMessage` lambda (`qt/test/util.cpp:22`), inside the wizard prompt's `exec()` | 163 passed, no ASan report |
  | ASan+UBSan | Unix | passes | 163 passed, no ASan report |
  | Default depends build, `--enable-werror` | Unix | passes | 163 passed, 0 failed |
  | Fedora RPM flags under valgrind memcheck, `QT_ENABLE_REGEXP_JIT=0` | GLib | n/a (hangs) | 163 passed, 0 errors |

  - A gdb breakpoint on `QMessageBox::buttonClicked` during the hang showed the test's "Cancel" click, then two clicks from `ConfirmMessage`'s lambda in the same `exec()`.
  - With `scope` replaced by `nullptr`, the new address book check fails with either dispatcher: "Compared pointers are not the same", or a crash in the stale callback at `util.cpp:22`.
  - UBSan reports the existing misaligned store in the vendored X11 code (`crypto/x11/sph_types.h:613`), with and without this PR. On this PR's base it also reports one invalid `bool` load at `qt/transactionrecord.h:70`, in code this PR does not touch: `decomposeTransaction()` copies a record into its list before setting its uninitialised `involvesWatchAddress`. With Qt's regexp JIT left on, valgrind's only reports are uninitialised-value warnings in JIT-generated code. With the JIT off there are none.
  - clang-format-diff (18.1.8) and `lint-whitespace.py` are clean.

  ## Breaking Changes

  None. Test code only.

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [ ] I have made corresponding changes to the documentation
  - [x] 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: d68bacc7990c68c1966cf0d020795d7638b29b9f49d5bc42620189fffa3afb36585bcbab9653f95bccfbda1f9dcf8d0fd05ea7e2cd3c3955f762230c186e4b11
(cherry picked from commit 6cedd34)
@PastaPastaPasta PastaPastaPasta added this to the 24 milestone Nov 12, 2026
@thepastaclaw

thepastaclaw commented Nov 12, 2026 •
edited
Loading

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 32a1553. Normal review starts when eligible; priority review starts as soon as a slot is available.

thepastaclaw
thepastaclaw suggested changes Nov 12, 2026
View reviewed changes

@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 the finding against exact head abe4b37: the commitment-cleanup backport introduces two references to an undeclared processor member, preventing validation.cpp from compiling. Current CI shows lint and Cppcheck passing, with source builds and several dependency builds still pending; this verification was static and no local builds or tests were run.

🔴 1 blocking

Review provenance

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

  • Triage: critical by gpt-6.1-sol (effort low) — This is a large, cross-cutting backport whose diff changes critical peer-facing validation and consensus-sensitive paths, including InstantSend lock deserialization/validation in src/instantsend/net_instantsend.cpp and quorum commitment processing in src/llmq/blockprocessor.cpp.
  • 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 quota lookup failed), glm-5.3-flash (not used above high effort; tier asks max)
  • Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
  • Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
  • Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — backport-reviewer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort xhigh); agent phase2-reviewer
🤖 Prompt for all review comments with AI agents 
These findings are from an automated code review (PastaClaw). How to work through them:
- Verify each finding against the code; fix only what is real, as narrowly as the finding.
- If a finding is wrong, decline it on its thread in one or two sentences; the bot answers once and stops.
- To defer one, link a tracking issue on its thread (or reply asking the bot to file one).
- Suggestions don't block the merge, but most are worth fixing in this PR; judge each on its merits.
- Keep the PR's scope; don't grow it to satisfy hardening suggestions.
- Carried items were raised in an earlier round: read the thread first, and don't re-fix what you already fixed, declined or deferred there. Disputed and deferred items are already handled and are not listed.

In `src/validation.cpp`:
- [BLOCKING] src/validation.cpp:2928: Adapt both commitment-cleanup calls to the v24 Chainstate API
  Both this new call in ConnectBlock and the new call in RollforwardBlock at line 5222 reference `m_special_tx`, which is not declared on this branch. Chainstate instead accesses the processor through `m_chain_helper->special_tx`, as the existing ProcessSpecialTxsInBlock calls at lines 2674 and 5213 demonstrate. These two undeclared-identifier references prevent validation.cpp from compiling. Change both cleanup calls to `m_chain_helper->special_tx->RemoveMineableCommitmentsForBlock(block, pindex);` so the backport uses the release branch's processor API.

Comment thread src/validation.cpp Resolved
@thepastaclaw thepastaclaw added the pastaclaw:changes-requested thepastaclaw's latest review requested changes label Nov 12, 2026
@thepastaclaw thepastaclaw removed the pastaclaw:changes-requested thepastaclaw's latest review requested changes label Nov 12, 2026
thepastaclaw
thepastaclaw approved these changes Nov 12, 2026
View reviewed changes

@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

Static review of b2b83b8 found no actionable in-scope defects: all ten picks preserve their source transformations, with only the documented release-branch adaptations. Current CI has passed all seven dependency builds and the aarch64 and fuzz source builds; other source builds remain pending, while the ARM64 slim-container failure is unrelated infrastructure trouble—the job log reports curl exit 7 connecting to apt.llvm.org in the unchanged Dockerfile. Approval applies only to this head and does not waive the four declared source-PR merge gates.

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

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: backport-reviewer); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); reviewer 4: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: backport-reviewer); reviewer 7: 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: critical by gpt-6.1-sol (effort low) — The intricate contribution re-reservation changes in src/qt/sharedmncreatedialog.cpp directly alter coin selection and reuse of funding inputs and change outputs, meeting the critical-surface bar beyond the PR's overall size.
  • 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 quota lookup failed), glm-5.3-flash (not used above high effort; tier asks max)
  • Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
  • 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 xhigh); agent phase2-reviewer, gpt-6.1-sol — backport-reviewer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — backport-reviewer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort xhigh); 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.
Out-of-scope follow-up suggestions (1)

These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.

  • Handle incomplete Platform entries in provider JSON rendering — The unchanged GetPlatformPort() implementation in src/evo/core_write.cpp assumes nonempty extended Evo network data includes both required Platform entries. Deserialization does not enforce that completeness, so rendering malformed provider payloads with deprecated service fields enabled can throw; the REST block handler and HTTP worker do not contain that exception, allowing node termination. This is separate from the unsupported-version/null-netInfo case fixed here.
    • Follow-up: Open a separate provider-rendering fix that tolerates incomplete extended network data, with RPC and REST regression coverage.

@thepastaclaw thepastaclaw added pastaclaw:approved thepastaclaw's latest review approved this PR and removed pastaclaw:approved thepastaclaw's latest review approved this PR labels Nov 12, 2026
PastaPastaPasta and others added 2 commits November 11, 2026 22:14
@PastaPastaPasta
Merge dashpay#8104: fix: keep final commitments mineable when their b…
d00c4e6 
…lock is rejected

3936953 fix: keep final commitments mineable when their block is rejected (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  `CQuorumBlockProcessor::ProcessCommitment()` removed a block's final commitments from the mineable pool while the block was still being validated. `ConnectBlock()` processes quorum commitments early, before inputs, scripts, the coinbase amount/payee checks and several special-transaction checks. A block carrying the commitment of a finished DKG that then failed one of those later checks was never connected, yet every node that validated it (compact blocks are relayed before validation) had dropped the commitment from its pool. Nothing re-adds it, so that quorum was not mined in that DKG cycle.

  ## What was done?
  - `ProcessCommitment()` no longer touches the mineable pool, apart from dropping its caches.
  - New `CQuorumBlockProcessor::RemoveMineableCommitmentsForBlock()` removes the same entries as before, called once `ConnectBlock()` has accepted the block (next to where the BLS scheme switch is committed) and in `RollforwardBlock()`.
  - Check-only validation (`fJustCheck` / `TestBlockValidity`) returns before that point and never touches the pool. `UndoBlock()` still re-adds the commitments of a disconnected block, so reorgs behave as before.

  Re-adding on the failure path instead was rejected: a block can fail partway through its commitments, which could re-pool a commitment that was never verified. This doesn't overlap with dashpay#8048, which changes `AddMineableCommitment` only.

  ## How Has This Been Tested?
  `feature_llmq_dkgerrors.py`: after the existing `invalidateblock` check, the test submits the same commitment-carrying block with the coinbase paying 1 duff too much, expects `bad-cb-amount`, and asserts the commitment is still listed as mineable in `quorum dkgstatus`. It fails on `develop` and passed three times with this change. A five-node reproduction also fails on `develop`; with the fix every node keeps the commitment and the quorum is mined within its window. `llmq_*`, `evo_*` and `validation_chainstatemanager_tests` unit tests pass, as do `feature_llmq_rotation.py`, `feature_llmq_is_cl_conflicts.py`, `feature_llmq_evo.py`, `feature_llmq_signing.py`, `feature_llmq_simplepose.py`, `feature_llmq_singlenode.py`, `feature_llmq_dkg_intake.py`, `feature_dip4_coinbasemerkleroots.py`, `feature_quorum_proof_chain.py`, `rpc_quorum.py`, `feature_new_quorum_type_activation.py` and `feature_abortnode.py`.

  ## Breaking Changes
  None.

  ## 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 _(if you have permission to set one)_

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

Top commit has no ACKs.

Tree-SHA512: a306a6fbf6613ca25dfce8665815c94833ebeb012642f492db882ea7154ac1a987bfe5c0afbebcc7d09762fcf8373e664ea3bb2a82e0c5921d87723426e2f4b6
v24.0.x adaptations: Chainstate has no m_special_tx member here, so
ConnectBlock() and RollforwardBlock() call the processor through
m_chain_helper->special_tx like the neighbouring special-tx calls.
CQuorumBlockProcessor keeps the v24.0.x GetCommitmentsFromBlock(), and
RemoveMineableCommitmentsForBlock()'s lock annotation drops
!m_commitment_payload_mutex, which only exists with dashpay#7936.

(cherry picked from commit ce9ef66)
@PastaPastaPasta @codex
fix: match governance voting keys by registration hash across reorgs
117a6cb 
Resolve updated masternodes in the destination list and match their previous entries by registration hash. Skip missing entries when removing invalid votes.

Co-Authored-By: GPT 6.1 Sol <noreply@openai.com>
(cherry picked from commit 9071a08)
This was referenced Nov 12, 2026
Draft
Draft
PastaPastaPasta and others added 5 commits November 11, 2026 22:50
@PastaPastaPasta @codex
fix: skip undecoded provider payloads in transaction JSON
1c128c3 
Unsupported provider transaction versions can deserialize without network information. Omit their optional decoded payload objects instead of letting JSON rendering throw through the REST block handler, and retain the raw payload bytes.

Co-Authored-By: GPT 6.1 Sol <noreply@openai.com>
(cherry picked from commit 9560439)
@PastaPastaPasta
fix(llmq): retry pending EHF signals on new tips
0a26d57 
Allow members to re-sign pending EHF signals after incomplete signing sessions expire. Exercise recovery with the same quorum after isolation and timeout in both connection modes.

(cherry picked from commit b619a4f)
@PastaPastaPasta
fix(llmq): keep verified contributions cached through DKG sessions
692935e 
Share verified vectors with their sessions and retain them until their owners release them. Update cached secret contributions on every verified write, while preserving the database format and cold-read fallback. Guard legacy decoding and scheme-sensitive cached hashes so shared vectors match cold database reads.

(cherry picked from commit 96b68d0)
@PastaPastaPasta
test(llmq): cover verified contribution cache writes and lifetime
d3e78cc 
(cherry picked from commit 04e93df)
@PastaPastaPasta @claude
fix(llmq): process our own DKG messages ahead of queued peer messages
32a1553 
A node's own contribution, complaint, justification and premature
commitment went to the back of the pending queue, so they were only
verified and relayed after every peer message already waiting. Verifying a
premature commitment of a 400-member quorum takes a noticeable fraction of
a second, so on a busy or slow node a backlog could hold its own commitment
back for much of a short commit phase.

Put messages this node produced at the front of the queue.

(cherry picked from commit 68de86b)

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@UdjinM6 UdjinM6 Awaiting requested review from UdjinM6 UdjinM6 is a code owner
@knst knst Awaiting requested review from knst knst is a code owner
1 more reviewer
@thepastaclaw thepastaclaw thepastaclaw approved these changes
Reviewers whose approvals may not affect merge requirements

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

24

Development

Successfully merging this pull request may close these issues.

3 participants

@PastaPastaPasta @thepastaclaw @knst