--- Log opened Thu Feb 10 00:00:55 2022
02:15 < darosior> sipa: so, your non-recursive GenNode() is doing with the loop something close to what i was doing when i told you i tried to make it always generate valid nodes (except i was using a hardcoded map from Type to NodeType to select what fits, so it was likely more efficient). If we decide to not try to be too smart by trying to generate valid
02:15 < darosior> fragments, and just hint the fuzzer to what is invalid (as discussed here https://gnusha.org/miniscript/2022-01-28.log) i think we can do something simpler. Based on your non-recursive GenNode() and your idea of a binary encoding i took a shot at a simplification of the fuzz target, where we read nodes according to a pre-defined encoding and we
02:15 < darosior> just don't try to generate valid (by type or properties) subs at all: https://github.com/darosior/bitcoin/commits/miniscript_random_fuzz
02:18 < darosior> By running it from scratch it is able to decently find valid nodes. But thanks to the predefined encoding, we could with some efforts create some initial seeds from the former random tests.
02:33 -!- darosior3 [~darosior@194.36.189.246] has joined ##miniscript
02:41 -!- Netsplit *.net <-> *.split quits: roconnor, darosior
02:45 -!- darosior3 is now known as darosior
02:46 -!- roconnor [~roconnor@coq/roconnor] has joined ##miniscript
06:32 <@sipa> Yeah,l; it's also possible to drop some of the type propagation rules in it.
06:33 <@sipa> I found it does find things significantly faster to do it this way, but it does mean more review to be sure it's correct.
06:33 <@sipa> We could also do both.
07:00 < darosior> Yeah. My version gets very close to the miniscript_decode target, but better. I think that actually it could replace it at least for the satisfaction part (keep the miniscript_decode target only for parsing). What do you think?
07:01 < darosior> Or we could ditch miniscript_decode altogether (which i'm, btw, very confused about the state of) but i think it's interesting to fuzz FromScript() directly from the fuzzer'sd output
07:01 <@sipa> I'll have another look this weekend.
08:36 < andytoshi> ``23:00 < sanket1729> I mean how they do parse a descriptor that is pre taproot
08:36 < andytoshi> hmmm, i had it in my mind that the caller would know whether it was pre-taproot
08:37 < andytoshi> if they want to parse everything, they'd parse a generic Descriptor type, and if they wanted taproot functionality they'd call a (fallible) conversion function
08:37 < andytoshi> it may turn out this is an impractical api
11:57 -!- devrandom [~devrandom@2001:470:69fc:105::d4d] has joined ##miniscript
12:21 < sanket1729> andytoshi: Is this what you expect?
12:21 < sanket1729>  https://github.com/rust-bitcoin/rust-miniscript/pull/297
12:37 < andytoshi> sanket1729: almost
12:38 < andytoshi> so, i guess an "AddressableDescriptor" is anything except Bare?
12:38 < andytoshi> i think this shouldn't have `script_code` because that method is not available for taproot
12:39 < andytoshi> tbh i don't know if we need a whole AddressableDescriptor trait, i'd be ok with just panicking if you call address() on a bare descriptor
13:06 < sanket1729> I can remove the Addressable descriptor
13:07 < sanket1729> So I am doing a impl PreTaprootDescriptor for Descriptor?
13:09 < sanket1729> So, you can users to <Descriptor as PreTaprootDescrpitor>::method()? 
13:09 < andytoshi> sanket1729: yep
13:10 < andytoshi> wait
13:10 < andytoshi> no .. because Descriptor has a taproot variant
13:10 < sanket1729> Yup
13:10 < andytoshi> and you can't compute a script_code on that
13:10 < sanket1729> And that would panic then!
13:10 < andytoshi> i'd rather the type system prevent you from even calling this
13:11 < sanket1729> I am not sure how you can parse a pre-taproot descriptor without using the Descriptor enum
13:11 < andytoshi> sanket1729: there are a couple strategies we could take
13:11 < andytoshi> one is to have a PretaprootDescriptor enum
13:11 < andytoshi> another is to let Descriptor be convertible to a TaprootDescriptor type (fallibly)
13:12 < sanket1729> What is the TaprootDescriptor type?
13:12 < andytoshi> sanket1729: my thinking is, if you want to parse an arbitrary descriptor, you can parse a Descriptor. If you want to parse only a taproot one you can parse a TaprootDescriptor
13:12 < andytoshi> sanket1729: an enum of Tr and RawTr i guess
13:13 < sanket1729> It is already possible to match descriptor and get Tr descriptor
13:13 < sanket1729> There is also DescriptorType API that tells you the type of descriptor
13:14 < sanket1729> It is somewhat indirect
13:16 < andytoshi> sanket1729: ok, but you can still call script_code on a taproot descriptor
13:16 < andytoshi> which ideally would be impossible
13:23 < sanket1729> andytoshi: I don't completely understand your other solution
13:23 < sanket1729> "Descriptor be convertible to a TaprootDescriptor type"
13:24 < sanket1729> How do I parse a pre-taproot descriptor from string?
13:24 < sanket1729> in that case
14:13 < andytoshi> sanket1729: what are you trying to parse it as?
14:13 < andytoshi> a generic descriptor? a descriptor that you know is pre-taproot?
14:26 < sanket1729> a descriptor that you know is pretaproot
14:26 < sanket1729> a descriptor that you know is pretaproot
15:48 < andytoshi> sanket1729: in that case, i'd like there to be a PreTaprootDescriptor enum and you'd call FromStr on that
15:48 < andytoshi> which would basically just be a subset of the existing Descriptor::FromStr
15:50 < sanket1729> andytoshi: So, we need a new enum right
15:51 < sanket1729> Which is the first solution you suggested
15:59 < sanket1729> What do you think about a macro for the code that implements a trait for enum where all variants impl that trait
16:21 < andytoshi> sanket1729: yeah, i think it's worth having that macro
16:22 < andytoshi> one of these days we should come up with a language extension for it
16:22 < andytoshi> and yes my proposal is basically to have three macros
16:22 < sanket1729> three macros?
16:22 < andytoshi> TaprootDescriptor, PretaprootDescriptor, and Descriptor ... where the third is a union of the toher two
16:22 < andytoshi> sorry, three structs
16:23 < andytoshi> sanket1729: what would be nice is if we had an extension that would impl a method or trait by just passing through to the method of the same name on every variant
16:23 < andytoshi> i'm sure that one exists, it'd be worth taking another dependency for this because i swear 80% of our codebase is this sort of repeated code..
16:23 < sanket1729> TaprootDescriptor is already Tr. 
16:24 < sanket1729> I don't think we want a separate RawTr. That should be contained inside Bare?
16:24 < andytoshi> sanket1729: right, but i think we'll have RawTr later?
16:24 < andytoshi> ah interesting
16:24 < andytoshi> very interestign..
16:24 < andytoshi> so, RawTr implies an address
16:24 < andytoshi> and Bare does not
16:24 < sanket1729> True, that is an important distinction
16:25 < andytoshi> i'm a little tempted to just drop Bare tbh
16:25 < andytoshi> i guess, we have it for block-explorer type applications
16:25 < sanket1729> It has the pk() descriptor. I don't know if it is useful or not
16:25 < andytoshi> ah yeah
16:26 < andytoshi> sanket1729: it is used by extremely old wallets
16:26 < andytoshi> but ok, Core supports it
16:26 < andytoshi> so i guess we ought to as well
16:26 < sanket1729> I think it might be okay to drop that Descriptor enum and have another enum that contains it
16:26 < andytoshi> it just sucks that address() has to be fallible
16:27 < andytoshi> sanket1729: can you elaborate
16:28 < sanket1729> Because we are going the macro route, we can afford these additional custom structs. 1) Remove Bare from Descriptor enum and descriptor has all non-raw things 2) RawDescriptor has support for Raw things
16:29 < sanket1729> If people are using Raw stuff, we have indirect support for doing everything, just that DescriptorTrait won't be implemented for RawDescriptor
16:29 < sanket1729> Maybe, SolvableDescriptor might be how core names it
16:29 < andytoshi> sanket1729: ok, i like where this is going
16:29 < andytoshi> but i guess, the idea is that Bare is a RawDescriptor?
16:29 < andytoshi> can you get an address from a RawDescriptor?
16:30 < sanket1729> Only RawTr
16:30 < andytoshi> ok, so RawDescriptor would have an address() method that returns a Result say
16:30 < andytoshi> or you are forced to extract the RawTr from the RawDescriptor 
16:30 < andytoshi> either is good by me
16:30 < andytoshi> or both
16:31 < sanket1729> Even if the raw API is awkward, it is okay. It's good that descriptor's don't fail on address
16:31 < andytoshi> yep agreed on both counts
16:31 < andytoshi> i'm fine with the raw API being awkward
16:32 < andytoshi> ok, i like this hierarchy
16:32 < sanket1729> Raw things are also useful for watch-only stuff
16:33 < sanket1729> But that is rare. Ideally, I would assume that you watch something that has address
16:33 < sanket1729> not a script pubkey
16:33 < sanket1729> Does RawDescriptor only have Raw stuff or everything?
16:34 < andytoshi> sanket1729: let's say only raw stuff
16:34 < andytoshi> and we'd have a catchall Descriptor type that includes everything
16:34 < sanket1729> What would that be called?
16:34 < andytoshi> that you can parse to, and lift from, etc., if you are really trying to be maximally generic
16:34 < andytoshi> sanket1729: i think Descriptor
16:34 < andytoshi> so we'd have TaprootDescriptor, PreTaprootDescriptor, RawDescriptor, all contained inside Descriptor
16:35 < sanket1729> I thought we needed a enum with Solvable descriptors
16:35 < sanket1729> that don't fail addr
16:36 < sanket1729> I think that is the most common use-case for wallets signing/recovery etc.
16:37 < sanket1729> So, the PreTaprootDescriptor does not contain Bare?
16:38 < andytoshi> sanket1729: ah right ok
16:38 < andytoshi> alright, then let's say RawDescriptor contains everything :)
16:38 < andytoshi> So we have RawDescriptor containing Bare, RawTr, Descriptor
16:39 < andytoshi> and Descriptor contains PreTaprootDescriptor and TaprootDescriptor
16:39 < sanket1729> Yup, on the same page
16:39 < andytoshi> and ordinary users will be able to use Descriptor -- and we could actually put all the methods on that, just have some of them be fallible
16:39 < andytoshi> e.g. script_code
16:39 < andytoshi> but if they know what they're working with, they can parse to (and use) the more specific typse, and get nonfallible methods
16:40 < sanket1729> Sounds great
16:43 < andytoshi> :) awesome. my brain is a bit fried today but hopefully i'll be back to reviewing things tomorrow
16:43 < andytoshi> e.g. the simplictiy gets pr
16:43 < andytoshi> jets
16:43 < andytoshi> and this
16:47 < sanket1729> cool. No rush
21:06 -!- warren [~warren@fedora/wombat/warren] has joined ##miniscript
--- Log closed Fri Feb 11 00:00:56 2022