Skip to content

fix: bound pending DKG message retention per proTxHash - #7583

Merged
PastaPastaPasta merged 2 commits into
dashpay:developfrom
PastaPastaPasta:fix/dkg-per-protx-pending-bound
Aug 12, 2026
Merged

PastaPastaPasta merged 2 commits into
dashpay:developfrom
PastaPastaPasta:fix/dkg-per-protx-pending-bound

Conversation

@PastaPastaPasta

Copy link
Copy Markdown
Member

Issue being fixed or feature implemented

The DKG pending-message retention quota in CDKGPendingMessages is keyed by NodeId, which resets on every reconnect. A single masternode identity can therefore retain an effectively unbounded number of pending DKG messages on a victim masternode by reconnecting between sends.

This extracts the DoS-relevant part of #7557 so it can ship in the release; the deserialize-once/framing intake redesign remains in #7557 for post-RC.

What was done?

  • Rekeyed the retention quota to the sender's MNAuth-verified proTxHash (locally produced messages charge this node's own proTxHash). The quota is cumulative for the round and not refunded on pop, so neither reconnects nor queue drains regain retention slots. The MNAuth gate pins quota keys to registered masternode identities.
  • The duplicate-hash check now runs before the quota charge so resent hashes don't burn budget, and quota-dropped hashes are not marked seen, so another peer with remaining budget can re-deliver the message.
  • PopAndDeserializeMessages and the framing/deserialize-once changes from perf: deserialize DKG messages once (framing-only intake) #7557 are intentionally not included.

How Has This Been Tested?

  • New unit tests in llmq_dkg_tests: quota survives NodeId changes, quota is per-proTx, duplicates are rejected before charging the quota, own-message enqueue path shares the quota.
  • Extended feature_llmq_dkg_intake.py with a late-message scenario: a reconnecting identity cannot exceed its budget across 2*llmq_size fresh NodeIds, over-quota drops are silent (banscore 0), a distinct proTx has its own budget, and round-start clearing discards retained messages without scoring.
  • make -j5, test_dash --run_test=llmq_dkg_tests, test_runner.py feature_llmq_dkg_intake.py, lint-python, lint-whitespace.

Breaking Changes

None. Honest masternodes send one message per type per round, far under the unchanged size * 2 budget; the only behavior change is that a reconnecting identity can no longer refill its retention quota.

Checklist:

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

The DKG pending-message retention quota was keyed by NodeId, which resets on reconnect: a single masternode identity could retain an unbounded number of pending messages across reconnects. Key the quota by the sender's proTxHash instead, which survives reconnects and is pinned to registered masternode identities by the MNAuth gate (develop already rejects pushed DKG messages from peers without a verified proRegTxHash). The quota is cumulative for the round and not refunded on pop, so draining the queue does not regain retention slots.

Also check the duplicate-hash set before charging the quota so resent hashes don't burn budget, and do not mark a quota-dropped hash as seen so another peer with remaining budget can re-deliver it. Locally produced messages (from=-1) are enqueued under this node's own proTxHash and charged like any other sender's.

Extracted from the DKG intake redesign in dashpay#7557; the deserialize-once/framing changes there are deliberately not included.
@thepastaclaw

thepastaclaw commented Aug 12, 2026 •

Copy link
Copy Markdown
Collaborator

🔍 Review in progress — actively reviewing now (commit cf85283)
Stage: Codex precheck starting
ETA: complete ~17:12 UTC (median 17m across 30 recent reviews)
Running 4m · Last checked: 2026-08-12 17:00 UTC

@coderabbitai

coderabbitai Bot commented Aug 12, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8f263fe9-c5de-4931-a1d7-9f967283628e

📥 Commits

Reviewing files that changed from the base of the PR and between 5e46608 and cf85283.

📒 Files selected for processing (1)
  • src/test/llmq_dkg_tests.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/test/llmq_dkg_tests.cpp

Walkthrough

CDKGPendingMessages now enforces cumulative retention quotas per sender proTxHash. It rejects duplicate hashes before quota accounting and clears per-sender counters during cleanup. DKG paths pass proTxHash values for locally generated and authenticated peer messages. Unit and functional tests cover reconnects, independent identities, duplicate messages, queue draining, and round resets.

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

Sequence Diagram(s)

