fix: guard VerifyContributionShares against an empty verification-vector set

[PastaPastaPasta]

Issue being fixed or feature implemented

CBLSWorker::AsyncVerifyContributionShares() terminates the node when handed an empty
verification-vector set.

The path is degenerate rather than obviously wrong, which is why it survives review:

• VerifyVerificationVectors() returns true for an empty span — its loop body never runs.
• ContributionVerifier::Start() then schedules zero work (batchCount == 0 when aggregated,
  or a zero-length one-by-one batch when not).
• Nothing ever invokes doneCallback. No scheduled lambda captured shared_from_this(), so
  the ContributionVerifier is released as soon as Start() returns, the promise held by the
  future-based wrapper is destroyed without set_value(), and .get() throws
  std::future_error(broken_promise).

The DKG phase handler in net_dkg.cpp only catches AbortPhaseException, so that throw
propagates out of the phase-handler thread and std::terminate aborts the node.

Reachability

ActiveDKGSession::VerifyPendingContributions() is the only caller that can produce an empty
input. It early-returns on pendingContributionVerifications.empty(), but then filters the
pending set by m->bad || m->weComplain. If every pending member was marked bad between
enqueue and flush — a duplicate contribution triggering MarkBadMember() is the way in — the
filtered vvecs is empty while the pre-filter check passed.

Getting there is hard, and the constraints are worth spelling out so the severity is not
over-estimated:

• An attacker can only mark its own masternodes bad. Message signatures are batch-verified
  (BatchVerifyMessageSigs) before ReceiveMessage() runs, so the duplicate contribution that
  triggers MarkBadMember() must be validly signed by that member's own operator key. Honest
  members cannot be poisoned into the bad set.
• Our own contribution is always in the pending list. EnqueueOwn() feeds the local
  contribution through the same ReceiveMessage() path at the start of the contribute phase,
  and we never mark ourselves bad. The filtered set can therefore only be empty if the
  32-entry flush inside ReceiveMessage() already cleared our own entry — i.e. the node must
  have accepted at least 33 contributions.
• That rules out every quorum smaller than 33 outright: all test/devnet params and
  llmq_25_67 cannot reach this code path at all. Only sizes 50/60/100/400 are candidates,
  and there an attacker additionally has to place its members entirely in the post-flush tail
  (N - t ≡ 0 mod 32 → 18/50, 28/60, 4/100, 16/400 members) while winning an ordering race
  against state it cannot directly observe.
• The 32-entry flush path cannot produce the empty case by itself: each member enqueues at
  most once, and the flush fires synchronously on the 32nd enqueue, so the just-enqueued
  member is never bad at that instant. Only the VerifyAndComplain() call can observe an
  all-bad pending set.

The practical reading: this is defensive hardening of a crash path, not a readily exploitable
DoS. It is still a std::terminate reachable from network input, the guard costs three lines,
and the same degenerate input will bite the next caller that shows up.

Note that the reachability constraints above are also why no existing test caught this — the
functional-test quorums are far too small to ever flush the pending set.

What was done?

Two commits, at two layers:

1. CBLSWorker::AsyncVerifyContributionShares() short-circuits on empty input and
   invokes doneCallback with an empty result. This mirrors the empty-input handling that
   AsyncBuildQuorumVerificationVector() and AsyncAggregateHelper() already have, so the
   worker API is now consistent about degenerate input.

2. ActiveDKGSession::VerifyPendingContributions() skips the worker call entirely when
   the post-filter member set is empty, rather than relying on the guard above. This avoids a
   pointless async round-trip and keeps the call site correct on its own terms.

The first commit is the fix; the second is defence in depth.

How Has This Been Tested?

Unit tests in src/test/bls_tests.cpp:

• bls_verify_contribution_shares_empty_vvecs_tests — without the fix this fails with
  std::future_error: The associated promise has been destructed prior to the associated state becoming ready., the exact production failure. With the fix it returns an empty
  result on both the aggregated and non-aggregated paths.
• bls_verify_contribution_shares_honest_tests — asserts that acceptance behaviour for
  non-empty input is unchanged, so the guard cannot mask a real verification failure. The
  quorum size (10) is deliberately larger than ContributionVerifier's batch size of 8, so
  corrupting one share in the first batch must fail that batch, fall back to per-contribution
  verification, blame exactly the corrupted index, and leave the second batch accepted
  wholesale. That property previously had no coverage under make check at all — only
  src/bench/bls_dkg.cpp exercised it, and benches do not run in CI.

Full bls_tests suite passes. Built and run on macOS/arm64 against current develop.

Breaking Changes

None. The change only affects an input shape that previously terminated the process.

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

[thepastaclaw]

✅ Final review complete — no blockers (commit 6cc098e)

[thepastaclaw]

Final validation — Codex + Sonnet

The empty-input short-circuit correctly completes callback- and future-based callers, and the DKG call-site guard safely clears a fully filtered pending batch. The only reported concern—placing the regression test before its fix—is explicitly permitted by current develop-branch guidance when the failure is documented, as it is here. No in-scope defects remain.

Source: reviewers codex/general=gpt-5.6-sol(completed); codex/dash-core-commit-history=gpt-5.6-sol(completed); claude/general=claude-sonnet-5(failed); claude/dash-core-commit-history=claude-sonnet-5(completed); claude/general=claude-sonnet-5(completed); verifier=codex/final-verifier=gpt-5.6-sol(completed) fallback_for_sonnet_verifier=true; coordinator=openclaw-agent/cliproxy/gpt-5.6-sol(orchestration-only).

Review provenance

• Codex reviewers: gpt-5.6-sol — general (completed), gpt-5.6-sol — dash-core-commit-history (completed)
• Verifier: gpt-5.6-sol — final-verifier (fallback)
• Sonnet reviewers: claude-sonnet-5 — general (failed), claude-sonnet-5 — dash-core-commit-history (completed), claude-sonnet-5 — general (completed)

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6ed9fc8a-f8eb-490d-88e3-7f6138b9f572

📥 Commits

Reviewing files that changed from the base of the PR and between 2d1d08d and 6cc098e.

📒 Files selected for processing (4)

• src/active/dkgsession.cpp
• src/bls/bls_worker.cpp
• src/bls/bls_worker.h
• src/test/bls_tests.cpp

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

• src/active/dkgsession.cpp
• src/bls/bls_worker.cpp

---

Walkthrough

The change adds immediate empty-input handling to asynchronous BLS contribution-share verification. It defines a shared batch-size constant and uses it for contribution verification. It also prevents DKG pending verification from invoking the BLS worker when no eligible members remain. Regression tests cover empty inputs, both verification paths and BLS schemes, multi-batch honest shares, and isolated rejection of a corrupted share.

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

Suggested reviewers: thepastaclaw

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main fix: guarding contribution-share verification against empty verification-vector input.
Description check	✅ Passed	The description directly explains the crash, the layered fix, reachability, tests, and compatibility impact.
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

• X
• Mastodon
• Reddit
• LinkedIn

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

[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 see https://github.com/UdjinM6/dash/commits/review/7486-suggestions/

[UdjinM6]

utACK 6cc098e