Skip to content

fix: bound SPORK signature deserialization - #7438

Merged
PastaPastaPasta merged 3 commits into
dashpay:developfrom
thepastaclaw:fix/spork-signature-vector-bound
Jul 10, 2026
Merged

fix: bound SPORK signature deserialization#7438
PastaPastaPasta merged 3 commits into
dashpay:developfrom
thepastaclaw:fix/spork-signature-vector-bound

Conversation

@thepastaclaw

@thepastaclaw thepastaclaw commented Jul 10, 2026

Copy link
Copy Markdown

Depends on #7439. Please review only the two SPORK-specific commits here.

What changed

  • bound CSporkMessage::vchSig to CPubKey::COMPACT_SIGNATURE_SIZE before vector allocation using #7439s LIMITED_VECTOR formatter
  • punish peers that send malformed, truncated, or oversized SPORK messages
  • add unit coverage for the exact CompactSize(MAX_SIZE) trigger and a P2P disconnect regression test

Why

A malformed SPORK message could declare a large signature length in a small payload. Generic byte-vector deserialization would allocate its first 5,000,000-byte chunk before discovering the payload was truncated. The resulting exception was caught by the generic message-processing handler without penalizing the peer, allowing repeated attempts on the same connection.

Valid SPORK signatures are 65-byte compact signatures on every network. The testnet distinction changes the signing preimage, not the signature encoding. Shorter well-formed signatures continue through the existing invalid-signature path and are punished there; the formatter prevents oversized declarations from allocating first.

The reusable bounded-vector serialization primitives are introduced separately in #7439.

Validation

  • src/test/test_dash --run_test=spork_tests,serialize_tests
  • test/functional/feature_sporks.py

The exact unit and functional regressions were also run against the vulnerable implementation: the unit test observed the old EOF-after-allocation path, and the malformed P2P peer was not disconnected.

@thepastaclaw
thepastaclaw force-pushed the fix/spork-signature-vector-bound branch 2 times, most recently from a31a24c to d73610b Compare July 10, 2026 02:36
@thepastaclaw

Copy link
Copy Markdown
Author

CI diagnosis: no branch change is needed. The linux64_sqlite failure is the known pre-existing feature_dip3_v19.py SPORK-sync flake tracked in #6702, not a regression in this PR.

The test exhausted all three retries waiting for wait_for_sporks_same after v19 activation setup. Detailed logs show valid 86-byte SPORK messages were deserialized and accepted normally; the one lagging node received the revert-SPORK inventory and requested the object from another peer, but never received/processed it before timeout. There was no malformed-SPORK exception, deserialization rejection, or peer penalty. The exact upstream/develop worktree at 2bbf4a4 passed feature_dip3_v19.py locally, and the identical timeout has prior occurrences on unrelated wallet, release-note-only, superblock, and LLMQ PRs.

The PR is proper as-is and the branch remains untouched. Maintainers can rerun CI if concerned.

@kwvg kwvg left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

spork_tests can be dropped, feature_sporks addition is sufficient.

Comment thread src/test/spork_tests.cpp Outdated
// A valid 65-byte signature vector round-trips through serialize/unserialize
// with the wire bytes unchanged — the fix must not have moved the wire format
// for legit messages.
BOOST_AUTO_TEST_CASE(spork_signature_exact_size_roundtrip)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The fix doesn't have the means to modify the payload contents, it's unclear what the test is supposed to guard against

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agreed and addressed in aa6b6041714. I removed the redundant unit suite; the functional test remains the end-to-end guard for rejecting an oversized serialized signature vector.

Comment thread src/test/spork_tests.cpp Outdated
// (allocating before it discovers there is no data) and then throws a generic
// stream underflow. On the fix, LimitedVectorFormatter rejects the count
// before any allocation via ios_base::failure.
BOOST_AUTO_TEST_CASE(spork_signature_max_size_rejected_before_allocation)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Check is identical to feature_sporks, can be dropped

@thepastaclaw thepastaclaw Jul 10, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in aa6b6041714: dropped src/test/spork_tests.cpp and its build entry. The retained feature_sporks.py coverage exercises the concrete oversized-SPORK disconnect path, while serialize_tests retains the bounded-vector boundary coverage.

PastaPastaPasta added a commit that referenced this pull request Jul 10, 2026
af6a0fd feat(serialize): add bounded-vector deserialization primitives (PastaClaw)

Pull request description:

  ## Issue being fixed or feature implemented

  Network messages often have protocol-specific vector limits below the generic serialization limit. Callers need to enforce those limits before vector allocation and element decoding, without changing the wire format.

  ## What was done?

  - Factor the existing batched vector element decoder into a shared internal helper.
  - Add `UnserializeVectorWithMaxSize` for runtime bounds.
  - Add `LIMITED_VECTOR` / `LimitedVectorFormatter` for compile-time bounds in `READWRITE` declarations.
  - Keep serialization byte-for-byte compatible with ordinary vectors; only deserialization is bounded.
  - Compare CompactSize counts before narrowing or allocating, including counts at and above `MAX_SIZE`.

  ## Stacked adopters

  Each consumer remains a separate command-specific PR:

  - #7416 — quorum-data response vectors
  - #7418 — LLMQ signing message vectors
  - #7419 — CoinJoin message vectors
  - #7438 — SPORK signature vector

  Reviewing #7439 first leaves each child PR with only its protocol-specific policy, punishment, and regression tests.

  ## How Has This Been Tested?

  - `src/test/test_dash --run_test=serialize_tests`
  - Exact and over-limit boundaries, zero limits, custom element formatters, `MAX_SIZE` and `MAX_SIZE + 1` declarations, 64-bit CompactSize counts, wire compatibility, and rejection before element decode are covered.

  ## Breaking Changes

  None. Existing vector serialization and deserialization behavior is unchanged.

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

Top commit has no ACKs.

Tree-SHA512: 61e4183a5a2a173254f3105d30969704c2c53dac3ce421f0b3e8e989bc87ec1a94d2757944694db219802403a668d7460943d507ab7948db30b97145d57f26d6
@github-actions

Copy link
Copy Markdown

