release: prepare v23.1.8

[PastaPastaPasta]

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

• backport: Merge bitcoin/bitcoin#26326: net: don't lock cs_main while reading blocks #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: backport: compact block relay hardening (bitcoin#26898, #27626, #27743, #26969, #29412, #32646, #33296) #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

• fix: limit signing share sessions per peer #7351 (fix: limit signing share sessions per peer) — cherry-picked as a single
  commit and placed before fix: bound pending recovered sig queue to prevent remote OOM #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.
• fix(qt): handle pixel-sized fonts when scaling widgets #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:

• fix: punish invalid dstx messages #7347 (fix: punish invalid dstx messages)
• fix: penalize oversized notfound messages #7348 (fix: penalize oversized notfound messages)

Adaptations worth flagging

• fix: empty platformP2PPort deprecated field in protx listdiff results #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 fix: empty platformP2PPort deprecated field in protx listdiff results #7360 — reading the live port from netInfo instead of the always-zero scalar — is retained.

• fix: bound pending sig share queue #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 fix(qt): handle pixel-sized fonts when scaling widgets #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 fix(qt): handle pixel-sized fonts when scaling widgets #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.

[thepastaclaw]

⛔ Blockers found — Sonnet deferred (commit 24920a0)
Canonical validated blockers: 4

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 59f079c780

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Purge banned queues before processing more signatures

When signature verification bans a peer, this cleanup does not reliably discard its remaining backlog: ProcessPendingRecoveredSigs() runs before the cleanup on every iteration, the cleanup is throttled to once per five seconds, and after the peer is finalized PeerIsBanned() returns false because its PeerRef no longer exists. Consequently, a disconnected attacker can still force processing of up to the per-peer/global caps of expensive recovered signatures. Purge the queue when the peer is banned/finalized, or perform this check before draining using ban state that survives peer removal.

Useful? React with 👍 / 👎.

[UdjinM6]

From claude:

• feature_llmq_simplepose.py — diff between 01fc719 (v23.1.7) and origin/develop: 0 lines. Byte-identical. The 66-line rewrite in Merge fix: bound DKG contribution blob intake #7408 had no structural driver at all.
• walletman.h::addWallet — returns void on the base and on develop; only the release branch has bool. The void→bool signature change, the duplicate-registration guard, and the OverviewPage toggle work in Merge fix: dangling point to cj client #7259 are net-new on this branch, not reconciliations.
• wait_for_quorum_commitment — the base has the inline commitment loop; develop refactored it to call node_has_quorum_commitment. The release-branch version is a third variant that matches neither: base's inline loop plus new count-based semantics. Adapting would mean converging on one of the two existing forms.
• p2p_governance_invs.py (Merge fix(net): authorize governance inv responses via the net-layer per-peer request tracker #7442, +152 lines) — upstream 13b9071 touches no functional test whatsoever, so there's no upstream hunk that needed reshaping.

Overall, it's pretty strange to see large blocks of completely new code suddenly appearing out of nowhere in backport commits. Is smth broken in your backport workflow?

[thepastaclaw]

You're right — the release backport workflow folded standalone patches into aggregate commits without recording their provenance. I traced exact patch identities and rebuilt the history to remove them rather than treating them as conflict resolutions:

• fix: bound DKG contribution blob intake #7408 now changes only src/llmq/dkgsessionmgr.cpp and feature_llmq_dkg_intake.py; the unrelated feature_llmq_simplepose.py / quorum-helper rewrite is gone.
• The test: migrate governance inv cache coverage to unit tests #7387 subset prerequisite now drops the obsolete cache-based p2p_governance_invs.py, matching upstream's coverage migration.
• fix(net): authorize governance inv responses via the net-layer per-peer request tracker #7442 keeps the per-peer authorization adapted to the existing v23 architecture, without the new functional-test rewrite or the unrelated refactor: separate governance from consensus code #7330 routing migration.
• fix: dangling point to cj client #7259 keeps its 13 source commits and necessary v23 architecture adaptation, but removes the net-new addWallet return/duplicate-registration work and both extra Qt toggle-state patches.

Rebuilt head: 0a6c70984a8dc6505176a5db1f0a332c937c38ba on https://github.com/thepastaclaw/dash/tree/tracker-2520.

Validation passed: exact reviewed tree match, 25-commit range-diff (only the four intended logical commits changed), git diff --check, Python compile/flake8/mypy, and lint-includes. The current credential cannot update PastaPastaPasta/release/v23.1.8, so Pasta still needs to replace the PR branch with this rewritten history. The title check also needs chore: prepare v23.1.8 release (the configured semantic types do not include release).

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d77f57361f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Bound per-object fulfilled-request entries

When a peer requests vote sync for many distinct known governance objects, every hash creates a unique entry in CNetFulfilledRequestManager; that store is an unbounded nested map, retains mainnet entries for an hour, and keys its outer map by peer.addr, so reconnecting with new source ports allows the same attacker to multiply the retained entries without triggering the repeat-request penalty. Since the governance object store itself can contain up to one million entries, this reintroduces a remote memory-exhaustion path; keep this throttle in bounded per-peer state or impose a total/per-peer cap and clear it on disconnect.

Useful? React with 👍 / 👎.

[PastaPastaPasta]

Heads up: I rebuilt this branch from scratch, so it's worth a fresh look rather than a re-read of the old diff.

Why: some commits titled Merge #NNNN turned out to contain code that isn't in the upstream PR at all. Two clear examples — feature_llmq_simplepose.py is byte-identical between v23.1.7 and develop, but the old Merge #7408 rewrote 66 lines of it; and p2p_governance_invs.py doesn't exist on develop at all, but had grown to 148 lines.

What that cost us:

• fix(net): bound governance vote signature deserialization #7440 was missing from the branch entirely
• 11 consecutive commits didn't compile (code written against newer upstream APIs that v23.1.x doesn't have)

