backport: Merge bitcoin/bitcoin#26749

[vijaydasmp]

bitcoin backports

[github-actions]

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

[thepastaclaw]

✅ Review complete (commit 5aa137e)

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5aa137ee7d

ℹ️ 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]

Update remaining CheckQueueControl callers

Changing CCheckQueueControl::Add to accept only std::vector<T>&& leaves existing Dash-specific LLMQ callers uncompilable: git grep "Add(vChecks" still finds queue_control->Add(vChecks) in src/llmq/commitment.cpp lines 76 and 90, and an lvalue vector cannot bind to this rvalue-reference parameter. Please either keep an lvalue-compatible overload or move/update those call sites as part of this API change.

Useful? React with 👍 / 👎.

[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: f3e93beb-9d06-4b8a-9e48-2144d412d4e2

📥 Commits

Reviewing files that changed from the base of the PR and between ebfdb8b and 5aa137e.

📒 Files selected for processing (6)

• src/bench/checkqueue.cpp
• src/checkqueue.h
• src/test/checkqueue_tests.cpp
• src/test/fuzz/checkqueue.cpp
• src/validation.cpp
• src/validation.h

---

Walkthrough

The PR replaces swap-based patterns with move semantics throughout the check queue infrastructure. CScriptCheck loses its default constructor, swap method, and copy semantics, gaining defaulted move construction/assignment. CCheckQueue::Add and CCheckQueueControl::Add now accept std::vector<T>&& instead of lvalue references; internally, batch insertion uses move iterators and batch extraction uses iterator-range move construction. All callers—validation.cpp, the benchmark, unit tests, and fuzz tests—are updated to pass vectors via std::move. Auxiliary check types (FakeCheck, FailingCheck, UniqueCheck, FrozenCleanupCheck, DumbCheck, PrevectorJob) have their swap methods removed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

• PastaPastaPasta
• thepastaclaw
• knst

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 46.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	The description is related to the change set but too vague to describe the actual backport or code changes.	Replace it with a short summary of the backport, such as the move-semantics and checkqueue refactors it contains.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies this as a backport of Bitcoin PR #26749, which matches the change set.
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]

Code Review

Policy gate: an agent-reported missing upstream prerequisite was restored as a blocking finding. This PR is a full Bitcoin backport, and omitted upstream hunks require their prerequisite PRs unless the finding is explicitly allowlisted (intentional_exclusion / policy_override). The agent's original evidence is preserved in the finding(s) below.

Prior verifier summary (overridden by policy gate): Backport of bitcoin#26749 changes CCheckQueue/CCheckQueueControl::Add to take std::vector&&, but the Dash-specific LLMQ caller in src/llmq/commitment.cpp was not adapted — both call sites still pass lvalues and will fail to compile. A secondary concern is that the Dash-specific BlsCheck element type has a user-declared destructor that suppresses implicit move operations, so the LLMQ check path will silently fall back to copies even after the call sites are fixed, defeating the refactor's purpose on this path.

🔴 6 blocking | 🟡 2 suggestion(s)

Findings not posted inline (4)

These findings could not be anchored to the current diff, but they are still part of this review.

