BIP-379: add test vectors from rust-miniscript - #2240
Conversation
jonatack
added
Proposed BIP modification
|
Concept ACK. It may be handy to provide a file here that runs/verifies the test vectors. |
3ef799a to
9334aa2
Compare
That's a great idea. I've added that in the latest push. $ cd bip-0379; go run main.go
Checking the BIP379 test vectors in . against
github.com/btcsuite/btcd/descriptors/miniscript.
Testing vectors in valid_from_alloy.txt [positive]: 5896/5896 done (type column not compared)
Testing vectors in valid_8f1e8_from_alloy.txt [positive]: 3492/3492 done (type column not compared)
Testing vectors in malleable_from_alloy.txt [positive/negative]: 7024/7024 + 7024/7024 done
Testing vectors in conflict_from_alloy.txt [positive]: 1773/1773 done (1586 of them are rejected by the sanity checks)
Testing vectors in edge_cases.txt [positive]: 42/42 done
Testing vectors in opcodes.txt [positive]: 18185/18185 done (type and op count columns not compared)
Testing vectors in invalid.txt [negative]: 5574/5574 done
Testing vectors in props_from_rust.tsv [positive]: 8198/8198 done (op_count and exec_stack columns not compared)
Testing vectors in props_from_rust_tap.tsv [positive/negative]: 6728/6728 + 1470/1470 done (op_count and exec_stack columns not compared)
Testing vectors in scripts_from_rust.tsv [positive]: 8198/8198 done
Testing vectors in scripts_from_rust_tap.tsv [positive/negative]: 6728/6728 + 1470/1470 done
Testing vectors in redeem.json [positive/negative]: 12/12 + 34/34 done (each spend is executed in the script engine)
All 81848 vectors passed (0 skipped).
|
|
cc Authors: @sipa, @apoelstra, @sanket1729, @darosior, @achow101 |
|
I tried to run these tests through the Bitcoin Core miniscript implementation, but quickly hit the problem that the typing rules are inplemented inconsistently. This isn't a problem in production, because the differences are all within expressions that are invalid (for the correctness-related typing rules) or malleable (for the malleability-related typing rules) anyway, but it does mean that exact type tests like the ones in In fact, rust-miniscript, Bitcoin Core, and the BIP text are all inconsistent with each other, in minor irrelevant ways. If we want to go the direction of having strict test vectors (which I support), I think we also need to first align all implementations and specifications. My suggestion would be to make everything match the BIP text. I have an (LLM written) patch for Bitcoin Core to make it do that (which passes all its unit & fuzz tests) which I'm happy to PR, but someone else will need to do the same for rust-bitcoin. It differs from the BIP in one rule: the |
(non-author) approach ACK |
|
Thanks a lot for testing the test vectors with Bitcoin Core, @sipa. I'm also in favor of attempting to align everything with the BIP text.
Will attempt that soon-ish. EDIT: Here's the BIP amendment PR and the |
ade6aa3 to
3526e43
Compare
|
I I've pushed up a fixup commit (3526e43) that addresses #2240 (comment) (and also fixes the Golang import issues I didn't realize were there). |
|
concept ACK. Great effort. I'm sure a LLM can fill in the documented gaps in a followup PR. I didn't look at the test vectors or the Go code, but the markdown updates look good to me. Will review rust-bitcoin/rust-miniscript#1032 more carefully. |
Both tables give the properties of a fragment in function of those of its subexpressions, and both are written assuming that the conditions in the "Requires" column of that fragment hold. Where they do not, the tables still assign values, but the values carry none of the meaning the descriptions of the properties give them, and implementations are free to differ on them. thresh is the case that surfaced this. Its rule is "e=all are s", so thresh(2,or_i(pk(A),pk(B)),a:or_i(pk(C),pk(D))) is "e" even though each or_i child has two unconditional dissatisfactions, giving the threshold four, which is the opposite of the "unique unconditional dissatisfaction" that the description of "e" promises. The threshold fails the "e" requirement the same table imposes, though, so it is malleable and the value is a don't-care. Say so below both tables, so that implementations which disagree on the type of an invalid or malleable expression are not read as contradicting this document. Bitcoin Core (bitcoin/bitcoin#36028) and rust-miniscript are aligning on the values the tables give, and the test vectors proposed in bitcoin#2240 leave out the properties that are not meaningful: z/o/n/d/u for invalid expressions and s/f/e for malleable ones.
|
I've updated the test vectors in a fixup commit to match the discussion in #2267 and the proposed changes in rust-bitcoin/rust-miniscript#1037. |
5 participants
The BIP-379 currently has a
TBDin the test vector section.I have extracted a bunch of test vectors from the
rust-miniscriptproject, either directly or by slightly modifying test code to dump them into text/TSV files.They have been very helpful for re-implementation in another language, so I think they should be made easily available here.