Skip to content

fix(net): bound bloom message vectors before allocation - #7444

Merged
PastaPastaPasta merged 3 commits into
dashpay:developfrom
thepastaclaw:fix/bloom-message-vector-bounds
Jul 11, 2026
Merged

fix(net): bound bloom message vectors before allocation#7444
PastaPastaPasta merged 3 commits into
dashpay:developfrom
thepastaclaw:fix/bloom-message-vector-bounds

Conversation

@thepastaclaw

Copy link
Copy Markdown

Prevent peer-declared FILTERADD and CBloomFilter vector lengths from allocating before their command-specific limits are checked.

The shared CBloomFilter bound covers FILTERLOAD and governance sync. Each P2P entrypoint catches malformed bounded reads locally and applies the existing 100-point punishment, preventing replay through the outer message-processing catch. The getmerkleblocks RPC keeps its prior complete-oversized, truncated, noncanonical, and above-MAX_SIZE behavior.

Adds exact CompactSize(MAX_SIZE) regressions for FILTERADD, FILTERLOAD, and governance sync while retaining valid boundary coverage.

Validation:

  • make -C src dashd dash-cli test/test_dash
  • test_dash --run_test=bloom_tests (14 cases)
  • test/functional/p2p_filter.py
  • test/functional/p2p_govsync_bloom.py
  • test/lint/lint-python.py
  • test/lint/lint-whitespace.py
  • git diff --check
  • manual getmerkleblocks RPC matrix for complete/truncated boundary and malformed-prefix behavior
  • independent exact-range review: ship, no significant findings

A peer-controlled CBloomFilter and the FILTERADD data element were decoded
in full before their size was checked. The length is a CompactSize up to
MAX_SIZE, so a raw count with the bytes omitted forced a large speculative
allocation and then threw std::ios_base::failure into net_processing's
outer message-processing catch, which drops without punishing the peer and
so allowed indefinite replay.

Bound the wire count before any element is decoded using LIMITED_VECTOR:
CBloomFilter::vData at MAX_BLOOM_FILTER_SIZE in its serialization method and
the FILTERADD data element at MAX_SCRIPT_ELEMENT_SIZE at the call site. The
wire format is unchanged, so IsWithinSizeConstraints() still guards the
exact boundary and nHashFuncs.

At FILTERLOAD, FILTERADD, and the MNGOVERNANCESYNC consumer of the shared
type, catch the local std::ios_base::failure and apply the existing
100-point punishment; the govsync catch is scoped to the filter read so
unrelated nProp handling is unchanged. Because CBloomFilter is now globally
bounded, getmerkleblocks prechecks the leading vData count without consuming
the stream: a fully present oversized filter keeps its historical
RPC_INVALID_PARAMETER response, a truncated oversized declaration reproduces
the original DataStream end-of-data error, and malformed, noncanonical, and
above-MAX_SIZE prefixes fall through to normal deserialization unchanged.

Add exact-trigger regressions to p2p_filter.py and p2p_govsync_bloom.py
(via msg_generic) that declare a CompactSize(MAX_SIZE) length with the
bytes omitted and assert the peer is punished; existing valid-boundary
coverage is retained.

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

thepastaclaw commented Jul 10, 2026

Copy link
Copy Markdown
Author

✅ Review complete (commit 5b6ac38)

@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: 0042e268-5baf-4ae0-92e3-5c233dc5cecb

📥 Commits

Reviewing files that changed from the base of the PR and between 862e7b0 and 5b6ac38.

📒 Files selected for processing (1)
  • test/functional/rpc_getmerkleblocks.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/functional/rpc_getmerkleblocks.py

Walkthrough

Bloom-filter deserialization now bounds vector allocation through LIMITED_VECTOR. Governance sync, filterload, and filteradd handlers catch deserialization failures and penalize affected peers. getmerkleblocks prechecks oversized or truncated filters while preserving distinct error behavior. Functional tests cover oversized network payloads and RPC boundary cases, and the default test runner now executes the new RPC test.

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

Possibly related PRs

  • dashpay/dash#7397: Modifies governance sync bloom-filter handling for oversized filters.

Suggested reviewers: knst, PastaPastaPasta, kwvg

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: bounding bloom-related message vectors before allocation.
Description check ✅ Passed The description is directly related to the changeset and accurately describes the bloom-filter bounds and test coverage.
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.

@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

This PR closes a pre-allocation DoS gap by switching CBloomFilter's vData, FILTERADD's data, and MNGOVERNANCESYNC's filter deserialization to bounded reads, with local try/catch blocks applying the Misbehaving(100) punishment that the outer message-processing catch (net_processing.cpp:5761-5772) silently skips. All boundary math was independently verified: SpanReader::size() correctly reflects post-read remaining bytes (confirmed via read()'s subspan advance), the LimitedVectorFormatter throws before allocation, and the getmerkleblocks RPC precheck's OVERSIZED_COMPLETE/OVERSIZED_TRUNCATED logic exactly reproduces prior behavior. The msg_generic slots fix is a correct necessary prerequisite for the new P2P tests. Both reviewer lanes agree the change is sound; the only outstanding item is a missing automated regression test for the RPC's non-trivial byte-level precheck.

