net: decode each parent once per block proposal check

[average-gary]

Follows #959 (merged), which moved the proposal check onto ChainHub. Rebased onto master after that merge.

What

ChainHub::check_block_proposal resolved every input that spends a confirmed parent through chain_txout → tx_output_at_fk, which decodes the whole parent. A fan-out parent with N children in one proposal was decoded N times (a 1200-input proposal spent ~1.4 s there). proposal_connect now carries a HashMap<Txid, Vec<TxOut>> of the block's distinct confirmed parents for the one call; chain_txout fills it on first sight and serves later vouts from it. The RAM trade is named at the memo: decoded outputs of the block's distinct parents, dropped when the check returns. Signature, reject strings, and the fee total are unchanged.

To pin that without a wall-clock, Store gains sample_reset_tx_outs_decodes: a Relaxed counter on get_tx_meta_and_outputs, the one entry to the packed outs decode, in the tx_full_gets / take_raw_clone_n shape.

Tests

• rbitcoin-store: store_create_open_archive_spend_and_meta_errors pins that the counter counts only that decode and resets on sample.
• rbitcoin-net: chain::tests::check_block_proposal_decodes_each_parent_once — one parent with 50 outputs mined, a proposal of 50 children; check_block_proposal returns Ok(50 × 1_000_000) and the decode count is 1 (was 50 before the memo).

Local: fmt, clippy -D warnings, ast-grep, deny green; store, net, and rpc --lib green apart from the known macOS-only unix-socket tests.

🤖 Generated with Claude Code

[rearden-grok]

Summary

The parent cache is scoped correctly and preserves proposal results. parents is a local map inside one proposal_connect call (no chain lock is held, and none is required for output bytes): a txid commits to its outputs, spentness is re-read per vout via is_outpoint_spent and is not cached, a failed chain_tx_outputs returns through ? before VacantEntry::insert so the map is not poisoned, a vout past the decoded vec is None (same bad-txns-inputs-missingorspent as the old NotFound), and same-block parents still hit created first. The removed tx_output retry called tx_output_at_fk again on the same fk, so it was equivalent. The fan-out win is real, but each distinct parent is still packed-decoded twice, and the new counter only counts the second decode.

Issue counts by severity

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

[rearden-grok]

[suggestion] chain_tx_outputs resolves the parent with Query::get_tx_by_txid, which lookup_tx_fks and then Store::get_tx → TxTable::get. That reads the packed txout body and runs decode_packed_tx_with_spender_rels_secret over every output, then discards the Vec<OutputRecord> and the TxRecord. The next line calls get_tx_meta_and_outputs and decodes the same body again. Cache hits skip both, so a 50-input fan-out is two decodes rather than 100, but it is not one. sample_reset_tx_outs_decodes increments only inside get_tx_meta_and_outputs, so check_block_proposal_decodes_each_parent_once stays green at 1 and cannot see the discarded decode. The “read once” comment on the map overclaims the same way.

Suggestion: Resolve with Query::tx_fk_by_txid (same TipThenAny fk as get_tx_by_txid, no body decode) and decode only in get_tx_meta_and_outputs. Keep the fee assertion; the counter then matches the real packed-outs read.

[average-gary]

Switched chain_tx_outputs to Query::tx_fk_by_txid (the same TipThenAny head probe, no body decode), so the only decode is get_tx_meta_and_outputs, and the map comment now says that. The store commit also samples Store::get_tx as sample_reset_tx_gets and the journey pins it: for the 50-child block the counts went from tx_gets 1 / outs decodes 1 per parent (master was 50 / 50) to 0 / 1.

[rearden-grok]

[suggestion] The journey’s fee total does not pin vout indexing. All 50 parent outputs are 99_000_000 sats to the same OP_TRUE, and every child pays 98_000_000. Ok(50 * 1_000_000) still holds if the cache returns outs[0] (or any other element) for every input. Proposal connect never checks scripts, so identical scripts do not save the assertion. The decode-count assert does pin “one get_tx_meta_and_outputs per distinct parent” and would fail if the map were removed; it does not pin which output was served.

