rpc: submitblock leaves a side-branch block's inputs to connect

[niteshbalusu11]

Summary

submitblock runs a cheap input check before accept. When the parent is off the best chain, that check reads only the store. The store doesn't hold a held side-branch parent's txs, so a child spending its side parent's output came back bad-txns-inputs-missingorspent and was cached invalid. Core checks inputs only in ConnectBlock, when the branch activates.

 cheap_submit_tx_reject (mine.rs)
   merkle, mutation, CheckBlock structure
+  parent off the best chain (no spend height)  -> no input verdict; accept decides
   inputs: missing / spent / premature coinbase / in < out

Deferring those checks exposed two reason gaps on the connect path. A heavier side branch that failed there returned consensus: in < out or consensus: double spend in block, where master's cheap check had given Core's reason. Both are fixed here:

 block_reject_reason (consensus/error.rs)
+  BadTx("in < out")              -> bad-txns-in-belowout
+  BadTx("double spend in block") -> bad-txns-inputs-missingorspent
 accept_branch connect failure (net/chain.rs)
-  ConnectFailed { msg: e.to_string() }   // "consensus: consensus: <reason>"
+  ConnectFailed { msg: <bare reason> }   // as connect_failed_for_header does

The cache made it stick. After that one submitblock, the node refused the same chain from peers as well and stayed on the old tip. Follows 3cae0e8, which covered a sibling spending a coin the tip also spent.

Evidence

Regtest, both nodes on A101. B1 is a sibling of A101 (parent: block 100) holding X, which spends the block-1 coinbase. B2 sits on B1 and spends X:0.

	Core 31.1	master	this branch
submitblock B1	inconclusive	inconclusive	inconclusive
submitblock B2	null, reorgs to B2	bad-txns-inputs-missingorspent	null, reorgs to B2
resubmit B2	duplicate	duplicate-invalid	duplicate
then B1..B4 arrive from a Core peer	—	stays at A101 for 240 s; Core at 104	follows to 104

Control: the same peer sequence without the submitblock follows to 104 on master too, so the P2P path was already right.

Heavier side branch with one bad tx (B1 equal-work sibling of the tip, then B2 on it), submitblock B2:

bad tx in B2	Core 31.1	master	this branch
spends more than its input	bad-txns-in-belowout	bad-txns-in-belowout	bad-txns-in-belowout
two txs spend one coin	bad-txns-inputs-missingorspent	bad-txns-inputs-missingorspent	bad-txns-inputs-missingorspent
spends a confirmed immature coinbase	bad-txns-premature-spend-of-coinbase	consensus: bad-txns-premature-spend-of-coinbase	bad-txns-premature-spend-of-coinbase
spends B1's own coinbase	bad-txns-premature-spend-of-coinbase	bad-txns-inputs-missingorspent	bad-txns-premature-spend-of-coinbase

The tip stays put in every row, on all three.

• New test submitblock_child_spends_its_side_parent_output. It runs the B1/B2 reorg, then four heavier side branches with one bad tx each: overspend, double spend, an immature confirmed coinbase, and the side parent's own coinbase. Each must answer Core's reason, leave the tip on B2, and answer duplicate-invalid on resubmit. A last beat checks that an overspend on the tip still gets bad-txns-in-belowout from the early check.
  • Before: B2 gets "bad-txns-inputs-missingorspent". With only the cheap-check change, the first bad branch gets "consensus: in < out".
  • After: passes.
• fmt, clippy, ast-grep, cargo test --workspace, and ci-os-smoke.sh pass locally.

Merge Danger

Door: two-way

Blast Radius: RPC and block-reject reasons

submitblock of a block whose parent is off the best chain no longer gets an early input verdict. Such a block is held (inconclusive), or connected when its branch has more work. A bad input then fails on connect and is remembered as invalid, as before. Blocks on a best-chain parent keep the cheap check. The two reason mappings and the bare ConnectFailed message also change the reason in debug log lines for these connect failures.

🤖 Generated with Claude Code

[rearden-grok]

Summary

Skipping the cheap input walk when the parent is off the best chain does not accept a block early. Merkle, the mutated-duplicate check, and validate_block_structure still run before the new return. connect_at still checks missing inputs, spent coins, immature coinbase, cross-tx double spend, and in < out before that block can be the tip. A failed connect still records the failing block hash. The two new BadTx arms match the strings connect emits, and ConnectFailed no longer stacks a second consensus: on a reason that does not already have one.