Source (experiment sonnet-primary-opus-quarter-sample-20260710, cohort sonnet_primary, bucket 3): reviewers codex/general=gpt-5.6-sol(completed); sonnet5/general=claude-sonnet-5(completed); codex/dash-core-commit-history=gpt-5.6-sol(completed); sonnet5/dash-core-commit-history=claude-sonnet-5(completed); verifier=verifier-sonnet5-7444-1783722240=claude-sonnet-5; orchestrator=openai/gpt-5.6-sol reasoning=high (orchestration-only, not a reviewer/verifier).

🟡 1 suggestion(s)

🤖 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 `src/rpc/blockchain.cpp`:
- [SUGGESTION] src/rpc/blockchain.cpp:855-882: getmerkleblocks oversized/truncated-filter precheck has no automated test
  This block hand-rolls CompactSize parsing via a manual SpanReader to distinguish OVERSIZED_COMPLETE from OVERSIZED_TRUNCATED and preserve the exact pre-PR error for each case. It's the most intricate addition in the PR — byte-offset arithmetic, trailer-size accounting (FILTER_TRAILER_SIZE), and manual exception construction — yet per the PR's own validation notes it was only exercised with a manual RPC matrix. There's no functional test file for getmerkleblocks (test_runner.py still has a 'TODO: implement functional tests for getmerkleblocks') and no C++ unit test covers this path. A future refactor of CBloomFilter's serialization or this precheck could silently change RPC error behavior (e.g. RPC_INVALID_PARAMETER vs a raw ios_base::failure) without CI catching it.

Comment thread src/rpc/blockchain.cpp

@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 28d2c6c

getmerkleblocks hand-parses the leading CompactSize vData count so that, now
that CBloomFilter deserialization is bounded, an oversized filter still
reproduces the historical RPC errors instead of a generic length-limit
failure. That byte-level precheck (distinguishing a fully present oversized
filter from a truncated one, including the trailing fixed-field boundary) had
no automated coverage.

Add rpc_getmerkleblocks.py, which asserts:
  - a fully present oversized filter -> RPC_INVALID_PARAMETER
    ("Filter is not within size constraints");
  - an oversized declaration with the vData bytes omitted, or with the trailer
    short by one byte, -> the original DataStream end-of-data RPC_MISC_ERROR;
  - a well-formed in-bounds filter falls through and is handled normally.

Register the test in BASE_SCRIPTS and drop the now-satisfied getmerkleblocks
RPC-coverage TODO, since the passing filter call exercises the command.

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

@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 latest test-only commit resolves the sole prior finding by adding functional coverage for the complete-oversized, truncated-body, one-byte-short trailer, and valid-filter paths. The assertions match the current C++ boundary arithmetic and RPC error mapping; Python lint passes and the two-commit history is clean.

Prior finding reconciliation: FIXED — “getmerkleblocks oversized/truncated-filter precheck has no automated test” is covered by test/functional/rpc_getmerkleblocks.py.

Carried-forward prior findings: None.

New findings in latest delta: None.

Source: reviewers codex/general=gpt-5.6-sol (parseable output, nonzero ACP wrapper exit); sonnet5/general=claude-sonnet-5 (completed); codex/dash-core-commit-history=gpt-5.6-sol (parseable output, nonzero ACP wrapper exit); sonnet5/dash-core-commit-history=claude-sonnet-5 (completed); verifier sonnet5=claude-sonnet-5 (completed); orchestrator openai/gpt-5.6-sol reasoning=high (orchestration-only, not reviewer/verifier). Experiment sonnet-primary-opus-quarter-sample-20260710, cohort sonnet_primary, bucket 2.

@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 862e7b0

knst
knst previously approved these changes Jul 11, 2026

@knst knst left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