This pull request has conflicts, please rebase.

thepastaclaw and others added 3 commits July 10, 2026 10:51
Add an idiomatic serialization wrapper LIMITED_VECTOR / LimitedVectorFormatter
analogous to LIMITED_STRING and backed by the existing
UnserializeVectorWithMaxSize primitive. Serialization preserves the ordinary
std::vector wire format so it is a drop-in wrapper for fields declared as an
unbounded vector but bounded in practice; deserialization rejects a
CompactSize above the compile-time limit before any element allocation via
std::ios_base::failure.

Apply it to CSporkMessage::vchSig with CPubKey::COMPACT_SIGNATURE_SIZE (65)
as the cap. Prior to this change a peer could hand us a bogus wire count on
a SPORK message, causing the vector to be resized in chunks well before we
had any chance to reject the message on signature verification. Shorter
well-formed signatures continue to deserialize and are rejected by the
existing cryptographic invalid-spork path.

Catch malformed / truncated / oversized SPORK deserialization locally in
the SPORK handler and Misbehaving(100) the peer so replay is punished
rather than silently dropped by the generic ProcessMessages catch. The
catch is narrowed to std::ios_base::failure so unrelated internal failures
during message dispatch are not attributed to the peer.

Wire format for valid 65-byte messages is unchanged on every network,
including testnet's different signing preimage.
Add a Boost unit suite that pins the LimitedVectorFormatter gate applied
to CSporkMessage::vchSig:

- the vulnerability report's exact trigger — a canonical CompactSize
  encoding of MAX_SIZE with no signature bytes — must be rejected before
  allocation; the predicate matches on the formatter's "Vector length
  limit exceeded" so the assertion fails on the vulnerable implementation
  (which throws a downstream stream-underflow) and passes only on the fix;
- a +1 boundary with a full payload short-circuits before element
  decoding even when every promised byte is on the wire;
- a valid 65-byte signature round-trips through serialize/unserialize
  with the wire bytes unchanged, guaranteeing wire compatibility.

Shorter well-formed signatures are intentionally left to the existing
cryptographic invalid-spork punishment path, so no deserialize-time
too-short assertion is added.

Extend feature_sporks.py with the smallest practical p2p assertion: send
a malformed SPORK carrying the oversized count through a dedicated peer
and require the node to disconnect it. Existing propagation, GETSPORKS,
persistence, and multi-node relay coverage is preserved.
Reviewer feedback (kwvg) on dashpay#7438: the CSporkMessage unit checks in
spork_tests.cpp are redundant and unclear. The generic bounded-vector
primitive is already exercised thoroughly by serialize_tests.cpp
(within-limit round trip, wire compatibility, exact/one-over boundary,
no-element-decode-on-reject, MAX_SIZE / 0xfe / 0xff CompactSize gate,
unbounded serialization), and the concrete CSporkMessage oversized-signature
path is covered end-to-end by feature_sporks.py, which sends a MAX_SIZE
signature-length prefix and asserts the peer is disconnected rather than
silently dropped.

Remove the file and its src/Makefile.test.include entry. The deserialization
bound itself (LIMITED_VECTOR on CSporkMessage::vchSig), the serialize.h
primitives, the net_processing peer-attribution, and the serialize_tests /
feature_sporks coverage are all retained.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@thepastaclaw
thepastaclaw force-pushed the fix/spork-signature-vector-bound branch from aa6b604 to 3cb73fd Compare July 10, 2026 15:53
@PastaPastaPasta
PastaPastaPasta marked this pull request as ready for review July 10, 2026 16:03

@PastaPastaPasta PastaPastaPasta left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

utACK 3cb73fd

@kwvg kwvg left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

utACK 3cb73fd

@thepastaclaw

thepastaclaw commented Jul 10, 2026

Copy link
Copy Markdown
Author

✅ Review complete (commit 3cb73fd)

@coderabbitai

coderabbitai Bot commented Jul 10, 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

Run ID: d3bc157c-01df-48b5-a4c6-7afcffc7fe77

📥 Commits

Reviewing files that changed from the base of the PR and between 9474fc5 and 3cb73fd.

📒 Files selected for processing (3)
  • src/net_processing.cpp
  • src/spork.h
  • test/functional/feature_sporks.py

Walkthrough

SPORK message signatures are now serialized and deserialized with a bounded size. SPORK processing catches deserialization failures, marks the peer misbehaving with a score of 100, and stops processing the message. The functional test constructs an oversized signature payload and verifies that the peer disconnects.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: PastaPastaPasta, UdjinM6

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately summarizes the main change: bounding SPORK signature deserialization.
Description check ✅ Passed The description directly matches the changeset and explains the SPORK bounds, peer punishment, and regression tests.
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

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

❤️ Share

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

@knst knst left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM 3cb73fd

though, I'd suggest to squash 2nd and 3rd commits

@PastaPastaPasta
PastaPastaPasta merged commit 4537f37 into dashpay:develop Jul 10, 2026
40 of 43 checks passed

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Code Review

The production change correctly bounds SPORK signatures and penalizes malformed messages without changing valid serialization. Two in-scope suggestions remain: directly test the bound on CSporkMessage, and correct a commit message that claims to introduce primitives already present at the base.

Source: experiment sonnet-primary-opus-quarter-sample-20260710; reviewers codex/gpt-5.6-sol (general and dash-core-commit-history, high reasoning, ok) and sonnet5/claude-sonnet-5 (general and dash-core-commit-history, high effort, failed quota); final verifier codex/gpt-5.6-sol (Sol fallback after sonnet5/claude-sonnet-5 failed quota); orchestrator openai/gpt-5.6-sol (high reasoning, orchestration-only).

🟡 2 suggestion(s)

1 additional finding(s) omitted (not in diff).

