[niteshbalusu11]

Summary

-    } else if script.is_op_return() {
+    } else if script.is_op_return()
+        && bitcoin::Script::from_bytes(&script.as_bytes()[1..]).is_push_only()
+    {
         "nulldata"

Core's Solver returns NULL_DATA only when the bytes after the leading OP_RETURN pass IsPushOnly. Otherwise the script falls through to NONSTANDARD. rbitcoin looked only at the first byte, so 6aac and 6a4c were nulldata. rust-bitcoin's is_push_only fails in the same places as Core's GetOp, and it counts OP_0, OP_1NEGATE, OP_RESERVED, and OP_1–OP_16 as pushes, as Core does.

Evidence

• Before: decodescript_nulldata_type_matches_core failed on 6a61: left: String("nulldata"), right: String("nonstandard"). After: test methods::decodescript_nulldata_type_matches_core ... ok
• Differential corpus: 1,098 scripts vs Bitcoin Core 31.1 decodescript on regtest. In the OP_RETURN branch, mismatches went from 887 to 0, and all 887 were Core nonstandard vs rbitcoin nulldata. The one mismatch outside the branch, 5101ab51ae (Core nonstandard, rbitcoin multisig), is the multisig branch that rpc: scriptPubKey type finds bare multisig as Core does #1183 fixed. That fix is now on master.
• Permanent test rows:

script	Core / this PR	master
6a	nulldata	nulldata
6a0051504f60	nulldata	nulldata
6a4c01cd (non-minimal PUSHDATA1)	nulldata	nulldata
6a4c64 + 100 bytes	nulldata	nulldata
6a4d0100ab (PUSHDATA2)	nulldata	nulldata
6a4e01000000ab (PUSHDATA4)	nulldata	nulldata
6a4d0902 + 521 bytes	nulldata	nulldata
6a61	nonstandard	nulldata
6aac	nonstandard	nulldata
6a6a	nonstandard	nulldata
6a01abac	nonstandard	nulldata
6a01 (truncated push)	nonstandard	nulldata
6a4c (truncated PUSHDATA1)	nonstandard	nulldata
6a4d0500ab (truncated PUSHDATA2)	nonstandard	nulldata
6a4e05000000ab (truncated PUSHDATA4)	nonstandard	nulldata

• Re-verified in an isolated build: the test fails with the fix reverted and passes with it, and a fresh corpus run reproduces 887 → 0. A separate review re-checked 2,424 scripts against Core 31.1 and found zero mismatches in the OP_RETURN branch.
• Local gates, in an isolated target dir: cargo fmt --all -- --check, ./scripts/ast-grep.sh, cargo clippy --workspace --all-targets -- -D warnings, cargo test --workspace --quiet --no-fail-fast, and ./scripts/ci-os-smoke.sh all pass. cargo deny is not installed here and did not run.

Merge Danger

Door: two-way
Blast Radius: RPC
scriptPubKey.type changes from nulldata to nonstandard for any script that starts with OP_RETURN and then has a non-push opcode or a truncated push. That affects decodescript, decoderawtransaction, getrawtransaction, getblock verbosity 2, gettxout, and REST getutxos. Push-only OP_RETURN outputs, including the witness commitment and pushes of any size, stay nulldata. Consensus, policy, and gettxout's unspendable rule (IsUnspendable, leading OP_RETURN) do not change.

🤖 Generated with Claude Code

[rearden-grok]

Verdict

APPROVE. I would merge this as-is.

script_core_type is the type string on scriptPubKey JSON. The patch returns nulldata only when the script starts with OP_RETURN and the bytes after that are push-only. That is Bitcoin Core 31.1 Solver: size >= 1 && script[0] == OP_RETURN && IsPushOnly(begin+1) (src/script/solver.cpp, src/script/script.cpp). rust-bitcoin 0.32.102 Script::is_push_only walks instructions() with minimal-push checks off and accepts any opcode <= 0x60, which is Core's opcode > OP_16 rule, including OP_RESERVED (0x50) and OP_1NEGATE (0x4f). A truncated push makes the iterator fail, so the type is nonstandard, same as GetOp failing.

The change is RPC output only. Consensus and relay still treat a leading OP_RETURN as unspendable through is_unspendable (leading OP_RETURN, or a script longer than 10,000 bytes). gettxout returns null for those confirmed outputs before it builds scriptPubKey. The new type is visible on decodescript, decoderawtransaction, getrawtransaction, getblock verbosity 2, and a mempool coin from gettxout / REST getutxos. scriptSig objects have no type. A leading OP_RETURN cannot fall through into pubkey or multisig.

An empty script makes is_op_return false, so the [1..] slice is not evaluated. A PUSHDATA4 length of 0xffffffff stays a short script and comes back nonstandard. No new dependency, unsafe block, or second classification path. The changelog fragment matches changelog.d/ format.

Findings

No bugs.

Nit: the committed rows stop at direct pushes and PUSHDATA1. They omit PUSHDATA2 / PUSHDATA4, a push over 520 bytes, and a length that does not fit. A throwaway probe of those cases on this commit matched Core. I would not hold the PR for more rows.

What was built and tested

Worktree at 20ab353da61e7671fcb8d99ad726422982a20a5b, then restored. Nothing committed or pushed. No GitHub comment.

• cargo fmt --all -- --check: exit 0.
• cargo test -p rbitcoin-rpc --test suite decodescript_nulldata_type_matches_core -- --quiet: exit 0.
• Same test with the push-only conjunct removed, leaving is_op_return() alone: exit 101. First failure is 6a61, left nulldata, right nonstandard. Production file restored after.
• Throwaway probe, not left in the tree: OP_RESERVED, OP_1NEGATE, OP_16, OP_VER, two pushes, 75- and 76-byte pushes, a 521-byte PUSHDATA2, truncated PUSHDATA2, empty and non-minimal PUSHDATA4, PUSHDATA4 length 0xffffffff, and a 10,000-byte push. Exit 0.
• cargo test -p rbitcoin-rpc --test suite -- --quiet: exit 0 (134 tests).
• cargo clippy -p rbitcoin-rpc --all-targets -- -D warnings: exit 0.

Core functional was not run here. CI skipped that job on this SHA. decodescript stays on the node, and the in-tree RPC suite is the pin for this path. GitHub test, qc, macos, windows, and pread were already green.

[coreyphillips]

Two independent reviews, nothing blocking a merge.

[niteshbalusu11]

@reardencode this is rebased onto current master (4036f76) and ready. The only conflict was in the test file, where #1176 added its desc test in the same spot, and both tests are kept. rearden-grok's nit is addressed in 823dbea. The nulldata test now also covers PUSHDATA2 and PUSHDATA4 pushes, a 521-byte push, and PUSHDATA2/PUSHDATA4 lengths that run past the end of the script, all pinned to Core 31.1's types. Required checks are green.

[niteshbalusu11]

@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 mine.rs change is the same as before. Ready, and required checks are green.

[niteshbalusu11]

@reardencode rebased onto current master (e5079de) now that #1186 has merged. #1186's witness arm sits right next to the nulldata arm in script_core_type, so mine.rs conflicted. The resolution keeps master's v0/unknown witness split and this PR's push-only check, and the test file keeps both tests. Ready, and required checks are green.

[niteshbalusu11]

@reardencode rebased onto current master (4f400f4) now that #1188 has merged. The only conflict was in the test file, where both tests are kept. mine.rs merged cleanly next to #1188's p2pk_key arm, and the change is the same push-only check as before. This is the last of the scriptPubKey type PRs. Ready, and required checks are green.