fix(net): bound CoinJoin message vector intake

[thepastaclaw]

Depends on #7439. Please review only the three CoinJoin-specific commits here.

Issue being fixed or feature implemented

CoinJoin server messages could deserialize or process peer-controlled input/output vectors before enforcing CoinJoin session bounds. This hardens the final-signature and entry intake paths by separating the wire-safety bound from the per-entry semantic bound.

What was done?

• Accept DSSIGNFINALTX only during signing and only from an active session participant.
• Bound DSSIGNFINALTX input deserialization at the largest complete CoinJoin size: currently 20 participants × 9 inputs = 180.
• Apply the same 180-element wire-safety cap to both CCoinJoinEntry vectors, rejecting larger declarations before vector materialization.
• Keep the existing semantic limit of 9 inputs and 9 outputs per entry in AddEntry().
• Allow declarations from 10 through 180 to deserialize safely and reach semantic rejection, consuming collateral only when it matches a collateral accepted for the active session.
• Remove the special MAX + 1 input rule, oversized-output flag, and asymmetric input/output deserialization behavior.
• Add focused boundary, participant, session-state, collateral, and round-trip tests.

The reusable bounded-vector deserialization primitive is introduced separately in #7439.

How Has This Been Tested?

• make -C src -j15 test/test_dash
• ./src/test/test_dash --run_test=coinjoin_inouts_tests (8/8 passed)
• test/lint/lint-whitespace.py
• Focused LogPrint format-string lint
• Focused clang-format-diff.py
• git diff --check
• Independent exact-range Codex review (9474fc5e2a1..48a1eb95838): ship, 0 findings

Breaking Changes

None.

Checklist

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

[coderabbitai]

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: 65229016-5aa0-49e3-aa27-068d0280f327

📥 Commits

Reviewing files that changed from the base of the PR and between 48a1eb95838ba3ce1ef1bdee48657e30bb6f05e8 and 45ee9af.

📒 Files selected for processing (4)

• src/coinjoin/coinjoin.cpp
• src/coinjoin/coinjoin.h
• src/coinjoin/server.cpp
• src/test/coinjoin_inouts_tests.cpp

🚧 Files skipped from review as they are similar to previous changes (4)

• src/coinjoin/coinjoin.cpp
• src/test/coinjoin_inouts_tests.cpp
• src/coinjoin/server.cpp
• src/coinjoin/coinjoin.h

---

Walkthrough

CoinJoin entry serialization now uses explicit methods with bounded input and output vector deserialization. DSSIGNFINALTX processing validates signing state and peer participation, bounds transaction-input counts, and passes peer context through the server. AddEntry rejects oversized inputs or outputs and consumes matching collateral. Tests cover stream handling, session preservation, oversized vectors, and boundary round trips.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
participant Peer
participant CCoinJoinServer
participant CDataStream
Peer->>CCoinJoinServer: DSSIGNFINALTX message
CCoinJoinServer->>CCoinJoinServer: Validate state and participant address
CCoinJoinServer->>CDataStream: Read bounded transaction-input count
alt oversized count
CCoinJoinServer-->>Peer: Reject with ERR_MAXIMUM
else valid count
CCoinJoinServer->>CCoinJoinServer: Process input signatures
end

Loading

Suggested reviewers:udjinm6, knst, kwvg

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 18.18% 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 is concise and accurately summarizes the main change: bounding CoinJoin message vector intake.
Description check	✅ Passed	The description clearly matches the CoinJoin hardening changes and test coverage in the pull request.
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

• X
• Mastodon
• Reddit
• LinkedIn

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

[thepastaclaw]

🔍 Review in progress — actively reviewing now (commit 45ee9af)

[github-actions]

⚠️ Potential Merge Conflicts Detected

This PR has potential conflicts with the following open PRs:

• feat: coinjoin promotion / demotion #7052 - feat: coinjoin promotion / demotion

