MiniTapscript: port Miniscript to Tapscript (Part 2) (
descriptors) Apr 12, 2023
The PR branch HEAD was 6e3b37b at the time of this review club meeting.
See notes from the
first part of this review club. Questions
Did you review the PR?
Concept ACK, approach ACK, tested ACK, or NACK?
What was your review approach? In Miniscript, we have type modifiers, which guarantee additional properties for an expression.
866284d makes the wrapper “ d:” have the “ u” property under Tapscript.
What is the “
d:” wrapper and the “ u” type modifier? Why is it that we can make
d: have the u property here? Why not in non-Tapscript Miniscript? This PR adds some logic for statically ensuring no spending path exceeds the stack size at execution time:
Why does this matter for Tapscript?
What’s the approach taken by this PR? What are the pros/cons? (hint:
efdd154) Can you think of an alternative approach to ensure no spending path exceeds the stack size?
e81635c, the scripts are optionally padded during fuzzing. Why? What is the most significant change to the descriptor logic in this PR (hint:
08db38a). Why is it needed? Meeting Log
1 17:00 <josie> #startmeeting
4 17:02 <josie> planning to review <a href=' https://bitcoincore.reviews/27255-2' target='blank'>https://bitcoincore.reviews/27255-2</a> , which is a follow up to last weeks PR review club: https://bitcoincore.reviews/27255
5 17:03 <josie> although, it's awfully quiet in here...
9 17:05 <josie> i recognize most of the names from last time, but I'll ask any way: did everyone here get a chance to attend part 1?
10 17:05 <josie> and if so, have you had a chance to review the questions for part 2?
12 17:06 <kevkevin> yup was in part 1 didnt get too much of a chance to look at part 2's questions
14 17:09 <josie> cool, let's start with question 2: In Miniscript, we have type modifiers, which guarantee additional properties for an expression. commit 866284d makes the wrapper “d:” have the “u” property under Tapscript
15 17:10 <josie> first off, what are the "d:" wrapper and "u" fragment?
16 17:11 <abubakar> from miniscript website "d" is a dissatisfiable: does not include a signature or a hash preimage cannot rely on timelocks for being satisfied
18 17:12 <abubakar> "u" when satisfied the expression will have 1 on the stack
19 17:13 <josie> also - I typed that incorrectly: that should be "what is the 'u' type modifier"
20 17:13 <stickies-v> abubakar: perhaps confusingly, there's a difference between `d` the property and `d:` the wrapper
21 17:13 <stickies-v> as in - they're entirely unrelated
22 17:14 <abubakar> stickies-v: ohthanks
23 17:15 <josie> abubakar: I made the same mistake when writing the questions! it is confusingly named
24 17:15 <stickies-v> properties/type modifiers describe attributes of fragments (expressions), e.g. the `z` property says that a fragment with that property always consumes exactly 0 stack elements
25 17:15 <stickies-v> wrappers however take a fragment and add some script before/after it
26 17:16 <josie> stickies-v: that's a great explanation on the difference between wrappers and type modifiers, thanks!
27 17:17 <stickies-v> the `d:X` wrapper takes fragment X, and adds `DUP IF` before and `ENDIF` after the fragment, so you end up with `DUP IF [X] ENDIF`
28 17:19 <abubakar> stickies-v: thanks
29 17:19 <josie> what about the type modifier, "u"?
30 17:20 <stickies-v> fragments with type modifier `u` always put `1` on the stack when satisfied
31 17:20 <abubakar> i think its the one I reffered to earlier
32 17:20 <stickies-v> ah yes exactly you did!
33 17:21 <josie> abubakar: sorry, i missed this, but yes! when satisfied, expressions with the u property leave exactly 1 on the stack
34 17:22 <josie> so leading into the second part: why is okay for us to say the d:X fragment has the u property under tapscript?
35 17:22 <abubakar> It is called unit because of the 1 it leaves right?
36 17:23 <josie> abubakar: that's how I understood it: unit as in exactly 1
37 17:25 <abubakar> josie: okay
38 17:26 <stickies-v> before taproot, OP_IF would evaluate to true for any value > 0, whereas in taproot we require the argument to be either 0/1. Which means that in case of true, the argument is definitely 1. Because the `d:` wrapper duplicates the argument (OP_DUP), we can state that in case of satisfaction this fragment will put 1 on the stack
39 17:27 <josie> stickies-v: "OP_IF would evaluate true for any value > 0" .. was this true for _any_ transaction?
40 17:29 <stickies-v> hmmmm, i thought so but it sounds like i'm missing something?
41 17:30 <josie> perhaps a better way to phrase my question: was there anything pre-tapscript that would have prevented "regular" users from constructing transactions that used this OP_IF behavior?
42 17:31 <instagibbs> MINIMALIF was a standardness check for p2wsh iirc
43 17:32 <josie> instagibbs: bingo! the main problem here is miners are not bound by standardness, only validity
44 17:33 <josie> I don't have a link to it, but darosier has shown some nasty examples where miners could exploit this to steal funds in scripts that use thresholds
45 17:34 <josie> so if people remember from last week, MINIMALIF was added as a consensus rule in tapscript, which means it is now safe to say that d:X fragments can have the u property
46 17:34 <josie> moving on to question 3: "This PR adds some logic for statically ensuring no spending path exceeds the stack size at execution time"
47 17:35 <josie> why does this matter for tapscript?
48 17:36 <kevkevin> didn't CHECKMULTISIG have a limit of 20, and CHECKSIGADD removed that limit so now we're bound by the stack size limit, not sure if I'm remembering correctly
49 17:37 <instagibbs> miniscript does more than 20 signatures fine I think. BIP342 introduces "It is extended to also apply to the size of initial stack" which means it can't exceed limits ever and succeed. Not sure that answers it
51 17:37 <abubakar> because someone might use a script that add elements that are larger than stack size, standarness were lifted
52 17:39 <stickies-v> miniscript also checks if a script is sane, which (amongst others) also means that it is consensus valid and standard - this helps a lot with analysis of scripts. for example, you may not care about the entire script, just that you have a certain spending path to which you control the keys. but if it then turns out that that spending path is actually not spendable (because of stack size limit), that's a problem
53 17:39 <josie> kevkevin: CHECKMULTISIG did have a limit, but I think its more correct to say CHECKMULTISIG was disabled in tapscript and CHECKSIGADD was added, and CHECKSIGADD doesn't have a limit (aside from the stack size limit)
54 17:41 <kevkevin> josie: ok ya that makes more sense, thanks
55 17:41 <josie> stickies-v: great point re: miniscript. I think tapscript's decision to remove some of the "arbitrary" limits from before makes being able to reason about sanity much more straightforward
56 17:43 <josie> moving on the the second part of the question: "What’s the approach taken by this PR? What are the pros/cons?"
57 17:44 <josie> by approach, we are referring to how this PR ensures no spending path exceeds the stack size limit at execution time
58 17:50 <josie> for this one, the commit message is pretty helpful
59 17:53 <josie> haha the silence is pretty deafening on this one (which admittedly is a pretty in the weeds question), so lets finish with the last question about descriptors
60 17:53 <josie> What is the most significant change to the descriptor logic in this PR?
61 17:54 <stickies-v> sorry josie I didn't review this far ahead so just looking at the commit now - if you wouldn't mind summarizing your answer i'd appreciate that?
62 17:54 <stickies-v> on the approach and pros/cons
63 17:58 <josie> sure! basically, it tracks how the stack will be affected after the script executes, and also how many items will be pushed on the stack during execution. the second part is the part I don't fully understand, but it also checks the maximum witness size for a fragment and takes the lesser. regarding pros and cons, it mentions this being a conservative approach because the max
64 17:58 <josie> witness might not always happen along with the max stack size
65 17:59 <stickies-v> i think we also distinguish between satisfactions and dissatisfactions, right?
66 17:59 <josie> as a con, you could say since this is a conservative approximation, which might not let you do a script that you would otherwise be able to do with more precise accounting
67 18:00 <josie> stickies-v: I believe so, but this where my knowledge of script execution gets fuzzy
68 18:01 <josie> we'll stop here. thanks everyone for attending part 2! this is a really dense topic so I appreciate everyone muscling through
69 18:01 <josie> #endmeeting
70 20:58 <sipa> @josie kevkevin Arguably, CHECKMULTISIG has no more or no less a limit (20) than CHECKSIGADD has a limit (1).
71 20:59 <sipa> The point is that multi maps to just one CHECKMULTISIG, and thus inherits the 20 limit.
72 20:59 <sipa> While multi_a maps to many CHECKSIGADD opcodes, so it is not bound to its 1 limit.
73 21:00 <sipa> We could arguably define a multi() fragment that intelligently maps to multiple CHECKMULTISIG opcodes, and that wouldn't be bound by the 20 limit either, but that's just hiding a lot of complexity that can equally well be captured at a higher level (forcing the miniscript to combine multiple multi() fragments explicitly).