🤖 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 `test/functional/feature_sporks.py`:
- [SUGGESTION] test/functional/feature_sporks.py:123-129: Directly test the CSporkMessage allocation bound
  This test verifies disconnection but does not prove that CSporkMessage rejects the declared signature size before allocation. If its field reverted to ordinary vector deserialization while the new local catch remained, the MAX_SIZE input would allocate the first 5 MiB batch, fail on missing payload bytes, and still disconnect the peer. The generic serialization tests validate LimitedVectorFormatter itself, not its application to CSporkMessage. Retain a focused unit test that deserializes a complete 66-byte signature and expects std::ios_base::failure; that fails with the unbounded field and directly protects the security invariant without depending on an exception-message sentinel.

In `<commit:d2e600f>`:
- [SUGGESTION] <commit:d2e600f>:1: Correct the commit message's prerequisite attribution
  The opening paragraph says this commit adds LIMITED_VECTOR and LimitedVectorFormatter, but both were introduced by af6a0fda99a and are already present at base 9474fc5e2a1. This commit only applies the existing formatter to CSporkMessage and adds peer punishment. Reword the paragraph so the permanent history accurately describes the diff.

Comment on lines +123 to +129
# Oversized signature length prefix must trigger disconnect, not silent drop.
MAX_SIZE = 0x02000000
bad_spork = msg_spork_raw()
bad_spork.raw = struct.pack("<iqq", 10001, 0, 0) + ser_compact_size(MAX_SIZE)
bad_peer = self.nodes[0].add_p2p_connection(SporkP2PInterface())
bad_peer.send_message(bad_spork)
bad_peer.wait_for_disconnect()

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🟡 Suggestion: Directly test the CSporkMessage allocation bound

This test verifies disconnection but does not prove that CSporkMessage rejects the declared signature size before allocation. If its field reverted to ordinary vector deserialization while the new local catch remained, the MAX_SIZE input would allocate the first 5 MiB batch, fail on missing payload bytes, and still disconnect the peer. The generic serialization tests validate LimitedVectorFormatter itself, not its application to CSporkMessage. Retain a focused unit test that deserializes a complete 66-byte signature and expects std::ios_base::failure; that fails with the unbounded field and directly protects the security invariant without depending on an exception-message sentinel.

source: ['codex-general']

@UdjinM6 UdjinM6 added this to the 24 milestone Jul 10, 2026
thepastaclaw pushed a commit to thepastaclaw/dash that referenced this pull request Jul 22, 2026
af6a0fd feat(serialize): add bounded-vector deserialization primitives (PastaClaw)

Pull request description:

  ## Issue being fixed or feature implemented

  Network messages often have protocol-specific vector limits below the generic serialization limit. Callers need to enforce those limits before vector allocation and element decoding, without changing the wire format.

  ## What was done?

  - Factor the existing batched vector element decoder into a shared internal helper.
  - Add `UnserializeVectorWithMaxSize` for runtime bounds.
  - Add `LIMITED_VECTOR` / `LimitedVectorFormatter` for compile-time bounds in `READWRITE` declarations.
  - Keep serialization byte-for-byte compatible with ordinary vectors; only deserialization is bounded.
  - Compare CompactSize counts before narrowing or allocating, including counts at and above `MAX_SIZE`.

  ## Stacked adopters

  Each consumer remains a separate command-specific PR:

  - dashpay#7416 — quorum-data response vectors
  - dashpay#7418 — LLMQ signing message vectors
  - dashpay#7419 — CoinJoin message vectors
  - dashpay#7438 — SPORK signature vector

  Reviewing dashpay#7439 first leaves each child PR with only its protocol-specific policy, punishment, and regression tests.

  ## How Has This Been Tested?

  - `src/test/test_dash --run_test=serialize_tests`
  - Exact and over-limit boundaries, zero limits, custom element formatters, `MAX_SIZE` and `MAX_SIZE + 1` declarations, 64-bit CompactSize counts, wire compatibility, and rejection before element decode are covered.

  ## Breaking Changes

  None. Existing vector serialization and deserialization behavior is unchanged.

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

Top commit has no ACKs.

Tree-SHA512: 61e4183a5a2a173254f3105d30969704c2c53dac3ce421f0b3e8e989bc87ec1a94d2757944694db219802403a668d7460943d507ab7948db30b97145d57f26d6
thepastaclaw pushed a commit to thepastaclaw/dash that referenced this pull request Jul 22, 2026
3cb73fd test: drop redundant spork_tests unit suite (PastaClaw)
44a8fe1 test: regression coverage for CSporkMessage signature-size bound (PastaClaw)
d2e600f fix: bound CSporkMessage signature vector allocation (PastaClaw)

Pull request description:

  Depends on dashpay#7439. Please review only the two SPORK-specific commits here.

  ## What changed

  - bound `CSporkMessage::vchSig` to `CPubKey::COMPACT_SIGNATURE_SIZE` before vector allocation using #7439s `LIMITED_VECTOR` formatter
  - punish peers that send malformed, truncated, or oversized SPORK messages
  - add unit coverage for the exact `CompactSize(MAX_SIZE)` trigger and a P2P disconnect regression test

  ## Why

  A malformed SPORK message could declare a large signature length in a small payload. Generic byte-vector deserialization would allocate its first 5,000,000-byte chunk before discovering the payload was truncated. The resulting exception was caught by the generic message-processing handler without penalizing the peer, allowing repeated attempts on the same connection.

  Valid SPORK signatures are 65-byte compact signatures on every network. The testnet distinction changes the signing preimage, not the signature encoding. Shorter well-formed signatures continue through the existing invalid-signature path and are punished there; the formatter prevents oversized declarations from allocating first.

  The reusable bounded-vector serialization primitives are introduced separately in dashpay#7439.

  ## Validation

  - `src/test/test_dash --run_test=spork_tests,serialize_tests`
  - `test/functional/feature_sporks.py`

  The exact unit and functional regressions were also run against the vulnerable implementation: the unit test observed the old EOF-after-allocation path, and the malformed P2P peer was not disconnected.

ACKs for top commit:
  PastaPastaPasta:
    utACK 3cb73fd
  kwvg:
    utACK 3cb73fd