Please coordinate with the authors of these PRs to avoid merge conflicts.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:fa8feddcd3

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[thepastaclaw]

Code Review

Source: reviewers claude/general=opus(failed), codex/general=gpt-5.5(completed), claude/dash-core-commit-history=opus(failed), codex/dash-core-commit-history=gpt-5.5(completed); verifier=gpt-5.5

The PR addresses the intended CoinJoin intake hardening, but the new bounded deserialization introduces a portable build regression. The new braced size_t initializers from ReadCompactSize() are ill-formed on supported 32-bit targets because ReadCompactSize() returns uint64_t.

Verified findings

• [BLOCKING] src/coinjoin/coinjoin.h:182-194: Avoid narrowing list-initialization from ReadCompactSize
  ReadCompactSize() returns uint64_t, but these new size_t variables are initialized with braces. On supported 32-bit builds, size_t cannot represent every uint64_t value, so const size_t x{ReadCompactSize(s)}; is ill-formed before tests can run. The value is already range-checked by ReadCompactSize() against MAX_SIZE, which fits in 32-bit size_t, so ordinary copy-initialization preserves the intended behavior without triggering C++ narrowing rules.
• [BLOCKING] src/coinjoin/server.cpp:228: Avoid narrowing list-initialization in DSSIGNFINALTX
  ReadCompactSize() returns uint64_t, so list-initializing a size_t with braces is a narrowing conversion on 32-bit targets and fails to compile. Dash still carries i686 depends hosts, so this new CoinJoin server path needs to use a form that remains valid when size_t is 32 bits.

Note: posted as a COMMENT review because GitHub does not allow approving/requesting changes on my own PR via the normal review event.

[thepastaclaw]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[thepastaclaw]

Handled the self-review finding by replacing the braced size_t initializers for compact-size counts with copy-initialization (a031fe718e8). git diff --check passes. I also tried the local unit-test build target in this worktree, but it is not configured with a src/test/test_dash rule.

[thepastaclaw]

Code Review

Source: reviewers claude/general=opus(failed: extra-usage quota), codex/general=gpt-5.5(ok), claude/dash-core-commit-history=opus(failed: extra-usage quota), codex/dash-core-commit-history=gpt-5.5(ok); verifier=gpt-5.5 (Opus verifier failed: extra-usage quota)

Reviewed commit: a031fe71.

The two prior blocking portability findings from fa8feddc are fixed at the current head: all new ReadCompactSize() counts now use copy-initialization instead of braced size_t initialization. I found no remaining code-level correctness issue in the changed CoinJoin bounds handling.

Prior Finding Reconciliation

• FIXEDsrc/coinjoin/coinjoin.h:182-194 — txdsin_size and txout_size now use = ReadCompactSize(s).
• FIXEDsrc/coinjoin/server.cpp:228 — txins_size now uses = ReadCompactSize(vRecv).

Carried-forward prior findings: none.

New Findings In Latest Delta

• [SUGGESTION] <commit:a031fe7>: Fold compact-size narrowing fix into the introducing commits
  Commit a031fe718e8 only changes ReadCompactSize() count initializers introduced by the previous two commits. If the stack is merged as-is, the permanent history contains intermediate commits with a portable-build regression that is fixed only by the final cleanup commit. Fold the src/coinjoin/server.cpp change into 450b3bb7f39 and the src/coinjoin/coinjoin.h changes into fa8feddcd3f so each substantive hardening commit is buildable on its own.

CodeRabbit context: no concrete inline findings; no reactions posted.

[thepastaclaw]

Resolved the self-review commit-history feedback by folding the compact-size narrowing cleanup into the commits that introduced the reads:

• 7704aa835bf folds the DSSIGNFINALTX count initialization fix into fix: bound CoinJoin final signature intake.
• d80de2aec27 folds the CCoinJoinEntry count initialization fixes into fix: bound CoinJoin entry intake.

