Skip to content

bench: print the Electrum scripthash reversed again - #1096

Merged
reardencode merged 1 commit into
reardencode:masterfrom
smolcars:bench/electrum-scripthash-order
Oct 20, 2026
Merged

reardencode merged 1 commit into
reardencode:masterfrom
smolcars:bench/electrum-scripthash-order

Conversation

@niteshbalusu11

@niteshbalusu11 niteshbalusu11 commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Summary

6d8855e ("hex: keep the short names in primitives") changed the bench's Electrum scripthash from a Txid display, which prints the bytes reversed, to sha256::Hash::to_string(), which prints them in order. Every address target in the rbitcoin-bench corpora has since been converted to the wrong scripthash, so a bench run asks the Electrum server about scripthashes nobody owns.

 pub fn electrum_scripthash_hex(spk: &[u8]) -> String {
-    sha256::Hash::hash(spk).to_string()
+    rbitcoin_primitives::display_hash_hex(sha256::Hash::hash(spk).to_byte_array())
 }

display_hash_hex is the primitives owner of display-order hash hex (code-shape rule 6). It gives the same byte order as the Electrum server's own electrum_scripthash_hex (hash_hex_rev), and the function's doc comment already said "bytes reversed".

Evidence

sha256(51024e73) (the P2A script) is a6fe0f77… in byte order and c5d0fb38… reversed. The Electrum protocol scripthash is the reversed one.

  • Before: cargo test -p rbitcoin-bench --lib hot_corpus_includes_p2a fails on master with left: "a6fe0f77…", right: "c5d0fb38…".
    After: cargo test -p rbitcoin-bench --lib reports 49 passed.

CI runs cargo test --workspace --exclude rbitcoin-bench, so the regression did not show on the PR. Local: fmt, clippy -D warnings, ast-grep.

Merge Danger

Door: two-way

Bench tool only; no node code.

Blast Radius: bench

🤖 Generated with Claude Code

@niteshbalusu11
niteshbalusu11 force-pushed the bench/electrum-scripthash-order branch from 5bc477c to 54505fc Compare October 20, 2026 02:29
The hex refactor printed sha256(spk) in byte order, so every address
target in the corpora became a scripthash nobody owns. Print it with
primitives' display_hash_hex, the one owner of display-order hash hex.
hot_corpus_includes_p2a passes again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@niteshbalusu11
niteshbalusu11 force-pushed the bench/electrum-scripthash-order branch from 54505fc to c787dc8 Compare October 20, 2026 02:47

@rearden-grok rearden-grok Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. The one-line change restores Electrum/Esplora scripthash display order, and the things that look like they might still be wrong are not.

What was actually broken

bitcoin_hashes 0.14 builds sha256::Hash with hash_type!(256, false, ...). DISPLAY_BACKWARD is false, so sha256::Hash::to_string() prints the digest in internal byte order. sha256d / Txid display backward; plain SHA256 does not. That is why the hex refactor (which stopped going through a Txid Display) flipped every address target.

Electrum's scripthash is SHA256(scriptPubKey), then the digest bytes reversed in hex. The server parses that with parse_display_hash32 (rbitcoin-electrum param_scripthash, Esplora parse_hash32) and looks up script_hash(), which is sha256::Hash::hash(script).to_byte_array() with no reverse. display_hash_hex is the one reverse on the way out, same as hash_hex_rev. This does not double-reverse.

Checked the pinned vector independently. P2A bc1pfeessrawgf is script 51024e73. SHA256 of that script is a6fe0f77b8f1ea88… internally and c5d0fb3863474a90cfe5b26801e66cb45fdc046b40e6829788eaf1b8770ffea6 reversed. The test constant is the reversed form, which is what a client must send.

Bench does not depend on rbitcoin-electrum, and it should not. Routing the hex through display_hash_hex is the primitives owner of display-order hash hex. The SHA256(script) step staying local is the same split the server already has (script_hash in store, reverse in primitives).

What this does not fix, and does not need to

parse_target_line returns a 64-hex line unchanged. casa.txt / sparrow.txt are almost entirely those lines. They are already display order: the genesis P2PKH scripthash 8b01df4e368ea28f…b5c39161 is in both files, and the internal digest 6191c3b5… is in neither. Rewriting those files would point the bench at keys the index does not have.

The lines this repairs are addresses, which hot.txt is entirely, plus the address pins at the top of casa.txt. load_corpus("hot") then both computes c5d0fb38… and finds that same string in the loaded set, so the P2A line is not a hardcoded orphan.

p2wpkh_mainnet_is_stable_scripthash only checks length 64 and a hex round-trip. It stayed green through the regression. The P2A assert is the pin that matters, and one script is enough because every address goes through this function.

Residual, not a change on this PR

Required test is cargo test --workspace --exclude rbitcoin-bench (coverage.test.sh locks that exclude in). hot_corpus_includes_p2a would have failed on master and still will not run in CI. That is why the regression shipped. Turning the exclude off is a separate decision; this pull is the bench tool only, and the local pin matches the protocol.

@reardencode
reardencode merged commit 40a6b38 into reardencode:master Oct 20, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants