--- Day changed Fri Jun 26 2020
01:10 -!- dr-orlovsky [~dr-orlovs@xdsl-188-155-56-253.adslplus.ch] has joined ##miniscript     
02:12 -!- dr-orlovsky [~dr-orlovs@xdsl-188-155-56-253.adslplus.ch] has quit [Quit: My MacBook has gone to sleep. ZZZzzz…]     
03:23 -!- dr-orlovsky [~dr-orlovs@xdsl-188-155-56-253.adslplus.ch] has joined ##miniscript     
04:39 -!- dr-orlovsky [~dr-orlovs@xdsl-188-155-56-253.adslplus.ch] has quit [Quit: My MacBook has gone to sleep. ZZZzzz…]     
04:49 -!- dr-orlovsky [~dr-orlovs@xdsl-188-155-56-253.adslplus.ch] has joined ##miniscript     
06:21 -!- dr-orlovsky [~dr-orlovs@xdsl-188-155-56-253.adslplus.ch] has quit [Quit: My MacBook has gone to sleep. ZZZzzz…]     
08:03 -!- jb55 [~jb55@gateway/tor-sasl/jb55] has joined ##miniscript     
10:45 < andytoshi> lol ok, finally looking at these now
10:45 < andytoshi> FYI if you are ever blocked on my review, it's fine to poke me here, and on signal, and on rocket ... but i will try to be more on top of things
10:45 < sipa> tomorrow morning at 5 am you wake up, sanket1729 banging loudly on your door
10:49 < andytoshi> hahah
10:49 < andytoshi> ok merged the deep nesting one. reading through scriptcontext
10:50 < andytoshi> needs rebase btw
10:50 < andytoshi> oh whoaaa we can do miri tests on travis now?
10:56 < sanket1729> Yeah, their README had clear instructions for travis
11:06 < andytoshi> lol wow miri has a readme now
11:07 < andytoshi> this is all much better than when i last looked at it :P
11:14 < sanket1729> andytoshi: 97 rebased, build passing
11:31 < andytoshi> dope
12:07 < sanket1729> Btw, our lexing relies on distinguishing public keys by 33 bytes and hashes as 32 bytes
12:08 < sanket1729> Which won't be true after the 32 byte pub key in tapscript
12:12 < sipa> i think pubkeys in miniscript-taproot will remain 33 bytes and xpubs
12:13 < sipa> they'll just be encoded in script as 32 bytes
12:13 < sipa> or are you talking about the script-to-miniscript transformation?
12:18 < sanket1729> yeah, script to miniscript decoding. 
12:20 < sipa> right, that will need something smarter
12:30 < andytoshi> sanket1729: oh good catch
12:30 < andytoshi> yeah we'll just have to move the hash/pubkey distinguishing from the lexer into the parser
12:30 < andytoshi> the parser has enough context to know which is which
12:33 < sanket1729> Why should a rational (fee optimizing) user use MULTISIG(based on CHECKSIGADD) from taproot compared to one in segwit?
12:33 < sanket1729> Seems like that costs more bytes?
12:34 < sanket1729> even tough the verification is batched, which the end user does not care about. 
12:34 < sipa> the signatures are still smaller; unsure if that's enough to compensate
12:34 < sipa> 64 bytes instead of 72
12:35 < sipa> another reason is that they may be able to separate the common case to the key path
12:35 < andytoshi> 64 vs 72 should pretty-much always be enough to compensate
12:35 < sanket1729> That should compensate. 
12:36 < andytoshi> i guess, checksigadd costs an additional 2 bytes per key, while the sigs save us 8 bytes per signature
12:36 < andytoshi> so you can certanly set up policies where checksigadd is worse, but probably not realistic ones
12:37 < sanket1729> I was wondering if while creating miniscript from policy, is it ever useful to create segwitv0 when we are allowed by the user to create tapscript
12:37 < andytoshi> i think as design decision we should say no
12:37 < andytoshi> but it is a good question whether we're screwing the user, ever, by making that decision..
12:38 < andytoshi> and i don't -think- so
12:38 < sipa> well if there is no just-single-aggregated-key branch that's pretty likely, taproot may well be wrose
12:38 < sipa> in terms of fees
12:39 < andytoshi> ah yeah that's true
12:39 < sipa> though you can probably argue that over time the privacy gains are worth it
12:39 < sipa> but especially if musig isn't applicable for some reason... it's hard
12:42 < andytoshi> yeah
12:43 < andytoshi> I think, i'm comfortable saying "my software prioritizes privacy over space efficiency, with a potential waste of 32 bytes or so, and if you don't like it you have to go elsewhere"
12:53 < sanket1729> another bip taproot question(probably discussed before), why do we have a checksigadd after every key and not a CHECK_AGGR_MULTISIG? Witness elements will still be n with empty sigs used to denote which keys to aggregrate over. 
12:54 < sipa> sanket1729: that'd require key aggregation in consensus... which is a great idea but also brings a bigger design space
12:54 < sipa> and it's something we'll need anyway for cross-input aggregation
13:07 < sanket1729> Thanks, even though the script interpreter says that there are k checksigs, the node software would only do with one aggregated check, is my understanding correct?
13:07 < sipa> yeah
13:08 < sipa> https://bitcointalk.org/index.php?topic=1377298.0 for history
13:09 < sipa> note that that is using H(P1)*P1 + H(P2)*P2 + ... instead of musig, because we weren't yet aware of the wagner attack on that
13:11 < sipa> it could also use BN06 instead of musig, which would be more conservative
13:39 < sanket1729> Nice
13:39 < sanket1729> andytoshi: bugging again for #97 :P 
14:02 < andytoshi> lol thanks sanket1729. let me just start laundry then i'll get on it
14:02 < andytoshi> qutebrowser says i'm 6% down the diff page
14:02 < andytoshi> so, at this rate it'll be 14 more hours ;0
14:04 < sipa> i suggest shrinking your font size
14:09 < andytoshi> i think the Result<Result> construction is the opposite of what makes sense
14:10 < andytoshi> trying to find our earlier discussion of it..
14:11 < andytoshi> hmm, maybe not, i dunno
14:12 < andytoshi> i think the Error should be on the outside
14:13 < sanket1729> <andytoshi> sanket1729: i think we should go with the Result<Result<Miniscript, Error>, Error>>
14:13 < sanket1729> <andytoshi> where the outside error is the one from FPk/FPkh
14:13 < andytoshi> hmmm
14:13 < andytoshi> yeah, i see, i think my intent was that the caller could tack a ? onto the call and get their own errors
14:14 < andytoshi> but i think it maybe makes more sense to let them call .unwrap() first
14:14 < andytoshi> like, i expect that most users will have no uncompressed keys anywhere in their codebase
14:14 < andytoshi> but i dunno
14:16 < andytoshi> this is kinda a shitty situation, i wish that we had moved compressedness to the type level in rust-bitcoin
14:16 < andytoshi> so people who wanted to be old-school were forced to use an enum and do things unergonomically
14:16 < andytoshi> and the rest of us could just go on with our lives
14:26 < sanket1729> All alternatives seem bad, I don't know how to evaluate. I can change the outer error, should not be hard to to. 
14:37 < andytoshi> ok, thanks
14:37 < andytoshi> i don't feel very strongly about this
14:40 < sanket1729> Do you have any other review changes for the PR?
14:42 < sanket1729> I am not even against panic there
14:42 < andytoshi> still at 20% :)
14:42 < andytoshi> yeah, i'm musing about panicking there
14:42 < andytoshi> because like, the user can always add their own compressedness check if they don't want to panick
14:42 < andytoshi> in their compression function
14:42 < andytoshi> maybe we should just document this
14:42 < sanket1729> If we expect most users to unwrap, it's same as panic
14:42 < andytoshi> right
14:44 < andytoshi> added a comment on github suggesting we panic
15:01 < sanket1729> andytoshi: As a general practice, I tried to derive everything that can be derived, not only those which are required. 
15:01 < sanket1729> Is that like a bad practice?
15:04 < andytoshi> sanket1729: no, in general that's reasonable
15:04 < andytoshi> but in this case these types should never be instantiated
15:04 < andytoshi> and never be directly used
15:04 < andytoshi> so i think they should implement no traits at all, except for private::Sealed
15:08 < andytoshi> lol this diff is so big
15:50 < sanket1729> andytoshi: Unfortunately, I do need those bounds. 
15:51 < sanket1729> Ord for comparing miniscripts, Eq for equality, Cloning , debug for printing miniscript
15:52 < sanket1729> even though it is a phantomData, rust is complaining if I remove any of those
17:01 < andytoshi> really?
17:01 < andytoshi> wtf     
17:01 < andytoshi> i'll play with this a bit
17:02 < andytoshi> i don't remember ever having trouble with phantoms ... like, we don't have any of these traits derived in rust-secp
17:23 < sanket1729> https://github.com/rust-lang/rust/issues/52079
17:24 < sanket1729> was closed as "This is working as intended"
17:27 < sanket1729> linked to open issue since 5 years :P, marked as hard to fix
17:27 < sanket1729> https://github.com/rust-lang/rust/issues/26925
17:52 < andytoshi> lol ugh
17:52 < andytoshi> but yeah i do remember this actually
17:53 < andytoshi> ok fine you can keep the derives :)
17:54 < andytoshi> but i think it's double-bullshit that you need to derive things for a type that can't be instantiated
17:54 < andytoshi> and a type that's only accessed thru a phantom
19:17 -!- harrigan [~harrigan@ptr-93-89-242-235.ip.airwire.ie] has quit [Quit: ZNC 1.7.5 - https://znc.in]     
19:17 -!- harrigan [~harrigan@ptr-93-89-242-235.ip.airwire.ie] has joined ##miniscript     
19:24 < sanket1729> let me know once you are done completely reviewing the PR.
19:24 < sanket1729> I will address of the comments together