Conversation
077e368 to
86f8ebd
Compare
7e05e83 to
718f0aa
Compare
718f0aa to
d3d7ce3
Compare
raphjaph
left a comment
There was a problem hiding this comment.
Ok so we need a bunch more new tests + tweak old tests.
The first group of tests is around the core interpreter path (p2wsh):
- interpreter_p2wsh_single_key_valid — witness script OP_CHECKSIG (miniscript pk()), witness [sig, script], sig over the BIP-143 P2WSH sighash. Assert Verification::Valid. The basic proof the fallback works at all — currently missing.
- interpreter_p2wsh_unsatisfied_rejected — same script with a corrupted/missing signature (well-formed spend, interpreter parses it, sig check fails). Assert Err(Error::ScriptNotSatisfied) — and critically, never Valid. Pins the "iterator exhaustion --> satisfied" invariant the whole feature rests on.
- interpreter_p2wsh_hashlock_valid — OP_SHA256 OP_EQUAL with a clean [preimage, script] witness. Assert Valid. (Deliberately distinct from the existing lib.rs:1661 test, whose witness has an extra empty stack item — see "existing tests to re-verify" below.)
- interpreter_rejects_non_sighash_all — pk() script signed with SIGHASH_NONE. Assert Err(Error::SigHashTypeUnsupported). Mirrors verify_rejects_non_sighash_all_signatures (lib.rs:1039) for the interpreter path.
- interpreter_rejects_high_s — pk() script with a high-S ECDSA signature (reuse the high-S construction from lib.rs:1686). Assert Err(Error::SignatureInvalid). The interpreter itself doesn't enforce low-S; this pins the policy check at verify.rs:392.
- interpreter_cltv_satisfied_valid — script OP_CHECKLOCKTIMEVERIFY OP_DROP OP_CHECKSIG, to_sign built with LockParams whose locktime satisfies the script and lock-enabled sequence. Assert Valid. Pins the intended timelock-verification behavior.
- interpreter_cltv_unsatisfied_rejected — same script, locktime below the script's value. Assert Err(Error::ScriptNotSatisfied).
- interpreter_inexpressible_script_is_inconclusive — a script miniscript can't structure (e.g. containing an opcode outside the miniscript fragment set). Assert Verification::Inconclusive. Pins the spec-mandated escape hatch
The next group is for taproot script-path. At the moment it can't even reach that because src/verify.rs:556–559 explicitly doesn't allow this. Your PR description states you want to add this but the code doesn't actually allow it. So here are the tests:
- interpreter_p2tr_script_path_valid — tr(internal_key, pk(leaf_key)) tree spent via script path (control block built with TaprootSpendInfo), schnorr sig with SIGHASH_DEFAULT. Assert Valid.
- interpreter_p2tr_script_path_tampered_rejected
- interpreter_p2tr_inexpressible_tapscript_inconclusive — valid taproot spend whose leaf isn't miniscript-expressible → Inconclusive.
Finally, we should test the legacy coverage just to be safe:
1.interpreter_p2sh_single_key_valid — bare P2SH with a pk() redeem script, scriptSig [sig, redeem_script] → Valid (exercises the legacy sighash branch of the interpreter).
2. interpreter_p2sh_p2wsh_single_key_valid — nested variant → Valid.
|
Thank you for the review. Group 1 and the bare-P2SH case are done. Three things before I continue.
|
Summary
Scripts outside the template paths reported inconclusive. They now go to a script interpreter, which verifies hashlocks, timelocks, and single-key witness scripts. Scripts it cannot express still report inconclusive, as the spec permits.
Rebased on #75.
Changes
verify_with_interpreterto evaluate scripts the templates cannot classify.verify_inputinto template dispatch and interpreter fallback.verify_full_p2trreturnsInconclusivefor multi-item witnesses so script-path spends reach the interpreter.Notes for reviewers
Closes #76.