Remediate 14 of 15 open issues: 9 correctness bugs across all 7 tiers + design features#136
Open
icellan wants to merge 91 commits into
Open
Remediate 14 of 15 open issues: 9 correctness bugs across all 7 tiers + design features#136icellan wants to merge 91 commits into
icellan wants to merge 91 commits into
Conversation
…131) All-0xffffffff input sequences make nLockTime a consensus no-op, so a locktime-gated method was script-enforced (extractLocktime) but not consensus-enforced. Add CallOptions.sequence and default every input to 0xfffffffe when a non-zero locktime is set (else keep 0xffffffff). Applied to buildCallTransaction (all three input sites) and the terminal-path tx builder via the shared resolveInputSequence helper. TypeScript tier only.
…TXOs (#133) The non-terminal call path forwarded every wallet UTXO (minus the contract UTXO) as funding, producing an (N+1)-input tx no matter how little funding the call needed. Replace the sweep with smallest-sufficient largest-first selection via selectUtxos (the strategy deploy already uses), covering only the output/fee shortfall the contract's own input value does not. Add CallOptions.maxFundingInputs to cap funding inputs and fail loudly when the cap cannot cover outputs + fee. TypeScript tier only.
…ript RunarContract.fromUtxo (and fromTxId, which delegates to it) filled the constructor argument list with 0n placeholders, so every restored contract operated on zeros: the ANF interpreter computed the wrong state continuation (readonly ctor params feed the continuation formula) and adjustCodeSepOffset computed a zero shift, yielding the wrong OP_CODESEPARATOR / OP_PUSH_TX offset for methods after a ctor slot. Restored stateful spends were unspendable. Add restoreConstructorArgs(): parse the deployed locking script via the existing extractConstructorArgs() and order the values by ABI param (paramIndex). Params without a constructor slot (mutable state fields) fall back to 0n and are overwritten by extractStateFromScript. RED->GREEN: a stateful contract with a readonly ctor slot, restored via fromUtxo, now reports the real slot value and round-trips through @bsv/sdk Spend for both public methods.
…er (#134) deploy() and prepareCall()'s three funding-signing loops signed every P2PKH funding input with the connected method signer and pushed that signer's pubkey, so funding coins owned by a different key failed OP_EQUALVERIFY. Add DeployOptions.fundingSigner / CallOptions.fundingSigner; the funding loops now use fundingSigner ?? signer while the method's own Sig args keep the connected signer. Defaults to the connected signer (zero behaviour change). TypeScript tier only.
…ript getSubscriptForSigning already byte-walks the real _codeScript via findCodesepOffsets when the contract is chain-loaded (#42/#47), but its sibling computeOpPushTxWithCodeSep still derived the OP_CODESEPARATOR offset from adjustCodeSepOffset, which recomputes the shift from the in-memory constructorArgs. When those args don't match the bytes baked into _codeScript (the restore path, or any caller building a RunarContract with a real _codeScript but wrong/placeholder args), the derived offset is wrong and the OP_PUSH_TX signature is computed over the wrong scriptCode. Byte-walk _codeScript for the true offset (index by method) when it is available, mirroring getSubscriptForSigning; keep adjustCodeSepOffset only as the deploy-time template fallback (_codeScript === null). Independent of #119: a unit test forces placeholder constructorArgs on a contract with a real _codeScript and asserts the OP_PUSH_TX preimage/sig are unchanged from the real-args contract. RED before, GREEN after.
…eeUtxo (#118) A true terminal method pays out the full contract balance, so fee == 0 and ARC rejects; the covenant asserts its exact output set, so no change output can absorb a fee. Add CallOptions.feeUtxo: a plain P2PKH input added to the terminal tx BEFORE the OP_PUSH_TX preimage is computed (so hashPrevouts covers it), consumed entirely as fee with no change output. The covenant's output assertions are untouched — only the input side grows. Signed with fundingSigner ?? signer, composing with #134. TypeScript tier only.
The ANF loop node carried only an iteration count and always lowered the iterator as i = 0..count-1, so non-zero-start and countdown loops diverged from interpreter semantics. The front-end had been rejecting those shapes outright. Add explicit start (bigint) and step (1|-1) to the Loop ANF node (compiler + runar-ir-schema TS type, JSON schema, README). extractLoopShape computes the real start/step/count; stack lowering pushes start + i*step per unrolled iteration; the SDK ANF interpreter honors start/step. Zero-start counting-up loops (start=0, step=1) reduce to BigInt(i), preserving byte-identical output. Remove the now-obsolete validate/anf-lower rejections. TS reference tier only; the other six tiers need matching ports.
…he param (#130) A method param whose name collides with a mutable state property was compiled to the stale deserialized on-chain state, not the witness value, when referenced by a bare identifier (e.g. as an addOutput positional value or an assignment RHS). Two layers were involved: 1. ANF resolution: declared params were never registered via addParam, so a bare identifier fell through isProperty -> load_prop. Register method params so isParam wins. Also reorder the property_access lowering to check isProperty before isParam, so explicit this.x still resolves to the property once params are registered (the isParam branch is only for the implicit this.txPreimage member). 2. Stack lowering: deserialize_state pushes each mutable property onto the stack under its own name, colliding with the same-named param slot; name lookups pick the shallower deserialized slot, so load_param read stale state. Rename a colliding param's slot to a reserved name at method entry and map it in lowerLoadParam. Only triggers on an actual param/mutable-prop name collision, so all other contracts are byte-identical. TS reference tier only; the other six tiers need matching ports.
The stateful continuation folded a P2PKH change output into the hashed output set unconditionally, but the SDK's buildCallTransaction omits the change output (and passes _changeAmount = 0) when change <= 0. An exact-cover call (funding == outputs + fee, change 0) could therefore never validate: the script demanded a change output the SDK never built. Wrap the change segment in a runtime ANF if (_changeAmount != 0n): the hashed set carries the P2PKH change output when non-zero and empty bytes (cat with empty is a no-op) when zero, matching the SDK at the exact-zero boundary. The change > 0 execution path is unchanged in behaviour (verified by the #99/#100 exec tests) but the EMITTED bytes now carry the guard. NOTE: this necessarily changes the deployed bytes of every stateful contract that emits a change output, so the checked-in conformance goldens (expected-script.hex for the stateful fixtures) and the 6 non-TS frontends must be regenerated/ported together. Not done on this TS-only branch — the task's 'byte-identical for change>0' expectation cannot hold for a runtime conditional. TS unit goldens (06-emit, 04-anf-lower structure) updated in place; conformance golden FILES intentionally left untouched.
Add explicit start (big.Int) and step (+1/-1) to the Go ANF loop node, mirroring the TS reference. extractLoopShape computes the real start/step/count; stack lowering pushes start + i*step per unrolled iteration; the Go SDK ANF interpreter honors start/step. Zero-start counting-up loops (start=0, step=1) reduce to i, preserving byte-identical output. Remove the obsolete validate/anf-lower non-zero-start and countdown rejections. Loop start is serialized as a bare JSON number (int64 range) or an Nn decimal string, byte-identical to the TS ANF JSON. Verified Go IR + hex are byte-identical to TS for bounded-loop, a countdown loop, and a non-zero-start loop. The bounded-loop expected-ir.json golden now legitimately mismatches (loop node gains start/step) in both TS and Go; regeneration is a later coordinated step. Hex golden is unchanged.
Add explicit start (bigint) and step (1|-1) to the Rust ANF Loop node, mirroring the TS reference. extract_loop_shape computes the real start/step/count from the for-statement init/condition/update; stack lowering pushes start + i*step per unrolled iteration; the SDK ANF interpreter honors start/step. Zero-start counting-up loops (start=0, step=1) reduce to i, preserving byte-identical output. Remove the now-obsolete validate/anf-lower countdown + non-zero-start rejections. Verified byte-identical to TS (hex + IR) for bounded-loop, a countdown loop, and a non-zero-start loop. bounded-loop expected-ir.json now lags the emitted IR by the added start/step fields (script hex unchanged); left un-regenerated pending the other 5 tiers.
…he param in Go (#130) Two layers, mirroring the TS reference: 1. ANF resolution: register declared method param NAMES via addParam so a bare identifier resolves to load_param before falling through to load_prop. Reorder property_access lowering to check isProperty before isParam so explicit this.x still resolves to the property. 2. Stack lowering: deserialize_state pushes each mutable property onto the stack under its own name, colliding with the same-named param slot. Rename a colliding param's slot to a reserved __param_<name> at method entry and map it in lowerLoadParam so load_param reads the witness value, not stale state. Only triggers on an actual param/mutable-prop name collision, so all other contracts are byte-identical (63/64 goldens unchanged; bounded-loop mismatch is the separate #121 start/step change). Verified the retire(balance) shadow case lowers the bare RHS to load_param in both Go and TS.
…Rust A method param whose name collides with a mutable state property compiled to the stale deserialized on-chain state, not the witness value, when referenced by a bare identifier. Port the two-layer TS fix: 1. ANF resolution: register declared method param NAMES via add_param so a bare identifier resolves to load_param before falling through to load_prop. Reorder PropertyAccess / lower_member_expr this.x lowering to check is_property before is_param, so explicit this.x still resolves to the property once params are registered (is_param is only for the implicit this.txPreimage member). 2. Stack lowering: rename a colliding param's slot to a reserved __param_<name> at method entry (before deserialize_state pushes the same-named property slot) and map it in lower_load_param. Only triggers on an actual param/mutable-prop name collision, so all other contracts are byte-identical. Verified: bare identifier resolves to load_param, this.x to load_prop; a readonly-shadow contract is byte-identical Rust==TS; conformance goldens unchanged (59/64, only #121 bounded-loop stale).
#116) Wrap the change-output segment in a runtime ANF if (_changeAmount != 0): the hashed continuation set carries the P2PKH change output when non-zero and empty bytes (cat with empty is a no-op) when zero, matching the SDK's BuildCallTransaction which omits the change output on an exact-cover call (change == 0). For any change > 0 the hashed bytes are unchanged; only the emitted script gains the OP_IF/ELSE/ENDIF guard. Mirrors the TS reference byte-for-byte: verified Go hex == TS hex for all 22 change-output stateful conformance fixtures (stateful-counter, token-ft, auction, ...), each differing from the old golden by the same guard wrapper. Updated the AddDataOutput continuation-ordering structural test to expect the change segment as an if node (buildChangeOutput lives in its then-branch). NOTE: this changes the deployed bytes of every stateful contract that emits a change output. The conformance expected-script.hex goldens for those fixtures now legitimately mismatch (in both TS and Go) and need regeneration as a later coordinated step; not done here.
… != 0 to Rust The stateful continuation folded a P2PKH change output into the hashed output set unconditionally, but the SDK's build_call_transaction omits the change output (and passes _changeAmount = 0) when change <= 0. An exact-cover call (change 0) could therefore never validate. Wrap the change segment in a runtime ANF if (_changeAmount != 0): the hashed set carries the P2PKH change output when non-zero and empty bytes (cat with empty is a no-op) when zero, matching the SDK at the exact-zero boundary. Change > 0 behaviour is unchanged; only the emitted script gains the guard. Verified byte-identical Rust==TS (hex + IR) for stateful-counter, token-ft, and a mutable-shadow repro. Updates the emit.rs inline byte golden in place (mirroring the TS 06-emit unit-golden update). Conformance golden FILES intentionally left untouched: this changes every stateful+change fixture's bytes, so all 7 tiers must be regenerated together (21 hex+IR fixtures pending; see report).
…n Go (#131) Add CallOptions.Sequence (+ BuildCallOptions.Sequence) and a shared resolveInputSequence helper: default every input to 0xfffffffe when a non-zero locktime is set (so nLockTime is consensus-enforced), else 0xffffffff (final, legacy); an explicit sequence always wins. Applied to BuildCallTransaction (all three input sites) and the terminal-path tx builder, and threaded from CallOptions through the non-terminal build + rebuild sites and the terminal site. Mirrors the TS reference.
…equences to Rust All-0xffffffff input sequences make nLockTime a consensus no-op, so a locktime-gated method was script-enforced (extractLocktime) but not consensus-enforced. Add CallOptions.sequence and default every input to 0xfffffffe when a non-zero locktime is set (else keep 0xffffffff). Applied to build_call_transaction_ext (all three input sites) and the terminal-path tx builder via a shared resolve_input_sequence helper. Tests: non-zero locktime defaults inputs to 0xfffffffe; explicit sequence overrides; resolve_input_sequence matches the TS default table.
…TXOs in Go (#133) The non-terminal call path forwarded every wallet UTXO (minus the contract UTXO) as funding, producing an (N+1)-input tx no matter how little funding the call needed. Replace the sweep with smallest-sufficient largest-first selection via SelectUtxos (the strategy deploy uses), covering only the output/fee shortfall the contract's own input value does not. Add CallOptions.MaxFundingInputs to cap funding inputs and fail loudly when the cap cannot cover outputs + fee. Mirrors the TS reference.
… Rust The non-terminal call path forwarded every wallet UTXO (minus the contract UTXO) as funding, producing an (N+1)-input tx no matter how little funding the call needed. Replace the sweep with smallest-sufficient largest-first selection via select_utxos (the strategy deploy uses), covering only the output/fee shortfall the contract's own input value does not. Add CallOptions.max_funding_inputs to cap funding inputs and return an error when the cap cannot cover outputs + fee. Tests: a call with 3 spare 100k UTXOs builds a 2-input tx (not 4); the cap errors when insufficient and is honored when sufficient.
… script in Go (#119) FromUtxo and FromTxId filled the constructor argument list with int64(0) placeholders, so every restored contract operated on zeros: the ANF interpreter computed the wrong state continuation (readonly ctor params feed the continuation formula) and the OP_CODESEPARATOR / OP_PUSH_TX offset was wrong for methods after a ctor slot. Restored stateful spends were unspendable. Add restoreConstructorArgs(): parse the deployed script via the existing ExtractConstructorArgs and order values by ABI param (paramIndex). Params without a constructor slot (mutable state fields) fall back to 0 and are overwritten by ExtractStateFromScript. Applied to both FromUtxo and FromTxId (Go's FromTxId does not delegate to FromUtxo, so both were fixed). Mirrors the TS reference.
…er in Go (#134) deploy() and prepareCall()'s funding-signing loops (non-terminal, rebuild, and terminal) signed every P2PKH funding input with the connected method/deploy signer and pushed that signer's pubkey, so funding coins owned by a different key failed OP_EQUALVERIFY. Resolve fundingSigner = options.FundingSigner ?? signer and use it in all funding-signing loops; the method's own Sig args keep the connected signer. Adds DeployOptions.FundingSigner / CallOptions.FundingSigner (struct fields landed with the #131 type additions). Defaults to the connected signer (zero behaviour change). Mirrors the TS reference.
deploy() and prepare_call()'s three funding-signing loops signed every P2PKH funding input with the connected method signer and pushed that signer's pubkey, so funding coins owned by a different key failed OP_EQUALVERIFY. Add DeployOptions.funding_signer / CallOptions.funding_signer; the funding loops now use funding_signer ?? signer while the method's own Sig args keep the connected signer. Defaults to the connected signer (zero behaviour change). Idiomatic divergence from TS: the signer is stored as a FundingSigner newtype wrapping Arc<dyn Signer> (Signer is not Debug), so CallOptions / DeployOptions keep deriving Debug/Clone/Default without a lifetime param. DeployOptions gains a Default derive; existing struct-literal constructions gain funding_signer: None. Tests: the funding input's scriptSig carries the funding signer's pubkey when funding_signer is set, and the method signer's pubkey otherwise.
The Go SDK already resolves the OP_CODESEPARATOR offset by byte-walking the real code script (getCodeSepIndex -> findCodesepOffsets) whenever codeScript is set, only falling back to the template adjustCodeSepOffset when codeScript == '' (deploy-time). Unlike the TS reference, Go has no divergent public sibling that bypassed the byte-walk, so no production change is needed — the #42/#47 port already centralised the fix. Add a regression test that pins the invariant: with a real codeScript present, the resolved offset is the byte-walked position and is independent of placeholder constructor args.
…n tier (#121) Port the TS-reference #121 fix to the Python compiler + SDK ANF interpreter. Add explicit start/step to the ANF Loop node (ir/types.py + loader). Replace _extract_loop_count with _extract_loop_shape (+ _extract_loop_step): computes real start/step/count for counting-up and countdown loops. Stack lowering pushes start + i*step per unrolled iteration. Serialize start/step in both --emit-ir and the --ir artifact (as plain integers, matching TS after the conformance runner's bigint reviver). SDK anf_interpreter honors start/step. constant_fold and the value-clone carry start/step through. Drop the now- obsolete validate/anf-lower non-zero-start + countdown rejections. Extend the Python parser's range() to accept a negative unit step (range(a, b, -1)) -> countdown loop. Zero-start counting-up loops (start=0, step=1) stay byte-identical; the bounded-loop conformance golden's ANF gains start/step (needs regen).
…he param — Python tier (#130) Port the TS-reference #130 fix to the Python compiler. ANF resolution: register declared method param NAMES via add_param so a bare identifier resolves to load_param before falling through to load_prop; reorder the property_access lowering to prefer a real property over a same-named param (explicit this.x still loads the property). Stack lowering: a param colliding with a MUTABLE property gets a duplicate stack slot once deserialize_state pushes that property under the same name. Rename the colliding param's slot to __param_<name> at method entry and target it in _lower_load_param. Only fires on an actual collision; non-colliding contracts are byte-identical.
…#118) CallOptions.FeeUtxo adds a single plain P2PKH input to a terminal call tx purely to pay the miner fee: added BEFORE the OP_PUSH_TX preimage is computed (so hashPrevouts covers it) and consumed entirely as fee with no change output. The covenant's terminal output assertions are untouched; only the input side grows. Signed with fundingSigner ?? signer (composes with #134). The FeeUtxo field + terminal merge into the shared terminal P2PKH funding-input path (reconciling with the pre-existing FundingUtxos mechanism) landed with the #131 type additions and the #134 terminal-path edit; this commit adds the regression tests. Mirrors the TS reference.
…thon tier (#116) Port the TS-reference #116 fix to the Python compiler. Wrap the P2PKH change segment of the stateful continuation in a runtime ANF `if (_changeAmount != 0)`: the hashed output set carries the change output when non-zero and empty bytes (cat with empty is a no-op) when zero, matching the SDK's build_call_transaction which omits the change output on an exact-cover call. change > 0 behaviour is unchanged; the emitted bytes gain the OP_IF/ OP_ELSE/OP_ENDIF guard. Necessarily changes deployed bytes of every stateful contract that emits a change output: the checked-in conformance goldens (expected-ir.json + expected-script.hex for those fixtures) need regen. Op-count-pinned unit tests updated (+11 ops).
…op start/step
Add an additive, non-breaking injection point so the SDK and a downstream layer (e.g. wallet-toolbox) can broadcast through one shared Broadcaster instance/config rather than WalletProvider's hardcoded ARC URL. When both layers share a broadcaster, parent and child txs land at the same ARC deployment, avoiding the SEEN_IN_ORPHAN_MEMPOOL divergence. - New minimal sync Broadcaster trait (Result<String,String>) matching the SDK's blocking (ureq) transport and Provider::broadcast error shape. The canonical bsv::transaction::Broadcaster is async (#[async_trait]); adopting it would force an async runtime into the sync SDK, so we define the minimal injection interface instead (permitted by the issue). - WalletProvider::with_broadcaster(Box<dyn Broadcaster>) builder method. - Provider::broadcast delegates to the injected broadcaster when present, flattening failures into the existing Err(String); else the hardcoded ARC path is unchanged. Provider trait signature untouched. Tests: injected broadcaster used instead of hardcoded ARC; failure flattens to Err; no-injection path still uses the default ARC.
TypeScript twin of the Rust injection point. Add an optional broadcaster to WalletProviderOptions so broadcast() delegates to an injected @bsv/sdk Broadcaster when provided, letting the SDK and a downstream layer (e.g. wallet-toolbox) share one broadcaster instance/config. Parent and child txs then land at the same ARC deployment, avoiding SEEN_IN_ORPHAN_MEMPOOL. - WalletProviderOptions.broadcaster?: Broadcaster (from @bsv/sdk). - broadcastTx delegates to the injected broadcaster after EF source-tx assembly; a BroadcastFailure return is flattened into a thrown Error to preserve the existing broadcast() contract. Additive, non-breaking; the hardcoded ARC fetch path is unchanged when no broadcaster is injected. Tests: injected broadcaster used instead of the hardcoded ARC fetch path; BroadcastFailure flattens to a thrown Error; default path unchanged.
…/ @bsv/sdk Spend) Adds runTriModalExecution: reuses the bi-modal compile + witness + interpreter + ScriptVM legs, then runs the SAME deployed bytes through the upstream @bsv/sdk Spend interpreter as an independent third engine. Spend is written by a different team against the consensus rules and enforces the clean-stack and push-only rules the hand-rolled ScriptVM does not, so a disagreement is a strong miscompile signal that the two in-repo engines could both mirror. buildWitness now emits consensus minimal-push encoding (OP_1NEGATE / OP_0 / OP_1..OP_16 for small ints) — Spend rejects non-minimal pushes; the ScriptVM and interpreter accept both forms identically, so this is semantics-preserving. Tests cover accept/reject agreement on arithmetic, loops (non-zero start + countdown + post-loop param read), and byte-ops (cat/substr/len over a ByteString param).
Add a producer-side convention for the deliberately-empty branch of an OR-CHECKSIG method (checkSig(sigA, pkA) || checkSig(sigB, pkB)). The failing branch must push an empty signature (OP_0) or BIP146 NULLFAIL rejects the spend; today both Sig modes (Auto, Bytes) push non-empty bytes. - types.rs: add SdkValue::EmptySig unit variant - contract.rs: encode_arg emits OP_0 for EmptySig; prepare_call skips it in the auto-sig collector (never signed) since it is not Auto; soft-warn when >=2 Auto Sig slots remain after resolution - anf_interpreter.rs: EmptySig -> Undefined (never feeds state/data outputs) Purely additive: existing Auto/Bytes callers unchanged. [Auto, EmptySig] signs only the Auto slot; the EmptySig slot stays OP_0. tests/issue_106_empty_sig.rs proves the wire-level RED->GREEN through bsv-sdk Spend: [Auto, EmptySig] validates (branch B empty); [Auto, Auto] duplicates Alice's sig into branch B (the non-empty invalid push NULLFAIL rejects). Note: bsv-sdk 0.1.72 does not implement NULLFAIL, so the interpreter-level rejection is proven on the TS tier (@bsv/sdk).
…ost-loop param reads Adds ForStmt to the language-neutral contract IR (non-zero-start count-up + countdown loops, matching the shapes #121 unrolls) and a dedicated arbExecCase arbitrary for the tri-modal execution oracle. Each case is a stateless contract combining: - a bounded for-loop accumulating into a mutable local, - substr/cat/len byte-ops over ByteString PARAMETERS (with statically-safe, in-range substr bounds from fixed-length ByteString inputs), and - a terminal assert that always reads a parameter AFTER the loop. Cases carry concrete constructor + method inputs so fast-check property mode shrinks both structure and inputs to a minimal repro. ForStmt renders fully in the TypeScript renderer (the oracle is TS-only) and is rejected loudly by the six native cross-tier renderers, which deliberately do NOT emit loops (that path requires 7-compiler byte-identical parity, out of scope here). A 300-case fixed-seed property test confirms all three engines agree on the fixed base.
runTriModalDifferential drives arbExecCase through fc.check (property mode) and runs every generated spend through all three engines (interpreter / ScriptVM / @bsv/sdk Spend). A divergence is shrunk to a minimal (contract, inputs) counterexample, reproduced to capture per-engine verdicts, written to conformance/fuzz-findings-trimodal/, and exits 1. Wired as '--tri-modal' in the fuzzer CLI (--num = property runs, --seed reproduces). Clean over 200 (seed 424242) + 4x500 cases across seeds 1/2/999/20250707.
Add a producer-side convention for the deliberately-empty branch of an
OR-CHECKSIG method (checkSig(sigA, pkA) || checkSig(sigB, pkB), where ||
lowers to the non-lazy OP_BOOLOR). The failing branch must push an empty
signature (OP_0) or BIP146 NULLFAIL rejects the spend; today Sig params
are either null (auto-sign) or explicit hex bytes.
- contract.ts: export EMPTY_SIG (a global-registry unique symbol) +
isEmptySig guard; encodeArg emits OP_0 for EMPTY_SIG; the resolution
loop skips it (not null → never signed) so it reaches encodeArg as OP_0;
soft-warn when >=2 auto-signed Sig slots remain after resolution
- index.ts: export EMPTY_SIG / isEmptySig
Purely additive: existing null/explicit-bytes callers unchanged.
call('m', [null, EMPTY_SIG]) signs only slot 0.
__tests__/issue-106-empty-sig.test.ts compiles a real OR-CHECKSIG
contract, deploys/calls via the SDK, and replays the built tx through
@bsv/sdk Spend: [null, EMPTY_SIG] validates with an OP_0 failing branch;
[null, null] duplicates Alice's sig into the failing branch (the non-empty
invalid push a NULLFAIL-enforcing node rejects). @bsv/sdk 2.0.7 does not
implement NULLFAIL, so the RED baseline is asserted at the wire level.
…issue #124) PR gate: --tri-modal --num 200 --seed 424242 (fast-check property mode, fixed seed, literal argv — no untrusted input). Nightly: --tri-modal --num 1000 with the already-validated rotating $SEED. Both hard-fail (exit 1) on any interpreter-vs-ScriptVM-vs-Spend disagreement and upload the shrunk repro under conformance/fuzz-findings-trimodal/. TS-only oracle — no native compiler build needed. Also gitignores the new findings dir.
…edAlways Add optional embedAlways flag to PropertyNode in both AST definitions (runar-compiler + runar-ir-schema, kept in sync). The .runar.ts parser detects a /** @embedAlways */ JSDoc directive (or // @embedAlways line comment) immediately preceding a field and sets the flag. Comment-directive form (not a decorator) keeps it portable across all nine surface formats.
…on stripped fields A readonly field no method references is eliminated by dead-binding DCE (its load_prop is dead, so no constructor slot is emitted), silently dropping deploy-time metadata authors intend to recover from the on-chain script. Two surfaces: - Preservation (Option 1): ANF lowering injects a load_prop + @ref alias for each @embedAlways readonly field into the first public method — the exact shape 'const _bind = this.field;' produces. The alias keeps the load_prop alive through DCE; stack lowering emits the constructor-slot placeholder and NIPs the value at method end, so the field's bytes remain in the deployed locking script. - Warning (Option 4): compile() emits a warning when an un-annotated, unreferenced readonly field (no initializer) is eliminated, pointing at the @embedAlways directive. Rides the existing diagnostics channel. Purely additive/opt-in: no existing fixture uses the directive, so ANF IR and hex output are byte-identical (zero golden churn).
…rArgs Drives the full path: compile -> splice real args into the deployed locking script (RunarContract.getLockingScript) -> recover with extractConstructorArgs. Asserts the annotated metadataId value survives and the un-annotated control is eliminated (not recoverable).
Adds the /** @sigHash <FLAGS> */ two-surface directive (JSDoc + leading trivia, mirroring #109's @embedAlways) on public methods, parsed into a numeric BIP-143 sighash type on MethodNode. Flag grammar lives in the new sighash-directive module: name->value mapping plus combo validation that rejects unknown flags, zero/multiple base types (ALL|NONE checked on NAMES to catch the 0x03 aliasing collision), and duplicates. Absent directive = default ALL|FORKID (0x41), so existing contracts are unchanged.
Swaps the hardcoded 0x41 in the auto-injected preimage-type assert (04-anf-lower) for the method's declared sighash mode, and threads that mode into the OP_PUSH_TX binding blob (oppushtx-codegen appends the mode's flag byte instead of a fixed 0x41) via a new optional CheckPreimage.sighashFlag carried through 05-stack-lower. Manual checkPreimage calls pick up the mode from the lowering context. The assembler surfaces a non-default mode as ABIMethod.sigHashType so the SDK can build the matching preimage. Default (ALL|FORKID) omits every new field → byte-identical ANF/script/ABI; all 3558 runar-compiler tests incl. cross-tier golden hex pass unchanged.
New sighash-validate pass (wired into 02-validate) rejects, at compile time with source locations, every preimage-field read / output binding that a relaxed sighash flag makes unsound: - ANYONECANPAY: reject extractHashPrevouts and extractPrevOutputScript (hashPrevouts zeroed -> input set uncommitted -> companion-input checks trivially bypassable). - hashSequence: reject extractHashSequence unless pure SIGHASH_ALL (NONE / SINGLE / ANYONECANPAY all zero it). - NONE: reject state-continuation, addOutput/addRawOutput/addDataOutput, requireOutputP2PKH, extractOutputHash/extractOutputs (NONE commits to no outputs -> output bindings unenforceable). - SINGLE: reject >1 committed output (SINGLE binds only the same-index output; extra outputs are attacker-controllable). Walk is transitive through private helpers. Default ALL|FORKID methods (no directive) are never flagged.
computeOpPushTx takes an optional sigHashType (default 0x41) driving both the @bsv/sdk BIP-143 scope (which fields get zeroed) and the appended DER sighash byte. computeOpPushTxWithCodeSep resolves the mode from ABIMethod.sigHashType by public-method index, so every call/finalize path picks it up with no call-site churn. mock-preimage mirrors the BIP-143 zeroing (hashPrevouts under ANYONECANPAY, hashSequence unless pure ALL, hashOutputs under NONE) and stamps the declared flag into the sighashType field + signature byte. End-to-end proofs (replayed through @bsv/sdk Spend): a SINGLE|FORKID exact-cover continuation validates with preimage sighash byte 0x43; an ANYONECANPAY (0xC1) crowdfund-style method validates; the default stays 0x41. All green.
… bypass, requireOutputP2PKH, mock F1 (P1): reject the mutate-only stateful auto-continuation under @sigHash SINGLE. Its output value is the caller-chosen _newAmount, which BIP-143 SINGLE never pins, so a spender can dust _newAmount + zero change + append a draining output and still pass the covenant (value skim). Allow an explicit single addOutput/ addRawOutput SINGLE covenant (the legitimate pairwise input↔output case) but emit a value-pinning WARNING. F2 (P2): reject any @sigHash flag set lacking FORKID. The whole OP_PUSH_TX / BIP-143 machinery is FORKID-only on BSV, so a FORKID-less mode deploys to brick. F3 (P2): the transitive field-read walk now covers for-loop init + condition and the assignment target (sighash-validate.ts scanMethod), and side-effect-summary walks the same for-header + assignment target so output intrinsics cannot hide in a loop header. F4 (P2): reject requireOutputP2PKH under SINGLE — a fixed output index cannot be proven equal to this input's index, the only output SINGLE commits to. F5 (P2): mock-preimage SINGLE now hashes ONLY output[inputIndex] (or ZERO32 when out of range), matching BIP-143, instead of digesting the whole output set. Each fix has a RED→GREEN test. Default 0x41 path untouched (zero golden churn).
The @sigHash (#123, per-method sighash type) and @embedAlways (#109, readonly-field DCE opt-out) comment directives are implemented only in the TypeScript compiler. The Go/Rust/Python/Zig/Ruby/Java frontends ignore comments, so they would silently drop these directives and change signing / DCE semantics (e.g. a @sigHash SINGLE contract compiled by Go would fall back to the default ALL|FORKID). Until the ports land, each non-TS parse entry now scans the source for the directive markers (word-boundary matched to mirror the TS /@sigHash\b/ and /@embedAlways\b/ scans) and errors with a clear message pointing at the issue and telling the author to use the TypeScript compiler. The scan runs for every format the tier accepts, so a .runar.ts fed to a non-TS compiler is caught too. No conformance fixture uses either directive, so the parser-only matrix and Stack-IR / hex parity are unaffected. One guard test per tier covers a @sigHash and an @embedAlways contract plus a word-boundary negative. Guards: - Go frontend.ParseSource (parser.go) - Rust parse_source (parser.rs) - Python parse_source (parser_dispatch.py) - Zig parseSource + CLI compileFromSource (compiler_api.zig, main.zig, shared helper in frontend/input_limits.zig) - Ruby _parse_source (compiler.rb) - Java ParserDispatch.parse (ParserDispatch.java)
The #116 change-output gate added an IF/ELSE/ENDIF around the stateful change output, altering the compiled bytecode. The main conformance goldens (expected-ir.json / expected-script.hex) were regenerated, but the analyzer subsuite's own goldens (conformance/analyzer/<fixture>/expected-analyzer-report.json, verified by packages/runar-rs tests/analyzer_fixtures.rs) were left stale, so `auction` and `stateful_counter` failed on the clean tree. Regenerated via the canonical TS reference generator (conformance/analyzer/scripts/generate-goldens.ts), which reads the current expected-script.hex and re-emits all 8 goldens; only these two changed. The Rust analyzer now matches byte-for-byte (8/8 green), confirming the TS and Rust analyzers agree on the new bytecode. Delta matches the added change-output gate: - auction scriptSize 1782 -> 1794 (+12), offsets shifted +12 - stateful-counter scriptSize 1851 -> 1875 (+24), offsets shifted +12, and a 9th branch point appears (new "2^9 = 512 paths, truncated to 256" finding) — the extra IF/ELSE branch around the change output.
The #116 change-output gate adds 24 bytes per change-emitting method. The Python TicTacToe byte-count lock was stale at 9425; the Python and TS compilers now both emit 9449 bytes for the canonical fixtures (matching the Ruby tier's 9449). Update EXPECTED_BYTES and the test-name/docstring references.
…S tiers A for-loop whose bound is not an integer literal (runtime member access `this.x`, or a bare identifier `const N`) cannot be unrolled into fixed Bitcoin Script. The #121 loop port made the 6 non-TS anf-lowers PANIC on such a bound (previously a silent 0-iteration miscompile). Move the rejection to the validator so compilation fails gracefully with a compile-time-constant diagnostic, matching the TS reference's observable behavior: only literal bounds compile. - Go/Rust/Python/Ruby/Java: tighten isCompileTimeConstant to accept only integer literals (drop the bare-identifier trust), so anf-lower's extractLoopShape is gated before it can panic. - Zig: the parser collapses the bound to an i64, so add a bound_is_const flag (set in parse_ts) and reject at the validator instead of silently compiling a runtime bound to 0 iterations. - Flip the stale "identifier bound accepted" tests (Rust/Go/Python) to assert rejection; add rejection tests for Ruby/Java/Zig. Verified across all 7 compilers: runtime `this.x` and identifier `N` bounds reject cleanly (no panic); literal `5n` bounds compile to byte-identical hex. No conformance fixture uses for-loops, so no golden churn.
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
1 participant
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.
Remediate 14 of 15 open issues: 9 correctness bugs across all 7 tiers + design features
Closes the correctness backlog filed as #106–#134. Nine "tests-green / chain-wrong" divergence bugs are fixed across all seven compiler/SDK tiers (TS, Go, Rust, Python, Zig, Ruby, Java) with the conformance suite regenerated and green in both fold modes; four design/RFC features land at their reference tiers behind fail-closed guards; and the differential fuzz harness is extended to a tri-modal (interpreter / ScriptVM / @bsv/sdk
Spend) gate that mechanically catches this bug class.Staged deliverable (per maintainer decision): this PR is the verified 7-tier correctness work + reference-tier design features. A follow-up PR #2 ports the two new author-facing directives (
@sighash,@embedAlways) and the EmptySig marker to the remaining tiers. Until then, non-TS compilers fail closed (error) on those directives — no silent cross-tier divergence.Correctness bugs — fixed in all 7 tiers, conformance green
forloops silently compiledi=0..count-1Loopnode gainsstart+step; lowering pushesstart + i*step; runtime (non-literal) bounds now reject cleanly in every tier (were a silent 0-iteration miscompile)addOutput/state to the OLD state valuethis.xstill resolves to the property; stack-lowering renames colliding param slotsif (_changeAmount != 0)gate around the change segment, matching the SDK boundary exactlyfromUtxo/fromTxIdfilled constructor args with0nplaceholdersextractConstructorArgs), paramIndex-orderedcomputeOpPushTxWithCodeSepused a placeholder-args offset on the restore path → invalid OP_PUSH_TX sigs for every method after a ctor slot_codeScriptfor the codesep offset (mirrorgetSubscriptForSigning); template path is the deploy-time fallback. (Already correct in every non-TS tier — only TS had the divergent path.)CallOptions.locktimenot consensus-enforcedCallOptions.sequence; default0xfffffffewhen locktime set & non-zerocall()swept every wallet UTXO as a funding inputselectUtxos) +CallOptions.maxFundingInputsCallOptions.fundingSigner/DeployOptions.fundingSignerCallOptions.feeUtxo: a P2PKH input added before the OP_PUSH_TX preimage, consumed as feePorts also fixed 3 latent pre-existing bugs: a Java
IndexOutOfBoundswhen the wallet had more UTXOs than selected (#133 path), a Zig<=/>=loop off-by-one, and the #132 codesep asymmetry itself.Test infra & design features
@bsv/sdkSpendas a third, independent engine that enforces clean-stack/push-only/minimal-push), with loop / byte-op / post-loop-read generators and a 200-case fast-check PR gate. Proven: a re-injected Loop lowering silently miscompiles non-zero-start and countdown loops #121-class miscompile goes RED with a shrunk repro.WalletProvider.withBroadcaster(...)injection (Rust + TS, additive, no trait break). (Subsumes [feat] CallOptions::verify_inputs_on_chain — opt-in pre-broadcast ARC probe of input parents #108 — declined.)EmptySigmarker for OR-CHECKSIG NULLFAIL branches (Rust + TS)./** @embedAlways */readonly-field DCE opt-out + a compile warning (TS reference)./** @sighash <FLAGS> */per-method sighash declaration with mode-aware field-usage validation (TS reference). The validation was hardened after an adversarial audit found a P1: a SINGLE mutate-only continuation is value-skimmable, so it's now rejected; FORKID is required; the field-read walk covers loop headers;requireOutputP2PKHis rejected under SINGLE.Not in this PR
Known follow-ups (tracked, non-blocking)
feeUtxois burned entirely to the miner (inherent — the exact-output covenant binding forbids a change output). Documented; a size-sanity warning is a follow-up.extractSequence < 0xffffffff; documented as authoring guidance (compiler auto-injection is a follow-up).deploy()fundingSigner+ theprepareCallmulti-signer path; thelowerLoadPropplaceholder harden (fromUtxo fills constructor args with zeros instead of parsing them from the deployed script #119 tail).Verification (full matrix, all green)
expected-ir.json+ 22expected-script.hex+ Rust analyzer reports + script-size baseline); all 7 tiers byte-identical.