--- Day changed Wed Nov 04 2020
03:51 -!- jonatack [~jon@88.124.242.136] has quit [Ping timeout: 260 seconds]     
03:51 -!- jonatack [~jon@213.152.161.101] has joined ##miniscript     
05:38 < sanket1729> andytoshi: https://github.com/rust-bitcoin/rust-miniscript/pull/166 this is more invasive than I initially thought
05:38 < sanket1729> do you think it is worth it?
07:27 -!- andytoshi [~apoelstra@unaffiliated/andytoshi] has quit [Ping timeout: 265 seconds]     
07:59 -!- andytoshi [~apoelstra@wpsoftware.net] has joined ##miniscript     
07:59 -!- andytoshi [~apoelstra@wpsoftware.net] has quit [Changing host]     
07:59 -!- andytoshi [~apoelstra@unaffiliated/andytoshi] has joined ##miniscript     
09:03 < andytoshi> sanket1729: i think everything should take a ctx rather than a &-reference to one
09:04 < andytoshi> also you don't need to make a local variable for NullCtx, you can just directly give NullCtx to the functions
09:04 < sanket1729> Yeah, makes sense.
09:05 < sanket1729> But do you think the tradeoffs are worth it?
09:05 < sanket1729> All APIs now take additional parameter. 
09:18 < andytoshi> still looking through this
09:18 < andytoshi> my first inclination is yes
09:19 < andytoshi> but yeah, e.g. i wasn't expecting the Satisfier trait to have to do this
09:20 < andytoshi> although i do like that you can now impl Satisfier on a secret key
09:21 < andytoshi> and have it use the context to take a secp context and a sighash
09:22 < sanket1729> I guess the basic question is should DescriptoPublicKey implement ToPublicKey ? 
09:23 < sanket1729> It has all the information to do so. So, it makes sense have it
09:25 < sanket1729> But, on the other hand, the context creation creates akward APIs which might not be that useful. That is you can the same APIs on translated bitcoin::PublicKey cheaply
09:29 < andytoshi> yeah, it's a good question
09:30 < andytoshi> it's a bit more general than DescriptorPublicKey
09:30 < andytoshi> we have the same issue with the Liquid keys, where it'd be nice if we didn't have to translate them from peer names to keys (we cauld give the name->key map as context)
09:30 < andytoshi> and it would be nice if we didn't have to use translate_pk to do the pay-to-contract transformation
09:31 < andytoshi> but "would be nice" maybe isn't a good reason
09:31 < andytoshi> would also be nice if we could impl Satisfier for secret keys, where it would take a sighash and sign on the fly
09:31 < andytoshi> or for a HWW handle
09:32 < andytoshi> which would call the HWW on-the-fly
09:33 < sanket1729> yeah, I did not consider the satisfier uses. 
09:33 < andytoshi> for Satisfier we can maybe do something more straightforward
09:33 < andytoshi> rather than passing context to every single lookup_sig call etc
09:33 < andytoshi> we could add a method to the trait
09:35 < andytoshi> hmm actually maybe not
09:37 < andytoshi> i wsih i could easily get a diff of the public API
09:37 < sanket1729> public API?
09:40 < andytoshi> like, how much user-facing code will have to change
09:53 -!- shesek [~shesek@164.90.217.137] has joined ##miniscript     
09:53 -!- shesek [~shesek@164.90.217.137] has quit [Changing host]     
09:53 -!- shesek [~shesek@unaffiliated/shesek] has joined ##miniscript     
13:20 < andytoshi> sanket1729: i'm gonna review and merge https://github.com/rust-bitcoin/rust-miniscript/pull/150
13:20 < andytoshi> let me know in the next half hour if you want me to wait on you
13:21 < andytoshi> probably it's gonna break your ctx-refactor PR, but that's fine, i think it's easier for us to rebase than to make darosior rebase
14:24 < andytoshi> i'm confused about d: http://bitcoin.sipa.be/miniscript/
14:24 < andytoshi> d:X is DUP IF [X] ENDIF
14:24 < andytoshi> which shuold be satisfied by satX, no
14:26 < andytoshi> oh never mind d: is a cast from V to B
14:26 < andytoshi> and it makes the user add the 1 themselves on the witness stack
14:34 < sipa> yeah, satisfy with "sat(X) 1"
14:55 < andytoshi> sanket1729: can i merge 150
16:03 -!- jonatack [~jon@213.152.161.101] has quit [Quit: jonatack]     
16:10 -!- jonatack [~jon@88.124.242.136] has joined ##miniscript     
16:15 -!- jonatack [~jon@88.124.242.136] has quit [Ping timeout: 258 seconds]     
16:15 -!- jonatack [~jon@213.152.161.30] has joined ##miniscript     
17:19 < andytoshi> sanket1729: :anxious_face: https://github.com/rust-bitcoin/rust-miniscript/pull/149/commits/eb6b56af8ee48b3cb9616bb214a0be225db6bba8#r517725732
17:19 < andytoshi> it turns out our code panics if you try to estimate the number of witness elems needed to dissatisfy a malleable or_i
17:20 < andytoshi> and our recent changes to check these limits during parsing, mean that you can decode a malleable or_i and then it'll panic
17:21 < andytoshi> so maybe, max_dissatisfaction_witness_elements (and max_dissatisfaction_witness_size) need to return a Result, whic his pretty shitty, because they already return an Option (if called on something that can't be dissatisfied)
17:21 < andytoshi> or maybe we should just give a worst-case estimate in this scenario, like we do with the checks in the type system
17:21 < andytoshi> or     
17:22 < andytoshi> we should get rid of these functiosn entirely, now that we track these things inthe type system there is no reason to have these recursive calls
17:33 < andytoshi> ok i'm getting a bit ahead of myself -- we need to merge 150, *then* we can drop these panicking methods because their functionality has been moved into the type system (with darosior's worst-case logic rather than panicks)
18:30 < sanket1729> yeah, 150 addresses some of these things. I remember reviewing some aspects related to these panics. 
18:45 < andytoshi> i think 150 completely fixes this
18:46 < sanket1729> re the fuzz failure
18:47 < sanket1729> I cannot find it on ZI history
18:47 < sanket1729> *CI     
18:47 < sanket1729> https://github.com/rust-bitcoin/rust-miniscript/pull/157#issuecomment-722054832
18:48 < sanket1729> What do you think about adding a doc folder to the repo?
18:49 < sanket1729> just as a place for various things we have written about miniscript. 
18:59 < sipa> stuff that doesn't belong on the site?
19:00 < sanket1729> like roconnor's document about reasoning about Miniscript. 
19:01 < sipa> that doesn't sound specific to rust-miniscript
19:02 < sanket1729> yeah, it is not specific to rust-miniscript. But about general reasoning how to arrive at policies. 
19:02 < sanket1729> I don't know if it belongs there, but it got lost in chats somewhere and I wanted to have it a place where we may not forget it existed
19:05 < sipa> feel free to PR things to add some doc directory to https://github.com/sipa/miniscript/ and link it from the site or so
19:05 < sanket1729> sure :) 
19:05 < sanket1729> I think miniscript site is probably a better place
19:05 < sanket1729> *link from the site
19:08 < andytoshi> sanket1729: yeah i blew away the fuzz failure
19:08 < andytoshi> i saved the vector
19:10 < sanket1729> while both of you are here, I like get a consensus on the following
19:10 < sanket1729> https://github.com/rust-bitcoin/rust-miniscript/pull/167#issue-515398884
19:10 < sanket1729> ^ sipa, andytoshi 
19:10 < andytoshi> we obviously shouldn't assume something is unsolvable just because we don't know a hash preimage
19:11 < sanket1729> on the other hand, we don't sign miniscripts like or_i(time, hash)
19:12 < sanket1729> which seems restricting.
19:12 < andytoshi> there's no inconsistency there
19:12 < andytoshi> or do you mean, we don' tfactor the time in, when a locktime may have been signed that eliminates it? we shuold
19:13 < sanket1729> the current signer would choose the time branch becuase it's cheaper to satisfy. 
19:14 < sanket1729> But, it can sign the nlocktime/nsequence to be dissatisfied. and choose the hash preimage for a non-malleable satisfaction. 
19:14 < sanket1729> this is a separate issue. 
19:15 < sanket1729> * sorry, our rust-miniscript signer would fail on that
19:15 < sanket1729> because both don't have has_sig.
19:16 < andytoshi> failing on it is somewhat reasonable
19:16 < andytoshi> though we should factor in the locktime of the tx
19:16 < andytoshi> though as you say, that's a separate issue
19:17 < sanket1729> Yeah, maybe the we really need a malleable signer implemented
19:17 < sanket1729> because I think and(pk(),or(hash,time)) is a common script
19:17 < andytoshi> well, it's malleable
19:17 < andytoshi> but yeah, we should just pay the fee assuming it gets malleated
19:18 < andytoshi> although .. are you aware of anyone using that?
19:19 < sanket1729> nope. I  just thought it seems like a natural construction. 
19:21 < sanket1729> will await opinion from sipa about this before we decide to split Witness:Unavailable into Unavailable + Impossible. 
19:22 < sanket1729> And related to timelock issue, it can Impossible if the top level has has_sig
19:22 < andytoshi> yeah. frustrating that this is nonlocal
19:22 < sanket1729> We have o(1) checks for this
19:23 < andytoshi> oh that's true, it's in the type system isn't it
19:23 < sanket1729> in type system
19:23 < andytoshi> err no, i don't think it is
19:23 < andytoshi> you have to somehow go to the top level (or some higher level) to find hassig
19:23 < sanket1729> Before we call satisfy on the top level, we pass this information
19:24 < sanket1729> isn't hassig same as checking whether all possible paths require signature
19:24 < andytoshi> yeah
19:24 < andytoshi> so we can pass -that- when we first call satisfy
19:24 < andytoshi> but we don't know which sigs are actually used, until after satisfy is done :P
19:25 < sanket1729> does that matter? we know atleast one sig will be used
19:25 < andytoshi> yes, but if we don't know that, maybe a sig gets used nonetheless
19:30 < sanket1729> here is a new suggestion. check top level has_sig. If true, set after(), older() to have has_sig true. and use the same algorithm
19:56 < sipa> i don't know what you need my opinion on
21:02 < andytoshi> ccccccfcjbdblvncuctkhtcdbfrhuftnhfjbvhhuggek
21:02 < andytoshi> sorry
23:14 -!- shesek [~shesek@unaffiliated/shesek] has quit [Remote host closed the connection]