fix(log): report tx_state in the ConnectBlock CheckTxInputs failure log

[PastaPastaPasta]

Issue being fixed or feature implemented

In Chainstate::ConnectBlock, the error path for Consensus::CheckTxInputs logs the wrong validation state:

TxValidationState tx_state;
if (!Consensus::CheckTxInputs(tx, tx_state, view, pindex->nHeight, txfee)) {
    // Any transaction validation failure in ConnectBlock is a block consensus failure
    LogError("%s: Consensus::CheckTxInputs: %s, %s\n", __func__, tx.GetHash().ToString(), state.ToString());
    return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS,
                tx_state.GetRejectReason(), tx_state.GetDebugMessage());
}

CheckTxInputs fills in tx_state, not state. At the point the log fires, the BlockValidationState& state has not been invalidated yet — that happens on the very next statement. ValidationState::ToString() short-circuits on IsValid() and returns the literal string "Valid", so every one of these block-connect failures logs:

ERROR: ConnectBlock: Consensus::CheckTxInputs: <txid>, Valid

The actual reason (bad-txns-inputs-missingorspent, bad-txns-in-belowout, bad-txns-inputvalues-outofrange, …) is dropped. The reason still reaches the BlockValidationState for the caller, so this is a diagnostics-only defect — but it silently removes the useful half of the log line in exactly the situation where you need it.

The mistake dates to the backport of bitcoin#15921, which introduced the separate tx_state but left the log reading state. Upstream later reordered state.Invalid() ahead of the log, so by then state carried the copied reject reason; Dash instead collapsed it into a single return state.Invalid(...), which fixes the opposite ordering. Reading tx_state directly is the smaller fix and does not disturb the return.

Split out of #7683, where it was an unrelated drive-by change in an otherwise mechanical error(...) → LogError(...) refactor. The two branches no longer touch the same line and can land in either order.

What was done?

Log tx_state.ToString() instead of state.ToString() in the ConnectBlock CheckTxInputs failure path.

How Has This Been Tested?

Inspection of the surrounding code path and ValidationState::ToString() in src/consensus/validation.h, which returns "Valid" whenever IsValid() holds. Both variables are in scope at the call site (tx_state is declared two lines above, state is the function parameter), so the change is type- and scope-identical; no behavior outside the log string changes. CI covers the build and test suites.

Breaking Changes

None. Log-string only; the BlockValidationState handed back to the caller is unchanged.

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

🤖 Generated with Claude Code

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: fb377789-b4cb-4b27-81fa-14c91773e019

📥 Commits

Reviewing files that changed from the base of the PR and between 7513e52 and e750ed4.

📒 Files selected for processing (1)

• src/validation.cpp

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

Walkthrough

Chainstate::ConnectBlock now logs tx_state when Consensus::CheckTxInputs fails. The log therefore reports the transaction-specific validation reason and debug message. The subsequent return state.Invalid(...) behavior is unchanged.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Merge Risk: ⚪ Minimal · up to e750e

This correction makes transaction rejection logs show the actual validation reason without changing transaction acceptance or rejection behavior. The change is ready to merge.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description clearly explains the logging defect, the tx_state fix, and the unchanged consensus behavior. It is directly related to the changeset.
Title check	✅ Passed	The title is concise, specific, and accurately identifies the ConnectBlock CheckTxInputs logging fix.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
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 a new PR

---

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]

⚠️ DEGRADED — Queued for automated review — 6th in line, estimated start in ~5 h (commit e750ed4)
Estimated review time once started: ~1.9 h (two-phase automated review; median of recent runs).
The primary review models are currently out of quota; this review will run on stand-in models and be marked as degraded.

• Request priority review — click to move this review to the front of the queue.

[knst]

utACK

[knst]

never had been fixed by mainstream, eventually code is removed / replaced by bitcoin#31112

It seems as no relevant backport to fix