overall LGTM 862e7b0 ; fix copyright year

} catch (const std::ios_base::failure& e) {
// An oversized filter now throws pre-allocation; punish here instead of the outer catch.
m_peer_manager->PeerMisbehaving(peer.GetId(), 100, strprintf("misformatted govsync bloom filter. peer=%d error=%s", peer.GetId(), e.what()));
return;

@knst knst Jul 11, 2026

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.

@PastaPastaPasta

nit: why won't re-throw exception here? instead return; ?

So this exception will be caught in the call-stack higher and logged:


LogPrint(BCLog::NET, "%s(%s, %u bytes): Exception '%s' (%s) caught\n", __func__, SanitizeString(msg.m_type), msg.m_message_size, ....

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.

PeerMisbehaving(..., e.what()) already logs the exception detail through Misbehaving, so rethrowing would produce a second outer-catch log for the same failure. Returning here is intentional and matches the filterload/filteradd deserialization handlers added by this PR.

Comment thread test/functional/test_runner.py
Comment thread test/functional/rpc_getmerkleblocks.py Outdated
@@ -0,0 +1,90 @@
#!/usr/bin/env python3
# Copyright (c) 2024 The Dash Core developers

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.

@thepastaclaw for new files specify 2026 year.

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.

Fixed in 5b6ac38.

@thepastaclaw
thepastaclaw dismissed stale reviews from knst and PastaPastaPasta via 5b6ac38 July 11, 2026 19:42

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

utACK 5b6ac38

@PastaPastaPasta
PastaPastaPasta merged commit b9d956d into dashpay:develop Jul 11, 2026
41 of 42 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

Current-head cumulative review found no code-correctness, security, or test-coverage issues in the bloom-vector bounds or their regression coverage. Prior reconciliation: the review at 862e7b0 had no findings, so there are no carried-forward prior findings. New finding in the latest delta: the one-line copyright-year correction is a standalone fixup commit that should be squashed into the immediately preceding test commit. Additional cumulative findings: none.

Source: Sol reviewer (gpt-5.6-sol), Sonnet reviewer (claude-sonnet-5), and Opus reviewer (claude-opus-4-8) for general and dash-core-commit-history lanes; final verifier: claude-sonnet-5. Orchestrator: openai/gpt-5.6-sol, reasoning high, orchestration-only (not reviewer/verifier evidence).

🟡 1 suggestion(s)

🤖 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/rpc_getmerkleblocks.py`:
- [SUGGESTION] test/functional/rpc_getmerkleblocks.py:1-2: Squash the copyright-year fixup into the commit that introduced the file
  Commit 5b6ac384d7f ("test: fix getmerkleblocks copyright year") only changes the header of test/functional/rpc_getmerkleblocks.py from '# Copyright (c) 2024' to '# Copyright (c) 2026'. That file was created one commit earlier in this same, unmerged PR by 862e7b0e890 ("test: cover getmerkleblocks oversized bloom-filter prechecks") with the wrong year — it has never shipped to develop. Since this only corrects a mistake made earlier in the same stack rather than being an independently reviewable change, and Dash merges commits without squashing (so this becomes permanent git log/blame/bisect noise on a file born one commit earlier), fold it into 862e7b0e890 via `git rebase -i --autosquash` (or a manual fixup) so the file lands with the correct copyright year in a single commit. Verified directly: `git show 862e7b0e890` shows the file added with '(c) 2024', and `git show 5b6ac384d7f` shows the sole change is the year correction, with 5b6ac384d7f not yet reachable from develop.

Comment on lines +1 to +2
#!/usr/bin/env python3
# Copyright (c) 2026 The Dash Core developers

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: Squash the copyright-year fixup into the commit that introduced the file

Commit 5b6ac38 ("test: fix getmerkleblocks copyright year") only changes the header of test/functional/rpc_getmerkleblocks.py from '# Copyright (c) 2024' to '# Copyright (c) 2026'. That file was created one commit earlier in this same, unmerged PR by 862e7b0 ("test: cover getmerkleblocks oversized bloom-filter prechecks") with the wrong year — it has never shipped to develop. Since this only corrects a mistake made earlier in the same stack rather than being an independently reviewable change, and Dash merges commits without squashing (so this becomes permanent git log/blame/bisect noise on a file born one commit earlier), fold it into 862e7b0 via git rebase -i --autosquash (or a manual fixup) so the file lands with the correct copyright year in a single commit. Verified directly: git show 862e7b0e890 shows the file added with '(c) 2024', and git show 5b6ac384d7f shows the sole change is the year correction, with 5b6ac38 not yet reachable from develop.

source: ['codex-dash-core-commit-history', 'sonnet5-dash-core-commit-history', 'opus-dash-core-commit-history']

@UdjinM6 UdjinM6 added this to the 24 milestone Jul 12, 2026
thepastaclaw pushed a commit to thepastaclaw/dash that referenced this pull request Jul 22, 2026
…cation

5b6ac38 test: fix getmerkleblocks copyright year (PastaClaw)
862e7b0 test: cover getmerkleblocks oversized bloom-filter prechecks (PastaClaw)
28d2c6c fix(net): bound CBloomFilter and filteradd vectors before allocation (PastaClaw)

Pull request description:

  Prevent peer-declared FILTERADD and CBloomFilter vector lengths from allocating before their command-specific limits are checked.

  The shared CBloomFilter bound covers FILTERLOAD and governance sync. Each P2P entrypoint catches malformed bounded reads locally and applies the existing 100-point punishment, preventing replay through the outer message-processing catch. The getmerkleblocks RPC keeps its prior complete-oversized, truncated, noncanonical, and above-MAX_SIZE behavior.

  Adds exact CompactSize(MAX_SIZE) regressions for FILTERADD, FILTERLOAD, and governance sync while retaining valid boundary coverage.

  Validation:
  - make -C src dashd dash-cli test/test_dash
  - test_dash --run_test=bloom_tests (14 cases)
  - test/functional/p2p_filter.py
  - test/functional/p2p_govsync_bloom.py
  - test/lint/lint-python.py
  - test/lint/lint-whitespace.py
  - git diff --check
  - manual getmerkleblocks RPC matrix for complete/truncated boundary and malformed-prefix behavior
  - independent exact-range review: ship, no significant findings

ACKs for top commit:
  knst:
    utACK 5b6ac38

Tree-SHA512: 8c433ff069021ccbc2fbaed92764704028a00fddfd9391644e00a3a70c768328a4f479142d8feebfd54a10d8c126ddcf77770f05cb3635a39def89ff320dba2e
thepastaclaw pushed a commit to thepastaclaw/dash that referenced this pull request Jul 22, 2026
…cation

5b6ac38 test: fix getmerkleblocks copyright year (PastaClaw)
862e7b0 test: cover getmerkleblocks oversized bloom-filter prechecks (PastaClaw)
28d2c6c fix(net): bound CBloomFilter and filteradd vectors before allocation (PastaClaw)

Pull request description:

  Prevent peer-declared FILTERADD and CBloomFilter vector lengths from allocating before their command-specific limits are checked.

  The shared CBloomFilter bound covers FILTERLOAD and governance sync. Each P2P entrypoint catches malformed bounded reads locally and applies the existing 100-point punishment, preventing replay through the outer message-processing catch. The getmerkleblocks RPC keeps its prior complete-oversized, truncated, noncanonical, and above-MAX_SIZE behavior.

  Adds exact CompactSize(MAX_SIZE) regressions for FILTERADD, FILTERLOAD, and governance sync while retaining valid boundary coverage.

  Validation:
  - make -C src dashd dash-cli test/test_dash
  - test_dash --run_test=bloom_tests (14 cases)
  - test/functional/p2p_filter.py
  - test/functional/p2p_govsync_bloom.py
  - test/lint/lint-python.py
  - test/lint/lint-whitespace.py
  - git diff --check
  - manual getmerkleblocks RPC matrix for complete/truncated boundary and malformed-prefix behavior
  - independent exact-range review: ship, no significant findings

ACKs for top commit:
  knst:
    utACK 5b6ac38

Tree-SHA512: 8c433ff069021ccbc2fbaed92764704028a00fddfd9391644e00a3a70c768328a4f479142d8feebfd54a10d8c126ddcf77770f05cb3635a39def89ff320dba2e
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
…cation

Backport of dashpay#7444 (upstream merge b9d956d, cherry-picked with -m1).

v23.1.x adaptation: the two new Misbehaving call sites use the NodeId overload (pfrom.GetId()) since develop's Peer& overload does not exist on this branch. All other hunks unchanged from upstream.

(cherry picked from commit b9d956de36a4a2d0a2ba0ecc46a2c5083f8058bf)
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jul 29, 2026
…cation

Backport of dashpay#7444 (upstream merge b9d956d, cherry-picked with -m1).

v23.1.x adaptation: the two new Misbehaving call sites use the NodeId overload (pfrom.GetId()) since develop's Peer& overload does not exist on this branch. All other hunks unchanged from upstream.

(cherry picked from commit b9d956de36a4a2d0a2ba0ecc46a2c5083f8058bf)
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jul 29, 2026
…cation

Backport of dashpay#7444 (upstream merge b9d956d, cherry-picked with -m1).

v23.1.x adaptation: the two new Misbehaving call sites use the NodeId overload (pfrom.GetId()) since develop's Peer& overload does not exist on this branch. All other hunks unchanged from upstream.

(cherry picked from commit b9d956de36a4a2d0a2ba0ecc46a2c5083f8058bf)
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jul 29, 2026
…cation

Backport of dashpay#7444 (upstream merge b9d956d, cherry-picked with -m1).

v23.1.x adaptation: the two new Misbehaving call sites use the NodeId overload (pfrom.GetId()) since develop's Peer& overload does not exist on this branch. All other hunks unchanged from upstream.

(cherry picked from commit b9d956de36a4a2d0a2ba0ecc46a2c5083f8058bf)
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 removed this from the 24 milestone Jul 30, 2026
@UdjinM6 UdjinM6 added this to the 23.1.8 milestone Jul 30, 2026
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.

4 participants