Tree-SHA512: d547f589c3920baaacb121c618b812314901639c449bd562ea150de3a8743e3fbff10c1c92f2e192de776452d74ad7dc9be02622ddd29d9e5f7e43eacd966fd2
thepastaclaw added a commit to thepastaclaw/dash that referenced this pull request Jul 22, 2026
Match the v23.1.8 release tree file mode for the SPORK functional test
script after the dashpay#7438 backport resolution.
thepastaclaw pushed a commit to thepastaclaw/dash that referenced this pull request Jul 22, 2026
af6a0fd feat(serialize): add bounded-vector deserialization primitives (PastaClaw)

Pull request description:

  ## Issue being fixed or feature implemented

  Network messages often have protocol-specific vector limits below the generic serialization limit. Callers need to enforce those limits before vector allocation and element decoding, without changing the wire format.

  ## What was done?

  - Factor the existing batched vector element decoder into a shared internal helper.
  - Add `UnserializeVectorWithMaxSize` for runtime bounds.
  - Add `LIMITED_VECTOR` / `LimitedVectorFormatter` for compile-time bounds in `READWRITE` declarations.
  - Keep serialization byte-for-byte compatible with ordinary vectors; only deserialization is bounded.
  - Compare CompactSize counts before narrowing or allocating, including counts at and above `MAX_SIZE`.

  ## Stacked adopters

  Each consumer remains a separate command-specific PR:

  - dashpay#7416 — quorum-data response vectors
  - dashpay#7418 — LLMQ signing message vectors
  - dashpay#7419 — CoinJoin message vectors
  - dashpay#7438 — SPORK signature vector

  Reviewing dashpay#7439 first leaves each child PR with only its protocol-specific policy, punishment, and regression tests.

  ## How Has This Been Tested?

  - `src/test/test_dash --run_test=serialize_tests`
  - Exact and over-limit boundaries, zero limits, custom element formatters, `MAX_SIZE` and `MAX_SIZE + 1` declarations, 64-bit CompactSize counts, wire compatibility, and rejection before element decode are covered.

  ## Breaking Changes

  None. Existing vector serialization and deserialization behavior is unchanged.

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

Top commit has no ACKs.

Tree-SHA512: 61e4183a5a2a173254f3105d30969704c2c53dac3ce421f0b3e8e989bc87ec1a94d2757944694db219802403a668d7460943d507ab7948db30b97145d57f26d6
thepastaclaw pushed a commit to thepastaclaw/dash that referenced this pull request Jul 22, 2026
3cb73fd test: drop redundant spork_tests unit suite (PastaClaw)
44a8fe1 test: regression coverage for CSporkMessage signature-size bound (PastaClaw)
d2e600f fix: bound CSporkMessage signature vector allocation (PastaClaw)

Pull request description:

  Depends on dashpay#7439. Please review only the two SPORK-specific commits here.

  ## What changed

  - bound `CSporkMessage::vchSig` to `CPubKey::COMPACT_SIGNATURE_SIZE` before vector allocation using #7439s `LIMITED_VECTOR` formatter
  - punish peers that send malformed, truncated, or oversized SPORK messages
  - add unit coverage for the exact `CompactSize(MAX_SIZE)` trigger and a P2P disconnect regression test

  ## Why

  A malformed SPORK message could declare a large signature length in a small payload. Generic byte-vector deserialization would allocate its first 5,000,000-byte chunk before discovering the payload was truncated. The resulting exception was caught by the generic message-processing handler without penalizing the peer, allowing repeated attempts on the same connection.

  Valid SPORK signatures are 65-byte compact signatures on every network. The testnet distinction changes the signing preimage, not the signature encoding. Shorter well-formed signatures continue through the existing invalid-signature path and are punished there; the formatter prevents oversized declarations from allocating first.

  The reusable bounded-vector serialization primitives are introduced separately in dashpay#7439.

  ## Validation

  - `src/test/test_dash --run_test=spork_tests,serialize_tests`
  - `test/functional/feature_sporks.py`

  The exact unit and functional regressions were also run against the vulnerable implementation: the unit test observed the old EOF-after-allocation path, and the malformed P2P peer was not disconnected.

ACKs for top commit:
  PastaPastaPasta:
    utACK 3cb73fd
  kwvg:
    utACK 3cb73fd

Tree-SHA512: d547f589c3920baaacb121c618b812314901639c449bd562ea150de3a8743e3fbff10c1c92f2e192de776452d74ad7dc9be02622ddd29d9e5f7e43eacd966fd2
thepastaclaw added a commit to thepastaclaw/dash that referenced this pull request Jul 28, 2026
…lization

8f0b813 fix(governance): bound vote signature deserialization (PastaClaw)

Pull request description:

  Uses the shared bounded-vector deserialization primitive merged in dashpay#7439.

  ## Motivation

  Governance vote signatures were deserialized through the generic byte-vector path. A peer could declare a very large signature length, causing allocation before the stream reported truncation. The outer message-processing catch did not score or disconnect the peer, allowing repeated malformed messages.

  ## Changes

  - bound network governance-vote signature reads to 96 bytes before allocation
  - require one of the two structurally valid encodings: 65-byte compact ECDSA or 96-byte BLS
  - score malformed or truncated governance vote messages with 100 misbehavior points
  - preserve disk, hash, and outbound serialization behavior
  - add focused unit coverage

  ## Testing

  - `./src/test/test_dash --run_test=governance_vote_wire_tests` (4/4 tests)
  - `./src/test/test_dash --run_test=serialize_tests` (10/10 tests)
  - `test/lint/lint-python.py`

Tree-SHA512: backported to v23.1.x by cherry-picking 8f0b813 (applies cleanly).

Backport note for v23.1.8
-------------------------

This was missing from the original v23.1.8 branch while its test follow-up
 dashpay#7450 ("test: make governance vote fixtures wire-valid", 915566d) was
already included. That ordering was inverted: dashpay#7450 exists solely to adapt the
 dashpay#7442 governance-inv fixtures to the bound that dashpay#7440 introduces.

Verified by removing dashpay#7450's SetSignature() line and rebuilding: without dashpay#7440
present the fixtures pass regardless, and re-adding dashpay#7440 reproduces exactly
the six governance_inv_tests failures dashpay#7450's description cites. So the branch
was shipping the compensating test change for a hardening fix it did not have,
leaving CGovernanceVote::vchSig unbounded on the network path.

