--- Log opened Mon Feb 14 00:00:59 2022
00:04 -!- elsirion [~quassel@gateway/tor-sasl/elsirion] has quit [Remote host closed the connection]
00:04 -!- elsirion [~quassel@gateway/tor-sasl/elsirion] has joined ##miniscript
12:30 -!- clewe [~clewe@s66-183-0-205.bc.hsia.telus.net] has joined ##miniscript
12:31 < andytoshi> hi clewe 
12:32 < andytoshi> clewe is an intern at blockstream working with me sanket1729 and roconnor on extensions to miniscript to support a simplicity backend
13:48 < sanket1729> Hello clewe 
13:58 < sanket1729> andytoshi: Does this comment still apply in light of the following review? https://github.com/rust-bitcoin/rust-miniscript/pull/297/files/6da402ac79a273952783aa02e0ec634d2ee24f3b..2fd4127c9b07493c6894ee6cf63accb13fc884ff#r806241705
13:59 < andytoshi> sanket1729: you mean, should we still do the complicated macro, even without so many passthrough methods?
13:59 < andytoshi> yes .. because i think there are a shitload of other passthrough methods already in the library, and i'd like to clean those up eventually
14:00 < andytoshi> so i guess, if you can come up with a macro that would also work for e.g. passing through Miniscript methods to Descriptors, then we should do that
14:00 < andytoshi> but if that isn't workable then i guess not
14:01 < sanket1729> Would that not make calling the macro complicated? Because there are vastly different things we are impl Descriptor for Wsh/Sh etc 
14:03 < sanket1729> I don't see how to avoid pass-through "ness"
14:03 < sanket1729> It is an enum with some variants. We have to call the same method on all variants that is inherantly pass-through
14:04 < sanket1729> Nvm, I was misunderstanding you comment I think. What you want is also to take a list of function name and function sig in macro so that is general 
14:04 < sanket1729> I think that should be possible
14:04 < andytoshi> yep
14:05 < andytoshi> i still might be wrong because sometimes we do subtly different things on individual variants
14:05 < andytoshi> but e.g. in descriptor/mod.rs
14:05 < andytoshi>  381     fn sanity_check(&self) -> Result<(), Error> {
14:05 < andytoshi>  382         match *self {
14:05 < andytoshi>  383             Descriptor::Bare(ref bare) => bare.sanity_check(),
14:05 < andytoshi>  384             Descriptor::Pkh(ref pkh) => pkh.sanity_check(),
14:05 < andytoshi> ...
14:06 < andytoshi> oh lol that is DescriptorTrait
14:06 < andytoshi> but i think the story for the other descriptor traits will be the same
14:06 < sanket1729> If all variants impl the same trait. I think we can do this cleanly
14:07 < sanket1729> Which they do in our case. It is calling individual expressions
14:07 < andytoshi> yeah
14:07 < sanket1729> Calling the macro for impl DescriptorTrait on Desciprtor would be somewhat complicated because now we have to list 8 methods and signatures
14:07 < andytoshi> i'm jumping through match expressions in the rest of the codebase and unfortunately i don't think any of them are this clear-cut though
14:08 < andytoshi> sanket1729: i think that's ok, it'd be 8 lines of code
14:08 < andytoshi> which is much less than the 50+ it'd be to impl these methods explicitly
14:08 < sanket1729> I don't think we can avoid this impl DescriptorTrait for Pkh, Wpkh etc. 
14:08 < sanket1729> Just for Descriptor, PreTaprootDescriptor
14:09 < andytoshi> mm, yeah, i think you're right
14:09 < andytoshi> ok, maybe the macro is more work that it's worth
14:10 < andytoshi> i had an idea that we could compress huge parts of the library, which is mostly enum-matching code, but i think i was wrong
14:11 < sanket1729> Are you suggesting to remove the new macros too? Or have the current have to Descriptor/PreTaprootDescriptor
14:11 < andytoshi> i am suggestign to remove the new macros, yeah
14:11 < sanket1729> Also, if you noted I removed the RawDescriptor enum. Descriptor now fails on address API and would contains raw descriptors too 
14:12 < andytoshi> i didn't notice any change to RawDescriptor (and i didn't know that RawDescriptor was even in the lib already)
14:12 < andytoshi> lemme re-read this :)
14:12 < sanket1729> It was there in the initial version of the PR
14:12 < sanket1729> But regardless
14:13 < andytoshi> ah yeah i gotcha
14:13 < sanket1729> <andytoshi> So we have RawDescriptor containing Bare, RawTr, Descriptor
14:13 < andytoshi> thanks :) i was about to find/quote that
14:13 < sanket1729> I think it is kinda odd to have Descriptor not parse raw things
14:14 < andytoshi> yeah, i agree
14:14 < andytoshi> ok, yeah, let's go with this strategy
14:14 < andytoshi> the one where Descriptor includes raw stuff
14:15 < andytoshi> including Bare
14:16 < sanket1729> andytoshi: Also  see https://github.com/bitcoin/bitcoin/issues/24114#issuecomment-1039150866
14:17 < andytoshi> sanket1729:  yeah, i saw that -- does it lead to anything actionable for us? we dan't support `addr` (should we? i guess not..)
14:17 < andytoshi> but maybe we could support `raw` inside sh/wsh
14:18 <@sipa> anything you can do with addr, you can do with raw as well
14:18 <@sipa> but addr is a bit more readable
14:18 <@sipa> not sure what your opposition is to supporting addr
14:19 <@sipa> (i'd understand not wanting to support any kind of partial descriptors, including raw and addr, and all the other variants proposed there, but that doesn't seem to be the issue?)
14:19 < andytoshi> sipa: almost none of the methods on our Descriptor trait (interface) would be supportible
14:19 < andytoshi> no, that is my issue
14:19 < andytoshi> ah
14:19 <@sipa> but you support raw()?
14:19 < andytoshi> actually let me think
14:19 < andytoshi> sipa: no, we don't support raw
14:19 <@sipa> ah, ok
14:20 <@sipa> then the entire discussion is moot, i think
14:20 < sanket1729> Only raw "miniscripts"
14:20 < andytoshi> but lemme organize my thoughts here
14:20 < andytoshi> because i think we could maybe treat addr as a sort of partial descriptor
14:20 <@sipa> if you don't want partial descriptors, don't - but that means no rawtr, no rawnode, no rawpkh, ...
14:20 < sanket1729> Those are really limited to pk, multi upto 3
14:20 < andytoshi> so, i *do* think we want partial descriptors :)
14:21 < sanket1729> We want partial descriptors because we want infer descriptor feature from witness/script sig/script pubkey
14:21 <@sipa> if you do want to support partial descriptors, i think it makes sense to go all the way... addr, raw, ... and whatever flavor of combinations we decide to settle on of those discussed in that issue
14:21 < andytoshi> sipa: sanket and i are talking about something slightly different though -- we support the "bare" descriptor which lets you use raw script in scriptpubkeys .. but this descriptor is unique in not having any address format, which makes our API kinda messy
14:22 < andytoshi> sipa: yeah, that makes conceptual sense to me
14:22 < andytoshi> hopefully it doesn't make our API too messy
14:23 <@sipa> what are bare descriptors?
14:23 < andytoshi> sanket1729: is this also your understanding/intention? that we support partial descriptors, and then also support all the raw descriptors since they're basically just extreme cases of partial descriptors?
14:23 < andytoshi> sipa: literally miniscript code in a scriptpubkey
14:23 < andytoshi> we use this to implement the pk() and multi() descriptors
14:23 < andytoshi> iirc
14:23 <@sipa> is that how you handle pk/pkh (at the top level)?
14:23 <@sipa> i see
14:23 < sanket1729> andytoshi: Our bare is really misleading, because it can be confused with raw. We don't support raw descriptors, but allow bare miniscripts descriptors for pk, mulit
14:24 < sanket1729> Yup
14:24 < andytoshi> sanket1729: cool. now, what if we dropped Bare and just made Multi/Pk be top-level descriptors
14:24 < andytoshi> i guess there still wouldn't be any address format for those
14:24 < andytoshi> so it wouldn't gain us much
14:25 <@sipa> in the c++ code, there are descriptors for raw,pk,pkh,sh,wsh,addr,wpkh, and then inside wsh miniscript it supported (which also has pk and pkh, but currently that functionality in duplicated somewhat)
14:25 <@sipa> not necessarily the best way of implementing things, but it is a possibility
14:27 < sanket1729> Yup, does not gain us much. We are trying to combine many things into a descriptor, can we satisfy it(with necessary information), can we have an address for it, does it have script code or not. Might be worth taking a step back and re-designing the DescriptorTrait
14:28 < andytoshi> sanket1729: agreed. we could make a tracking issue or a gist for this
14:28 < andytoshi> but (a) we want to move some methods to (Pre)TaprootDescriptor, (b) we should rethink it with partial descriptors in mind
14:30 < sanket1729> There are some partial descriptors like rawnode/rawleaf/ possibly rawtr/ that we can satisfy, but raw/addr we can never satisfy. 
14:30 < sanket1729> Will make a gist
14:31 <@sipa> Well rawtr you may be able to satisfy in some cases.
14:31 <@sipa> (if you have the tweaked private key)
14:32 <@sipa> rawnode similar, if you happen to have a satisfaction in a leaf that is not covered
14:35 < sanket_cell> sipa: The current workflow in rust-miniscript. Psbt -> infer descriptor -> satisfy descriptor. 
14:35 < sanket_cell> This requires rawtr, rawnode. We can have support for raw/addr, but thought it was not useful because we can never satisfy it with psbt information.
14:35 <@sipa> Right, that's fair.
14:36 <@sipa> But there may be different workflows where raw/addr descriptors are useful.
14:36 <@sipa> And if you already accept the fact that satisfaction will not always be possible with the information present in a descriptor, maybe it's ok to also support some that are never satisfiable.
14:39 < sanket1729> fair point. I think we can have it
16:05 -!- meshcollider [meshcollid@user/meshcollider] has quit [Ping timeout: 240 seconds]
16:15 -!- meshcollider [meshcollid@jujube.ircnow.org] has joined ##miniscript
21:06 <@sipa> I'm unsure about whether to propose adding rawsh(), rawwsh(), rawwpkh(). They're not strictly necessary in that their function can be covered by raw() and/or addr(), without loss of information.
21:07 <@sipa> But on the other hand, it does seem like we need a rawpkh to represent pkh's inside miniscript with only a known hash (i don't think we want to permit raw() inside miniscript... eek).
21:07 <@sipa> And for consistency it feels then that that practice should be generalized.
21:08 <@sipa> (I'm just using rawX as nomenclature now to refer to the "fragment representing the same as X but with only known hash; the actual notation could be different)
21:10 <@sipa> In particular, using the same fragment name for these things, but with a specially-prefixed/wrapped argument would be good too, and result in less proliferation of fragment names.
21:10 <@sipa> pkh(hash160(...)) is a possibility, but kind of abuse of notation
21:11 <@sipa> pkh(~...) or pkh(@...) or pkh($...) are also possible.
21:14 <@sipa> And with such an approach, I'd feel less bad about adding versions for everything (including wpkh, wsh, sh)
21:17 <@sipa> raw* is better in the sense that it's a clearer indication of "avoid if you can".
21:17 < sanket1729> Agreed that this practice should be generalized. Need to think more about special prefix thing
21:19 <@sipa> or perhaps pkh(raw:...) etc
21:20 <@sipa> though inside wsh(), where miniscript is possible, that'd collide with miniscript wrappers
21:21 <@sipa> or pkh_raw(...)
22:26 < sanket1729> I think we might also take the opportunity to somehow represent "yes/maybe/no" keys in descriptor
22:27 < sanket1729> that should help solve the estimating the witness size problem/fee problem
--- Log closed Tue Feb 15 00:00:59 2022