• [BLOCKING] src/llmq/commitment.cpp:76: queue_control->Add(vChecks) passes an lvalue — won't compile after the signature change — This PR changes CCheckQueueControl::Add (src/checkqueue.h:233) and CCheckQueue::Add (src/checkqueue.h:163) from taking std::vector& to std::vector&&. Every upstream call site in the diff was updated to use std::move(vChecks), but this Dash-specific LLMQ call site in CFinalCommitment::...
• [BLOCKING] src/llmq/commitment.cpp:90: Second LLMQ Add() call also passes an lvalue — same compile failure — Same problem as line 76: queue_control->Add(vChecks) passes a non-movable lvalue to the new Add(std::vector&&) overload. Both LLMQ call sites in this function must be updated together for the file to compile.
• [SUGGESTION] src/llmq/utils.h:45-58: BlsCheck suppresses implicit moves — LLMQ check path silently copies — After this backport, CCheckQueue::Loop uses std::make_move_iterator to drain the internal queue, and Add likewise inserts with std::make_move_iterator. Upstream prepared CScriptCheck for this by deleting copy ops and defaulting move ops. The Dash-specific BlsCheck (the element type used by m_bls_...
• [BLOCKING] src/llmq/commitment.cpp:76-90: Incomplete Dash adaptation for bitcoin#26749 — bitcoin#26749 changes CCheckQueueControl::Add and CCheckQueue::Add from taking std::vector& to std::vector&& and upstream updates all affected call sites to pass std::move(vChecks). Dash has an additional CCheckQueue user for LLMQ BLS checks that does not exist upstream. In CFinalCommitment...

🤖 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/commitment.cpp`:
- [BLOCKING] src/llmq/commitment.cpp:76: queue_control->Add(vChecks) passes an lvalue — won't compile after the signature change
  This PR changes CCheckQueueControl<T>::Add (src/checkqueue.h:233) and CCheckQueue<T>::Add (src/checkqueue.h:163) from taking std::vector<T>& to std::vector<T>&&. Every upstream call site in the diff was updated to use std::move(vChecks), but this Dash-specific LLMQ call site in CFinalCommitment::VerifySignatureAsync was not. The lvalue vChecks cannot bind to the new rvalue reference parameter, so this file will fail to build. Upstream review couldn't catch this because the LLMQ caller is Dash-only.
- [BLOCKING] src/llmq/commitment.cpp:90: Second LLMQ Add() call also passes an lvalue — same compile failure
  Same problem as line 76: queue_control->Add(vChecks) passes a non-movable lvalue to the new Add(std::vector<T>&&) overload. Both LLMQ call sites in this function must be updated together for the file to compile.
- [BLOCKING] src/llmq/commitment.cpp:76-90: Incomplete Dash adaptation for bitcoin#26749
  bitcoin#26749 changes CCheckQueueControl::Add and CCheckQueue::Add from taking std::vector<T>& to std::vector<T>&& and upstream updates all affected call sites to pass std::move(vChecks). Dash has an additional CCheckQueue user for LLMQ BLS checks that does not exist upstream. In CFinalCommitment::VerifySignatureAsync, both queue_control->Add(vChecks) calls still pass an lvalue vector. With the new signature in src/checkqueue.h, these calls cannot bind to std::vector<utils::BlsCheck>&&, so the backport is incomplete and this Dash-specific file will not compile. This is a merge/adaptation gap rather than a missing upstream prerequisite; upstream had no corresponding LLMQ call site to update.

  ---
  **Policy gate (backport-prereq-restore):** For full upstream backport PRs, a missing prerequisite is blocking unless the finding is explicitly allowlisted (e.g. `intentional_exclusion: true` or a matching entry in `policy_overrides`). The agent's original evidence above is the basis for this block; either backport the prerequisite or annotate the intentional exclusion in the PR description.
- [BLOCKING] src/llmq/commitment.cpp:76: queue_control->Add(vChecks) passes an lvalue — won't compile after the signature change
  This PR changes CCheckQueueControl<T>::Add (src/checkqueue.h:233) and CCheckQueue<T>::Add (src/checkqueue.h:163) from taking std::vector<T>& to std::vector<T>&&. Every upstream call site in the diff was updated to use std::move(vChecks), but this Dash-specific LLMQ call site in CFinalCommitment::VerifySignatureAsync was not. The lvalue vChecks cannot bind to the new rvalue reference parameter, so this file will fail to build. Upstream review couldn't catch this because the LLMQ caller is Dash-only.
- [BLOCKING] src/llmq/commitment.cpp:90: Second LLMQ Add() call also passes an lvalue — same compile failure
  Same problem as line 76: queue_control->Add(vChecks) passes a non-movable lvalue to the new Add(std::vector<T>&&) overload. Both LLMQ call sites in this function must be updated together for the file to compile.
- [BLOCKING] src/llmq/commitment.cpp:76-90: Incomplete Dash adaptation for bitcoin#26749
  bitcoin#26749 changes CCheckQueueControl::Add and CCheckQueue::Add from taking std::vector<T>& to std::vector<T>&& and upstream updates all affected call sites to pass std::move(vChecks). Dash has an additional CCheckQueue user for LLMQ BLS checks that does not exist upstream. In CFinalCommitment::VerifySignatureAsync, both queue_control->Add(vChecks) calls still pass an lvalue vector. With the new signature in src/checkqueue.h, these calls cannot bind to std::vector<utils::BlsCheck>&&, so the backport is incomplete and this Dash-specific file will not compile. This is a merge/adaptation gap rather than a missing upstream prerequisite; upstream had no corresponding LLMQ call site to update.

  ---
  **Policy gate (backport-prereq-restore):** For full upstream backport PRs, a missing prerequisite is blocking unless the finding is explicitly allowlisted (e.g. `intentional_exclusion: true` or a matching entry in `policy_overrides`). The agent's original evidence above is the basis for this block; either backport the prerequisite or annotate the intentional exclusion in the PR description.

In `src/llmq/utils.h`:
- [SUGGESTION] src/llmq/utils.h:45-58: BlsCheck suppresses implicit moves — LLMQ check path silently copies
  After this backport, CCheckQueue::Loop uses std::make_move_iterator to drain the internal queue, and Add likewise inserts with std::make_move_iterator. Upstream prepared CScriptCheck for this by deleting copy ops and defaulting move ops. The Dash-specific BlsCheck (the element type used by m_bls_queue in src/llmq/blockprocessor.h) still declares a user-defined destructor (utils.h:54, defined at utils.cpp:537), which suppresses implicit move ctor/assignment. Elements will fall back to copying signature, pubkey vector, hash, and id string per element — the LLMQ path silently loses the performance benefit this PR was meant to deliver. The now-unused BlsCheck::swap should also be removed for consistency with the upstream refactor.
- [SUGGESTION] src/llmq/utils.h:45-58: BlsCheck suppresses implicit moves — LLMQ check path silently copies
  After this backport, CCheckQueue::Loop uses std::make_move_iterator to drain the internal queue, and Add likewise inserts with std::make_move_iterator. Upstream prepared CScriptCheck for this by deleting copy ops and defaulting move ops. The Dash-specific BlsCheck (the element type used by m_bls_queue in src/llmq/blockprocessor.h) still declares a user-defined destructor (utils.h:54, defined at utils.cpp:537), which suppresses implicit move ctor/assignment. Elements will fall back to copying signature, pubkey vector, hash, and id string per element — the LLMQ path silently loses the performance benefit this PR was meant to deliver. The now-unused BlsCheck::swap should also be removed for consistency with the upstream refactor.