The prerequisite dashpay#7439 (LIMITED_VECTOR) is already present via 099b99d, as
are the sibling bounding backports dashpay#7416/dashpay#7418/dashpay#7419/dashpay#7438/dashpay#7444, so this
restores the intended set rather than widening release scope.

Reported-by: UdjinM6
Co-Authored-By: Claude <noreply@anthropic.com>
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jul 29, 2026
af6a0fd feat(serialize): add bounded-vector deserialization primitives (PastaClaw)

Pull request description:

  ## Issue being fixed or feature implemented

  Network messages often have protocol-specific vector limits below the generic serialization limit. Callers need to enforce those limits before vector allocation and element decoding, without changing the wire format.

  ## What was done?

  - Factor the existing batched vector element decoder into a shared internal helper.
  - Add `UnserializeVectorWithMaxSize` for runtime bounds.
  - Add `LIMITED_VECTOR` / `LimitedVectorFormatter` for compile-time bounds in `READWRITE` declarations.
  - Keep serialization byte-for-byte compatible with ordinary vectors; only deserialization is bounded.
  - Compare CompactSize counts before narrowing or allocating, including counts at and above `MAX_SIZE`.

  ## Stacked adopters

  Each consumer remains a separate command-specific PR:

  - dashpay#7416 — quorum-data response vectors
  - dashpay#7418 — LLMQ signing message vectors
  - dashpay#7419 — CoinJoin message vectors
  - dashpay#7438 — SPORK signature vector

  Reviewing dashpay#7439 first leaves each child PR with only its protocol-specific policy, punishment, and regression tests.

  ## How Has This Been Tested?

  - `src/test/test_dash --run_test=serialize_tests`
  - Exact and over-limit boundaries, zero limits, custom element formatters, `MAX_SIZE` and `MAX_SIZE + 1` declarations, 64-bit CompactSize counts, wire compatibility, and rejection before element decode are covered.

  ## Breaking Changes

  None. Existing vector serialization and deserialization behavior is unchanged.

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

Top commit has no ACKs.

Tree-SHA512: 61e4183a5a2a173254f3105d30969704c2c53dac3ce421f0b3e8e989bc87ec1a94d2757944694db219802403a668d7460943d507ab7948db30b97145d57f26d6
(cherry picked from commit 9474fc5)
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jul 29, 2026
Backport of dashpay#7438 (upstream merge 4537f37, cherry-picked with -m1).

v23.1.x adaptations: (1) on this branch spork messages are deserialized in CSporkManager::ProcessSpork (src/spork.cpp), not in net_processing's SPORK handler as on develop - the same try/catch is applied there, reporting the failure as MisbehavingError{100, ...} through the existing MessageProcessingResult path (equivalent to develop's Misbehaving(*peer, 100, ...)). (2) The functional test's msg_spork_raw/SporkP2PInterface helpers come from dashpay#7343 (commit 60efd84), which is not on this branch; they are included verbatim, and the new test registers MESSAGEMAP[b"spork"] itself since dashpay#7343's test method (which did the registration on develop) is absent here. The spork.h LIMITED_VECTOR bound is unchanged from upstream.

(cherry picked from commit 4537f37d51d5b62be3aca4bfc9e40404db76ffc9)
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jul 29, 2026
af6a0fd feat(serialize): add bounded-vector deserialization primitives (PastaClaw)

Pull request description:

  ## Issue being fixed or feature implemented

  Network messages often have protocol-specific vector limits below the generic serialization limit. Callers need to enforce those limits before vector allocation and element decoding, without changing the wire format.

  ## What was done?

  - Factor the existing batched vector element decoder into a shared internal helper.
  - Add `UnserializeVectorWithMaxSize` for runtime bounds.
  - Add `LIMITED_VECTOR` / `LimitedVectorFormatter` for compile-time bounds in `READWRITE` declarations.
  - Keep serialization byte-for-byte compatible with ordinary vectors; only deserialization is bounded.
  - Compare CompactSize counts before narrowing or allocating, including counts at and above `MAX_SIZE`.

  ## Stacked adopters

  Each consumer remains a separate command-specific PR:

  - dashpay#7416 — quorum-data response vectors
  - dashpay#7418 — LLMQ signing message vectors
  - dashpay#7419 — CoinJoin message vectors
  - dashpay#7438 — SPORK signature vector

  Reviewing dashpay#7439 first leaves each child PR with only its protocol-specific policy, punishment, and regression tests.

  ## How Has This Been Tested?

  - `src/test/test_dash --run_test=serialize_tests`
  - Exact and over-limit boundaries, zero limits, custom element formatters, `MAX_SIZE` and `MAX_SIZE + 1` declarations, 64-bit CompactSize counts, wire compatibility, and rejection before element decode are covered.

  ## Breaking Changes

  None. Existing vector serialization and deserialization behavior is unchanged.

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

Top commit has no ACKs.

Tree-SHA512: 61e4183a5a2a173254f3105d30969704c2c53dac3ce421f0b3e8e989bc87ec1a94d2757944694db219802403a668d7460943d507ab7948db30b97145d57f26d6
(cherry picked from commit 9474fc5)
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jul 29, 2026
Backport of dashpay#7438 (upstream merge 4537f37, cherry-picked with -m1).

v23.1.x adaptations: (1) on this branch spork messages are deserialized in CSporkManager::ProcessSpork (src/spork.cpp), not in net_processing's SPORK handler as on develop - the same try/catch is applied there, reporting the failure as MisbehavingError{100, ...} through the existing MessageProcessingResult path (equivalent to develop's Misbehaving(*peer, 100, ...)). (2) The functional test's msg_spork_raw/SporkP2PInterface helpers come from dashpay#7343 (commit 60efd84), which is not on this branch; they are included verbatim, and the new test registers MESSAGEMAP[b"spork"] itself since dashpay#7343's test method (which did the registration on develop) is absent here. The spork.h LIMITED_VECTOR bound is unchanged from upstream.

(cherry picked from commit 4537f37d51d5b62be3aca4bfc9e40404db76ffc9)
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jul 29, 2026
af6a0fd feat(serialize): add bounded-vector deserialization primitives (PastaClaw)

