Repository navigation
rpc: scriptPubKey type is nonstandard for a v0 witness program of another length - #1186
Conversation
There was a problem hiding this comment.
Verdict
APPROVE. I would merge this as-is.
script_core_type used to label every witness program that was not P2WPKH, P2WSH, taproot, or pay-to-anchor as witness_unknown. Bitcoin Core's Solver returns WITNESS_UNKNOWN only when witnessversion != 0. A version-0 program whose length is not 20 or 32 returns NONSTANDARD (nonstandard) and does not continue into nulldata, pubkey, pubkeyhash, or multisig. This patch does that, and only that.
Findings
No blocking issues.
The new arm runs only when witness_version() returns Some. In rust-bitcoin 0.32.102 that predicate is is_witness_program(): script length 4 through 42, a direct push of 2 through 40 bytes that consumes the rest, and a first byte of OP_0 (0x00) or OP_1 through OP_16 (0x51 through 0x60). That is the same set as Core CScript::IsWitnessProgram. witness_version uses TryFrom<Opcode>, so the instruction-parser path that treats an empty push as version 0 is not involved.
The surrounding order matches Core on this branch:
- 20-byte and 32-byte version 0 stay
is_p2wpkhandis_p2wsh, checked first. - 32-byte version 1 stays
is_p2tr. 51024e73staysis_p2anchor(the same four bytes asCScript::IsPayToAnchor), checked before the version split, so it staysanchor.- Every other version 1 through 16 program in the 2-through-40 window stays
witness_unknown. - Version 0 programs of length 2–19, 21–31, and 33–40 become
nonstandard. - A 43-byte script (push length 41) is outside 4–42, so
witness_version()isNoneand the previous fall-through tononstandardis unchanged. A non-minimalOP_PUSHDATA*program fails the direct-push check the same way and is unchanged.
script_core_type has one caller, script_pubkey_json. That object is what decodescript, decoderawtransaction, verbose getrawtransaction, getblock at verbosity 2 and above, gettxout, and REST getutxos emit. Consensus, libre-relay admission, wallet code, and descriptors do not read the string. Address insertion is still Address::from_script. WitnessProgram::new rejects version 0 unless the program is 20 or 32 bytes, so these outputs still have no address. The diff adds no panics, unwraps, allocation, or overflow, and it does not touch dependencies, workflows, or consensus.
Nit. The checked-in rows omit the changelog example 0003abcdef, the lengths next to the standard sizes (19, 31, 33), and a non-minimal push. The version-0 arm does not special-case those lengths, and a non-minimal push never enters it. The nine rows cover the label change and the neighbors this edit could have stolen: P2WPKH, P2WSH, anchor, an unknown version-1 program, version 16 at the 40-byte maximum, and a 43-byte non-program.
The PR's 924-script Core 31.1 corpus is not in the tree, and I did not replay it. I checked the nine rows against Solver, IsWitnessProgram, and IsPayToAnchor on Bitcoin Core master (same branch structure the PR attributes to 31.1) and against rust-bitcoin 0.32.102.
Built and tested
Head aec5cd37efbcb57fdb2c4ec1e8a11750b1044c32. Merge-base with origin/master is 8e62e4ce4d60c30abc851cb0893e4b80149ac9e4. The diff is the three files in the PR (+36/−2).
cargo fmt --all -- --check— exit 0cargo test -p rbitcoin-rpc --test suite --quiet -- decodescript_witness_program_type_matches_core decode_rpc_subset gettxout_script_pubkey_matches_getrawtransaction named_params_getblock_object— exit 0,4 passed; 0 failed; 0 ignored; 0 measured; 130 filtered outcargo clippy -p rbitcoin-rpc --all-targets -- -D warnings— exit 0
I did not run the workspace suite, cargo deny, or a live bitcoind.
aec5cd3 to
dafceda
Compare
|
@reardencode this is rebased onto current master (4036f76) and ready. The only conflict was in the test file, where #1176 added |
…ther length `type` called every witness program that was not P2WPKH, P2WSH, taproot, or an anchor `witness_unknown`. Core's solver does that only for versions 1 to 16. A version 0 program that is neither 20 nor 32 bytes, such as `0003abcdef`, is `nonstandard`, and the solver returns there without trying the later templates. `type` now reads the witness version and answers `nonstandard` for version 0. `address` was already absent for these scripts, as in Core. Expected types are Bitcoin Core 31.1 `decodescript` on regtest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dafceda to
4fcb0d6
Compare
|
@reardencode rebased again, onto current master (a9b3e49) now that #1183 has merged. The only conflict was in the test file, where #1183 added its multisig test in the same spot. Both tests are kept, and the |
Summary
} else if is_p2anchor(script) { "anchor" - } else if script.is_witness_program() { - "witness_unknown" + } else if let Some(version) = script.witness_version() { + if version == bitcoin::WitnessVersion::V0 { + "nonstandard" + } else { + "witness_unknown" + } } else if script.is_op_return() {Core's
SolverreturnsWITNESS_UNKNOWNonly forwitnessversion != 0(src/script/solver.cpp:172). A version 0 program that is neither 20 nor 32 bytes falls toreturn TxoutType::NONSTANDARD(solver.cpp:177) without trying the later templates.script_core_typecalled those scriptswitness_unknown. rust-bitcoin'switness_version()accepts exactly the scripts Core'sIsWitnessProgramaccepts: 4 to 42 bytes,OP_0orOP_1..OP_16, then one direct push of the rest.Evidence
assertion `left == right` failed: 0002abab/left: String("witness_unknown")/right: String("nonstandard")After:test methods::decodescript_witness_program_type_matches_core ... okdecodescript.typemismatches went from 39 to 0. All 39 were version 0 programs that are neither 20 nor 32 bytes.addresspresence matched Core on all 924, before and after. The corpus covered first bytes00/51/52/60with programs of 0–41 bytes, push bytes equal to the length and off by one or two, everyOP_1..OP_16at boundary lengths, version-adjacent first bytes, PUSHDATA-encoded programs, P2A and near-P2A, v1/32 off-curve programs, and tiny scripts.abrepeated as the program):0002+ 2 bytesnonstandardwitness_unknown0015+ 21 bytesnonstandardwitness_unknown0028+ 40 bytesnonstandardwitness_unknown0014+ 20 byteswitness_v0_keyhashwitness_v0_keyhash0020+ 32 byteswitness_v0_scripthashwitness_v0_scripthash5102+ 2 byteswitness_unknownwitness_unknown51024e73anchoranchor6028+ 40 byteswitness_unknownwitness_unknown5129+ 41 bytesnonstandardnonstandardcargo test --workspace --quiet --no-fail-fastpassed. A first run had one failure, the timing testrbitcoin-testintegration_multinode::mocktime_generate_keeps_ponging_peer; it passed 3/3 alone.cargo clippy --workspace --all-targets -- -D warnings,cargo fmt --all -- --check,./scripts/ast-grep.sh, and./scripts/ci-os-smoke.shall passed.cargo denyis not installed and was not run.Merge Danger
Door: two-way
Blast Radius: rpc
scriptPubKey.typechanges fromwitness_unknowntononstandardfor a version 0 witness program of 2–19, 21–31, or 33–40 bytes. That shows up indecodescript,decoderawtransaction,getrawtransactionverbose,getblockverbosity 2,gettxout, and RESTgetutxos. No other script changes type, andaddressis unchanged (already absent for these scripts).🤖 Generated with Claude Code