fix: intermittent incorrect logging of CheckQueue for invalid blocks

[knst]

Issue being fixed or feature implemented

ConnectBlock may return no-named failure instead proper error code such as:

2024-09-23T13:13:10Z ERROR: ConnectBlock: CheckQueue failed

What was done?

This PR fixes this intermittent failure

How Has This Been Tested?

N/A

Breaking Changes

N/A

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

[github-actions]

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

[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: 4c59195f-c7fe-4a7e-8f63-e19a1143e462

📥 Commits

Reviewing files that changed from the base of the PR and between 5fd84aa and 76e6645.

📒 Files selected for processing (1)

• src/validation.cpp

---

Walkthrough

The pull request modifies CChainState::ConnectBlock() in src/validation.cpp to reorder memory allocations. The std::vector<PrecomputedTransactionData> txsdata vector is now allocated before the CCheckQueueControl<CScriptCheck> control object is constructed, rather than after. This ensures that the transaction data vector exists prior to control initialization, preventing potential allocation-related side effects during script-check worker execution.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly addresses the main fix: ensuring proper error logging for CheckQueue failures in ConnectBlock, which matches the core change of moving txsdata allocation to prevent intermittent errors.
Description check	✅ Passed	The description clearly identifies the issue (intermittent CheckQueue failures with generic logging) and explains that the PR fixes this problem, which aligns with the changeset's objective of preventing CheckQueue execution issues.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

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

[thepastaclaw]

✅ Review complete (commit 76e6645)

[thepastaclaw]

Code Review

I reviewed the exact queued head 76e6645cd43e73847e1444bb1aef788f58e8c0cf and only src/validation.cpp changes in this PR. The change is a narrowly scoped fix to object lifetime ordering in CChainState::ConnectBlock: txsdata is now guaranteed to outlive CCheckQueueControl, which matches the existing comment and prevents queued CScriptCheck workers from observing destroyed PrecomputedTransactionData. I did not find any correctness, consensus, or Dash-specific interaction issues in the current patch.

Reviewed commit: 76e6645

[thepastaclaw]

Code Review

The single-line reordering in src/validation.cpp ensures txsdata is constructed before CCheckQueueControl<CScriptCheck> control and therefore destroyed after it, matching the in-source comment that txsdata must remain valid until control's destructor finishes running queued script checks. The fix is narrow, correct, and has no Dash subsystem interaction.

Reviewed commit: 76e6645

Fresh dispatcher run for this queue item. A same-SHA review already existed, so this records the fresh verification without duplicating inline threads.

Code Review

Reviewed commit: 76e6645

[UdjinM6]

utACK 76e6645