test: backport LLMQ recsig relay symmetry by thepastaclaw · Pull Request #7324 · dashpay/dash

thepastaclaw · 2026-05-15T21:47:57Z

Backport LLMQ recovered-sig relay fix

Summary

Backport revert: "fix: avoid improper dual way connection attempts" + tests #7289 to v23.1.x.
Restore full-mesh recovered-signature relay membership when
SPORK_21_QUORUM_ALL_CONNECTED is active.
Add the upstream regression check for symmetric QSENDRECSIGS negotiation in
feature_llmq_signing.py --spork21.

Context

feature_llmq_signing.py --spork21 is still flaky on the v23.1.x CI lane
because the #7289 fix is present on develop but not on the
release branch.

The latest hit was in #7322, whose branch only changes
.github/workflows/predict-conflicts.yml, so the LLMQ failure is unrelated to
that PR.

Validation

python3 -m py_compile test/functional/feature_llmq_signing.py
git diff --check upstream/v23.1.x...test/backport-7289-v23-llmq-signing

Code review gate:

code-review dashpay/dash \
  upstream/v23.1.x \
  test/backport-7289-v23-llmq-signing \
  "Backport PR #7289 to v23.1.x to fix the #7323 spork21 flake"

Result: Recommendation: ship

Functional test was not run locally; this workspace does not have a configured
Dash Core build directory for the v23.1.x worktree.

This reverts a57a811 from PR dashpay#6967. `relayMembers` and `connections` in EnsureQuorumConnections are not interchangeable. * `connections`: who this node should connect TO. For each pair (A, B) only the deterministic-outbound side is listed, so the pair results in one TCP connection. * `relayMembers`: who this node should ask to push recovered sigs. For every already-connected MN in the set we send QSENDRECSIGS=true, and the peer then flips m_wants_recsigs=true on its side. For the handshake to happen in both directions, the set must list every other quorum member -- not just the outbound half. After a57a811, only the outbound half is listed, so on the inbound half of each pair m_wants_recsigs stays false. RelayRecoveredSig only pushes QSIGREC to peers with m_wants_recsigs=true, so half of all proactive recovered-sig pushes are silently dropped. This only triggers with spork21 on (IsAllMembersConnectedEnabled returns true). In this case both the path that uses the half-mesh outbound subset and the path that relies on proactive QSIGREC. This fixes the functional test `feature_llmq_signing.py --spork21`, which times out in wait_for_sigs ~60% of the time while the non-spork21 variant passes on the same CI job. (cherry picked from commit 0e33aa2)

thepastaclaw · 2026-05-15T21:48:05Z

✅ Review complete (commit 15fd508)

github-actions · 2026-05-15T21:48:24Z

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

thepastaclaw · 2026-05-15T21:48:40Z

@coderabbitai review

coderabbitai · 2026-05-15T21:48:46Z

✅ Actions performed

Review triggered.

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 · 2026-05-15T21:53:22Z

Walkthrough

This PR refactors LLMQ quorum connection management to separate outbound connection topology from relay member topology. The core change in EnsureQuorumConnections now maintains a distinct relayMembers set computed based on quorum membership and the all-members-connected configuration, rather than reusing the outbound connections set. For members with all-members-connected enabled, relay becomes a full mesh; otherwise it uses relay-derived deterministic selection. Non-members retain watch-based relay topology. A new test assertion verifies QSENDRECSIGS messages flow symmetrically between quorum member peers.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'test: backport LLMQ recsig relay symmetry' accurately describes the primary change—adding a test assertion for QSENDRECSIGS symmetry as part of backporting the LLMQ relay fix.
Description check	✅ Passed	The description clearly explains the backport objective, links to upstream `#7289`, references the flaky test issue `#7323`, and outlines validation steps.
Linked Issues check	✅ Passed	The PR fully addresses the coding requirements from `#7323`: it backports the recovered-signature relay fix and adds the regression test for symmetric QSENDRECSIGS handshake.
Out of Scope Changes check	✅ Passed	All changes are scoped to the linked issues: modifications to EnsureQuorumConnections in utils.cpp implement the relay fix, and the test changes add the regression check.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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 and usage tips.}

coderabbitai

Actionable comments posted: 1

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 9958cbf1-ef3c-43bc-bd61-ca234947dacc

📥 Commits

Reviewing files that changed from the base of the PR and between b564a33 and 95b5b5d.

📒 Files selected for processing (2)

src/llmq/utils.cpp
test/functional/feature_llmq_signing.py

If relayMembers gets conflated with the outbound-only connections set, one direction of each MN-MN pair never sends QSENDRECSIGS, the inbound side keeps m_wants_recsigs=false, and half of all proactive QSIGREC pushes are silently dropped which could be visible only as slow/flaky signing. (cherry picked from commit c1cdb75)

thepastaclaw · 2026-05-15T23:15:19Z

/rerun

coderabbitai Bot reviewed May 15, 2026

View reviewed changes

Comment thread test/functional/feature_llmq_signing.py Outdated

thepastaclaw force-pushed the test/backport-7289-v23-llmq-signing branch from 95b5b5d to 15fd508 Compare May 15, 2026 22:12

thepastaclaw mentioned this pull request May 15, 2026

tests: feature_dip3_v19.py is flakey #6702

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: backport LLMQ recsig relay symmetry#7324

test: backport LLMQ recsig relay symmetry#7324
thepastaclaw wants to merge 2 commits into
dashpay:v23.1.xfrom
thepastaclaw:test/backport-7289-v23-llmq-signing

thepastaclaw commented May 15, 2026

Uh oh!

thepastaclaw commented May 15, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 15, 2026 •

edited

Loading

Uh oh!

thepastaclaw commented May 15, 2026

Uh oh!

coderabbitai Bot commented May 15, 2026

Uh oh!

coderabbitai Bot commented May 15, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

thepastaclaw commented May 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

thepastaclaw commented May 15, 2026

Backport LLMQ recovered-sig relay fix

Summary

Context

Validation

Uh oh!

thepastaclaw commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ No Merge Conflicts Detected

Uh oh!

thepastaclaw commented May 15, 2026

Uh oh!

coderabbitai Bot commented May 15, 2026

Uh oh!

coderabbitai Bot commented May 15, 2026

Walkthrough

Estimated code review effort

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thepastaclaw commented May 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

thepastaclaw commented May 15, 2026 •

edited

Loading

github-actions Bot commented May 15, 2026 •

edited

Loading