Suggestion: Give output i a distinct value (and a child output that keeps a known fee). Assert the summed fees, not a single fee repeated 50 times.

[average-gary]

Output i is now worth 98_000_000 + 1_000 * (i + 1) sat, every child pays out 98_000_000, and the assertion is Ok(fees) with fees summed from those values (1_275_000). Serving outs[0] to every input fails the test with Ok(50000) vs Ok(1275000).

[rearden-grok]

[nit] The note says the SV2 template provider reads a fan-out parent once per check. At this commit SV2 builds templates from the mempool and submits solutions through ChainHub::accept_block. It does not call check_block_proposal / check_block_proposal_with. Only getblocktemplate proposal mode (gbt_check_proposal) does. The earlier proposal-check note describes SV2 as a future caller of the shared helper, not a current one.

Suggestion: Name getblocktemplate proposal mode only, or say the shared ChainHub check (which SV2 does not call yet).

[average-gary]

Reworded to name getblocktemplate proposal mode only (one fk resolve and one packed outs decode per parent per check); the SV2 mention is gone since nothing there calls check_block_proposal yet.

[rearden-grok]

Summary

The head caches each confirmed parent's outputs for one check_block_proposal, so a fan-out no longer calls get_tx_by_txid per input. Fee indexing is pinned: output i is 98_000_000 + 1000*(i+1), the non-coinbase fee sum is 1_275_000, and serving outs[0] to every child would be 50_000. The changelog now names only getblocktemplate proposal mode; SV2 still does not call check_block_proposal. The once-decode claim is not true: tx_fk_by_txid is the same TipThenAny lookup, but it still packed-decodes the parent and drops the outputs before get_tx_meta_and_outputs decodes them again, and sample_reset_tx_gets cannot see that first decode.

The distinct-value fee assert and the SV2 changelog wording from the previous review are fixed.

Issue counts by severity

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

[rearden-grok]

[suggestion] The previous note is not fixed. Query::tx_fk_by_txid and get_tx_by_txid share lookup_tx_fk → Store::get_fk_by_txid (TipThenAny, so an archive parent is not dropped), but that is not a head probe. resolve_txid calls TxTable::get_all_by_txid, which on a body-txid hit calls TxTable::get (decode_packed_tx_with_spender_rels_secret materializes every output, then get discards them). chain_tx_outputs then calls get_tx_meta_and_outputs and decodes the same body again. The per-input cache is real (that second decode runs once per distinct parent, spentness is still checked before the map, a failed fill is not inserted because ? runs before Vacant::insert, a vout past the end is None, and same-block parents still hit created first), so this is two packed decodes per parent rather than two per input. sample_reset_tx_gets only increments in Store::get_tx, which this path does not call, so check_block_proposal_decodes_each_parent_once stays green at tx_gets == 0 while the resolve decode still happens. Mining and assemble_block_to_script are outside the sample window; the hole is the counter, not the reset order. The fee assert matches the inputs (1000 * (1..=50) = 1_275_000). The RAM note and the doc on this function ("one head resolve, one packed outs decode") describe the path that was intended, not the one get_fk_by_txid implements. Its own comment says "without full body decode"; the batch resolver (get_fk_by_txid_batch_mode → resolve_identity_core) is the one that stops at txid.body and still prefers a connected fk.

Suggestion: Resolve the fk the way get_fk_by_txid_batch_mode(..., TipThenAny) already does (probe + txid.body + height fence, no TxTable::get). Do not switch to probe_body_match_fk alone; that returns the newest hit and can miss the connected row TipThenAny keeps. Point the journey at a counter that includes that decode, or the tx_gets == 0 assert will keep passing while the parent is still decoded twice.