Repository navigation
rpc: gettxout is null for a confirmed unspendable output - #1095
Conversation
ac54854 to
ac87bd4
Compare
There was a problem hiding this comment.
Comment, not a change request. The confirmed-path filter is the right fix, and the new test should stay its own test on the shared mature pad.
Behavior
gettxout loads the Class A output and then checks spentness. An unspendable output is not a coin, so it is never spent, and master returns it. Connect already refuses a spend of one (bad-txns-inputs-missingorspent), and the BIP30 walk already skips policy::is_unspendable. This RPC was the surface that still treated the stored script as a coin.
The new check is in the right place: after the spent and mempool-spent returns, and only on the confirmed branch. It uses policy::is_unspendable (leading OP_RETURN, or length greater than 10,000), the same predicate as connect and blockstats. A script of exactly 10,000 bytes stays a coin. The mempool branch is untouched, which matches Core: AddCoin drops IsUnspendable, and CCoinsViewMemPool::GetCoin still returns whatever output the mempool tx has. include_mempool=false hits this same confirmed branch. REST /rest/getutxos calls gettxout, so a null coin becomes bitmap 0.
Leaving the store and the mempool view alone is correct. This is an RPC view of a rule the node already enforces.
Test time
gettxout_unspendable_confirmed_output_is_null already shares the pad. ctx_from_mature_pad copies the process-wide OnceLock from mature_regtest_pad (one generate 101 per test binary). This test does not remine that chain.
The added cost is one store copy, one hub and mempool open, and one generateblock. The 10,000-byte and 10,001-byte scripts are the boundary under test. About 20KB on that one block is not a second pad, and it is small next to the open.
Keep it a separate #[test]. gettxout_leftover_is_connected_not_unconfirmed is the nearest neighbor and pays for the same copy, but its story is a mempool spend and a leftover confirmation count. Distinct cases that only share a costly chain stay separate tests on that fixture.
rpc_regtest_mature_chain_ops is a worse home. That journey mines its own 130-block chain, not this OnceLock, and a fat script would sit on the chain the rest of the journey reads.
A one-block hand-built coinbase would skip the copy. It would also be a second block builder beside the generateblock this test already uses.
Follow-ups, same test, no extra open
These can land in this function. None of them need another pad or another #[test].
docs/rpc.mdowns thegettxoutcontract. That row already lists the other null cases (mempool-spent, disconnected, past the output count, genesis coinbase) and does not mention an unspendable confirmed output. One sentence there.- The evidence table's interesting case is a valued
OP_RETURN(master returned0.00001). Every extra output in the test isAmount::ZERO. Move 1,000 sats from vout 0 onto theOP_RETURN. Same tx, samegenerateblock. assert!(!boundary.is_null())accepts any object. AssertscriptPubKey.hexis 20,000 hex chars so a truncated 10,000-byte script cannot pass as "still a coin".- The witness commitment on the block
generateblockjust mined is the sameOP_RETURNpredicate, and REST is the same function. Onegettxouton that coinbase output and one/rest/getutxosbitmap fit here without another mine. The mempool "still listed" claim does not: a standard mempool tx cannot carry the 10,001-byte non-OP_RETURNscript, or a zero-value spendable 10,000-byte script. Leave that path to the early return above this check.
An OP_RETURN output, including the witness commitment, or a script over 10,000 bytes was reported as an unspent coin. Core never adds these to the coins view, and connect already refuses to spend them. The mempool view is unchanged, as in Core. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ettxout test The OP_RETURN now carries 1,000 sats, the case master reported as a coin. The 10,000-byte output must return its full script, and the REST bitmap must match gettxout. docs/rpc.md lists the unspendable null case. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ac87bd4 to
da72a87
Compare
|
Thanks. Rebased onto master and pushed
Witness commitment: I did not add this. The block Kept as its own |
There was a problem hiding this comment.
Approve. The confirmed-path filter is unchanged and still right, and the follow-ups land in the same test.
What I re-checked
The four-line change is still only on the confirmed branch, after spent and mempool-spent, using policy::is_unspendable (leading OP_RETURN, or length greater than 10,000). The mempool early return still lists every output. That is Core: AddCoin drops IsUnspendable, CCoinsViewMemPool::GetCoin does not.
The updated test pins both arms of that predicate, on one generateblock off the shared mature pad:
- Vout 1 is a 1,000-sat
OP_RETURN(6a0107). A length-only check still returns0.00001. Fee stays 100,000 sats: vout 0 is50 BTC − 101_000, plus this 1,000. - Vout 2 is a zero-value 10,001-byte script. A value-only check would still return it.
- Vout 3 must come back with
scriptPubKey.hexof 20,000 characters.>= 10_000, or a truncated script, fails that. - Default
gettxoutusesinclude_mempool=true. REST/rest/getutxoswithoutcheckmempoolcallsgettxoutwithinclude_mempool=false. Bitmap1001is vout 0 and vout 3..jsonis stripped bysplit_formatbefore the outpoints are parsed, so the lastnis3.
docs/rpc.md now lists the confirmed unspendable null and the mempool exception. The row matches the code.
Witness commitment
Agreed, this block has none. mine_regtest_paying calls apply_witness_commitment only when block_has_witness is true, and that walks inputs for a non-empty witness. This spend's witness is empty, so the coinbase is just the payment output. The commitment script (6a24aa21a9ed…) is the same leading-OP_RETURN arm vout 1 already fails closed. A second witness transaction would be another mine for a predicate this test already hits.
Still one private copy of the OnceLock pad. No second test.
Summary
gettxoutreported a confirmed unspendable output as an unspent coin. That covers anOP_RETURNoutput, including every coinbase witness commitment, and a script over 10,000 bytes. Core never adds these to the coins view, so it returnsnull. Consensus already agrees with Core: spending one isbad-txns-inputs-missingorspent(rule C43).gettxout (confirmed branch) resolve txid → fk, check vout range, spent, mempool-spent out = tx_output_at_fk(fk, n) + if policy::is_unspendable(out.script) → null # the rule C43 already uses build the coin JSONThe mempool branch is unchanged. Core's
CCoinsViewMemPool::GetCoinalso returns any output of a mempool transaction. REST/rest/getutxoscallsgettxout, so its bitmap follows.Evidence
The same block was sent to bitcoind 31.1 and rbitcoin. It holds one tx with these outputs:
OP_RETURNwith value 0,OP_RETURNwith value 1000, a 10,001-byte script, a 10,000-byte script, andOP_TRUE. The coinbase witness commitment of the next block was also queried.gettxoutOP_RETURN, 0 satOP_RETURN, 1000 satOP_TRUETest:
gettxout_unspendable_confirmed_output_is_null. A block fromgenerateblockholds a spend with anOP_TRUEoutput, a 1,000-satOP_RETURN, a zero-value 10,001-byte script, and a 10,000-byte script that must stay a coin with its full 20,000-hex-char script. REST/rest/getutxosover vouts 0–3 must give bitmap1001.unspendable vout 1 is not a coin: {…"hex":"6a0107"},"value":0.00001}. A length-only predicate (script.len() > 10_000) fails the same way.After: passes.
cargo test -p rbitcoin-rpc --libreports 125 passed.Local: fmt, clippy
-D warnings, ast-grep,cargo test --workspace --exclude rbitcoin-bench,ci-os-smoke.sh.Found while comparing crash-recovery state against Core: a full
gettxoutsweep differed only on these outputs.Merge Danger
Door: two-way
RPC and REST output only.
docs/rpc.mdlists the newnullcase. The store, consensus and the mempool view are unchanged.Blast Radius:
gettxoutand REST/rest/getutxosA caller that read
OP_RETURNoutputs back throughgettxoutnow getsnull, as it would from Core.🤖 Generated with Claude Code