spend_height == None is a missing header or a header with no best-chain height (held side parent, header-only parent, reorged-off header). An unknown parent never reaches this function: submit_received_block already returns prev-blk-not-found unless the parent header or a held body exists. A best-chain parent, including a non-tip ancestor, still gets Some and still walks inputs. Same-transaction duplicate inputs still die in the structure check as bad-txns-inputs-duplicate. Empty vin and vout are already rejected by check_tx_local.

A heavier branch is disconnected and connected one block at a time. A weaker branch is held, not written as best chain. There is no valid-block cache that would skip a later connect. The invalid set is keyed by block hash. ConnectFailed carries the failing block's hash, and remember_failed_accept stores that hash. A valid side child no longer takes the cheap reject, so it is not inserted. A bad heavier child still is.

ConnectFailed's Display writes consensus: {msg}. connect_at stores block_reject_reason output, which has no prefix of its own. Unwrapping Consensus and Mutated leaves the prefix once. Store and Cancelled return earlier as local faults.

The new mappings apply to every connect_at, including best-chain blocks. Peer block accept logs that string and keeps the session. Header and compact disconnects key off the invalid set, not these strings.

submitblock_child_spends_its_side_parent_output pins the bug. Regtest blocks share 0x207fffff, so work is block count. Equal-work b1 stays inconclusive. b1+b2 is two blocks against one, so b2 connects and becomes the tip. Each bad c1+c2 is two blocks against b2's one; the one-block prefix ties and is not kept, and the tip stays b2. The overspend is one sat above the subsidy. The double spend is two transactions, one outpoint. The immature coinbase is height 100 spent at 103, and maturity is 100, so 103 < 200. b2 spends b1's non-coinbase output.

Follow-ups, not merge blocks:

• The bad-branch cases never resubmit, so they do not show duplicate-invalid. The happy path also never asserts that b2 stayed out of the invalid set. The new tip is the pin.
• Nothing spends the side parent's own coinbase. That coin appears only after the parent connects, and at the next height it is immature.
• Nothing in this test forces the cheap walk on a best-chain parent. A tip-extension overspend should keep returning bad-txns-in-belowout.
• bad-txns-premature-spend-of-coinbase was already mapped. The bare ConnectFailed message is what makes that RPC string exact. The changelog lists it with the two new mappings.
• The other => to_string() arm would still double-prefix if a non-consensus error ever reached this wrap. It does not on the current path.

Issue counts by severity

• bugs: 0
• suggestions: 3
• nits: 2

[niteshbalusu11]

@rearden-grok Thanks. Taken in 1734fd6:

• Each failing branch is resubmitted and must answer duplicate-invalid, and b2 must not be in the invalid set.
• A fourth branch spends its side parent's own coinbase and must answer bad-txns-premature-spend-of-coinbase. Master answered bad-txns-inputs-missingorspent for that case.
• An overspend on the tip must still answer bad-txns-in-belowout through the early check.
• The changelog no longer lists the premature-coinbase reason as a new mapping.

Left as is: the other => to_string() arm. As you noted, nothing else reaches that wrap today, and I'd rather not add an arm for a case that can't happen.

Please re-review the latest commit (1734fd6).

[rearden-grok]

Summary

1734fd67 does not change the submit or connect path. The approval on de00b315 still holds. This commit only tightens the test and the changelog.

The new asserts match the code that is already there:

• After b2 reorgs in, is_block_invalid is false for that hash. A successful accept does not call note_invalid_block.
• Each heavier failure is submitted again. submit_received_block returns duplicate-invalid when the hash is already in the invalid set, before the cheap walk. ConnectFailed still records the failing block, so a removed note_invalid_block fails this assert.
• None spends c1's own coinbase from the next block. That coin exists only once c1 connects, and height +2 is inside the 100-block maturity window, so the reason is bad-txns-premature-spend-of-coinbase. A missingorspent answer fails the test.
• The tip-extension overspend spends the height-2 coinbase for one sat more than the subsidy. Its parent is b2, the tip, so cheap_spend_height is Some and the early walk still runs. The tip stays b2.

The changelog no longer lists the premature-coinbase string as a new mapping. That arm was already in block_reject_reason. The new text names the two mappings this PR adds, and the removal of the doubled consensus: prefix.

One limit of the new tip case: connect would also answer bad-txns-in-belowout and leave the tip put. The assert pins the outcome. It does not by itself prove the early walk ran. The parent being the tip is what selects that walk.

Issue counts by severity

• bugs: 0
• suggestions: 0
• nits: 1