Validation:

• git diff --check
• ./autogen.sh && ./configure --without-gui
• make -C src test/test_dash -j4
• src/test/test_dash --run_test=coinjoin_inouts_tests --catch_system_errors=no --log_level=test_suite

[thepastaclaw]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[thepastaclaw]

Code Review

Source: reviewers claude/general=opus(failed: extra-usage quota), codex/general=gpt-5.5(completed), claude/dash-core-commit-history=opus(failed: extra-usage quota), codex/dash-core-commit-history=gpt-5.5(completed); verifier=gpt-5.5(codex)

Reviewed commit: d80de2ae.

The current head rewrites the stack into two self-contained CoinJoin hardening commits. The prior commit-history suggestion from a031fe71 is fixed: the standalone compact-size cleanup commit is gone, and the ReadCompactSize() count initialization fixes are folded into the introducing commits. I found no in-scope issues in the cumulative CoinJoin vector-bound changes.

Prior Finding Reconciliation

• FIXED<commit:a031fe7> — Fold compact-size narrowing fix into the introducing commits. Current stack has only 7704aa835bf and d80de2aec27, both substantive/self-contained, and no separate compact-size cleanup commit remains.

Carried-forward prior findings: none.

New Findings In Latest Delta

None. git diff a031fe71..d80de2ae is tree-empty after the history rewrite, and the cumulative review found no new current-head issue.

CodeRabbit context: no concrete inline findings; no reactions posted.

Note: GitHub does not allow me to submit an approval review on my own PR, so this is posted as a COMMENT while preserving the verified clean result.

[thepastaclaw]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[thepastaclaw]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[UdjinM6]

pls consider a2c7b0a

[thepastaclaw]

Applied UdjinM6’s a2c7b0a39f79cf081a20686f00d5c4fa456dad89 suggestion and folded it into the existing CCoinJoinEntry intake commit.

Updated head: 1894eacb135

What changed:

• Added entry_accepts_max_sized_vectors, covering the inclusive COINJOIN_ENTRY_MAX_SIZE boundary for both inputs and outputs.

Validation passed:

• git diff --check origin/fix/coinjoin-message-vector-bounds..HEAD
• COMMIT_RANGE=origin/fix/coinjoin-message-vector-bounds..HEAD test/lint/lint-whitespace.py
• ./autogen.sh && ./configure --without-gui
• make -C src test/test_dash -j4
• src/test/test_dash --run_test=coinjoin_inouts_tests --catch_system_errors=no --log_level=test_suite

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:1894eacb13

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[thepastaclaw]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

✅ Action performed

Reviews resumed.

[knst]

LGTM d81ff3c

[knst]

nit: consider moving this to the common helper ; and inline MakeSecretKey there

 BOOST_REQUIRE(m_node.mn_sync);
m_node.mn_sync->SwitchToNextAsset();
BOOST_REQUIRE(m_node.mn_sync->IsBlockchainSynced());
CActiveMasternodeManager mn_activeman(*Assert(m_node.connman), *Assert(m_node.dmnman), MakeSecretKey());
TestableCoinJoinServer server(m_node.peerman.get(), *Assert(m_node.chainman), *Assert(m_node.connman),
*Assert(m_node.dmnman), *Assert(m_node.dstxman), *Assert(m_node.mn_metaman),
*Assert(m_node.mempool), mn_activeman, *Assert(m_node.mn_sync),
*Assert(m_node.llmq_ctx->isman));
// Same setup, but this time the oversized DSSIGNFINALTX comes from the
// session participant itself. It must still be rejected without materializing
// the txin vector and without collapsing the session for everyone else.
auto participant = MakePeer(/*id=*/7, /*ipv4=*/0x0a000001);
server.SeedParticipant(participant->addr);
server.EnterSigningState();

[coderabbitai]

✅ Action performed

Reviews resumed.

[UdjinM6]

utACK 45ee9af