Repository navigation
rpc: scriptPubKey carries Core's inferred desc - #1176
Conversation
script_pubkey_json returned hex, asm, type, and address, so a Core client reading scriptPubKey.desc got nothing from getrawtransaction, decoderawtransaction, getblock verbosity 2, decodescript, gettxout, or REST getutxos. It now adds the descriptor Core infers with no keystore: pk, multi, rawtr, addr, else raw, with the BIP380 checksum. Closes reardencode#1146
cb09382 to
79513ed
Compare
There was a problem hiding this comment.
Verdict
APPROVE. I would merge b6eb899ae8f38a4f40a716e1a7f4cae6921708b9 as-is.
script_pubkey_json now sets desc from Core 29.0 InferDescriptor with an empty signing provider (ScriptToUniv uses DUMMY_SIGNING_PROVIDER, which is an empty SigningProvider). getrawtransaction, decoderawtransaction, and getblock verbosity 2 go through tx_to_json; decodescript, gettxout, and REST getutxos (via gettxout) call the same helper. The diff is that helper, its call, the decodescript help line, docs/rpc.md, and a changelog fragment. No Cargo.toml or lockfile change.
Findings
Blocking: none.
Nit — address parsed twice. infer_body calls Address::from_script for the addr(...) fallback, and script_pubkey_json calls it again to fill address. Outputs that infer as pk, multi, or rawtr pay for one call. Every other output, including every standard address in getblock verbosity 2, pays for two. The strings match.
Nit — 17-key type (pre-existing). A 1-of-17 bare multisig infers desc multi(1,...), which matches Core MatchMultisig (OP_1–OP_16, or a minimal one-byte push of 17–20, max 20). type stays nonstandard because script.is_multisig only accepts OP_1–OP_16. Core Solver reports multisig. script_core_type is unchanged. The new test locks desc and leaves type unchecked.
Field order. Keys come out alphabetical: address, asm, desc, hex, type (no address when from_script fails). serde_json::Map is a BTreeMap; preserve_order is off, which the RPC tests already state. Core ScriptToUniv emits asm, desc, hex, address, type. The descriptor text matches Core. Key order follows this repo's existing JSON map.
DoS. One pass over the script, then a BIP380 checksum. getblock verbosity 2 already materializes hex and asm per output, and block scripts are consensus-bounded. decodescript of a large hex already holds the script and the hex field; raw(...) adds another hex-sized string. Same cost class as Core's per-output InferDescriptor. The checksum expect fires when a character is outside the BIP380 input charset. Base58 and bech32, including bcrt / bc / tb, sit inside that charset, so current networks do not hit it.
Scope. Top-level pkh / wpkh / sh / wsh / tr / miniscript need a keystore or a non-top parse context. Core's empty provider does not emit them. p2sh and segwit wraps stay omitted, as the help text and docs/rpc.md say. No second inference path.
Core correspondence
Checked against Bitcoin Core v29.0 InferScript, MatchPayToPubkey, MatchMultisig, GetScriptNumber, IsValidNonHybrid, and XOnlyPubKey::IsFullyValid (secp256k1_xonly_pubkey_parse):
pk: 33- or 65-byte direct push,OP_CHECKSIG, header length matchesCPubKey::GetLen, hybrid06/07rejected, curve unchecked. Off-curve03ff…ffispk.OP_PUSHDATA1andOP_CHECKSIGVERIFYareraw.multi: threshold and key count in[1, 20], minimal number encoding,m <= n,OP_CHECKMULTISIGat the end. A hybrid key drops the script toraw. Uncompressed and off-curve keys stay inmulti. A non-minimal count (OP_PUSHDATA1of 17, or a direct push of 1–16) israw.OP_PUSHDATA1key pushes still infermulti.rawtr: 34-byte v1 program whose program parses as an x-only point. 32 zero bytes andff×32 areaddr.addr: rust-bitcoinAddress::from_script, which lines up with CoreExtractDestinationand theGetScriptForDestinationround-trip for P2PKH, P2SH, witness v0 lengths 20 and 32, anchor51024e73, and other v1–v16 programs of 2–40 bytes. Witness v0 of any other length israwon both sides.- Otherwise
raw(<hex>). Empty script israw()#58lrscpx.
Checksum is Core DescriptorChecksum. The BIP380 vector raw(deadbeef)#89f8spxm matches an independent implementation of the BIP, and all 19 expected strings in the PR match that implementation. Address bodies match independent base58check and bech32/bech32m, including BIP173 bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t4 and regtest bcrt1pfeesnyr2tx. The mainnet checksum is a real BIP380 checksum of that address, so the row is a Core string even though the PR text says it was produced by a local port.
A throwaway probe (reverted, not part of the PR) also matched Core on: empty script, OP_PUSHDATA1 multisig keys, non-minimal 17-count, OP_CHECKMULTISIGVERIFY, v0 program length 2, 32 zero bytes, a trailing opcode, OP_PUSHDATA1 P2PK, 17-of-17, 1-of-20, 1-of-21, OP_1NEGATE, a non-minimal push of 16, v1 program length 31, uncompressed and off-curve multisig, OP_RETURN OP_CHECKSIG, and OP_CHECKSIGVERIFY.
Built and tested
rustc 1.95.0, worktree at b6eb899ae8f38a4f40a716e1a7f4cae6921708b9.
| Command | Result |
|---|---|
cargo test -p rbitcoin-rpc --test suite decodescript -- --quiet |
exit 0 |
cargo test -p rbitcoin-rpc --quiet |
exit 0 |
cargo fmt --all -- --check |
exit 0 |
cargo clippy -p rbitcoin-rpc --all-targets -- -D warnings |
exit 0 |
Workspace cargo test, cargo deny, and core-functional were left unrun. ./scripts/ast-grep.sh exited 1 because ast-grep is not installed in this environment. Nothing in the diff changes dependencies.
| if let Some(key) = taproot_output_key(script) { | ||
| return format!("rawtr({})", hex_encode(key.serialize())); | ||
| } | ||
| if let Ok(addr) = Address::from_script(script, network) { |
There was a problem hiding this comment.
Nit: script_pubkey_json calls Address::from_script again to fill address. Outputs that are not pk, multi, or rawtr parse the script twice, including every standard output in getblock verbosity 2. The address string is the same either way.
There was a problem hiding this comment.
Verdict
APPROVE. I would merge 79513ed3628ff457b9c7f731bab7aeb51653df5d as-is.
What changed since b6eb899
Both commits have parent 8e62e4ce4d60c30abc851cb0893e4b80149ac9e4. git diff b6eb899ae8f38a4f40a716e1a7f4cae6921708b9 79513ed3628ff457b9c7f731bab7aeb51653df5d is deletions only, and only in the two files whose sizes changed. The other four files in the PR are byte-identical.
crates/rbitcoin-rpc/src/methods/descriptor.rswent from +165 to +151. Removed the module docs and the Core citations onMAX_MULTISIG_KEYS,infer_descriptor,has_key_size,is_hybrid,p2pk_key,bare_multisig,multisig_count,taproot_output_key, andchecksum. No statement, constant, or control flow changed.crates/rbitcoin-rpc/tests/methods/mod.rswent from +84 to +83. Removed/// Expected strings are Bitcoin Core 29.0 decodescript output on regtest.The 18 regtest rows and the mainnet row are unchanged.
Findings
No blocking bug, risk, or weakened test.
Checked against Bitcoin Core 29.0 InferScript / Solver / ExtractDestination with an empty signing provider:
- Order is still
pk, baremulti,rawtronly whenXOnlyPublicKey::from_sliceaccepts the 32-byte v1 program,addrwhenAddress::from_scriptsucceeds, elseraw. - Hybrid
06/07keys are rejected for bothpkandmulti. Off-curve keys still infer aspk/multi, matchingInferPubkey→IsValidNonHybrid(no curve check).rawtrstill requires a fully valid x-only point. - Multisig counts match
GetScriptNumber:OP_1–OP_16, or a minimal one-byte push of 17–20. A direct push of 1–16 is non-minimal in Core and is rejected here (n > 16). Required cannot exceed the key count. Anything afterOP_CHECKMULTISIGrejects the match.is_p2tris exactly 34 bytes, so the[2..]slice cannot panic. - Witness v0 programs whose length is not 20 or 32: rust-bitcoin 0.32.102
WitnessProgram::newerrors (checked0002abcdand 19, 21, and 40-byte programs). Those fall through toraw(), matching Core (SolverreturnsNONSTANDARD,ExtractDestinationreturns false). Versions 1–16 with program length 2–40 still becomeaddr(on-curve v1/32 staysrawtr). - All 19 expected checksums match an independent BIP380 implementation. The
INPUT_CHARSETliteral is the 95-character BIP380 charset. Theexpectinchecksumcannot fire: every alphanumeric is in that charset, and every body is hex,(),, or a base58/bech32 address.
Nits, not merge blockers:
Address::from_scriptstill runs twice for standard scripts: once ininfer_body, once inscript_pubkey_jsonforaddress.- Pre-existing and untouched by either commit: a 17-key bare multisig infers
multi(...), butscript_core_typestaysnonstandardbecauseScript::is_multisigonly acceptsOP_1–OP_16. Core'stypefor that script ismultisig. - The deleted comments were the Core citations.
docs/rpc.mdstill states the contract.
No test vector or edge case was dropped or weakened. No new panic path, DoS, or behavior change versus the previously approved tree.
Build
Detached worktree at 79513ed3:
cargo test -p rbitcoin-rpc --quiet— exit 0cargo fmt --all -- --check— exit 0cargo clippy -p rbitcoin-rpc --all-targets -- -D warnings— exit 0
No GitHub comment was posted.
Summary
Closes #1146.
script_pubkey_jsonnow addsdesc: the descriptor Core'sInferDescriptorreturns with an empty signing provider, plus the BIP380 checksum. That one helper feedsgetrawtransaction,decoderawtransaction,getblockverbosity 2,decodescript,gettxout, and RESTgetutxos, so all six gain the key.The new module
methods/descriptor.rstries, in order:pk(<key>)for P2PK (CoreMatchPayToPubkey)multi(<k>,<keys>…)for bare multisig (CoreMatchMultisig)rawtr(<x-only>)for P2TR whose program is a valid x-only pointaddr(<address>)whenAddress::from_scriptsucceedsraw(<hex>)otherwiseWhere this differs from the guide comment
Core 29.0 says otherwise on two points, so this follows Core:
InferPubkeychecks onlyIsValidNonHybrid, which is length plus prefix byte, not the curve.decodescript 2102ff…ffacispk(02ff…ff)#ragaskvlin Core, and a multisig with an off-curve key is stillmulti(…).06/07key makes P2PK and multisigraw(…).rawtrdoes check the curve, as the guide says: an off-curve 32-byte v1 program isaddr(…).Multisig counts follow Core's
GetScriptNumber:OP_1–OP_16, or a minimal one-byte push of 17–20, so a 17-key bare multisig ismulti(…).requiredmust not exceed the key count, and nothing may followOP_CHECKMULTISIG.Evidence
decodescript_desc_matches_coredrivesdecodescriptover 18 scripts on regtest and one on mainnet. Every expected string is copied from Bitcoin Core 29.0decodescripton regtest. The mainnetbc1q…checksum is computed with a port ofdescsum_createthat reproduces Core's checksums for the regtest rows. Covered: P2WPKH, P2PKH, P2SH, P2WSH, on-curve and off-curve P2TR, compressed, uncompressed, off-curve and hybrid P2PK, 2-of-2, 17-key, hybrid-key and 3-of-2 multisig, pay-to-anchor, witness v2,OP_TRUE, andOP_RETURN. On master every row fails withdesc: null.Local: fmt, clippy
-D warnings,ast-grep.sh,unwrap-ratchet.sh,ci-os-smoke.sh, andcargo test -p rbitcoin-rpcpass. I ran the workspace suite on macOS. Its only failures are tests that need Linux or thethird_party/bitcoinsubmodule, plus timing tests that pass when rerun alone. No dependency changed.cargo denywas not available locally.Docs:
docs/rpc.md(decodescript,getrawtransaction, andgettxoutrows, plus the decode-dialect gap), thedecodescripthelp text, andchangelog.d/scriptpubkey-desc.md.Not in this PR:
decodescript'sp2sh/segwitwraps and miniscript, sorpc_decodescript.pystays skipped.