fix(sdk): MINIMALDATA-correct encode_push_data for 1-byte payloads in OP_N range#110
Open
E-Jacko wants to merge 1 commit into
Open
fix(sdk): MINIMALDATA-correct encode_push_data for 1-byte payloads in OP_N range#110E-Jacko wants to merge 1 commit into
E-Jacko wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
encode_push_datainpackages/runar-rs/src/sdk/state.rsdoes not apply BSV consensus ruleSCRIPT_VERIFY_MINIMALDATAfor single-byte payloads. When a 1-byte ByteString payload happens to be in the OP_N range (0x00,0x01..=0x10,0x81), the encoder produces a direct push (01 NN) which ARC, TAAL ARC, and WhatsOnChain all reject at the relay layer.Problem
Current
encode_push_dataatpackages/runar-rs/src/sdk/state.rs:450-462(upstream HEADa8187ab4):For a 1-byte payload
0x01, this returns"0101"(direct push). BSV consensus requires it to be"51"(OP_1). For0x00it must be"00"(OP_0), not"0100". For0x81it must be"4f"(OP_1NEGATE), not"0181".ARC empirical response when a non-minimal push reaches the relay:
The Rust SDK's
encode_script_number(contract.rs:1930) DOES enforce MINIMALDATA forSdkValue::Int(it short-circuits toOP_Nopcodes forn ∈ 1..=16). Butencode_push_datais the encoder forSdkValue::Bytes— and when an arbitrary ByteString arg happens to land on a single byte in the script-number range, the encoder produced a non-minimal direct push.How this surfaced
Surfaced during a mainnet broadcast where a stateful contract method took a ByteString argument carrying a value whose final byte landed on a payload in the OP_N range (
{0x00, 0x01..=0x10, 0x81}). The transaction built cleanly client-side, computed a valid sighash, and signed correctly. On submission to ARC, the relay rejected with the MINIMALDATA error above — the rejection happens at the script-verify stage before the tx reaches any miner.Other common ByteString payloads (signatures ≥ 70 bytes, pubkeys at 33 bytes, hashes at 32 bytes, full preimages) never tripped this because they're always multi-byte. The defect only surfaces when a contract author or framework happens to pass a single byte through the
SdkValue::Bytespath that falls in the OP_N range.Fix
packages/runar-rs/src/sdk/state.rs:Visibility raised from
pub(crate)topubso the regression test can call it directly from an integration test (matches the test-through-public-surface pattern used by other recent SDK fixes).Cross-language parity
The TS SDK's
encodeScriptNumberalready applies the same rule for numeric values; this PR closes the asymmetry on theSdkValue::Bytespath in the Rust port. A TS twin may want to auditencodePushDatafor the same case if the bytes-vs-numeric distinction is mirrored there.Verification
Reproducible in 60 seconds:
Fails without the patch (running the same test against bare HEAD, with
encode_push_datamadepubbut the MINIMALDATA short-circuit reverted):Passes with the patch:
Five tests cover: each of the three minimal-opcode cases (
OP_0,OP_1..OP_16,OP_1NEGATE), the regression boundary (bytes outside the OP_N range still take the direct push form), and the multi-byte path (lengths 2, 4, 75, 76 — covering the direct push / OP_PUSHDATA1 boundary).Backwards compatibility
Existing consumers see no behavioural change for multi-byte payloads (the common case — sigs, pubkeys, hashes, preimages). The only behavioural change is for single-byte payloads in the OP_N range, where the new output is the consensus-required form and the old output was being rejected at the relay anyway.
The
pub(crate)→pubvisibility change is additive — no existing caller is affected; only new callers gain access.Related
SdkValue::Intencoding path (encode_script_numberincontract.rs:1930) already enforces this rule. This PR brings theSdkValue::Bytespath to the same standard.