Skip to content

fix: bound pending sig share queue#7413

Closed
PastaPastaPasta wants to merge 1 commit into
dashpay:developfrom
PastaPastaPasta:codex/bound-pending-sig-shares
Closed

fix: bound pending sig share queue#7413
PastaPastaPasta wants to merge 1 commit into
dashpay:developfrom
PastaPastaPasta:codex/bound-pending-sig-shares

Conversation

@PastaPastaPasta

Copy link
Copy Markdown
Member

Summary

Bound pending LLMQ sig-share admission before worker verification so direct QSIGSHARE and batched QBSIGSHARES traffic cannot grow retained verification work without limit.

Root cause

Incoming sig shares were admitted after only cheap quorum/member checks and retained in pendingIncomingSigShares. Verification happens later through the dispatcher and worker pool, so peers could enqueue retained work faster than it drains. The queue had no per-peer or aggregate cap, and banning a peer did not immediately clear pending incoming shares.

Changes

  • Add MAX_PENDING_SIG_SHARES_PER_NODE and MAX_PENDING_SIG_SHARES_TOTAL bounds.
  • Track pending incoming sig-share counts per node plus globally, without repeated map scans.
  • Gate both direct and batched sig-share writers before pending admission; over-cap shares are dropped without misbehaviour scoring.
  • Clear pending incoming sig shares immediately when a peer is marked banned.
  • Skip banned node states while collecting pending shares for worker verification.
  • Add focused unit coverage for per-node cap behavior and counter cleanup.

Validation

  • git diff --check passed.
  • make -C src/test check could not run because this checkout is not configured (No rule to make target 'dash_test_check').
  • cmake -S . -B /private/tmp/dash-codex-build -DBUILD_TESTS=ON -DBUILD_GUI=OFF could not run because cmake is not installed.
  • ./autogen.sh could not run because autoconf is not installed.

The code path was manually rechecked to confirm direct and batched admission now use the capped helper, ban cleanup clears pending shares, and collection skips banned nodes.

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

PastaPastaPasta added a commit that referenced this pull request Jul 11, 2026
bdca48c refactor: use CountedBucketMap for counting total amount of pending sigs (Konstantin Akimov)
355da45 fix: cap sig-share verification batch by share count (PastaClaw)
08d9533 fix: cap amount of unverified batches by 4 (Konstantin Akimov)
e2862fe fix: the actual cap enforcement for pending signatures (Konstantin Akimov)
4666ef3 test: add regression tests for CountedBucketMap (Konstantin Akimov)
ec15f90 refactor: use dedicated util structure to count internal objects in signing-shares (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented

  Pending incoming LLMQ sig shares are admitted cheaply and verified later by worker threads. Direct and batched share traffic could grow retained pending verification work without per-node/global backpressure.

  This replaces closed PR #7413 and keeps the same scope: resource/backpressure hardening for LLMQ signing, with no consensus or user-facing behavior change.

  ## What was done?

  - Add per-node and global caps for pending incoming sig shares.
  - Track pending counts explicitly so cap checks do not require repeated scans.
  - Gate `QSIGSHARE` and `QBSIGSHARES` pending admission; over-cap shares are dropped without misbehavior scoring.
  - Clear pending incoming shares when a peer is banned and skip banned node states during collection.
  - Refuse new pending work for already-banned node states.
  - Add unit coverage for cap/drop/count cleanup behavior.

  ## How Has This Been Tested?

  - `./autogen.sh`
  - `./configure --without-gui --disable-bench --disable-fuzz-binary`
  - `make -C src test/test_dash -j$(sysctl -n hw.ncpu)`
  - `src/test/test_dash --run_test=llmq_utils_tests`
  - `git diff --check upstream/develop..HEAD`
  - `COMMIT_RANGE=upstream/develop..HEAD test/lint/lint-whitespace.py`

  ## Breaking Changes

  None.

  ## 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 _(for repository code-owners and collaborators only)_

ACKs for top commit:
  kwvg:
    utACK bdca48c

Tree-SHA512: e2403635a608aab5488dbf10b481a778e735a6e5b065eec5a4cc444818f61e89b3a688f82c14aaceae07fd714432d39c013769a54aa66e31bb8abddb3531aaf3
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.

1 participant