sequenceDiagram
  participant DKGPeer
  participant ProcessMessage
  participant CDKGPendingMessages
  participant DKGSession
  DKGPeer->>ProcessMessage: send authenticated DKG message
  ProcessMessage->>CDKGPendingMessages: enqueue payload with sender proTxHash
  DKGSession->>CDKGPendingMessages: enqueue locally generated message with session proTxHash
  CDKGPendingMessages->>CDKGPendingMessages: reject duplicate or over-quota hash
  CDKGPendingMessages-->>DKGSession: retain accepted message
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: limiting pending DKG message retention per sender proTxHash.
Description check ✅ Passed The description directly explains the proTxHash-based quota change, its purpose, implementation details, and test coverage.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/llmq/dkgsessionhandler.cpp`:
- Around line 40-44: Update the sender_protx validation in the DKG session
handler to use a direct null check and return immediately for a null identity,
avoiding Assume/assertion_fail abort behavior. Add a unit test covering a null
sender_protx and confirming the request is dropped without aborting.

In `@src/test/llmq_dkg_tests.cpp`:
- Around line 109-114: Add a third sender with available quota in the
pending-message test, submit hash5 through that sender after confirming
protx_b’s quota rejection, and assert that hash5 is retained/enqueued. Update
the expected PopPendingMessages(5) count to include the successfully accepted
redelivery while preserving the existing HasSeen assertions for the rejected
attempt.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 89d4a6d7-efef-4f73-a157-b36a14eff395

📥 Commits

Reviewing files that changed from the base of the PR and between c30e901 and 5e46608.

📒 Files selected for processing (5)
  • src/llmq/dkgsessionhandler.cpp
  • src/llmq/dkgsessionhandler.h
  • src/llmq/net_dkg.cpp
  • src/test/llmq_dkg_tests.cpp
  • test/functional/feature_llmq_dkg_intake.py

Comment thread src/llmq/dkgsessionhandler.cpp Resolved
Comment thread src/test/llmq_dkg_tests.cpp Resolved

@PastaPastaPasta PastaPastaPasta left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

LGTM; this simplifies the remaining / other PR

Unit tests: the quota is charged per proTxHash (surviving NodeId changes and not refunded on pop), duplicates are rejected before charging the quota, distinct proTxHashes have independent budgets, and locally produced messages share the quota path.

Functional test: extend feature_llmq_dkg_intake.py with a late-message scenario proving that a masternode identity reconnecting under fresh NodeIds cannot retain more than maxMessagesPerProTx contributions, that quota drops are silent (banscore stays 0), that a distinct proTx keeps its own budget, and that round-start clearing discards retained messages without them ever reaching a worker.

Ported from dashpay#7557, adapted to the pre-framing intake (well-formed zero-BLS payloads instead of BLS-invalid ones, since develop still deserializes a copy at intake).
@PastaPastaPasta
PastaPastaPasta force-pushed the fix/dkg-per-protx-pending-bound branch from 5e46608 to cf85283 Compare August 12, 2026 15:03
@PastaPastaPasta
PastaPastaPasta merged commit 856ce1d into dashpay:develop Aug 12, 2026
47 of 50 checks passed
@UdjinM6 UdjinM6 added this to the 24 milestone Aug 12, 2026
PastaPastaPasta added a commit that referenced this pull request Oct 29, 2026
cf85283 test: cover per-proTx DKG pending-message quotas (pasta)
4b7e21f fix: bound pending DKG message retention per proTxHash (pasta)

Pull request description:

  ## Issue being fixed or feature implemented

  The DKG pending-message retention quota in `CDKGPendingMessages` is keyed by NodeId, which resets on every reconnect. A single masternode identity can therefore retain an effectively unbounded number of pending DKG messages on a victim masternode by reconnecting between sends.

  This extracts the DoS-relevant part of #7557 so it can ship in the release; the deserialize-once/framing intake redesign remains in #7557 for post-RC.

  ## What was done?

  - Rekeyed the retention quota to the sender's MNAuth-verified proTxHash (locally produced messages charge this node's own proTxHash). The quota is cumulative for the round and not refunded on pop, so neither reconnects nor queue drains regain retention slots. The MNAuth gate pins quota keys to registered masternode identities.
  - The duplicate-hash check now runs before the quota charge so resent hashes don't burn budget, and quota-dropped hashes are not marked seen, so another peer with remaining budget can re-deliver the message.
  - `PopAndDeserializeMessages` and the framing/deserialize-once changes from #7557 are intentionally not included.

  ## How Has This Been Tested?

  - New unit tests in `llmq_dkg_tests`: quota survives NodeId changes, quota is per-proTx, duplicates are rejected before charging the quota, own-message enqueue path shares the quota.
  - Extended `feature_llmq_dkg_intake.py` with a late-message scenario: a reconnecting identity cannot exceed its budget across `2*llmq_size` fresh NodeIds, over-quota drops are silent (banscore 0), a distinct proTx has its own budget, and round-start clearing discards retained messages without scoring.
  - `make -j5`, `test_dash --run_test=llmq_dkg_tests`, `test_runner.py feature_llmq_dkg_intake.py`, `lint-python`, `lint-whitespace`.

  ## Breaking Changes

  None. Honest masternodes send one message per type per round, far under the unchanged `size * 2` budget; the only behavior change is that a reconnecting identity can no longer refill its retention quota.

  ## 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)_

Top commit has no ACKs.

Tree-SHA512: cf881cef9465b8b37f31b9630c937d35c40024ed86771d20d717b4a09a06b9c2467662d05c50a8b4a1f92b96d62dceafe565478eafb6f35610f2f994ef6e2ce0

v23.1.x adaptation: develop routes the quota through src/llmq/net_dkg.cpp
(NetDKG/EnqueueOwn), which does not exist in v23.1.x. The fix is re-threaded
through the v23 intake path instead: the sender proTxHash flows through
CDKGSessionHandler::ProcessMessage, CDKGSessionManager, and the own-message
path in active/dkgsession.cpp, with CDKGPendingMessages keyed by proTxHash.
The develop-only net_dkg.cpp change is dropped (file absent on v23.1.x) and the
feature_llmq_dkg_intake.py functional-test extension is not included; the three
upstream unit tests are ported to the v23 interface in llmq_dkg_tests.cpp.
PastaPastaPasta added a commit that referenced this pull request Oct 29, 2026
71b3f0c Merge #7556: fix: gate BLS vote-signature failure log behind gobject category (pasta)
14dddb1 Merge #7838: fix(wallet): authenticate unlock deadline extensions (pasta)
5f73355 Merge #7858: fix: do not InstantSend-lock provider transactions (pasta)
0d2e579 fix(evo): keep -allowprivatenet out of masternode address validation (Konstantin Akimov)
b4cd3fa fix(evo): only write credit pool snapshots inside a block-scoped EvoDB transaction (#7549) (pasta)
d215280 Merge #7846: fix: treat pending owner keys and Platform node IDs as one conflict namespace (pasta)
db9ec2b Merge #7826: fix: skip repeated authenticated Platform bans (pasta)
dcc9596 Merge #7867: fix: refuse masternode list RPCs for blocks outside the active chain (pasta)
197b14a Merge #7866: fix: stop `masternode payments` at the genesis block (pasta)
3055a19 Merge #7850: perf: let CHECKDATASIG use the signature cache (pasta)
4da0099 Merge #7821: fix: relock encrypted wallets after HD upgrade failures (pasta)
c7393c0 Merge #7833: fix: gate signing intake before quorum retrieval (pasta)
fdf40fe Merge #7828: fix: omit recovery words from invalid mnemonic errors (pasta)
8888af8 Merge #7753: fix: bound masternode list caches by size instead of by new block (pasta)
457af6d Merge #7738: refactor: avoid double lookups and default-inserting map access in governance (pasta)
be3b55e Merge #7662: fix: isolate missing quorums in InstantSend batches (pasta)
9bc1e77 Merge #7489: fix: conflict ProRegTx collateral reuse with in-mempool masternode updates (pasta)
5278ce8 Merge #7583: fix: bound pending DKG message retention per proTxHash (pasta)
9ebf78f Merge #7522: fix(net): bound verified inbound connections per proRegTxHash (pasta)
9f2946e Merge #7487: fix: bound DomainPort string deserialization and stop swallowing deser failures (pasta)
c077d32 Merge #7527: fix: require a valid masternode signature before caching an orphan governance vote (pasta)
dfa7db1 Merge #7518: fix: memoize governance vote signature checks to bound govsync cost (pasta)
bb09c49 Merge #7532: fix: bound DYNBITSET allocation against remaining stream size (pasta)
9739f35 Merge #7488: fix: validate ProUpServTx nType at mempool acceptance (pasta)
4687550 Merge #7385: fix: reject invalid extended masternode net info (Pasta)

Pull request description:

  ## Issue being fixed or feature implemented

  Backports for v23.1.9, on top of current `v23.1.x`. The base already carries the
  negative-nVersion consensus fix (#2), the invalid-DKG-share fix (#4), the
  per-sender sig-share scoping (#5), the individual DKG contribution verification
  (#7) and the provider-decode/orphan-cap hardening (#8), so those are not part of
  this diff.

  ## What was done?

  Cherry-picked (`git cherry-pick -m1 -x`) the develop merges below into `v23.1.x`,
  in develop merge order. Where a develop merge carried develop-only refactors or
  APIs absent on `v23.1.x`, only the fix was taken and the adaptation is noted in
  that commit's message.

  ### Included (cherry-picks of merged dashpay/dash PRs)

  - #7385 — `fix: reject invalid extended masternode net info`
  - #7488 — `fix: validate ProUpServTx nType at mempool acceptance`
  - #7532 — `fix: bound DYNBITSET allocation against remaining stream size`
  - #7518 — `fix: memoize governance vote signature checks to bound govsync cost`
  - #7527 — `fix: require a valid masternode signature before caching an orphan governance vote`
  - #7487 — `fix: bound DomainPort string deserialization and stop swallowing deser failures`
  - #7522 — `fix(net): bound verified inbound connections per proRegTxHash`
  - #7583 — `fix: bound pending DKG message retention per proTxHash`
  - #7489 — `fix: conflict ProRegTx collateral reuse with in-mempool masternode updates`
  - #7662 — `fix: isolate missing quorums in InstantSend batches`
  - #7738 — `fix: do not record a masternode vote before it is validated`
  - #7753 — `fix: bound masternode list caches by size instead of by new block`
  - #7828 — `fix: omit recovery words from invalid mnemonic errors`
  - #7833 — `fix: gate signing intake before quorum retrieval`
  - #7821 — `fix: relock encrypted wallets after HD upgrade failures`
  - #7850 — `perf: let CHECKDATASIG use the signature cache`
  - #7866 — `fix: stop 'masternode payments' at the genesis block`
  - #7867 — `fix: refuse masternode list RPCs for blocks outside the active chain`
  - #7826 — `fix: skip repeated authenticated Platform bans`
  - #7846 — `fix: treat pending owner keys and Platform node IDs as one conflict namespace`
  - #7858 — `fix: do not InstantSend-lock provider transactions` (with its functional test, ported to v23.1.x)
  - #7838 — `fix(wallet): authenticate unlock deadline extensions`
  - #7556 — `fix: gate BLS vote-signature failure log behind gobject category`

  ### Upstream-derived changes with no clean develop merge (own commits)

  - `fix(evo): only write credit pool snapshots inside a block-scoped EvoDB transaction (#7549)` — the develop fix landed inside #7639, which is not on this branch, so it is hand-ported (EvoDB transaction-depth counter + `HasActiveTransaction()` gate); develop commits `8503c5d211`/`0325195ecb` cited in the commit.
  - `fix(evo): keep -allowprivatenet out of masternode address validation` — the consensus portion of #7888 only (develop commit `e3e780801b`); the V24-gated ExtNetInfo parts are excluded.

  ### Adaptation notes
  - #7556 pairs with #7527 (already here): once orphan governance votes must pass `IsValid` before caching, a peer with a valid masternode outpoint and an invalid BLS signature can reach the BLS vote-signature failure log, so that log (and two sibling `ProcessVote` logs) is gated behind `BCLog::GOBJECT`.
  - #7662: the unit test calls `SetMinSporkKeys(1)`; v23.1.x defaults the spork signer threshold to `INT_MAX`, so without it `SPORK_2_INSTANTSEND_ENABLED` never activates and the regression check would be vacuous.
  - #7858 is taken with its functional test ported to v23.1.x: `wait_for_instantlock` is `(txid, node)` here and does not sync/bump mocktime itself, and develop's `DashTestFramework.bury_tx` does not exist, so the waits sync+bump per node and the bury step is inlined. `IsProviderTx` lists only the four v23.1.x provider special-tx types (the v24-only `DISSOLVE`/`UPDATE_SHARE`/`UPDATE_SHARED_REGISTRAR` are omitted).
  - #7385/#7488/#7489/#7846: kept v23.1.x `ProTxVersion::ExtAddr` equality and the atomic-pointer `dmnman`; dropped develop-only test setups and the shared-masternode owner-key branch.
  - #7527/#7738: v23.1.x governance lookup/`ProcessVote` shapes; develop-only refactors excluded.
  - #7583: re-threaded through `active/dkgsession.*`; #7867: `insert_or_assign`; #7821: `interfaces::MakeHandler`; #7753: dropped the `<atomic>` include/`DoMaintenance` test calls.

  ### Not included
  - #7822 (`fix: reject malformed detached quorum share batches`) — its one-line change (record the node in `badSources` rather than `BanNode`) is already present in the base via #5, so the pick is redundant and was dropped.
  - #7895 (`test: cover retroactive provider tx refusal`) — its functional test needs the single-member-quorum harness absent on `v23.1.x`.
  - Compact-block extra-tx fix (bitcoin bitcoin#35670) — dropped from this release as a latent, very-hard-to-trigger crash; opened as its own PR against `develop`.
  - #7861 (pruned-node historical-block reject) — deferred; also missing from `v24.0.0-rc.4` (should land in v24.0.0 final).
  - Develop-only regressions and V24-gated changes that do not affect `v23.1.x`.

  ## How Has This Been Tested?

  Rebased onto current `v23.1.x`; the only merge conflicts were the additive test appends in `src/test/evo_netinfo_tests.cpp` and `src/test/governance_inv_tests.cpp` (kept both the base's and the picks' cases). Assembled branch builds with `--enable-werror` (`dashd` + `test_dash`). The full `test_dash` unit suite passes, and `p2p_instantsend.py` (including the restored `test_provider_tx_not_locked`) passes. Each pick's changed lines were compared with its develop merge; differences are the adaptations above.

  ## Breaking Changes

  - #7858: provider transactions no longer receive InstantSend locks. Software that waited for an islock on a ProTx should wait for a ChainLock or confirmations instead.
  - #7828: invalid-mnemonic RPC errors no longer echo the mnemonic. #7821: encrypted wallets relock after a failed HD upgrade. #7867: `protx info/listdiff` and `masternode payments` error for blocks off the active chain. #7489/#7488: provider transactions with reused collateral or an invalid type are rejected at mempool acceptance. #7888: the RFC1918 masternode-address check applies even with `-allowprivatenet`. #7838: a wallet unlock deadline can no longer be extended without re-authenticating.

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

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

  <!-- This is an auto-generated comment: release notes by coderabbit.ai -->

  <!-- coderabbit-ai-generated:v1 -->

  ## Summary by CodeRabbit

  * **Behavior Changes**
    * Provider transactions no longer receive InstantSend locks. They are mined after 10 minutes in the mempool and then protected by a ChainLock; transactions spending their outputs can be locked once mined. Use ChainLocks or confirmations instead of waiting for an InstantSend lock on a provider transaction.
  * **Bug Fixes**
    * Masternode RPCs now reject blocks outside the active chain.
    * Wallet unlock attempts with an incorrect passphrase no longer alter an existing unlock period.
    * Improved validation and handling of governance votes, network addresses, and masternode connections.

  <!-- end of auto-generated comment: release notes by coderabbit.ai -->

Top commit has no ACKs.

Tree-SHA512: d48eaae945bcbad443cf3280c08d2ed00f06816bb09ef70f85fca497a864b89e502d09fb3c9c977f8d0a74d761dc3c29f3b11e217cc7e29c43d1ee691d4df572
PastaPastaPasta added a commit that referenced this pull request Oct 29, 2026
4722ad9 chore: prepare v23.1.9 release (pasta)
71b3f0c Merge #7556: fix: gate BLS vote-signature failure log behind gobject category (pasta)
14dddb1 Merge #7838: fix(wallet): authenticate unlock deadline extensions (pasta)
5f73355 Merge #7858: fix: do not InstantSend-lock provider transactions (pasta)
0d2e579 fix(evo): keep -allowprivatenet out of masternode address validation (Konstantin Akimov)
b4cd3fa fix(evo): only write credit pool snapshots inside a block-scoped EvoDB transaction (#7549) (pasta)
d215280 Merge #7846: fix: treat pending owner keys and Platform node IDs as one conflict namespace (pasta)
db9ec2b Merge #7826: fix: skip repeated authenticated Platform bans (pasta)
dcc9596 Merge #7867: fix: refuse masternode list RPCs for blocks outside the active chain (pasta)
197b14a Merge #7866: fix: stop `masternode payments` at the genesis block (pasta)
3055a19 Merge #7850: perf: let CHECKDATASIG use the signature cache (pasta)
4da0099 Merge #7821: fix: relock encrypted wallets after HD upgrade failures (pasta)
c7393c0 Merge #7833: fix: gate signing intake before quorum retrieval (pasta)
fdf40fe Merge #7828: fix: omit recovery words from invalid mnemonic errors (pasta)
8888af8 Merge #7753: fix: bound masternode list caches by size instead of by new block (pasta)
457af6d Merge #7738: refactor: avoid double lookups and default-inserting map access in governance (pasta)
be3b55e Merge #7662: fix: isolate missing quorums in InstantSend batches (pasta)
9bc1e77 Merge #7489: fix: conflict ProRegTx collateral reuse with in-mempool masternode updates (pasta)
5278ce8 Merge #7583: fix: bound pending DKG message retention per proTxHash (pasta)
9ebf78f Merge #7522: fix(net): bound verified inbound connections per proRegTxHash (pasta)
9f2946e Merge #7487: fix: bound DomainPort string deserialization and stop swallowing deser failures (pasta)
c077d32 Merge #7527: fix: require a valid masternode signature before caching an orphan governance vote (pasta)
dfa7db1 Merge #7518: fix: memoize governance vote signature checks to bound govsync cost (pasta)
bb09c49 Merge #7532: fix: bound DYNBITSET allocation against remaining stream size (pasta)
9739f35 Merge #7488: fix: validate ProUpServTx nType at mempool acceptance (pasta)
4687550 Merge #7385: fix: reject invalid extended masternode net info (Pasta)
58b9bce fix: keep flat-file positions 64-bit clean at the remaining raw seek/tell sites (Konstantin Akimov)
7a21752 Merge bitcoin#36667: net: enforce timeout interval for paused send peers (Konstantin Akimov)
58abb28 Merge bitcoin#36665: sock: align exception monitoring between poll() and select() backends (Konstantin Akimov)
8030deb Merge bitcoin#36669: refactor: store positions as a 64 bits integer in flatfiles (Konstantin Akimov)
b1178f9 fix(llmq): judge every sig-share sender by its own verified shares (pasta)
33ec74e fix(governance): cap the orphan parent objects re-requested per tick (pasta)
7fc0edb doc(rpc): mark provider payload network fields optional in the help (pasta)
2aa3a87 fix(rpc): don't dereference a missing netInfo when decoding provider payloads (pasta)
120b5db fix(llmq): verify DKG contributions individually when a batch vvec check fails (pasta)
808b105 Merge #7486: fix: guard VerifyContributionShares against an empty verification-vector set (pasta)
8351db9 fix(llmq): reject invalid decrypted DKG shares before aggregation (pasta)
07494dd fix(consensus): reject negative transaction nVersion after DIP3 (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  Merges the v23.1.9 release back into develop. v23.1.9 ships five security fixes and three Bitcoin Core network/storage backports that develop doesn't have yet. Without this merge, develop-based nodes (v24) stay exposed to all of them.

  Note on the base: `master` is still at 728f505 (v23.1.8 + crash fixes) and has not been fast-forwarded to the release yet. It is an ancestor of the signed `v23.1.9` tag, so this PR merges the tag commit (67c106f). That gives exactly the result of merging `master` once it is fast-forwarded.

  ## What was done?
  One merge commit. Most of v23.1.9 is backports of develop PRs that develop already has (#7385, #7486, #7487, #7488, #7489, #7518, #7522, #7527, #7532, #7556, #7583, #7662, #7738, #7753, #7821, #7826, #7828, #7833, #7838, #7846, #7850, #7858, #7866, #7867, and the `-allowprivatenet` fix from #7888). Those conflicts resolve to develop's code. The new payload, adapted to develop's layout:

  - **Sig-share sender aliasing** (`fix(llmq): judge every sig-share sender by its own verified shares`). The batch verifier is now keyed by `(NodeId, SigShareKey)`, so two peers pushing the same key can't be judged by each other's signature. Ported into develop's `NetSigning::ProcessPendingSigShares` (`net_signing.cpp`). The second half of the v23 fix (malformed share → `badSources`) was already on develop.
  - **Negative tx `nVersion`** (`fix(consensus): reject negative transaction nVersion after DIP3`). Applied as-is, plus `PackVersionAndType()` at all three pack sites.
  - **Invalid decrypted DKG shares** (`fix(llmq): reject invalid decrypted DKG shares before aggregation`). The QDATA check lived in `src/active/quorums.cpp`, which develop deleted; it is ported into `NetQuorum::ProcessContribQDATA` (`llmq/net_quorum.cpp`). The new `feature_llmq_dkgerrors.py` case passes numeric `dkgsimerror` rates, which develop's RPC requires.
  - **Batch vvec fallback** (`fix(llmq): verify DKG contributions individually when a batch vvec check fails`). The new `bls_tests` case calls develop's `CBLSWorker::Start(4)`, which takes an explicit worker count.
  - **Orphan governance re-request cap** (`fix(governance): cap the orphan parent objects re-requested per tick`). At most 100 orphan parents are re-requested per tick. The test helper uses develop's `governance::OrphanVote`.
  - **bitcoin#36665, bitcoin#36667**. Applied cleanly.
  - **bitcoin#36669**. Develop moved `TruncateFile`/`AllocateFileRange` into `util/fs_helpers.{h,cpp}`, so those hunks, `FileSeek64` and `FileTell64` land there.
  - **64-bit flat-file positions** (`fix: keep flat-file positions 64-bit clean at the remaining raw seek/tell sites`). Covers `txindex.cpp` and `blockstorage.cpp`, which now include `util/fs_helpers.h`. It also covers develop-only `BlockFilterIndex::LookupFilterRange` (from #7574's file-handle reuse), which seeks on a `FlatFilePos` the same way.
  - **Release bookkeeping**: v23.1.9 release notes, archived 23.1.8 notes, flatpak release entry and regenerated man pages. `configure.ac` keeps develop's 24.1.0 / `IS_RELEASE=false`.

  Deliberately resolved to develop's side:
  - `fix(rpc): don't dereference a missing netInfo when decoding provider payloads` and its help-text follow-up. On develop an unknown-version ProTx payload reaches `netInfo` only through `CHECK_NONFATAL`, so `decoderawtransaction` returns an RPC error instead of dereferencing null; the v23 commit message says this is the develop equivalent. Reporting the payload without network fields (as v23.1.9 now does) would be a nicer develop follow-up, but it doesn't belong in this merge.
  - `fix(evo): only write credit pool snapshots inside a block-scoped EvoDB transaction (#7549)`. This is a v23 hand-port of develop's own 8503c5d / 0325195.
  - `src/active/quorums.cpp` stays deleted.

  ## How Has This Been Tested?
  Linux x86_64, `--enable-debug --enable-werror`, built `dashd` and `test_dash`.

  - Unit suites, all passing: `llmq_utils_tests`, `transaction_tests`, `txvalidation_tests`, `sighash_tests`, `bls_tests`, `governance_inv_tests`, `llmq_dkg_tests`, `flatfile_tests`, `fs_tests`, `net_tests`, `sock_tests`, `evo_netinfo_tests`, `evo_assetlocks_tests`, `blockfilter_index_tests`, `txindex_tests`, `blockmanager_tests`.
  - Functional tests, all passing: `feature_llmq_dkgerrors.py` (including the new contribution-invalid case), `feature_llmq_signing.py` (both variants), `p2p_timeouts.py` (v1/v2), `mempool_accept.py`.
  - `git diff --remerge-diff` is empty on all eight v23.1.9 merge commits, so the release has no hidden merge-only changes.
  - An independent review compared each port with its v23.1.9 commit and checked develop for second copies of each bug: other `SigShareKey` batch verifiers, other decrypt-then-aggregate sites, other `fPauseSend` setters and other raw seeks on `FlatFilePos`. It also spot-checked that the PRs treated as "already on develop" have equivalent code there.

  I did not run the full functional suite locally; CI covers it.

  ## Breaking Changes
  Consensus: once DIP3 is active, transactions with a negative `nVersion` are rejected (`bad-txns-version`). Every v23.1.9 node already enforces this. Such transactions were already split-inducing, since a re-serialized copy changes `nType`.

  ## 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
  - [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)

ACKs for top commit:
  UdjinM6:
    utACK 257af26

Tree-SHA512: d6d19ecbfaa0a891b09887169ec2709a7dfde06af022203ca28e80fdf009ca360f2704ebadca1f830a97663dac09da78d05c979d9c30ee91780065550070b954
PastaPastaPasta added a commit that referenced this pull request Oct 29, 2026
…xes, #7947)

4722ad9 chore: prepare v23.1.9 release (pasta)
71b3f0c Merge #7556: fix: gate BLS vote-signature failure log behind gobject category (pasta)
14dddb1 Merge #7838: fix(wallet): authenticate unlock deadline extensions (pasta)
5f73355 Merge #7858: fix: do not InstantSend-lock provider transactions (pasta)
0d2e579 fix(evo): keep -allowprivatenet out of masternode address validation (Konstantin Akimov)
b4cd3fa fix(evo): only write credit pool snapshots inside a block-scoped EvoDB transaction (#7549) (pasta)
d215280 Merge #7846: fix: treat pending owner keys and Platform node IDs as one conflict namespace (pasta)
db9ec2b Merge #7826: fix: skip repeated authenticated Platform bans (pasta)
dcc9596 Merge #7867: fix: refuse masternode list RPCs for blocks outside the active chain (pasta)
197b14a Merge #7866: fix: stop `masternode payments` at the genesis block (pasta)
3055a19 Merge #7850: perf: let CHECKDATASIG use the signature cache (pasta)
4da0099 Merge #7821: fix: relock encrypted wallets after HD upgrade failures (pasta)
c7393c0 Merge #7833: fix: gate signing intake before quorum retrieval (pasta)
fdf40fe Merge #7828: fix: omit recovery words from invalid mnemonic errors (pasta)
8888af8 Merge #7753: fix: bound masternode list caches by size instead of by new block (pasta)
457af6d Merge #7738: refactor: avoid double lookups and default-inserting map access in governance (pasta)
be3b55e Merge #7662: fix: isolate missing quorums in InstantSend batches (pasta)
9bc1e77 Merge #7489: fix: conflict ProRegTx collateral reuse with in-mempool masternode updates (pasta)
5278ce8 Merge #7583: fix: bound pending DKG message retention per proTxHash (pasta)
9ebf78f Merge #7522: fix(net): bound verified inbound connections per proRegTxHash (pasta)
9f2946e Merge #7487: fix: bound DomainPort string deserialization and stop swallowing deser failures (pasta)
c077d32 Merge #7527: fix: require a valid masternode signature before caching an orphan governance vote (pasta)
dfa7db1 Merge #7518: fix: memoize governance vote signature checks to bound govsync cost (pasta)
bb09c49 Merge #7532: fix: bound DYNBITSET allocation against remaining stream size (pasta)
9739f35 Merge #7488: fix: validate ProUpServTx nType at mempool acceptance (pasta)
4687550 Merge #7385: fix: reject invalid extended masternode net info (Pasta)
58b9bce fix: keep flat-file positions 64-bit clean at the remaining raw seek/tell sites (Konstantin Akimov)
7a21752 Merge bitcoin#36667: net: enforce timeout interval for paused send peers (Konstantin Akimov)
58abb28 Merge bitcoin#36665: sock: align exception monitoring between poll() and select() backends (Konstantin Akimov)
8030deb Merge bitcoin#36669: refactor: store positions as a 64 bits integer in flatfiles (Konstantin Akimov)
b1178f9 fix(llmq): judge every sig-share sender by its own verified shares (pasta)
33ec74e fix(governance): cap the orphan parent objects re-requested per tick (pasta)
7fc0edb doc(rpc): mark provider payload network fields optional in the help (pasta)
2aa3a87 fix(rpc): don't dereference a missing netInfo when decoding provider payloads (pasta)
120b5db fix(llmq): verify DKG contributions individually when a batch vvec check fails (pasta)
808b105 Merge #7486: fix: guard VerifyContributionShares against an empty verification-vector set (pasta)
8351db9 fix(llmq): reject invalid decrypted DKG shares before aggregation (pasta)
07494dd fix(consensus): reject negative transaction nVersion after DIP3 (pasta)
11ea306 Merge #7947: fix: stop quorum key warming from stalling DKG on 2-core masternodes (pasta)
25d7c85 Merge #7826: fix: skip repeated authenticated Platform bans (pasta)
18073dd Merge #7850: perf: let CHECKDATASIG use the signature cache (pasta)
3759397 Merge #7833: fix: gate signing intake before quorum retrieval (pasta)
db98ee2 fix: keep flat-file positions 64-bit clean at the remaining raw seek/tell sites (Konstantin Akimov)
54c9738 Merge bitcoin#36667: net: enforce timeout interval for paused send peers (Konstantin Akimov)
f1adb2c Merge bitcoin#36665: sock: align exception monitoring between poll() and select() backends (Konstantin Akimov)
8becf6a Merge bitcoin#36669: refactor: store positions as a 64 bits integer in flatfiles (Konstantin Akimov)
4c0cbd6 fix(llmq): judge every sig-share sender by its own verified shares (pasta)
649259f fix(governance): cap the orphan parent objects re-requested per tick (pasta)
418f5ee fix(llmq): verify DKG contributions individually when a batch vvec check fails (pasta)
abf2eb0 fix(llmq): reject invalid decrypted DKG shares before aggregation (pasta)
39bda8f fix(consensus): reject negative transaction nVersion after DIP3 (pasta)

Pull request description:

  ## Issue being fixed or feature implemented

  Backports for v24.0.0-rc.5, on top of `v24.0.x` at the v24.0.0-rc.4 tag (0e7382d, #7917). This PR holds the must-haves for rc.5: the v23.1.9 fixes that v24.0.x does not have yet, and #7947 (DKG stalls that get 2-core masternodes PoSe-banned). Without the v23.1.9 part, rc.5 would be missing fixes that the already published v23.1.9 ships, and it would accept transactions that v23.1.9 nodes reject (see Breaking Changes).

  The remaining `backport-candidate-24.0.x` work (#7916 and its test prerequisite) will follow in a separate PR so this one stays reviewable.

  ## What was done?

  ### v23.1.9 fixes not yet on v24.0.x

  v23.1.9 (tag 67c106f) contains 46 commits since v23.1.8. Each one was checked by content against `v24.0.x`. Most are backports of develop PRs that `v24.0.x` already has, either from before its branch point or picked for rc.3/rc.4. The rest are picked here with `git cherry-pick -x` from `v23.1.x`, in v23.1.9 merge order:

  - `fix(consensus): reject negative transaction nVersion after DIP3` (07494dd), **consensus**. Context-only conflict in `primitives/transaction.h`.
  - `fix(llmq): reject invalid decrypted DKG shares before aggregation` (8351db9). The QDATA path that v23 has in `src/active/quorums.cpp` is `src/llmq/net_quorum.cpp` on this branch. The new `feature_llmq_dkgerrors.py` case passes numeric `dkgsimerror` rates.
  - `fix(llmq): verify DKG contributions individually when a batch vvec check fails` (120b5db). The new `bls_tests` case calls `CBLSWorker::Start(4)`, since `Start()` takes an explicit worker count here.
  - `fix(governance): cap the orphan parent objects re-requested per tick` (33ec74e). The test helper uses this branch's `governance::OrphanVote`.
  - `fix(llmq): judge every sig-share sender by its own verified shares` (b1178f9). Ported into `NetSigning::ProcessPendingSigShares` (`llmq/net_signing.cpp`). The second half of the v23 fix (malformed share → `badSources`) is already on this branch.
  - bitcoin#36669 (8030deb). The `util/system` hunks land in `util/fs_helpers.{h,cpp}`. Two upstream hunks that the v23 port dropped are restored here, because this branch has their equivalents: `BlockInfo::data_pos` becomes `uint64_t` (in `interfaces/chain.h`), and `AutoFile`'s XOR read/write paths use `FileTell64`.
  - bitcoin#36665 (58abb28), bitcoin#36667 (7a21752). Clean.
  - `fix: keep flat-file positions 64-bit clean at the remaining raw seek/tell sites` (58b9bce). Also covers `BlockFilterIndex::LookupFilterRange`, which only this branch has.
  - #7833 fix: gate signing intake before quorum retrieval
  - #7850 perf: let CHECKDATASIG use the signature cache
  - #7826 fix: skip repeated authenticated Platform bans

  The last three are develop merges, picked with `-m1 -x` from develop; they change exactly the same lines as their develop merges. Every adaptation above is described in its commit message, and each matches how the open v23.1.9 → develop merge (#7961) resolves the same commit.

  Already on `v24.0.x` in another form, so not picked:
  - The credit-pool snapshot fix for #7549 (b4cd3fa) is a v23 hand-port of develop's 8503c5d / 0325195 (#7639), which this branch already has.
  - `fix(evo): keep -allowprivatenet out of masternode address validation` (0d2e579) is already covered by #7888.
  - `fix(rpc): don't dereference a missing netInfo when decoding provider payloads` (2aa3a87) and its help-text follow-up (7fc0edb). Here those accesses go through `CHECK_NONFATAL`, so an unknown-version ProTx payload gets an RPC error instead of a crash. The v23 commit message names this as the develop equivalent.

  ### Merge of v23.1.9

  The last commit merges the v23.1.9 tag (67c106f, now `master`) with the tree unchanged (`-s ours`). `master` moved to v23.1.9 today, and `check_merge` requires `master` to fast-forward to `v24.0.x` plus the PR. The final v24.0.0 also has to contain every v23.1.9 commit. All of v23.1.9's content is already on the branch through the picks above, so every commit resolves to the `v24.0.x` side, as #7961 did on develop. Release bookkeeping (version, v23.1.9 release notes, man pages, flatpak entry) stays with v24's own release process. A trial real merge confirmed this: its only clean additions were those docs and three duplicate re-insertions of code the branch already has.

  ### #7947 fix: stop quorum key warming from stalling DKG on 2-core masternodes

  Picked with `-m1 -x`, before the merge. It changes exactly the same lines as its develop merge, and `v24.0.x` has 756fc4f (#7659), the commit it reverts.

  No version change is needed. `configure.ac` is already 24.0.0 with `CLIENT_VERSION_IS_RELEASE` false, so the build takes its version string from the `v24.0.0-rc.5` tag.

  ## How Has This Been Tested?

  Linux x86_64 against depends, `--enable-debug --enable-werror --disable-hardening --without-gui`: `dashd` and `test_dash` build cleanly.

  - Unit suites, all passing (149 cases): `transaction_tests`, `txvalidation_tests`, `sighash_tests`, `bls_tests`, `governance_inv_tests`, `llmq_utils_tests`, `llmq_dkg_tests`, `quorum_proofs_tests`, `flatfile_tests`, `fs_tests`, `net_tests`, `sock_tests`, `evo_netinfo_tests`, `evo_assetlocks_tests`, `blockfilter_index_tests`, `txindex_tests`, `blockmanager_tests`, `checkdatasig_tests`.
  - Functional tests, all passing: `feature_llmq_dkgerrors.py` (including the new contribution-invalid case), `feature_llmq_signing.py` (plain and `--spork21`), `feature_llmq_simplepose.py` (plain and `--disable-spork23`), `feature_llmq_rotation.py`, `feature_llmq_data_recovery.py`, `p2p_timeouts.py` (v1/v2), `mempool_accept.py`, `p2p_platform_ban.py`. In the first parallel run, `feature_llmq_data_recovery.py` (known flaky, see #7947), one `feature_llmq_signing.py --spork21` run and `p2p_platform_ban.py` failed while another build was loading the machine. All three pass when re-run on their own.
  - Each port was compared with its v23.1.9 commit and with #7961's develop resolution. The per-file changes are identical apart from the branch's own API differences listed above. A separate review checked v24-only call sites for second copies of each bug:
    - other `SigShareKey` batch verifiers;
    - other decrypt-then-aggregate paths;
    - other orphan re-request loops;
    - other tx version packing sites;
    - other raw `FlatFilePos` seeks.

    It also checked that the batch-vvec fallback cannot deadlock on a single-thread BLS pool; it runs on the DKG phase threads, not on a BLS worker.

  ## Breaking Changes

  - Consensus: once DIP3 is active, transactions with a negative `nVersion` are rejected (`bad-txns-version`). Every v23.1.9 node already enforces this, so mixed v23.1.9/rc.4 networks could disagree until rc.5 is deployed. Valid transactions and their txids are unchanged.
  - From #7947: quorum key share warming runs on its own `q-cache` thread again, which is one more thread per node.

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

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

Top commit has no ACKs.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants