Repository navigation
backport: v23.1.10 fixes for v24.0.x - #8016
Merged
PastaPastaPasta merged 26 commits intoNov 4, 2026
Merged
Conversation
…eNonLockedTx The backport of dashpay#7858 made RemoveNonLockedTx and HandleFullyConfirmedBlock take cs_timingsTxSeen and annotated them with !cs_timingsTxSeen, but on v23.1.x they are also called from other CInstantSendManager methods that were left unannotated. Clang's thread-safety analysis rejects those calls, so a clang build with --enable-werror (the default in CI) fails in instantsend.cpp. develop is not affected: its remaining same-class callers already carry the annotation. Add !cs_timingsTxSeen to the direct callers (ProcessInstantSendLock, RemoveConflictedTx, NotifyChainLock, UpdatedBlockTip) and to the callers of RemoveConflictedTx (RemoveMempoolConflictsForLock, ResolveBlockConflicts). Annotations only; no behavior change. Verified with a full clang build using --enable-werror. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ents A quorum member can copy another member's whole contribution, its verification vector and encrypted shares alike, and re-sign it with its own operator key. BLS-IES decryption is bound to the recipient, not the dealer, so every recipient decrypts a share that is consistent with the copied vvec: the copy passes the individual contribution check and nobody complains. SendCommitment then fails to build the quorum verification vector, because VerifyVerificationVectors rejects coefficients shared between members (and AggregateSecretKeys rejects the identical shares), so no honest member sends a premature commitment and a single member kills the DKG round. The copy and the original cannot be told apart, so VerifyAndCommit now marks every remaining member whose vvec shares a coefficient with another one as bad, after members with open complaints are excluded. Members that agree on the remaining members and hold the same contributions drop the same ones, so the quorum vvec can be built and they still agree on the commitment. A new CBLSWorker::FindVerificationVectorsWithSharedCoefficients() helper finds them with the same hash comparison VerifyVerificationVectors uses. PoSe trade-off: both members are invalid in the final commitment, so each gets the usual CalcPenalty(66) PoSe penalty for failed DKG participation. An attacker can therefore make a chosen honest co-member take that penalty (a ban if repeated in two DKGs close together), paying the same penalty itself and only when both are selected into the same quorum. Before this change the same attack prevented the commitment altogether. Possible follow-up: the cross-member duplicate checks look like input hygiene from the original DKG worker (3ee27c1) rather than a security requirement. Joint-Feldman DKG stays secure while one honest dealer's polynomial is secret, and a copied contribution gives the copier no control over the quorum key, so accepting duplicates would neutralize the attack without punishing anyone. That needs a cryptographic review and relaxing the shared duplicate checks in quorum vvec building and secret key aggregation, and nodes still running older code would keep failing on a duplicated set during a rollout, so it is not done here. Binding each contribution to its dealer would also exclude only the copier, but it changes the DKG protocol. Test: bls_verify_contribution_shares_duplicate_coefficient_tests now covers the full copy (every share verifies, the quorum vvec can't be built, the helper flags exactly that pair, and the vvec builds without them). The VerifyAndCommit marking itself is covered end to end by the feature_llmq_simplepose scenario added in the following commit, which fails without it. feature_llmq_dkgerrors, feature_llmq_rotation, feature_llmq_signing and feature_llmq_simplepose pass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add a regtest-only "contribution-clone" DKG sim error: a member simulating it derives its secret polynomial from the quorum hash alone, so every member simulating it deals the same contribution, as if it had copied another member's whole contribution. feature_llmq_simplepose (5-member llmq_test, minSize 3) now starts with a scenario where two masternodes deal cloned contributions. The quorum is mined with exactly the other three as valid members, and both cloners are PoSe-punished but not banned. The scenario then lets the penalty decay with DKG disabled, so the existing scenarios start from a clean state. Without the VerifyAndCommit marking from the previous commit, no member can build the quorum verification vector, no premature commitment is sent and the scenario fails waiting for the commit phase. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
During the DKG every member verifies the contributions it received with CBLSWorker::VerifyContributionShares. Its aggregated mode, which the DKG uses, summed the verification vectors and the secret-key shares of each batch of CONTRIBUTION_VERIFY_BATCH_SIZE dealers without weights and marked the whole batch valid when the sums matched. Two colluding dealers in the same batch could therefore hand a recipient wrong shares whose errors cancel (for example by swapping them): the recipient doesn't complain and stores wrong verified shares, and if one of the dealers is later excluded its quorum secret key share is wrong. Each batch is now verified by one worker task that multiplies every dealer's vvec coefficients and the public key of its share by the same fresh secret random 64-bit weight before aggregating, and checks that the public key share recovered from the aggregated vvec matches the aggregated public keys. A batch that fails falls back to verifying its contributions one by one, as before; a batch whose aggregation failed outright used to be marked invalid as a whole instead. The old unweighted aggregation flow for this path is removed, and MakeBatchVerificationWeight takes the weight size. 64-bit weights: a batch with a wrong contribution passes with probability about 2^-64. Colluding dealers lose nothing on a failed attempt (they can answer the complaint with a valid justification), so an attacker gets many free attempts, but the weights never leave the verifying node, so it can't test them offline, and 2^-64 per attempt stays negligible. 32-bit weights would have been about 1.9x faster but leave a small chance of success over the lifetime of the network. The weighting is only sound for points in the prime-order subgroup. Legacy-scheme deserialization skips that check, so CDKGContribution now decodes its whole vvec under a single scheme and records it in a non-serialized vvecLegacyScheme field (the wire format is unchanged). The session keeps it per member, and VerifyPendingContributions only uses the weighted batches when none of the pending vvecs was decoded under the legacy scheme, verifying them one by one otherwise. A point decoded under the basic scheme has always passed the subgroup check, whatever the chain did since, so this also covers a reorg across the v19 activation. Release build (-O2), one member verifying every dealer's contribution, all valid, 4 BLS worker threads: quorum unweighted (old) individual weighted (new) llmq_100_67 49 ms 292 ms 189 ms llmq_400_60 0.58 s 4.15 s 2.45 s llmq_400_85 0.82 s 5.92 s 3.46 s Also fix bench/bls_dkg.cpp, which passed its aggregated flag in the parallel position, so its "aggregated" and "simple" variants compared parallel against serial aggregated verification. Tests: bls_verify_contribution_shares_honest_tests now covers two dealers in one batch swapping shares (both blamed) and a missing share (only it blamed), with and without parallel verification, through the default arguments the DKG uses; the swap cases fail with all weights set to 1. llmq_dkg_tests/contribution_records_vvec_scheme round-trips a contribution under each scheme and checks the recorded scheme and the decoded point. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nnot cancel CBLSBatchVerifier and the DKG message batch (BatchVerifyMessageSigs) added signatures and public keys without weights and accepted the whole batch when the aggregate verified. Two signatures that are each wrong but whose errors cancel (S1 + T and S2 - T, for distinct messages or for the same message and key) therefore passed as one valid aggregate: Verify() returned before any per-source check, nobody landed in badSources, and the forged entries were stored. For LLMQ sig shares that lets a peer that has seen an honest share occupy the member's slot with a wrong one, so the honest copies are dropped as already known and recovery fails; recovered sigs, ISLOCKs and DKG messages go through the same aggregation. Multiply each signature and its public key by a secret random nonzero 128-bit weight before aggregating, which makes a cancelling combination negligibly unlikely. The per-source and per-message fallbacks still verify the original signatures. MulInsecure() is added to CBLSPublicKey and CBLSSignature for the scalar multiplication; the weights are drawn in the header-only verifier, so the consensus library gains no RNG dependency. BatchVerifyMessageSigs now also skips an invalid operator key instead of handing it to the multiplication. The new batch_verifier_tests cases (cancelling errors across two messages of one source, and across two sources signing the same message with the same key, in every verifier mode) and the new sig-share row fail without this change and pass with it. BatchVerifyMessageSigs is file-local and needs a full CDKGSession, so it is covered by feature_llmq_dkgerrors and feature_llmq_signing rather than a unit test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ity window Before the superblock maturity window opens, CreateSuperblockCandidate() returns nothing, yet VoteGovernanceTriggers() still voted NO-FUNDING on every active future trigger. Once the window opened, CreateGovernanceTrigger() picked up the existing trigger with the matching DataHash, and the YES path treated any earlier FUNDING vote of ours as final, so the early NO was never replaced by a YES. A single masternode could exploit this by relaying, just before the window, a trigger matching the payments every node is about to compute: all masternodes voted NO on it, no other trigger was created, and the superblock paid none of the approved proposals. Skip NO-FUNDING votes on triggers whose maturity window has not started yet, since there is no local candidate to compare them against. They still get a NO once the window opens if they don't match. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CreateSuperblockCandidate() returns nothing until the node is synced, but VoteGovernanceTriggers() still voted NO-FUNDING on every active future trigger, including ones loaded from governance.dat. A masternode restarting inside the maturity window, before it had voted YES, would therefore vote NO on the legitimate trigger and, once synced, treat that NO as its final vote for the identical trigger it computes. Skip trigger voting until we are synced, as there is no candidate to compare triggers to before then. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… partial aggregate CBLSPublicKey::AggregateInsecure() marks the result invalid if the underlying aggregation throws, and the next dealer's AggregateInsecure() then asserts on it. Return false instead, so the batch is verified one by one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…er's share A member builds its quorum secret key share by summing the shares the valid dealers sent it, through CBLSWorker::AggregateSecretKeys. That rejected the input outright if two shares were equal, so the member logged "failed to build own secret share" and sent no premature commitment (and the QDATA recovery path punished the honest peer that served the shares). Two colluding dealers, for example two masternodes of one operator in the same quorum, can deal a chosen member equal shares: pick polynomials that agree at its id. Both contributions verify, nobody complains, and the vvecs share no coefficient, so neither the batch vvec checks nor the shared coefficient marking in VerifyAndCommit see anything. With degree t-1 polynomials they can do this for up to t-1 members at once, which is enough to keep any LLMQ type from reaching minSize signers, and nobody is blamed. Duplicate shares are harmless here: the sum is the correct share either way, and it is checked against the quorum vvec when the premature commitment is verified. Aggregate secret key shares with a validity check only. Public key and signature aggregation keep their duplicate check. Test: bls_aggregate_colliding_contribution_shares_tests builds two dealers whose polynomials agree at the receiver's id, checks that every contribution verifies and the quorum vvec builds, and that the receiver's aggregated share matches the public key share of the quorum vvec. It fails without this change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…contribution copies and equal secret key shares dfd9820 fix(llmq): accept equal DKG secret key shares when aggregating a member's share (pasta) 1ec6aaa test: cover DKG members dealing identical contributions (UdjinM6) c8627a6 fix(llmq): drop DKG members whose verification vectors share coefficients (UdjinM6) 3174d5f fix(instantsend): propagate !cs_timingsTxSeen to the callers of RemoveNonLockedTx (UdjinM6) Pull request description: ## Issue being fixed or feature implemented Three things on `v23.1.x`, all DKG-related or blocking CI: 1. **`--enable-werror` clang builds are broken on `v23.1.x`.** The dashpay#7858 backport annotated `RemoveNonLockedTx` / `HandleFullyConfirmedBlock` with `!cs_timingsTxSeen`, but their same-class callers weren't annotated, so clang's thread-safety analysis fails in `instantsend.cpp` (4 errors, reproduced locally on `v23.1.x` = v23.1.9). develop is not affected. 2. **One member can kill a DKG round by copying another member's whole contribution** (vvec and encrypted shares) and re-signing it. BLS-IES decryption isn't bound to the dealer, so every share verifies and nobody complains, but the quorum vvec can't be built (duplicate coefficients), so nobody sends a premature commitment. This is the case the v23.1.9 fix (#7) didn't cover: #7 catches a member that plants a *colliding coefficient* with inconsistent shares, not a full copy. 3. **Two colluding members can keep chosen honest members from committing, silently** (new, found while reviewing 2). They pick polynomials that agree at the target's id, so the target receives two *equal* secret-key shares. Both contributions verify, the vvecs share nothing, but `CBLSWorker::AggregateSecretKeys` rejected duplicate inputs, so the target logs "failed to build own secret share" and sends no premature commitment. Degree t-1 polynomials let them do this to up to t-1 members at once, enough to keep any LLMQ type below `minSize`, and nobody is blamed or punished. Two masternodes of one operator in the same quorum suffice. The QDATA share-recovery path hit the same rejection and also punished the honest peer serving the shares. ## What was done? - UdjinM6's `fix(instantsend): propagate !cs_timingsTxSeen …`: annotations only. - UdjinM6's `fix(llmq): drop DKG members whose verification vectors share coefficients`: `VerifyAndCommit` marks every remaining member whose vvec shares a coefficient with another one as bad, after members with open complaints are excluded, via the new `CBLSWorker::FindVerificationVectorsWithSharedCoefficients()`. - UdjinM6's `test: cover DKG members dealing identical contributions`: regtest-only `contribution-clone` sim error and a `feature_llmq_simplepose` scenario. - `fix(llmq): accept equal DKG secret key shares when aggregating a member's share`: `AggregateSecretKeys` checks validity only, not duplicates. Equal shares are harmless in the sum, which is still checked against the quorum vvec when premature commitments are verified. Public key and signature aggregation keep their duplicate check. Commit messages carry the full reasoning. ## How Has This Been Tested? Combined tip of this PR, #16 and #17 (the merged tree equals the tested tree), clang 21, `--enable-werror --enable-debug`, with and without the GUI: - Builds clean with `--enable-werror`, including `dash-qt`. Reverting `instantsend.h` to `v23.1.x` reproduces the 4 `-Wthread-safety-analysis` errors. - `test_dash` full suite: no errors. - New `bls_tests/bls_aggregate_colliding_contribution_shares_tests`: fails without the `AggregateSecretKeys` change (`skShare.IsValid()`), passes with it. - Functional: `feature_llmq_simplepose.py` (both variants), `feature_llmq_dkgerrors.py`, `feature_llmq_signing.py` (+`--spork21`), `feature_llmq_rotation.py`, `feature_llmq_connections.py`, `feature_llmq_evo.py`, `feature_llmq_is_cl_conflicts.py`, `interface_zmq_dash.py`, and `feature_llmq_data_recovery.py` (3/3 on rerun; one earlier failure under `-j8` load was a member that never sent a contribution) pass. - `feature_llmq_chainlocks.py` fails at `test_bestCLHeightDiff` (line 301), but it fails identically 3/3 on unmodified `v23.1.x`, so it's not from this PR. ## Breaking Changes None. No wire-format or consensus change. Members copying a contribution, and the member they copied, are now excluded from the commitment and get the usual PoSe penalty for failed DKG participation (see the trade-off in the commit message; before this the round failed entirely). ## Forward-port Needed on `develop` and `v24.0.x` for the DKG commits (not public yet). The `instantsend.h` annotation fix is `v23.1.x`-only. ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] 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 _(for repository code-owners and collaborators only)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: c494c40b02adce2684b95e108d77886e92236fa029fe9551db77c71bd0ce090e919a658b02a2d9d9a931803c3860a49424de098be01cb878173eec492a6cc9c6
…oS from premature NO-FUNDING trigger votes bd948aa fix(governance): don't vote on triggers while not synced (pasta) 5d9fe74 fix(governance): don't vote NO-FUNDING on triggers before their maturity window (pasta) Pull request description: ## Issue being fixed or feature implemented Before the window opens, `CreateSuperblockCandidate()` returns nothing, but `VoteGovernanceTriggers()` still votes NO-FUNDING on every active future trigger. If a trigger whose payments match the expected superblock is relayed during that period, every masternode votes NO on it. Once the window opens, each node computes a candidate with the same DataHash and reuses the existing trigger instead of creating its own, and the YES path treats our earlier FUNDING vote as final without checking its outcome. The trigger never reaches the threshold. Affects v23.1.8, v23.1.9, v24.0.0-rc.x and develop. ## What was done? 1. `fix(governance): don't vote NO-FUNDING on triggers before their maturity window`: `VoteGovernanceTriggers()` takes the block height and skips the NO vote on a trigger until `height >= trigger height - nSuperblockMaturityWindow`. Before then there is no candidate to compare it to. Non-matching triggers still get their NO once the window opens, about 3 days before the superblock on mainnet. 2. `fix(governance): don't vote on triggers while not synced`: found in review. A masternode that restarts inside the window loads triggers from `governance.dat` and, before it's synced, has no candidate either, so it would vote NO on the correct trigger and keep that vote. Trigger voting now waits for sync. A synced masternode never goes back to unsynced, and the YES path already required sync. The functional test adds a scenario at the end of `feature_governance.py` (superblock 240). The next payee is isolated, mines into the window on its own fork so unmodified code signs a correct trigger, rolls back, and relays the trigger before the window. The test then checks there are no NO votes before the window, a YES from all 5 masternodes once it opens, and that the superblock pays. ## How Has This Been Tested? - Reproduced on regtest against unmodified develop: the early trigger ended at 1 YES / 4 NO and the superblock paid nothing. With the fix: 5 YES / 0 NO, paid. - v23.1.x with the fix: `feature_governance.py` 4x in each wallet mode, plus `feature_governance_cl.py` (both modes) and `feature_governance_objects.py`, all pass. Without commit 1, the new scenario fails as expected (NO votes before the window). - develop plus both commits, built with `--enable-werror`: `feature_governance.py`, `_cl` and `_objects` pass. Both commits also cherry-pick cleanly onto v24.0.x (not built there). - v23.1.x doesn't build with `--enable-werror` under clang 21 until #15 lands (4 thread-safety errors in `instantsend.cpp`, unrelated). `governance/signing.cpp` itself compiles clean under werror. - Independent review of both commits: no defects. Open points are under Breaking Changes. ## Breaking Changes None. No consensus, wire-format or RPC change; only when this node casts automatic trigger votes. Notes: - **Rollout:** unpatched masternodes still cast the early NO, so the issue remains until roughly 55% of the weighted masternode set runs the fix. - **Follow-up:** inside the window, a node that hasn't voted YES yet still votes NO on non-matching triggers, and that NO still blocks a later YES if its candidate changes to match. This matters mainly when no proposal is fundable as the window opens and one crosses the threshold later, after a matching trigger was already relayed. The fuller fix is letting a node replace its own automatic NO with a YES. That needs retry logic for the 1h vote-change limit and a decision on overriding manually cast NO votes, so it is left out of this point release. - **Not compared yet** with the reporter's suggested fix (the encrypted attachment wasn't available). Forward-port to develop and v24.0.x needed (both commits apply cleanly). ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] 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 _(for repository code-owners and collaborators only)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Bug Fixes** * Governance trigger voting now waits until masternode synchronization is complete and the superblock maturity window has opened, preventing premature NO-FUNDING votes. * Eligible triggers can receive YES votes once the voting window opens, helping valid superblocks pass as expected. <!-- end of auto-generated comment: release notes by coderabbit.ai --> ACKs for top commit: knst: utACK bd948aa Tree-SHA512: 976e117ece5eeb42cec5f7d09e24b8768ba86b6989e840d9a927a9d31eb76db86f316c784ab50e4fc3d6254b6d88356dada5e6170a26aaad72772dd48c9f78d6
…tures so complementary errors cannot cancel 593f252 fix(bls): weight batch-verified signatures so complementary errors cannot cancel (UdjinM6) Pull request description: ## Issue being fixed or feature implemented `CBLSBatchVerifier` (LLMQ sig shares, recovered sigs, ISLOCKs) and the DKG message batch (`BatchVerifyMessageSigs`) summed signatures and public keys without weights and accepted the whole batch if the aggregate verified. Two signatures that are each wrong but whose errors cancel (S1 + T and S2 − T) pass as one valid aggregate, so nobody lands in `badSources` and the forged entries are stored. For LLMQ sig shares, a peer that has seen an honest share can occupy that member's slot with a wrong one. The honest copies are then dropped as already known and recovery fails, which stalls InstantSend/ChainLock signing. Honest nodes can also be banned, or DKG members marked bad. The v23.1.9 per-sender scoping (#5) doesn't help, because the cancelling pair can come from one sender. Public reviewers (CodeRabbit, thepastaclaw) flagged the same aggregation on the public v23.1.9 merge-back, so this should ship soon. ## What was done? UdjinM6's `fix(bls): weight batch-verified signatures so complementary errors cannot cancel`. Each signature and its public key are multiplied by a secret random nonzero 128-bit weight before aggregating; the per-source and per-message fallbacks still verify the original signatures. Adds `MulInsecure()` to `CBLSPublicKey`/`CBLSSignature`. `BatchVerifyMessageSigs` also skips an invalid operator key. ## How Has This Been Tested? See the testing section of #15: one combined clang `--enable-werror` build of all three PRs. New `batch_verifier_tests` cases and the new `llmq_utils_tests` sig-share row fail without the change and pass with it. `feature_llmq_signing.py` (+`--spork21`), `feature_llmq_is_cl_conflicts.py`, `feature_llmq_dkgerrors.py` and `interface_zmq_dash.py` pass. `feature_llmq_chainlocks.py` fails identically on unmodified `v23.1.x`. Review notes for UdjinM6: - The same-key row in the batch-verifier test passes even without the fix in the two secure-mode runs (equal message hashes land in separate steps), so "fails in every verifier mode" in the commit message overstates it; the suite as a whole does fail without the fix. - Weighting assumes prime-order-subgroup points. G2 signatures are subgroup-checked under the basic scheme, so this only matters before V19 (regtest/devnets), where legacy decoding skips the check. Legacy G1 operator keys are fine, since the pairing kills a G1 cofactor component. - Cost: one 128-bit G2 and one G1 scalar multiplication per pushed message, not benchmarked. 64-bit weights, as the contribution PR uses, would roughly halve it. - `CBLSWorker::PushSigVerifyBatch` still aggregates without weights. Only `bench_dash` calls it today; it should be weighted or removed. ## Breaking Changes None. ## Forward-port Needed on `develop` and `v24.0.x` (not public yet). ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] 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 _(for repository code-owners and collaborators only)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: d76915072adc367c3c65c3d4d7fd875b49bbb7135749847f759c65c91bbbb3b66084232c6955ce8bd31ab58c124b6222de430207ce8a567c4755cddbe083d74f
…ution verification 95fbf22 fix(bls): stop weighted contribution batch verification on an invalid partial aggregate (pasta) 12bbd21 fix(llmq): weight batched DKG contribution verification (UdjinM6) Pull request description: **Stacked on #16**, which adds the shared `MakeBatchVerificationWeight` helper. Merge that first; this PR's base is that branch, so the diff shows only this change. ## Issue being fixed or feature implemented During the DKG every member verifies the contributions it received with `CBLSWorker::VerifyContributionShares`. Its aggregated mode, which the DKG uses, summed the vvecs and secret-key shares of each batch of 8 dealers without weights and accepted the whole batch when the sums matched. Two colluding dealers in the same batch can hand a recipient wrong shares whose errors cancel, for example by swapping them. The recipient doesn't complain and stores wrong "verified" shares, so its quorum secret key share is wrong. This is LQ4-C10, deferred from v23.1.9. It is a correctness/liveness issue: no share is published, so it doesn't leak the quorum key. The old flow also marked a whole batch invalid if aggregation itself failed. No peer can reach that path today, but this PR removes it. ## What was done? - UdjinM6's `fix(llmq): weight batched DKG contribution verification`. Each dealer's vvec and its share's public key are multiplied by the same secret random 64-bit weight before aggregating. A failing batch falls back to one-by-one verification. Weighting needs subgroup-checked points, so `CDKGContribution` records whether its vvec was decoded under the legacy scheme, and legacy vvecs are verified one by one. The wire format is unchanged. The commit also fixes the argument order in `bench/bls_dkg.cpp`. It includes a benchmark: about 1.6–1.7x slower than unweighted, about 1.6x faster than individual verification. - `fix(bls): stop weighted contribution batch verification on an invalid partial aggregate`: hardening. If an aggregation step ever leaves an invalid partial result, return false (falling back to one-by-one) instead of letting the next `AggregateInsecure()` assert. I found no way for a peer to reach this; it's defence in depth on a peer-data path. ## How Has This Been Tested? See the testing section of #15: one combined clang `--enable-werror` build of all three PRs. `bls_verify_contribution_shares_honest_tests` covers swapped shares (both blamed) and a missing share (only it blamed); the swap cases fail with unit weights. `llmq_dkg_tests/contribution_records_vvec_scheme` covers the scheme bookkeeping. The full `test_dash` suite and the LLMQ functional tests listed there pass. `feature_llmq_chainlocks.py` fails identically on unmodified `v23.1.x`. Gap: no test checks that a legacy vvec actually takes the one-by-one path, or uses a point outside the subgroup. ## Breaking Changes None. ## Forward-port Needed on `develop` and `v24.0.x` (not public yet). ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] 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 _(for repository code-owners and collaborators only)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) ACKs for top commit: knst: utACK 95fbf22 Tree-SHA512: d8a184c9574041afbc0c8bb30733b62f4a33a58f5cea35a5bc3b731840a11bdd6f1f1ab1ce09e6af13ce466c92292749a5fbe474c3d7a01af9af92ab15bf99bf
Bumps _CLIENT_VERSION_BUILD to 10 (_CLIENT_VERSION_IS_RELEASE stays true, no RC), adds the flatpak metainfo release entry, writes the v23.1.10 release notes and archives the v23.1.9 notes under doc/release-notes/dash/. Man pages carry the version bump only; no CLI options changed this release. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
01dc7e0 chore: prepare v23.1.10 release (pasta) Pull request description: ## Issue being fixed or feature implemented A superblock trigger relayed before its maturity window could collect NO-FUNDING votes from every masternode and then never reach the funding threshold, so the superblock paid no proposals. That fix, plus several DKG/BLS verification hardening fixes, is now on `v23.1.x`. This PR prepares the **v23.1.10** point release on top of them, following the v23.1.9 prep (4722ad9). ## What was done? - `configure.ac`: `_CLIENT_VERSION_BUILD` 9 → 10 (`_CLIENT_VERSION_IS_RELEASE` stays `true`, no RC). - `doc/release-notes.md`: v23.1.10 notes covering the fixes merged since v23.1.9: - governance: no NO-FUNDING trigger votes before the superblock maturity window, and no trigger voting until synced (a08d35e); - weighted BLS batch verification for sig shares, recovered sigs, ISLOCKs and DKG messages (5f17c2e); - weighted batched DKG contribution verification (aeffa7e); - DKG members sharing verification-vector coefficients are excluded, and equal secret-key shares are accepted when aggregating (a28c030). - Archived the v23.1.9 notes to `doc/release-notes/dash/release-notes-23.1.9.md` (verbatim copy), added v23.1.9 to "Older releases", and pointed the change log link at `v23.1.9...v23.1.10`. - `contrib/flatpak/org.dash.dash-core.metainfo.xml`: `23.1.10` release entry dated 2026-10-30. - `doc/man/*.1`: version string bump only; no CLI options changed this release. The `instantsend.h` thread-safety annotation fix (a28c030) is build-only (clang `--enable-werror`), so it is not in the notes. ## How Has This Been Tested? Release-mechanics only; no code changes. - Built the branch (Linux x86_64, wallet, no GUI); `dashd -version` reports `v23.1.10`. - Regenerated `dashd`, `dash-cli`, `dash-tx`, `dash-wallet` and `dash-util` man pages with `contrib/devtools/gen-manpages.py` from that build: byte-identical to this commit, so the CLI options didn't change. `dash-qt.1` got the same version-only bump and wasn't regenerated (no GUI build). - The notes were checked against the merged diffs for accuracy and kept to effects rather than exploit detail. The archived 23.1.9 notes are byte-identical to the previous `doc/release-notes.md`. **Known failure, not from this PR:** `feature_llmq_chainlocks.py` fails on `v23.1.x` (3/3 locally, at `test_bestCLHeightDiff`). The dashpay#7858 backport dropped its `test_framework.py` hunk that syncs mempools before burying a tx. Provider transactions are no longer InstantSend-locked, so ChainLock signers won't sign the block until they have seen the ProRegTx from `dynamically_add_masternode` for 10 minutes, and the mocktime bump happens before they see it. Adding `self.sync_mempools()` before that bump makes the test pass (2/2). That belongs in its own PR. ## Breaking Changes None in this PR. Two operator-visible behaviour changes ship in the fixes and are described in the notes: masternodes stop voting on triggers before the maturity window or while unsynced, and DKG members whose verification vectors share a coefficient (the copier and the copied member) are excluded from the commitment and receive the usual PoSe penalty. ## Checklist: - [x] 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 - [x] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: 12f907b61dcb4d004faf9159b83cdf2c21a48ba68ac11c7f326bc673fd4e55d0801f7a1d077d2faff2ac11a6c4ac04ea3d9402c8a82966392ff8bfe3132a396c
…ents A quorum member can copy another member's whole contribution, its verification vector and encrypted shares alike, and re-sign it with its own operator key. BLS-IES decryption is bound to the recipient, not the dealer, so every recipient decrypts a share that is consistent with the copied vvec: the copy passes the individual contribution check and nobody complains. SendCommitment then fails to build the quorum verification vector, because VerifyVerificationVectors rejects coefficients shared between members (and AggregateSecretKeys rejects the identical shares), so no honest member sends a premature commitment and a single member kills the DKG round. The copy and the original cannot be told apart, so VerifyAndCommit now marks every remaining member whose vvec shares a coefficient with another one as bad, after members with open complaints are excluded. Members that agree on the remaining members and hold the same contributions drop the same ones, so the quorum vvec can be built and they still agree on the commitment. A new CBLSWorker::FindVerificationVectorsWithSharedCoefficients() helper finds them with the same hash comparison VerifyVerificationVectors uses. PoSe trade-off: both members are invalid in the final commitment, so each gets the usual CalcPenalty(66) PoSe penalty for failed DKG participation. An attacker can therefore make a chosen honest co-member take that penalty (a ban if repeated in two DKGs close together), paying the same penalty itself and only when both are selected into the same quorum. Before this change the same attack prevented the commitment altogether. Possible follow-up: the cross-member duplicate checks look like input hygiene from the original DKG worker (3ee27c1) rather than a security requirement. Joint-Feldman DKG stays secure while one honest dealer's polynomial is secret, and a copied contribution gives the copier no control over the quorum key, so accepting duplicates would neutralize the attack without punishing anyone. That needs a cryptographic review and relaxing the shared duplicate checks in quorum vvec building and secret key aggregation, and nodes still running older code would keep failing on a duplicated set during a rollout, so it is not done here. Binding each contribution to its dealer would also exclude only the copier, but it changes the DKG protocol. Test: bls_verify_contribution_shares_duplicate_coefficient_tests now covers the full copy (every share verifies, the quorum vvec can't be built, the helper flags exactly that pair, and the vvec builds without them). The VerifyAndCommit marking itself is covered end to end by the feature_llmq_simplepose scenario added in the following commit, which fails without it. feature_llmq_dkgerrors, feature_llmq_rotation, feature_llmq_signing and feature_llmq_simplepose pass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit c8627a6)
Add a regtest-only "contribution-clone" DKG sim error: a member simulating it derives its secret polynomial from the quorum hash alone, so every member simulating it deals the same contribution, as if it had copied another member's whole contribution. feature_llmq_simplepose (5-member llmq_test, minSize 3) now starts with a scenario where two masternodes deal cloned contributions. The quorum is mined with exactly the other three as valid members, and both cloners are PoSe-punished but not banned. The scenario then lets the penalty decay with DKG disabled, so the existing scenarios start from a clean state. Without the VerifyAndCommit marking from the previous commit, no member can build the quorum verification vector, no premature commitment is sent and the scenario fails waiting for the commit phase. Backport note: this branch keeps the spork23-disabled run free of the contribution-miss banning, so test_cloned_contributions runs right after the sporks are set, in both modes, as in v23. dkgsimerror rates are numeric here. The simulated clone in active/dkgsession.cpp uses this branch's util::irange (util/helpers.h) instead of irange::range (util/irange.h). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 1ec6aaa)
…er's share A member builds its quorum secret key share by summing the shares the valid dealers sent it, through CBLSWorker::AggregateSecretKeys. That rejected the input outright if two shares were equal, so the member logged "failed to build own secret share" and sent no premature commitment (and the QDATA recovery path punished the honest peer that served the shares). Two colluding dealers, for example two masternodes of one operator in the same quorum, can deal a chosen member equal shares: pick polynomials that agree at its id. Both contributions verify, nobody complains, and the vvecs share no coefficient, so neither the batch vvec checks nor the shared coefficient marking in VerifyAndCommit see anything. With degree t-1 polynomials they can do this for up to t-1 members at once, which is enough to keep any LLMQ type from reaching minSize signers, and nobody is blamed. Duplicate shares are harmless here: the sum is the correct share either way, and it is checked against the quorum vvec when the premature commitment is verified. Aggregate secret key shares with a validity check only. Public key and signature aggregation keep their duplicate check. Test: bls_aggregate_colliding_contribution_shares_tests builds two dealers whose polynomials agree at the receiver's id, checks that every contribution verifies and the quorum vvec builds, and that the receiver's aggregated share matches the public key share of the quorum vvec. It fails without this change. Backport note: CBLSWorker::Start takes an explicit worker count on this branch, so the new bls_tests case passes 4. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit dfd9820)
…ity window Before the superblock maturity window opens, CreateSuperblockCandidate() returns nothing, yet VoteGovernanceTriggers() still voted NO-FUNDING on every active future trigger. Once the window opened, CreateGovernanceTrigger() picked up the existing trigger with the matching DataHash, and the YES path treated any earlier FUNDING vote of ours as final, so the early NO was never replaced by a YES. A single masternode could exploit this by relaying, just before the window, a trigger matching the payments every node is about to compute: all masternodes voted NO on it, no other trigger was created, and the superblock paid none of the approved proposals. Skip NO-FUNDING votes on triggers whose maturity window has not started yet, since there is no local candidate to compare them against. They still get a NO once the window opens if they don't match. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 5d9fe74)
CreateSuperblockCandidate() returns nothing until the node is synced, but VoteGovernanceTriggers() still voted NO-FUNDING on every active future trigger, including ones loaded from governance.dat. A masternode restarting inside the maturity window, before it had voted YES, would therefore vote NO on the legitimate trigger and, once synced, treat that NO as its final vote for the identical trigger it computes. Skip trigger voting until we are synced, as there is no candidate to compare triggers to before then. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit bd948aa)
…nnot cancel CBLSBatchVerifier and the DKG message batch (BatchVerifyMessageSigs) added signatures and public keys without weights and accepted the whole batch when the aggregate verified. Two signatures that are each wrong but whose errors cancel (S1 + T and S2 - T, for distinct messages or for the same message and key) therefore passed as one valid aggregate: Verify() returned before any per-source check, nobody landed in badSources, and the forged entries were stored. For LLMQ sig shares that lets a peer that has seen an honest share occupy the member's slot with a wrong one, so the honest copies are dropped as already known and recovery fails; recovered sigs, ISLOCKs and DKG messages go through the same aggregation. Multiply each signature and its public key by a secret random nonzero 128-bit weight before aggregating, which makes a cancelling combination negligibly unlikely. The per-source and per-message fallbacks still verify the original signatures. MulInsecure() is added to CBLSPublicKey and CBLSSignature for the scalar multiplication; the weights are drawn in the header-only verifier, so the consensus library gains no RNG dependency. BatchVerifyMessageSigs now also skips an invalid operator key instead of handing it to the multiplication. The new batch_verifier_tests cases (cancelling errors across two messages of one source, and across two sources signing the same message with the same key, in every verifier mode) and the new sig-share row fail without this change and pass with it. BatchVerifyMessageSigs is file-local and needs a full CDKGSession, so it is covered by feature_llmq_dkgerrors and feature_llmq_signing rather than a unit test. Backport note: BatchVerifyMessageSigs lives in src/llmq/net_dkg.cpp on this branch instead of src/active/dkgsessionhandler.cpp; the hunk is applied there unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 593f252)
During the DKG every member verifies the contributions it received with CBLSWorker::VerifyContributionShares. Its aggregated mode, which the DKG uses, summed the verification vectors and the secret-key shares of each batch of CONTRIBUTION_VERIFY_BATCH_SIZE dealers without weights and marked the whole batch valid when the sums matched. Two colluding dealers in the same batch could therefore hand a recipient wrong shares whose errors cancel (for example by swapping them): the recipient doesn't complain and stores wrong verified shares, and if one of the dealers is later excluded its quorum secret key share is wrong. Each batch is now verified by one worker task that multiplies every dealer's vvec coefficients and the public key of its share by the same fresh secret random 64-bit weight before aggregating, and checks that the public key share recovered from the aggregated vvec matches the aggregated public keys. A batch that fails falls back to verifying its contributions one by one, as before; a batch whose aggregation failed outright used to be marked invalid as a whole instead. The old unweighted aggregation flow for this path is removed, and MakeBatchVerificationWeight takes the weight size. 64-bit weights: a batch with a wrong contribution passes with probability about 2^-64. Colluding dealers lose nothing on a failed attempt (they can answer the complaint with a valid justification), so an attacker gets many free attempts, but the weights never leave the verifying node, so it can't test them offline, and 2^-64 per attempt stays negligible. 32-bit weights would have been about 1.9x faster but leave a small chance of success over the lifetime of the network. The weighting is only sound for points in the prime-order subgroup. Legacy-scheme deserialization skips that check, so CDKGContribution now decodes its whole vvec under a single scheme and records it in a non-serialized vvecLegacyScheme field (the wire format is unchanged). The session keeps it per member, and VerifyPendingContributions only uses the weighted batches when none of the pending vvecs was decoded under the legacy scheme, verifying them one by one otherwise. A point decoded under the basic scheme has always passed the subgroup check, whatever the chain did since, so this also covers a reorg across the v19 activation. Release build (-O2), one member verifying every dealer's contribution, all valid, 4 BLS worker threads: quorum unweighted (old) individual weighted (new) llmq_100_67 49 ms 292 ms 189 ms llmq_400_60 0.58 s 4.15 s 2.45 s llmq_400_85 0.82 s 5.92 s 3.46 s Also fix bench/bls_dkg.cpp, which passed its aggregated flag in the parallel position, so its "aggregated" and "simple" variants compared parallel against serial aggregated verification. Tests: bls_verify_contribution_shares_honest_tests now covers two dealers in one batch swapping shares (both blamed) and a missing share (only it blamed), with and without parallel verification, through the default arguments the DKG uses; the swap cases fail with all weights set to 1. llmq_dkg_tests/contribution_records_vvec_scheme round-trips a contribution under each scheme and checks the recorded scheme and the decoded point. Backport note: The new bls_tests loop uses this branch's util::irange instead of irange::range. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 12bbd21)
… partial aggregate CBLSPublicKey::AggregateInsecure() marks the result invalid if the underlying aggregation throws, and the next dealer's AggregateInsecure() then asserts on it. Return false instead, so the batch is verified one by one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 95fbf22)
Records v23.1.10 (703b2a0, now master) as merged, so master can fast-forward to v24.0.x again (check_merge) and the final v24.0.0 contains every v23.1.10 commit. The tree is unchanged: the content is already on this branch through the preceding cherry-picks, so every v23.1.10 commit resolves to the v24.0.x side, as dashpay#7965 did for v23.1.9: - the eight v23.1.10 code commits are the preceding cherry-picks, adapted to this branch's layout (BatchVerifyMessageSigs in llmq/net_dkg.cpp, util::irange, CBLSWorker::Start(4), numeric dkgsimerror rates); - 3174d5f (!cs_timingsTxSeen annotations) fixes a v23.1.x-only clang -Werror break; the methods it annotates don't exist on this branch; - release bookkeeping (configure.ac version, v23.1.10 release notes, man pages, flatpak entry) stays with v24's own release process. A trial real merge on top of the picks staged no clean additions; every remaining difference was release bookkeeping or a conflict re-inserting code the picks already carry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Collaborator
|
✅ Final review complete — no blockers (commit cdb250c) · triage: critical |
thepastaclaw
reviewed
Nov 2, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
Static verification of head 66bbbcc confirms that all eight code fixes and their source tests are retained with the documented branch adaptations; no blocking implementation defect was found. One non-blocking documentation omission remains: the behavioral v23.1.10 release notes have not been carried into the v24 notes. Current CI reports no failures and passing linters, but cppcheck and the main container manifest remain pending; builds and tests were not run locally in this static-review lane.
🟡 1 suggestion(s)
1 finding(s) not shown inline (the lines are not part of this PR's diff)
🟡 Suggestion: Carry forward the behavioral v23.1.10 release notes
doc/release-notes.md:632-636
Excluding release commit 01dc7e0 avoids importing v23 version metadata and generated artifacts, but it also leaves its substantive operator-facing notes absent. The v24 notes mention carried-forward fixes through v23.1.9 here and otherwise provide only a generic hardening statement; they do not explain the trigger-voting fixes, weighted signature and contribution verification, shared-coefficient exclusion and its PoSe consequences, acceptance of equal secret-key shares, or increased DKG/signing CPU work. Adapt that behavioral content into the v24 notes or add doc/release-notes-8016.md without copying the v23 release bookkeeping. If this documentation remains deferred to v24 release preparation, link a tracking issue or PR so the omission is tracked.
source: gpt-6.1-sol (phase2-reviewer: general, backport-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: 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:
criticalbygpt-6.1-sol(effort low) — The substantial, intertwined changes in src/bls/bls_batchverifier.h and src/bls/bls_worker.cpp alter cryptographic signature and DKG contribution batch verification, including randomized weighting and invalid-aggregate handling. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-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; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— backport-reviewer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— dash-core-commit-history (completed, effort xhigh); agentphase2-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 `doc/release-notes.md`:
- [SUGGESTION] doc/release-notes.md:632-636: Carry forward the behavioral v23.1.10 release notes
Excluding release commit 01dc7e058c avoids importing v23 version metadata and generated artifacts, but it also leaves its substantive operator-facing notes absent. The v24 notes mention carried-forward fixes through v23.1.9 here and otherwise provide only a generic hardening statement; they do not explain the trigger-voting fixes, weighted signature and contribution verification, shared-coefficient exclusion and its PoSe consequences, acceptance of equal secret-key shares, or increased DKG/signing CPU work. Adapt that behavioral content into the v24 notes or add doc/release-notes-8016.md without copying the v23 release bookkeeping. If this documentation remains deferred to v24 release preparation, link a tracking issue or PR so the omission is tracked.
thepastaclaw
added
pastaclaw:commented
thepastaclaw
approved these changes
Nov 2, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
Independently reviewed the complete diff at e2fed88 and compared all eight code backports with their v23.1.10 originals; the documented branch adaptations preserve the fixes and regression tests, and the release-tag merge is tree-neutral. No actionable defects were found, and the prior release-notes finding is fixed. Validation was static only: current CI reports successful lint and infrastructure checks with no failures, while platform dependency builds and Cppcheck remain pending, so CI build/test validation is not yet established.
🔴 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:
criticalbygpt-6.1-sol(effort low) — The intricate changes to src/bls/bls_batchverifier.h and src/bls/bls_worker.cpp alter cryptographic signature and DKG contribution batch verification, including randomized weighting and invalid aggregate handling. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-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; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— backport-reviewer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— dash-core-commit-history (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— backport-reviewer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— dash-core-commit-history (completed, effort xhigh); agentphase2-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.
The v24.0.0 notes carried the v23.1.9 fixes forward as one bullet framed by that release. Release notes only describe what an operator needs to act on, so keep the part that changes expectations - provider transactions are no longer InstantSend-locked, and software waiting for such a lock should wait for a ChainLock or confirmations instead - as a standalone note, and drop the patch-release framing and the negative-nVersion consensus fix, which affects no valid transaction. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PastaPastaPasta
force-pushed
the
backport/v24.0.x-v23.1.10
branch
from
November 2, 2026 20:41
e2fed88 to
18a8e40
Compare
Member
Author
|
The release-notes finding is addressed in 18a8e40, but against a narrower rule than the earlier fix. The v24.0.0 notes include a change from a v23.1.x patch release only if a v24 operator has to act on it or plan for it: required action, configuration, compatibility, upgrade behaviour or changed operational expectations. The notes no longer carry patch releases forward version by version, so e2fed88 is dropped and replaced by two doc commits after the tag merge. Coverage, checked against the v23.1.1 to v23.1.10 notes:
The archived v23.1.10 notes, the older-releases list and the change-log link stay with v24.0.0 release preparation, as for v23.1.9. 🤖 Posted autonomously by Claude on behalf of pasta. |
thepastaclaw
removed
the
pastaclaw:approved
thepastaclaw
reviewed
Nov 2, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
Verified the complete diff at 18a8e40: all eight source code/test picks are preserved with the documented v24 adaptations, and no blocking correctness or missing-code-prerequisite issues were found. One non-blocking documentation issue remains: the source release's governance-specific upgrade timing and mixed-version caveat are absent. This was static review only; current CI lint, cppcheck, and container builds pass, while source builds and several dependency builds remain pending, with no failed checks reported.
🟡 1 suggestion(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: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 5: gpt-6-astra (agent: phase2-reviewer, role: backport-reviewer); reviewer 6: gpt-6-astra (agent: phase2-reviewer, role: dash-core-commit-history); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: backport-reviewer); reviewer 10: 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:
criticalbygpt-6.1-sol(effort low) — The diff makes intricate cryptographic verification changes across src/bls/bls_batchverifier.h, src/bls/bls_worker.cpp and src/llmq/net_dkg.cpp, including randomized weighting of signature and DKG contribution batches and invalid aggregate handling. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-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; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— backport-reviewer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— dash-core-commit-history (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— general (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— backport-reviewer (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— dash-core-commit-history (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— backport-reviewer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— dash-core-commit-history (completed, effort xhigh); agentphase2-reviewer - Model comparison: every Phase-2 reviewer also ran on
gpt-6-astra; the verifier weighed both sets without knowing which model wrote which
🤖 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 `doc/release-notes.md`:
- [SUGGESTION] doc/release-notes.md:648-653: Carry forward the behavioral v23.1.10 release notes
The operator-focused rewrite covers the DKG penalties and increased CPU cost, but it still omits actionable governance rollout guidance. The Governance section in `01dc7e058c:doc/release-notes.md` explicitly recommends upgrading before the next superblock because masternodes without these fixes continue casting premature NO-FUNDING votes. The backported `VoteGovernanceTriggers` guards take effect immediately, independently of v24 activation, and upgrading one node does not prevent older nodes from casting those votes. The existing hard-fork upgrade deadline therefore does not communicate this governance-specific timing. Add a concise note explaining that fixed masternodes withhold trigger votes while unsynced and premature NO-FUNDING votes before the maturity window, together with the mixed-version caveat and upgrade recommendation for operators still running versions without these fixes. This fits the stated upgrade-planning rule without restoring patch-release chronology or cataloguing routine fixes.
Audited the v23.1.1 to v23.1.10 release notes for changes a v24 operator has to act on or plan for, and added those the v24.0.0 notes were missing, each under its v24 topic: - Mining and mempool policy: Asset Locks with more than 100 inputs or larger than 20480 bytes are non-standard (dashpay#7359, v23.1.4). - Changed options: -allowprivatenet no longer affects masternode address validation (dashpay#7888, v23.1.9). - Masternode configuration requirements: a DKG member sharing verification-vector coefficients with another is excluded together with it, and both get the usual PoSe penalty (v23.1.10). Routine bug, crash and denial-of-service fixes, GUI, build and performance changes, and fixes with no operator action (such as the superblock trigger voting fix, or the extra work of weighted BLS batch verification) are left out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PastaPastaPasta
force-pushed
the
backport/v24.0.x-v23.1.10
branch
from
November 2, 2026 22:00
18a8e40 to
cdb250c
Compare
thepastaclaw
removed
the
pastaclaw:commented
thepastaclaw
approved these changes
Nov 2, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
Static review of the complete assigned diff and comparison with all eight source patches found no actionable backport defects. The documentation requests do not establish missing operator guidance or prerequisites, and the prior governance finding is withdrawn. Current CI reports lint, Cppcheck, and check_merge passing with no failed checks; dependency and source-build jobs remain pending, so build and test validation is incomplete.
🔴 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:
criticalbygpt-6.1-sol(effort low) — The substantial, interdependent changes to src/bls/bls_batchverifier.h and src/bls/bls_worker.cpp alter cryptographic signature and DKG contribution verification, including randomized weighting to prevent complementary errors from cancelling. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-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; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— backport-reviewer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— dash-core-commit-history (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— backport-reviewer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— dash-core-commit-history (completed, effort xhigh); agentphase2-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.
This was referenced Nov 3, 2026
thepastaclaw
removed
the
pastaclaw:approved
PastaPastaPasta
added a commit
that referenced
this pull request
Nov 4, 2026
2bf133c docs: cite #8042 for the index fixes in the v24.0.0 release notes (pasta) 8aa11f6 docs: add the v24.0.0-rc.7 picks and index fixes to the v24.0.0 release notes (pasta) ad4e5d6 docs: drop the index caveat from the -reindex-chainstate man page entry (pasta) 19a90bb test: blocks orphaned while the address index is disabled are removed from it (pasta) d2f2a1f fix(index): don't adopt an empty or rebuilt async index as migrated legacy data (pasta) e3bafdb fix(index): reject an uncommitted tip the block index lost (pasta) 9198962 fix(index): check the blocks governance resumes from at startup (pasta) baff347 fix(index): resume from the tip a skipped commit leaves behind (pasta) 2dbf5ca fix(index): write a rewound block's erasures in one batch with its locator (pasta) 252acbe fix(index): move the committed locator below the entries a rewind erases (Konstantin Akimov) e9f9c8a Merge bitcoin#36150: indexes: set prune lock to genesis before first block (Konstantin Akimov) 2edeb98 partial Merge bitcoin#34897: indexes: Don't commit ahead of the flushed chainstate (Konstantin Akimov) 3eba84c Merge bitcoin#32875: index: handle case where pindex_prev equals chain tip in NextSyncBlock() (Konstantin Akimov) 399125c Merge bitcoin#32882: index: remove unnecessary locator cleaning in BaseIndex::Init() (Konstantin Akimov) bb724c6 Merge bitcoin#33212: index: Don't commit state in BaseIndex::Rewind (Konstantin Akimov) d5ff431 Merge bitcoin#32878: index: fix wrong assert of current_tip == m_best_block_index (Konstantin Akimov) 31c7d8f partial Merge bitcoin#32580: wallet, test: best block locator matches scan state follow-ups (Konstantin Akimov) 94f5c58 partial Merge bitcoin#30221: wallet: Ensure best block matches wallet scan state (Konstantin Akimov) 8e0b1e8 partial Merge bitcoin#32069: test: fix intermittent failure in wallet_reorgsrestore.py (Konstantin Akimov) 4d17947 Merge bitcoin#25193: indexes: Read the locator's top block during init, allow interaction with reindex-chainstate (Konstantin Akimov) 0127df8 Merge #8045: test: scale the mempool-loaded wait by timeout_factor so TSAN reindex restarts stop timing out (pasta) 62d0cef Merge #8036: fix(qt): let a shared masternode participant follow a revised invitation that keeps their share (merge-script) 775d31c Merge #8018: fix(consensus): require IPv4 masternode primaries until IPv4 is optional, pin mainnet ports (merge-script) e1b826e Merge #8026: fix(qt): keep shared masternode contribution fields locked while coins are reserved (merge-script) 9fa3df3 Merge #8020: fix(rpc): make protx shared_sign sign only dissolutions that return every principal (merge-script) 246a465 Merge #8024: fix(qt): keep the shared masternode dissolve dialog consistent with what it will send (merge-script) 590ad0a Merge #8021: fix(qt): don't load a newer shared masternode session copy that changes your own share (merge-script) 974aaa3 Merge #8034: fix(qt): check a shared masternode's service and refund addresses before coins are reserved (merge-script) 0026e3e Merge #8027: fix(qt): fund a shared masternode's service update from this wallet by default (merge-script) 6a5d2dc Merge #8035: fix(qt): don't strand coins reserved for a shared masternode session (merge-script) df6083a Merge #8032: fix(qt): don't offer a new operator key when reopening a shared masternode session (merge-script) 2b5bf30 Merge #8029: fix: let dashd -version, -help and first start work without an existing data directory (merge-script) e33d9b1 Merge #8037: docs: list the config file path each Linux init system uses (merge-script) 6030ea0 docs: add the #8014 migratewallet fixes to the v24.0.0 release notes (pasta) 1b1ea95 Merge #8014: backport: bitcoin#28609, bitcoin#34215, bitcoin#34226, bitcoin#34370, partials bitcoin#31423, bitcoin#34156 (migratewallet fixes from 28.x) (merge-script) 608ff63 Merge bitcoin#28542: wallet: Check for uninitialized last processed and conflicting heights in MarkConflicted (DCG-Claude) 3af6f63 Merge bitcoin#28038: wallet: address book migration bug fixes (DCG-Claude) 2fe41fb docs: add ecosystem notes on Asset Unlock txids, coinbase payees and RPC errors (pasta) 0e3f653 docs: correct operator guidance in the v24.0.0 release notes for rc.7 (pasta) 11c8140 Merge #8019: fix(qt): stop reporting refused shared masternode broadcasts as sent (merge-script) Pull request description: ## Issue being fixed or feature implemented Backports and release prep for v24.0.0-rc.7, on top of `v24.0.x` at 8d195f1 (v24.0.0-rc.6, #8013). This PR collects the develop merges labeled `backport-candidate-24.0.x`, the v24.0.x port of #8042 with the index fixes from #8033, and the release notes for them, so rc.7 has a single backport PR to merge. ## What was done? ### Picked Cherry-picked with `git cherry-pick -m1 -x`, in develop merge order, except that bitcoin#28038 goes before bitcoin#28542 as in Bitcoin Core. Each pick changes exactly the same lines as its develop merge, except as noted below for bitcoin#28542, #8014, #8020 and #8018. - #8019 fix(qt): stop reporting refused shared masternode broadcasts as sent - bitcoin#28038 (from #7863): prerequisite of #8014 - bitcoin#28542 (from #7762): prerequisite of #8014. Its `test_conflict_txs` migrates through `self.migrate_wallet`, as on develop since bitcoin#28868. - #8014 backport: bitcoin#28609, bitcoin#34215, bitcoin#34226, bitcoin#34370, partials bitcoin#31423, bitcoin#34156 (migratewallet fixes from 28.x), with the Dash adaptations below - #8037 docs: list the config file path each Linux init system uses - #8029 fix: let dashd -version, -help and first start work without an existing data directory - #8032 fix(qt): don't offer a new operator key when reopening a shared masternode session - #8035 fix(qt): don't strand coins reserved for a shared masternode session - #8027 fix(qt): fund a shared masternode's service update from this wallet by default - #8034 fix(qt): check a shared masternode's service and refund addresses before coins are reserved - #8021 fix(qt): don't load a newer shared masternode session copy that changes your own share - #8024 fix(qt): keep the shared masternode dissolve dialog consistent with what it will send - #8020 fix(rpc): make protx shared_sign sign only dissolutions that return every principal. Its `doc/release-notes-7437.md` sentence goes into the same paragraph of `doc/release-notes.md`, since that file was folded in for v24.0.0. - #8026 fix(qt): keep shared masternode contribution fields locked while coins are reserved (merged on develop as cc73a04; GitHub shows the PR as closed) - #8018 fix(consensus): require IPv4 masternode primaries until IPv4 is optional, pin mainnet ports, with the adaptation below - #8036 fix(qt): let a shared masternode participant follow a revised invitation that keeps their share - #8045 test: scale the mempool-loaded wait by timeout_factor so TSAN reindex restarts stop timing out ### Index fixes: #8042 port and #8033 The index commits are #8042 (merge 9e5e3d5), ported commit by commit to `v24.0.x`, plus the parts of #8033 that #8042 doesn't cover. #8042's v24.0.x port replaces #8033's bitcoin#33212, bitcoin#34897 and rewind-locator commits, and #8033's flushed-block announcement fix (e6a687b) is dropped: bitcoin#30221 removes the wallet's `ChainStateFlushed` locator write, and nothing else on `v24.0.x` consumes that notification. This PR replaces #8033, and #8031 comes in through it (adapted to the `v24.0.x` `CCoinsViewDB` constructor), not as a separate pick. - bitcoin#25193 (from #7883, via #8033): indexes: read the locator's top block during init, allow interaction with `-reindex-chainstate` - From #8042: partial bitcoin#32069 (only `TestNode.kill_process()`, with develop's `wait_until_stopped(**kwargs)` handling it needs), partial bitcoin#30221, partial bitcoin#32580, bitcoin#32878, bitcoin#33212, bitcoin#32882, bitcoin#32875, partial bitcoin#34897, bitcoin#36150, and #8042's own fixes: move the committed locator below the entries a rewind erases; write a rewound block's erasures in one batch with its locator; resume from the tip a skipped commit leaves behind; check the blocks an index resumes from at startup; reject an uncommitted tip the block index lost - From #8033: #8031 fix(index): don't adopt an empty or rebuilt async index as migrated legacy data; test: blocks orphaned while the address index is disabled are removed from it; docs: drop the index caveat from the `-reindex-chainstate` man page entry #### Dash adaptations for #8042 - The governance collateral index doesn't exist on `v24.0.x`, so its hunks are dropped; from the startup resume check only the `base.{h,cpp}` part is kept. - `feature_pruning.py`: node 5 syncs before node 0's pruned restart, and `test_rescan_blockchain` expects the governance-disabled stderr a pruned node prints on `v24.0.x`. - The wallet's `blockConnected` has no birthday skip on `v24.0.x`. - `TestNode.wait_until_stopped()` keeps `v24.0.x`'s `None` default for a clean exit. Apart from these, the `v24.0.x` index, validation and index test sources differ from develop after #8042 only by differences that already existed before it. #### Dash adaptations for #8018 `v24.0.x` predates bitcoin#26177, so it has no `src/kernel/chainparams.cpp` and no `GetBuriedDeployment()`. The four `IPv4OptionalHeight` values (mainnet not scheduled, testnet 0, devnet 0, regtest only through `-testactivationheight=ipv4_optional@<height>`) go into the matching classes in `src/chainparams.cpp`, and `ipv4_optional` is parsed in its `MaybeUpdateHeights()`. The `GetBuriedDeployment()` hunk is dropped; the `DeploymentName()` case is kept. Every other file, including all tests, changes exactly the same lines as the develop merge. The same notes are in the pick's commit message. #### Dash adaptations for #8014 #8014 doesn't cherry-pick cleanly onto `v24.0.x`. The conflicts were resolved against `v24.0.x` APIs. No prerequisite was backported, and no hunk or test was dropped. The same notes are in the pick's commit message. - `src/wallet/bdb.{cpp,h}`, `dump.cpp`, `sqlite.{cpp,h}`, `transaction.{cpp,h}`, `wallet.cpp`: the changed lines are identical to the develop merge. In `wallet.cpp`, the conflict was with develop-only context (`CanGrindR`), which stays out. - `src/wallet/db.h`: #8014 makes `WalletDatabase::Files()` pure virtual. `v24.0.x` still defines `DummyDatabase` in `db.h`, so it gets the `Files()` override there. - `src/wallet/test/wallet_tests.cpp`: the test database mock on `v24.0.x` is `FailDatabase`, so it gets the same `Files()` override. `src/wallet/salvage.cpp` and `src/wallet/test/util.h` keep the `v24.0.x` side, since develop's `DummyDatabase`/`MockableDatabase` placement doesn't exist here. - `test/functional/tool_wallet.py`, `wallet_backup.py`, `wallet_migration.py`: the test framework on `v24.0.x` has no `TestNode.wallets_path`/`datadir_path` (bitcoin#27884), so the new tests build these paths with `os.path.join` and pass RPC paths as strings. `wallet_migration.py` takes only #8014's new tests, not the develop-only tests around them. - `test/functional/wallet_migration.py`: `unsynced_wallet_on_pruned_node_fails` stops its pruned node with `EXPECTED_STDERR_NO_GOV_PRUNE`, because on `v24.0.x` a pruned node still prints the governance-disabled warning. The rc.6 picks handled the same warning the same way. ### Separate v24.0.x PRs, merged separately - #8016 backport: v23.1.10 fixes for v24.0.x - #8022 backport: bind CoinJoin signatures to the approved transaction (#7815) for v24.0.x, which depends on #7815 merging on develop #8016 and #8022 each merge cleanly with this branch in either order, and all three merge to the same tree in any order. ### Not picked - #8025: closed; superseded by #8014 (picked above) - #8031 is not picked separately; it comes through #8033's commits above. - #8039, #8040, #8041, #8043, #8044 (CI and tooling), #7905, follow-ups to #8042, and the PRs in the 24.1 milestone. - Explicitly deferred: #7870, #7679, #7675, #7220, #7217, #7109, #6988, #4974, #8028. ### Release notes `doc/release-notes.md` corrections, checked against the code on this branch: - **Version 2 Asset Unlocks:** version 1 Asset Unlocks stay valid after `v24` activation. Platform withdrawals keep using version 1 until a future Dash Platform release issues version 2, so the stable txid, InstantSend locking and instance-hash relay don't apply to them yet. - **Masternode requirements:** the startup refusals (#7754) and the DKG bad votes for missing transaction relay or compact filters (#7755) apply from the first start of v24.0.0, not from activation. Operators should remove conflicting settings and upgrade before the `v24` start date (November 9, 2026), so their masternodes are running when the activation signal is signed. `-peerblockfilters=0` is the new refusal; `-blockfilterindex=0` already failed on v23. - **How to Upgrade:** a pruned node with `-addressindex` or `-spentindex` now refuses to start; v23 accepted that combination. - **Downgrading:** there is no supported downgrade after `v24` activation: older versions can't follow the network, and they misread the masternode list data v24.0.0 writes. Before activation, downgrading to v23.1.x works with the listed caveats. Dash-Qt's dust-protection setting is migrated to the shared settings, so it reads as disabled after a downgrade. - **Dash Platform:** Platform/dashmate v5.0.0 and later handle the hidden `service` and `softforks` RPC fields. Older Platform versions need both `-deprecatedrpc` options. - **Compatibility:** users on macOS 13 or older must upgrade macOS, or move their node, before activation. - **Legacy wallets:** in v24, `createwallet` with `descriptors=false` also needs `load_on_startup`; before v24 this applied only to `descriptors=true`. The notes now give a working example. - **Wallet migration (#8014):** failure handling in `migratewallet`, `restorewallet` and `dash-wallet createfromdump` is fixed, and the new backup and watch-only/solvables wallet names are listed. - **Ecosystem notes:** the txid of a version 2 Asset Unlock zeroes `requestedHeight`, `quorumHash` and `quorumSig` before hashing (the full-bytes hash is `instanceHash`); pools should emit one coinbase output per `masternode[]` and `superblock[]` template entry, since merged or deduplicated payees are invalid after activation; client libraries that send numbers or booleans as strings fail (#7099); and some wallet RPC error messages changed with upstream backports. - **rc.7 picks:** `doc/release-notes-8018.md` and `doc/release-notes-8029.md` are folded in and removed. #8018's IPv4-primary and mainnet standard-port rules are under Extended addresses, with a pointer from the masternode upgrade notes; #8029's `dash.conf` change is under Updated settings. The other rc.7 picks fix code that is new in v24 and need no release note. - **Index fixes (#8042, #8031):** nodes that ran a release candidate with the address, spent or timestamp index should rebuild them once (with `-reindex`, or by starting once with them disabled, deleting their `indexes/` directories and starting with them enabled). The notes also cover `-reindex-chainstate` with indexes enabled, older versions refusing a migrated index until `-reindex`, the new "the block the index was last synced to is not found. Please rebuild the index." startup error, and the known limitation shared with earlier versions in the remaining cases. - All rc.7 TODO comments are resolved. `configure.ac` is already 24.0.0 with `CLIENT_VERSION_IS_RELEASE` false, so the version string comes from the `v24.0.0-rc.7` tag. The man pages carry #8033's `-reindex-chainstate` change; no other pick changes non-debug help text (#8018's `-testactivationheight` change is debug-only). ## How Has This Been Tested? On this head (2bf133c): - Every develop pick compared with its develop merge (`git patch-id --stable` and the set of changed lines). #8019, #8037, #8032, #8035, #8027, #8034, #8021, #8024, #8026, #8036 and #8045: identical. #8029: the same changed lines; only context differs in `feature_config_args.py`. #8020: the same changed lines, in `doc/release-notes.md` instead of `doc/release-notes-7437.md`. #8018: identical except the adaptation above, checked file by file against 900f085 and reviewed separately for consensus equivalence on mainnet, testnet, devnet and regtest. #8014 and its prerequisites: as in the earlier rounds (adaptations above). - #8042 port: `v24.0.x`'s divergence from develop, before #8042 (058b060) versus after it (9e5e3d5), is unchanged in `src/index/*`, `src/validation.{h,cpp}`, the index unit tests and `feature_coinstatsindex.py`, apart from the adaptations listed above. The #8031, test and man page commits have the same patch-id and message as on #8033. - Built with `--enable-debug --enable-werror` and Qt: no warnings. - Unit suites pass: `baseindex_tests`, `index_migration_tests`, `txindex_tests`, `blockfilter_index_tests`, `coinstatsindex_tests`, `validation_flush_tests`, `wallet_tests`, `interfaces_tests`, `evo_netinfo_tests` and `evo_snapshot_tests`. With the develop picks (7275005), every `evo_*` suite and `test_dash-qt` (15 classes, 152 passed, 0 failed) passed too. - Functional tests pass: `rpc_netinfo.py`, `feature_masternode_shares.py`, `wallet_migration.py`, `feature_coinstatsindex.py` (including the unclean restart after a reorg), `feature_addressindex.py`, `feature_config_args.py` and `feature_index_prune.py`. On the same C++ tree, these pass as well: `feature_spentindex.py`, `feature_timestampindex.py`, `feature_pruning.py`, `feature_txindex.py`, `feature_reindex.py`, `feature_init.py`, `feature_abortnode.py` and `wallet_reorgsrestore.py`. - Regtest upgrade gates, run on a build of this exact head: all pass. They cover v23.1.10 index migration, index contents vs v23 and `-reindex`, a kill mid-migration, 9 wallets, a pre-activation downgrade and round trip, and v22.1.3/v23.0.2 upgrades. The crash-after-reorg probe and the deleted-index probe are both required to be not reproduced: 0/10 stale (rc.6: about 4/10), and the plain-delete rebuild matches. - Man pages: `help2man` output of the built `dashd`, `dash-cli`, `dash-tx`, `dash-wallet` and `dash-util` matched `doc/man` on the develop picks (apart from the debug-build-only `lock` log category). No later commit changes non-debug help text, so nothing is regenerated beyond #8033's `-reindex-chainstate` change. - `git merge-tree` against #8016 (cdb250c) and #8022 (62609bf), heads fetched 05:23 UTC: clean in both orders, and all three merge to the same tree in every order. From the earlier rounds: the wallet suites and functional tests listed for #8014, bitcoin#28038 and bitcoin#28542 passed, and the `createwallet` example was checked against the official v24.0.0-rc.6 binary. ## Breaking Changes - From #8014: migration backups are now named `<name>_<time>.legacy.bak`, the unnamed wallet's watch-only and solvables wallets are named `default_wallet_watchonly` and `default_wallet_solvables`, and `restorewallet` accepts an existing empty directory. - From #8018: on mainnet, a version 3 ProRegTx or ProUpServTx must list an IPv4 address on the standard port as the first entry of each address list; testnet and devnets are unaffected. - From #8029: `dashd` no longer creates an empty `dash.conf`. - From #8031: older versions refuse an index migrated by v24.0.0 until they run `-reindex`; nodes that ran a release candidate should rebuild these indexes once. - From #8042: an index whose last synced block was lost in an unclean shutdown makes startup fail with a rebuild instruction. The release notes describe these. ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: 3c405ef570fc1c5b46d1ba8bb2e344213cf65018abe5ef4b94b7cf2b9aa7160ba2675f71a0a365c72bc8249a2f9a5defe774af2a4e33537245aeb420c5f66711
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
Brings the v23.1.10 fixes to
v24.0.x.masterwas fast-forwarded today to the signedv23.1.10tag (703b2a0).v24.0.xhas none of that release's code fixes, so v24.0.0 would ship without the DKG, BLS batch-verification and superblock-trigger fixes in v23.1.10's release notes. A masternode upgrading from v23.1.10 to v24.0.0 would lose them. Also,check_mergerequiresmasterto fast-forward tov24.0.x, which fails until v23.1.10 is recorded here.What was done?
This is not a merge of the v23 branch. Each v23.1.10 commit was checked by content against
v24.0.x, and none of the eight code commits were present. They are picked here withgit cherry-pick -xfromv23.1.x, in release order:fix(llmq): drop DKG members whose verification vectors share coefficients(c8627a6). Clean.test: cover DKG members dealing identical contributions(1ec6aaa). The newfeature_llmq_simplepose.pycase runs right after the sporks are set, in both spork23 modes as in v23, with numericdkgsimerrorrates. The simulated clone inactive/dkgsession.cppuses this branch'sutil::irange(util/helpers.h).fix(llmq): accept equal DKG secret key shares when aggregating a member's share(dfd9820). The newbls_testscase passesCBLSWorker::Start(4), becauseStart()takes an explicit worker count here.fix(governance): don't vote NO-FUNDING on triggers before their maturity window(5d9fe74) andfix(governance): don't vote on triggers while not synced(bd948aa). Clean.fix(bls): weight batch-verified signatures so complementary errors cannot cancel(593f252).BatchVerifyMessageSigslives insrc/llmq/net_dkg.cppon this branch, so its hunk is applied there unchanged.fix(llmq): weight batched DKG contribution verification(12bbd21). The newbls_testsloop usesutil::irange.fix(bls): stop weighted contribution batch verification on an invalid partial aggregate(95fbf22). Clean.Each adaptation is described in its commit message. They are the same as in the develop merge-back, #8015. The files these fixes touch are identical on
developandv24.0.x, apart from two unrelated lines.Not picked:
fix(instantsend): propagate !cs_timingsTxSeen to the callers of RemoveNonLockedTx(3174d5f). It fixes a clang-Werrorbreak that exists only on v23.1.x. The methods it annotates don't exist on this branch, which builds cleanly with-Werrorand-Wthread-safety.Merge of v23.1.10
The last commit merges the
v23.1.10tag with the tree unchanged (-s ours), as #7965 did for v23.1.9. That letsmasterfast-forward tov24.0.x, and the final v24.0.0 contains every v23.1.10 commit. A trial real merge on top of the picks staged no clean additions. Everything else it produced was release bookkeeping or conflicts re-inserting code the picks already carry.Release notes
The v24.0.0 notes include only what a v24 operator needs to act on or plan for: required action, configuration, compatibility, upgrade behaviour and changed operational expectations. They no longer carry patch releases forward one by one. I checked the v23.1.1 to v23.1.10 notes against that rule. Two doc commits follow the merge:
-allowprivatenetno longer affects masternode address validation (fix(consensus): canonical ExtNetInfo encoding and strict netInfo decoding #7888).Routine bug, crash and DoS fixes, GUI, build and performance changes, and fixes that need no operator action (including the superblock trigger voting fix, the negative-
nVersionrule and the extra work of weighted BLS batch verification, for which no node-wide CPU impact has been measured) are left out. The archived v23.1.10 notes, the older-releases list and the change-log link stay with v24.0.0 release preparation.No version change is needed.
configure.acstays at 24.0.0 withCLIENT_VERSION_IS_RELEASEfalse.How Has This Been Tested?
macOS arm64 against depends,
--enable-debug --enable-werror --disable-hardening --without-gui.dashd,test_dashandbench_dashbuild cleanly at the tip.dashdandtest_dashalso build at every pick.bls_tests,llmq_dkg_tests,llmq_utils_tests,governance_inv_tests,governance_validators_tests.feature_governance.py(with the new early-trigger case),feature_governance_cl.py,feature_llmq_simplepose.py(default and--disable-spork23, both with the new cloned-contribution case),feature_llmq_dkgerrors.py,feature_llmq_signing.py(default and--spork21),feature_llmq_data_recovery.py,feature_llmq_rotation.py.v23.1.9..v23.1.10, and each carries the same added and removed lines. The only exceptions are the adaptations listed above and the skipped instantsend commit.I did not run the full functional suite locally; CI covers it.
Breaking Changes
None. These are not consensus changes.
Checklist:
🤖 Generated with Claude Code