Pull request description:

  ## Issue being fixed or feature implemented

  Network messages often have protocol-specific vector limits below the generic serialization limit. Callers need to enforce those limits before vector allocation and element decoding, without changing the wire format.

  ## What was done?

  - Factor the existing batched vector element decoder into a shared internal helper.
  - Add `UnserializeVectorWithMaxSize` for runtime bounds.
  - Add `LIMITED_VECTOR` / `LimitedVectorFormatter` for compile-time bounds in `READWRITE` declarations.
  - Keep serialization byte-for-byte compatible with ordinary vectors; only deserialization is bounded.
  - Compare CompactSize counts before narrowing or allocating, including counts at and above `MAX_SIZE`.

  ## Stacked adopters

  Each consumer remains a separate command-specific PR:

  - dashpay#7416 — quorum-data response vectors
  - dashpay#7418 — LLMQ signing message vectors
  - dashpay#7419 — CoinJoin message vectors
  - dashpay#7438 — SPORK signature vector

  Reviewing dashpay#7439 first leaves each child PR with only its protocol-specific policy, punishment, and regression tests.

  ## How Has This Been Tested?

  - `src/test/test_dash --run_test=serialize_tests`
  - Exact and over-limit boundaries, zero limits, custom element formatters, `MAX_SIZE` and `MAX_SIZE + 1` declarations, 64-bit CompactSize counts, wire compatibility, and rejection before element decode are covered.

  ## Breaking Changes

  None. Existing vector serialization and deserialization behavior is unchanged.

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

Top commit has no ACKs.

Tree-SHA512: 61e4183a5a2a173254f3105d30969704c2c53dac3ce421f0b3e8e989bc87ec1a94d2757944694db219802403a668d7460943d507ab7948db30b97145d57f26d6
(cherry picked from commit 9474fc5)
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jul 29, 2026
Backport of dashpay#7438 (upstream merge 4537f37, cherry-picked with -m1).

v23.1.x adaptations: (1) on this branch spork messages are deserialized in CSporkManager::ProcessSpork (src/spork.cpp), not in net_processing's SPORK handler as on develop - the same try/catch is applied there, reporting the failure as MisbehavingError{100, ...} through the existing MessageProcessingResult path (equivalent to develop's Misbehaving(*peer, 100, ...)). (2) The functional test's msg_spork_raw/SporkP2PInterface helpers come from dashpay#7343 (commit 60efd84), which is not on this branch; they are included verbatim, and the new test registers MESSAGEMAP[b"spork"] itself since dashpay#7343's test method (which did the registration on develop) is absent here. The spork.h LIMITED_VECTOR bound is unchanged from upstream.

(cherry picked from commit 4537f37d51d5b62be3aca4bfc9e40404db76ffc9)
PastaPastaPasta added a commit that referenced this pull request Jul 30, 2026
24920a0 chore: prepare v23.1.8 release (pasta)
2194248 Merge #7348: fix: penalize oversized notfound messages (pasta)
f5c72c3 Merge #7347: fix: punish invalid dstx messages (pasta)
550caf7 Merge #7465: fix(qt): handle pixel-sized fonts when scaling widgets (pasta)
e203710 Merge #7419: fix(net): bound CoinJoin message vector intake (pasta)
5f5b960 Merge #7418: fix(net): bound signing message vector intake (Pasta)
7cc2cca Merge #7450: test: make governance vote fixtures wire-valid (Pasta)
f011c80 Merge #7440: fix(net): bound governance vote signature deserialization (Pasta)
4b4d96a Merge #7442: fix(net): authorize governance inv responses via the net-layer per-peer request tracker (Pasta)
f855b13 Merge #7444: fix(net): bound bloom message vectors before allocation (Pasta)
5b5c6fb Merge #7415: fix: bound pending sig share queue (Pasta)
9bbe808 Merge #7416: fix(net): bound quorum data response vectors (Pasta)
da42f50 Merge #7424: fix: bound ChainLock seen cache (Pasta)
5b310df Merge #7438: fix: bound SPORK signature deserialization (Pasta)
e118d0c Merge #7259: fix: dangling point to cj client (Pasta)
9921621 Merge #7439: refactor: add bounded vector deserialization (Pasta)
89bdf7c Merge #7414: fix(net): throttle per-object governance vote sync requests (Pasta)
44c396d Merge #7402: fix: bound pending recovered sig queue to prevent remote OOM (Pasta)
05cfe27 Merge #7351: fix: limit signing share sessions per peer (pasta)
3ef3a5b Merge #7408: fix: bound DKG contribution blob intake (pasta)
0ea6532 Merge #7387: test: migrate governance inv cache coverage to unit tests (Pasta)
8ffdf7f Merge #7398: backport: compact block relay hardening (bitcoin#26898, bitcoin#27626, bitcoin#27743, bitcoin#26969, bitcoin#29412, bitcoin#32646, bitcoin#33296) (Pasta)
2915142 backport: bitcoin#27608 - p2p: Avoid prematurely clearing download state for other peers (PastaClaw)
90b5473 Merge #7396: fix: run of circular-dependencies with python3.15 (Pasta)
b003cdc Merge #7395: ci: update GitHub Actions pins for Node 24 (pasta)
97c3dd1 Merge #7394: fix: stabilize par help text in manpages (pasta)
8f8616b Merge #7372: backport: bitcoin#32693: depends: fix cmake compatibility error for freetype (pasta)
48f72be Merge #7360: fix: empty platformP2PPort deprecated field in protx listdiff results (pasta)
a8cccff Merge #7298: fix(qt): keep PoSe score visible when hiding banned masternodes (pasta)