What's better now:

• Every commit diffed against its upstream merge commit; adaptations are only where v23.1.x predates a refactor
• fix(net): bound governance vote signature deserialization #7440 and test: migrate governance inv cache coverage to unit tests #7387 are back
• All 26 commits compile individually, not just the tip
• Unit + functional tests pass

@UdjinM6 your lock catch on the man pages carried over — it's fixed here too. Sorry that your comments now point at commits that no longer exist.

Two deliberate deviations from upstream are called out in the PR description (#7360's gate and #7415's second bound) — worth a look, since both would otherwise read as mistakes.

(Posted by claude)

[knst]

"s/knst/Konstantin Akimov"
-PastaClaw

?

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 21baf06ae2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Remove the unimplemented disk-read claim

For any requested block absent from m_most_recent_block, this claim is false: ProcessGetBlockData() still takes LOCK(cs_main) at src/net_processing.cpp:2748 and retains it through ReadBlockFromDisk() at line 2787, exactly as in the parent commit. Busy nodes therefore retain the lock contention this release advertises as fixed; either include the bitcoin#26326 backport or remove this release-note entry.

Useful? React with 👍 / 👎.

[knst]

need to updated if removed 7350 [due to multiple conflicts]

[knst]

nit: original 7418 has — no -

it is not supposed to be — but it should not be fixed here but backported 1:1 byte-to-byte to prevent conflicts.

[knst]

need to updated if removed 7350 [due to multiple conflicts]

[knst]

should be dash PR here; not bitcoin

[knst]

LGTM e669540

[UdjinM6]

utACK 5fb4639

[thepastaclaw]

Preliminary review — Codex only

Three in-scope blockers remain in the release stack's resource-hardening changes. Recovered-signature cleanup observes a transient flag, the #7415 adaptation leaves the worker task queue unbounded, and governance throttling stores attacker-controlled per-object keys in an unbounded retained map.

Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.

Review provenance

• Codex reviewers: gpt-5.6-sol — general (completed), gpt-5.6-sol — dash-core-commit-history (completed), gpt-5.6-sol — backport-reviewer (completed)
• Verifier: gpt-5.6-sol — verifier
• Sonnet: not run (deferred by blocker gate)

🔴 3 blocking

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 `src/llmq/net_signing.cpp`:
- [BLOCKING] src/llmq/net_signing.cpp:143-149: Recovered-signature cleanup checks a transient ban flag
  This sweep does not reliably remove a banned peer's pending recovered signatures. `PeerIsBanned()` delegates to `IsBanned()`, which only tests `Peer::m_should_discourage`; `MaybeDiscourageAndDisconnect()` clears that one-shot flag before disconnecting, and `FinalizeNode()` then removes the peer so subsequent checks also return false. Because this cleanup runs only every five seconds, it normally misses the brief observable interval and leaves the source's remaining backlog consuming BLS verification work and global queue capacity. Key the sweep on durable disconnection state, or purge the source immediately when batch verification identifies it as invalid.

In `src/llmq/signing_shares.cpp`:
- [BLOCKING] src/llmq/signing_shares.cpp:1760-1764: Incomplete #7415 adaptation leaves the worker task queue unbounded
  The adapted backport caps shares in the pending maps but not the processing tasks created for them. Every 10 ms, the dispatcher sees any remaining share and appends another `ProcessPendingSigSharesLoop()` closure to `workerPool`; the pool has only one to four threads, each executing worker can remain in its loop while incoming traffic replenishes the pending maps, and `ctpl::thread_pool::push()` stores every additional heap-allocated task in an uncapped `std::queue`. Sustained sig-share traffic can therefore keep all workers occupied while adding roughly 100 redundant tasks per second without a memory bound. Add outstanding processing-task accounting and refuse new dispatches once the worker limit is reached; each task should also process finite, share-count-bounded work so signing tasks can interleave.

In `src/governance/net_governance.cpp`:
- [BLOCKING] src/governance/net_governance.cpp:119-132: Per-object governance throttle creates an unbounded retained map
  Each first vote-sync request for a known object inserts a distinct hash-bearing string into `CNetFulfilledRequestManager`. That manager has no cardinality bound, retains mainnet entries for one hour, and keys its outer map by `CService`, whose ordering includes the remote port. An attacker can request every advertised object once, reconnect with a different source port or address, and repeat without receiving any misbehavior score. This creates retained and eventually serialized state proportional to syncable objects multiplied by connection identities. Use a bounded per-peer cache or rate bucket instead of inserting every object hash into the global fulfilled-request map.

[thepastaclaw]

🔴 Blocking: Recovered-signature cleanup checks a transient ban flag

This sweep does not reliably remove a banned peer's pending recovered signatures. PeerIsBanned() delegates to IsBanned(), which only tests Peer::m_should_discourage; MaybeDiscourageAndDisconnect() clears that one-shot flag before disconnecting, and FinalizeNode() then removes the peer so subsequent checks also return false. Because this cleanup runs only every five seconds, it normally misses the brief observable interval and leaves the source's remaining backlog consuming BLS verification work and global queue capacity. Key the sweep on durable disconnection state, or purge the source immediately when batch verification identifies it as invalid.

_{source: ['codex']}

[thepastaclaw]

🔴 Blocking: Per-object governance throttle creates an unbounded retained map

Each first vote-sync request for a known object inserts a distinct hash-bearing string into CNetFulfilledRequestManager. That manager has no cardinality bound, retains mainnet entries for one hour, and keys its outer map by CService, whose ordering includes the remote port. An attacker can request every advertised object once, reconnect with a different source port or address, and repeat without receiving any misbehavior score. This creates retained and eventually serialized state proportional to syncable objects multiplied by connection identities. Use a bounded per-peer cache or rate bucket instead of inserting every object hash into the global fulfilled-request map.

_{source: ['codex']}

[PastaPastaPasta]

added 7348 and 7347