Pull request description:

  Release PR for Dash Core v23.1.8, a patch release on top of v23.1.7.

  Fast-forwards from `v23.1.x` (currently at `chore: prepare v23.1.7 release`), 29 commits, no merge commits, no conflicts.

  ## Contents

  Backports of PRs already reviewed and merged on `develop`:

  `#7259` `#7347` `#7348` `#7351` `#7298` `#7360` `#7372` `#7387` `#7394` `#7395` `#7396` `#7398` `#7402` `#7408` `#7414` `#7415` `#7416` `#7418` `#7419` `#7424` `#7438` `#7439` `#7440` `#7442` `#7444` `#7450` `#7465`

  Plus `backport: bitcoin#27608`, a single commit taken from Dash #7237 because #7398's compact-block hardening depends on it. The rest of that v0.26 batch is intentionally not included on v23.1.x. The commit is byte-identical to its reviewed counterpart inside #7237.

  And release preparation: version bump, regenerated man pages, release notes, archived 23.1.7 notes.

  ## Note for reviewers: this branch was rebuilt

  An earlier revision of this PR was discarded and the branch rebuilt from scratch. Review comments on the previous revision point at commits that no longer exist, though the feedback itself was carried over (see below).

  The reason: several commits titled `Merge #NNNN` in the earlier revision contained substantial code that exists nowhere upstream — apparently written from a description of each PR rather than ported from its diff. For example, `feature_llmq_simplepose.py` is byte-identical between v23.1.7 and `develop`, yet the earlier `Merge #7408` rewrote 66 lines of it; `test/functional/p2p_governance_invs.py` does not exist on `develop` at all, yet had grown from 62 to 148 lines.

  That mislabeling matters because a commit titled `Merge #NNNN` invites less scrutiny, not more. It also had consequences: the earlier revision was **missing #7440 entirely**, and contained eleven consecutive commits that did not compile (code written against newer upstream APIs this branch does not have — `Misbehaving(Peer&)`, and `PeerIsBanned` used five commits before it was declared).

  Every commit on this branch has now been diffed against its upstream merge commit. Where a backport differs, it is because v23.1.x predates an upstream refactor and the change had to be applied to the pre-refactor file — for example #7418 and #7438 patch `signing_shares.cpp` / `spork.cpp` where upstream patches `net_signing.cpp` / `net_processing.cpp`.

  ## Dropped from this branch

  - **#7350** (`net: don't lock cs_main while reading blocks`) — dropped on review feedback. It is a 110-line lock-structure refactor of `ProcessGetBlockData` with no measured benefit, and it would add avoidable churn to the eventual master→develop merge-back. Nothing on this branch depends on it: #7398's compact-block work precedes it, and the remaining 14 commits replay with zero conflicts once it is removed. Thanks @knst.

  ## Added after the initial review pass

  - **#7351** (`fix: limit signing share sessions per peer`) — cherry-picked as a single
    commit and placed before #7402, matching upstream's merge order. The include block
    additionally carries `<ranges>`: upstream's diff adds only `<algorithm>` because develop
    already had it, whereas v23.1.x did not and the backported `GetSessionCount()` /
    `GetAnnouncementSessionCount()` use `std::ranges::count_if`.
  - **#7465** (`fix(qt): handle pixel-sized fonts when scaling widgets`) — cherry-picked from
    the five upstream commits. `optiontests.cpp` additionally includes `qt/guiutil_font.h`,
    because `fontsLoaded()` and `updateFonts()` are declared there on v23.1.x while develop
    declares them in `qt/guiutil.h`, which is all the upstream test includes.

  Two further backports were added later and applied without any adaptation --
  their diffs are byte-for-byte identical to upstream:

  - **#7347** (`fix: punish invalid dstx messages`)
  - **#7348** (`fix: penalize oversized notfound messages`)

  ## Adaptations worth flagging

  - **#7360** — upstream gates `platformP2PPort` / `platformHTTPPort` in `protx listdiff` behind `IsServiceDeprecatedRPCEnabled()`. On 23.x those deprecated fields are deliberately not enforced through gating (see `bbcd9d543e6`), so shipping the gate as-is would silently drop two fields that v23.1.7 always returned. Changed to `if (true)` with a comment, per review feedback, keeping the block aligned with `develop`. The substantive fix from #7360 — reading the live port from `netInfo` instead of the always-zero scalar — is retained.

  - **#7415** — the pending-map caps (`MAX_PENDING_SIG_SHARES_PER_NODE`, `MAX_PENDING_SIG_SHARES_TOTAL`) are backported. The additional bound upstream places on batches awaiting verification is not, because it guards a condition that does not exist here: upstream's dispatcher pushes one task per batch inside an inner loop, whereas v23.1.x pushes a single looping worker per 10 ms tick. There is no unbounded task queue to bound.

  - **Man pages** — regenerated without the `lock` debug category, which only exists under `DEBUG_LOCKCONTENTION` and so is absent from release binaries. Thanks @UdjinM6 for catching this.

  ## Known CI failure

  macOS jobs are expected to fail. `actions/upload-artifact@v6` rejects filenames containing `:`, and the Xcode SDK ships Perl man pages with `::` in the name. A release-branch-only workaround existed on the earlier revision but was dropped as it corresponds to no upstream PR. This is accepted for this release.

  ## Testing

  - Every commit through #7465 compiles individually (verified for 27 of the 29; the three additions below were verified at the tip) — verified individually, not just at the tip.
  - Full build clean; no new warnings.
  - Unit tests pass.
  - Functional tests pass: `feature_llmq_signing` (both variants), `feature_llmq_chainlocks`, `feature_llmq_dkgerrors`, `feature_llmq_is_cl_conflicts`, `p2p_instantsend`, `feature_dip3_deterministicmns` (both wallet types), `rpc_coinjoin`.
  - Qt unit tests pass (32 cases, run under the `cocoa` platform plugin so the pixel-sized
    font regression from #7465 actually executes rather than self-skipping).
  - Lint: one pre-existing `lint-cppcheck-dash` failure, identical on v23.1.7, in files this branch does not touch.

Top commit has no ACKs.

Tree-SHA512: 0fa469c9a33820aa85fbb8b90c5877409d09490f746f1300b05ceda470a600765f900bec42e5aad5d90a2d46b44e28c20e44dc4a8fe719553a072298069eaec4
@UdjinM6 UdjinM6 modified the milestones: 24, 23.1.8 Jul 30, 2026
PastaPastaPasta added a commit that referenced this pull request Aug 3, 2026
728f505 doc: record the v23.1.8 critical crash fixes in the release notes (pasta)
ca56af8 fix(llmq): reject parentless quorum base blocks instead of terminating (pasta)
a801d3f fix(llmq): reject unregistered LLMQ types from qsigshare before quorum lookup (pasta)
6c1f611 fix: skip already-removed conflicts when a ProTx key change clears the mempool (pasta)
24920a0 chore: prepare v23.1.8 release (pasta)
2194248 Merge #7348: fix: penalize oversized notfound messages (pasta)
f5c72c3 Merge #7347: fix: punish invalid dstx messages (pasta)
550caf7 Merge #7465: fix(qt): handle pixel-sized fonts when scaling widgets (pasta)
e203710 Merge #7419: fix(net): bound CoinJoin message vector intake (pasta)
5f5b960 Merge #7418: fix(net): bound signing message vector intake (Pasta)
7cc2cca Merge #7450: test: make governance vote fixtures wire-valid (Pasta)
f011c80 Merge #7440: fix(net): bound governance vote signature deserialization (Pasta)
4b4d96a Merge #7442: fix(net): authorize governance inv responses via the net-layer per-peer request tracker (Pasta)
f855b13 Merge #7444: fix(net): bound bloom message vectors before allocation (Pasta)
5b5c6fb Merge #7415: fix: bound pending sig share queue (Pasta)
9bbe808 Merge #7416: fix(net): bound quorum data response vectors (Pasta)
da42f50 Merge #7424: fix: bound ChainLock seen cache (Pasta)
5b310df Merge #7438: fix: bound SPORK signature deserialization (Pasta)
e118d0c Merge #7259: fix: dangling point to cj client (Pasta)
9921621 Merge #7439: refactor: add bounded vector deserialization (Pasta)
89bdf7c Merge #7414: fix(net): throttle per-object governance vote sync requests (Pasta)
44c396d Merge #7402: fix: bound pending recovered sig queue to prevent remote OOM (Pasta)
05cfe27 Merge #7351: fix: limit signing share sessions per peer (pasta)
3ef3a5b Merge #7408: fix: bound DKG contribution blob intake (pasta)
0ea6532 Merge #7387: test: migrate governance inv cache coverage to unit tests (Pasta)
8ffdf7f Merge #7398: backport: compact block relay hardening (bitcoin#26898, bitcoin#27626, bitcoin#27743, bitcoin#26969, bitcoin#29412, bitcoin#32646, bitcoin#33296) (Pasta)
2915142 backport: bitcoin#27608 - p2p: Avoid prematurely clearing download state for other peers (PastaClaw)
90b5473 Merge #7396: fix: run of circular-dependencies with python3.15 (Pasta)
b003cdc Merge #7395: ci: update GitHub Actions pins for Node 24 (pasta)
97c3dd1 Merge #7394: fix: stabilize par help text in manpages (pasta)
8f8616b Merge #7372: backport: bitcoin#32693: depends: fix cmake compatibility error for freetype (pasta)
48f72be Merge #7360: fix: empty platformP2PPort deprecated field in protx listdiff results (pasta)
a8cccff Merge #7298: fix(qt): keep PoSe score visible when hiding banned masternodes (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  Merges master (v23.1.8) back into develop so develop carries the v23.1.8 release and, most importantly, the three remotely reachable crash fixes shipped on top of it. Without this, develop-based nodes remain vulnerable to all three crashes.

  ## What was done?
  Merged `upstream/master` into `develop`. The genuinely new payload is:

  - **Three remote-crash fixes** (with their regression tests):
    - skip already-removed conflicts when a ProTx key change clears the mempool (`txmempool.cpp`)
    - reject unregistered LLMQ types from `qsigshare` before quorum lookup (`llmq/blockprocessor.cpp`, `llmq/quorumsman.cpp`)
    - reject parentless quorum base blocks instead of terminating (`llmq/commitment.cpp`, `llmq/utils.cpp`, `validation.{cpp,h}`)
  - **Release bookkeeping**: v23.1.8 release notes, archived 23.1.7 notes, flatpak release entry, regenerated v23.1.8 man pages, and the `configure.ac` version bump (develop keeps `IS_RELEASE=false` and its own configure flags).

  Everything else on master since the last merge (the v23.1.x security-hardening train, #7259#7465 and #7398) was dual-merged and already exists on develop as its own merge commits, so all conflicts from those files resolve to develop's side. Deliberate adaptations, itemized in the merge commit message:

  - The `qsigshare` LLMQ-type gate is ported into develop's `net_signing.cpp` and the parentless-base DKG check into develop's `net_dkg.cpp` (develop moved message processing out of `signing_shares.cpp`/`dkgsessionmgr.cpp`).
  - The defence-in-depth `find()` lookup is additionally applied to `CQuorumManager::GetCachedMutableQuorum()`, a develop-only method reached with a wire-supplied LLMQ type from the `QDATA` handler.
  - The new mempool regression test is adapted to develop's `CreateProRegTx`/`CreateProUpServTx`/`CreateProUpRevTx` helper signatures and `MemPoolOptionsForTest`.
  - `src/active/quorums.cpp` (deleted by develop's ActiveContext refactor) stays deleted; develop's #7416 equivalent already covers the new layout.

  ## How Has This Been Tested?
  Built with `--enable-debug` on macOS (arm64). Ran the new/extended suites — `evo_deterministicmns_tests`, `llmq_invalid_type_tests`, `evo_utils_tests` — plus adjacent ones (`llmq_signing_tests`, `llmq_dkg_tests`, `llmq_blockprocessor_tests`, `llmq_commitment_tests`, `llmq_utils_tests`, `mempool_tests`, `validation_tests`): all pass. The three regression tests abort the process on unpatched code.

  ## Breaking Changes
  None. `ChainstateManager::IsQuorumTypeEnabled()` loosens its `pindexPrev` parameter from `gsl::not_null` to a plain pointer (null now returns false instead of aborting); all existing callers are unaffected.

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

ACKs for top commit:
  UdjinM6:
    utACK e15bb64

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants