--- Day changed Wed Aug 05 2020
00:03 -!- jeremyrubin [~jr@2601:645:c200:f539:bd98:3c2c:9403:b8b3] has quit [Ping timeout: 240 seconds]     
00:53 -!- dr-orlovsky [~dr-orlovs@194.230.155.142] has joined ##miniscript     
01:24 -!- dr-orlovsky [~dr-orlovs@194.230.155.142] has quit [Quit: My MacBook has gone to sleep. ZZZzzz…]     
01:49 -!- afilini [~user@gateway/tor-sasl/afilini] has joined ##miniscript     
03:44 -!- jonatack [~jon@2a01:e0a:53c:a200:bb54:3be5:c3d0:9ce5] has quit [Quit: jonatack]     
05:19 -!- jonatack [~jon@2a01:e0a:53c:a200:bb54:3be5:c3d0:9ce5] has joined ##miniscript     
05:31 < sgeisler> andytoshi, sipa: I should ping you regarding https://github.com/rust-bitcoin/rust-miniscript/pull/116#issuecomment-668850056
05:32 < sgeisler> as I see it descriptors have at least two use cases: wallets that want to describe their set of addresses they use and can sign for and watchonly applications which will never do any signing
05:32 < sgeisler> the former obviously always knows the public keys for all descriptors
05:33 < sgeisler> but the latter may just have a bitcoin address which it wants to watch and thus only has a hash of some public key
05:34 < sgeisler> afaik the current descriptors in core support this use case for e.g. querying the UTXO set
05:35 -!- jonatack [~jon@2a01:e0a:53c:a200:bb54:3be5:c3d0:9ce5] has quit [Quit: jonatack]     
05:37 < sgeisler> This whole problem came up when I implemented https://github.com/MagicalBitcoin/rust-electrum-client/pull/22 for fetching address balances from arbitrary scripts. I use it in an asset manager to query the total holdings on a bunch of descriptors.
05:39 < sgeisler> But there might be the use case of old paper wallets that can't be migrated for now and for which one only has the address handy (= no pubkey). So in the end it's supposed to query a set of xpub descriptors and simple pkh(<hash>) descriptors.
05:40 < sgeisler> Is this use case  something we want to support? And if so is having a hash variant in `DescriptorPublicKey` a viable solution?
05:41 -!- jonatack [~jon@2a01:e0a:53c:a200:bb54:3be5:c3d0:9ce5] has joined ##miniscript     
06:11 < andytoshi> i think i'm okay with it
06:11 < andytoshi> but i recall sipa has argued against stuff like this in the past
07:33 < sgeisler> I have to admit that it would make the API less beautiful (may panic) or vastly more complicated (different key types with/without hash support). But I think it would be worth it. I'd be interested in sipa's counter arguments.
07:44 < afilini> andytoshi: do you think it would make sense to add a "network" type to descriptors? I mean, something like ScriptContext but for Bitcoin/Testnet/Regtest
07:45 < afilini> I could use it to check that the keys I'm parsing (wif/xpub/xprv) have the right prefix for the network
07:46 < afilini> as a side-effect the `Descriptor::address()` method that now takes a `network` parameter would not need it anymore
07:48 < afilini> or maybe before asking "how", I should ask whether I should even bother checking the prefixes. I think Core does it (i.e. you can't import a descriptor with a "tprv" in mainnet), do we want to have the same behaviour in rust-miniscript?
07:58 < sgeisler> I don't think we should get the network mixed up there. It doesn't belong there conceptually imo.
08:06 < afilini> Yeah, it also feels weird to me. But it also feels weird to blindly accept keys from any network.. maybe just check that they are all consistent with each other (any network is accepted but you can't mix them)?
08:21 < sgeisler> but that reminds me of something: we need to patch `Extended{Public,Private}Key` to accept ypubs etc. too
08:22 < sgeisler> currently you can't just use the export from electrum or similar wallets without converting for certain script/address types
08:25 < andytoshi> afilini: scripts, pubkeys, etc., have no notion of network
08:26 < andytoshi> private keys do but public keys don't, addresses do but scripts don't, etc
08:26 < andytoshi> we've considered this before and it becomes a nightmare trying to come up with a sane API
08:37 < sipa> sgeisler: can't you use the addr() or raw() descriptors?
08:40 < andytoshi> we don't have those descriptors in rust-miniscript because they'd disable the majority of the library (and make a lot of functions fallible that otherwise aren't)
08:41 < andytoshi> which i think is why sgeisler is trying to approach this by adding variants to a key type
08:41 < andytoshi> since the key types are easy for users to swap out if they don't like the API
08:41 < sipa> pkh(hash) has the same peoblem
08:41 < andytoshi> yes. we only support pkh(pubkey)
08:42 < sipa> yes     
08:42 < andytoshi> which i thought was the same as Core
08:42 < sipa> that's my point
08:42 < sipa> if you want to only have "functional" descriptors, you cannot support pkh(hash) or addr()
08:43 < andytoshi> hmm yeah i see
08:43 < sipa> if you're ok with non-functional ones (which don't let you sign, or predict sizes, ...), pkh(hash) has no advantages over addr() and you should just have that
08:43 < andytoshi> sgeisler: can you clarify how the `Hash` variant would actually work with the rest of the API
08:44 < sgeisler> Ok, that makes sense. So maybe the right approach is to wrap and extend the scriptor type in cases where `addr()` is wanted?
08:44 < andytoshi> sipa: that's a pretty strong claim, it wouldn't surprise me if it were possible to have a cleaner pkh(hash)-supporting API than to have a addr-supporting API
08:45 < sipa> andytoshi: perhaps some things
08:45 < andytoshi> sgeisler: maybe. i hope not, wrapping and extending types is pretty unergonomic :/
08:45 < sipa> andytoshi: i'm curious what you can come up with :)
08:46 < andytoshi> i'll try to think about this today. tbh i would like to have addr() and raw() so that we could implement scantxoutset
08:46 < sgeisler> andytoshi: the two key versions variant would work as `MiniscriptKey` only requires knowledge of a pubkey hash
08:46 < andytoshi> but every time i look at this, i don't like the available options :)
08:46 < sgeisler> only `ToPublicKey` requires pub key knowledge
08:47 < andytoshi> ah i see
08:47 < andytoshi> and iirc only conversion to Script requires ToPublicKey ?
08:47 < sipa> if the only thing you want is conversion to Script, addr() is enough ;)
08:47 < sgeisler> currently `DescriptorKey` impls both, but there could be a version for the mentioned use case that only impls `MiniscriptKey`
08:47 < andytoshi> sipa: hehe true :)
08:48 < andytoshi> i guess i mean, computation of witness/redeem script
08:48 < sgeisler> oh, I actually didn't check, that would make sense but would be annoying :/
08:48 < sgeisler> that sounds better
08:48 < sgeisler> the watchonly use case doesn't care about redeemscripts
08:49 < sipa> you can't compute the witness/scriptsig with just a pkh
08:49 < andytoshi> right
08:49 < sipa> you can't even predict its siz3
08:49 < andytoshi> i wonder if we want to change Descriptor to be a trait rather than an enum
08:49 < andytoshi> i forget whether we've tried that before
08:50 < andytoshi> because that would let us to special-purpose stuff for specific kinds of descriptors
08:50 < andytoshi> at the cost of making descriptor-generic stuff harder to use
08:50 < andytoshi> ah, no, we can't really do this ... what would the parsing function return? never mind
08:50 < sgeisler> Idk, do you have concrete use cases in mind?
08:51 < andytoshi> sgeisler: my concrete case is that i want addr() and raw(), and i want the compiler to prevent attempts to compute witness scripts etc on those
08:51 < andytoshi> rather than returning errors that the library user has to deal with
08:52 < andytoshi> also i'm trying to do some covenant stuff on Elements and it would be nice if i had a raw() descriptor that i could wrap/extend, it would save me a bunch of transaction/script-munging code..
08:52 < andytoshi> now i sound like jeremy :)
08:53 < sgeisler> I think we can already kinda support this with the 2 types: only the type with hash variant could parse `addr()` effectively disabling redeem script generation. The other one would return an error when parsing.
08:53 < sgeisler> the weird thing would be that the key parsing fails and has to tell you you cant use `addr` with it
08:54 < sgeisler> idk about raw
08:54 < andytoshi> do we want two totally distinct types or can we use a generic parameter on Descriptor?
08:54 < andytoshi> yeah there's gonna be weird things no matter what :/
08:54 < sgeisler> imo only changing the key parameter on `Descriptor<K>` would allow this distinction
08:54 < afilini> andytoshi: well, the `ToPublicKey` trait already achieves this. you can't call most methods on Descriptor<Pk: !ToPublicKey>. I guess the problem is when your `Pk` is an enum and some of its variants are not `ToPublicKey`, but I don't think there's any way to do compile-time checks on enum variants, right?
08:55 < andytoshi> afilini: correct
08:55 < afilini> "already achieves this" = i want the compiler to prevent attempts to compute witness scripts etc on those
08:55 < sgeisler> Yeah, that's the problem. But we could just have two kinds of descriptor keys fort that reason and share most of the code
08:56 < andytoshi> the issue with having two kinds of keys, is that it doesn't really match the user model
08:56 < andytoshi> abstractly, we're talking about different descriptor types -- addr() -- not different key types
08:56 < andytoshi> it just happens that our API is set up so we can easily disable/enable functionality for different keytypes
08:56 < andytoshi> and not set up for doing this for different descriptors
08:57 < sgeisler> the addr variant would fail to parse because the key type doesn't support it because it doesn't contain the pub key
08:58 < sgeisler> ah, I think I get what you are saying
08:58 < sgeisler> it's a bit weird
08:58 < sgeisler> we'd indirectly control capabilities via key types
08:59 < sgeisler> maybe just provide two type defs with some explanations?
08:59 < andytoshi> 15:57 < sgeisler> the addr variant would fail to parse because the key type doesn't support it because it doesn't contain the pub key
08:59 < andytoshi> heh that's super surprising
08:59 < andytoshi> hmmm maybe not
08:59 < andytoshi> basically when you try to parse addr(1234) as a Descriptor<bitcoin::PublicKey> it'll fail
09:00 < sgeisler> ahhh, it doesn't really work like that for addr I think :/
09:01 < sgeisler> because the key type won't yield tree nodes like p2pkh(hash)
09:02 < sgeisler> it wants to parse some key-like object, so that might need some more reengineering of the API
09:02 < sgeisler> p2pkh(hash) would work though
09:09 -!- jeremyrubin [~jr@2601:645:c200:f539:bd98:3c2c:9403:b8b3] has joined ##miniscript     
09:09 < afilini> that's probably stupid, but what about a macro (or more likely a proc_macro?) that disables some variants of an enum based on the `Pk` type? basically, we could add an `Addr(Script)` variant to the `Descriptor` enum, but that would only be accessible if the `Pk` type is not `ToPublicKey` which means that you can't call any of the potentially broken methods. then if you try to convert the
09:09 < afilini> descriptor to another `Pk` type you could return an error in `translate_pk` that already returns a `Result`
09:10 < sanket1729> still catching up the discussion, why is it a bad idea to support `addr(Script)` or `addr(has160)` as Descriptor enum and panic on whenever we try to do anything to requires ToPublicKey? 
09:11 < afilini> ah no, scrap that, the `address()` and `script_pubkey()` methods are also only available on `ToPublicKey` Pks. So basically you wouldn't be able to do anything on `addr()` descriptors because there are no methods available on them, and you also can't convert them to anything else
09:14 < sgeisler> please no macros, they still destroy my auto completion most of the time xD
09:35 < sanket1729> I am starting to think that adding `addr` as descriptor enum and changing the APIs to return Results is probably the cleanest way to do this. 
10:13 < afilini> this https://github.com/afilini/rust-miniscript/commit/81337a1de03211e58d312c87ae49f0d3c7872357 basically implements the "two Descriptor types" but with a phantom generic
10:13 < afilini> the naming sucks (as always), but `ScriptKnowledge` basically means "how much do we know about the script"
10:14 < afilini> if we know "everything" (Full), then we can also estimate the size, produce the script_sig, etc
10:14 < sipa> fwiw, in Bitcoin Core the terminology is "solvable" (which has been used sinc ebefore descriptors existed)
10:14 < sipa> solvable means you'd be able to spend an output, except for not knowing private keys
10:15 < afilini> if we know "something" (Limited) then we can only try to turn the descriptor into an address or a script_pubkey
10:16 < afilini> sipa: yeah, "solvable" is probably the right term here, thanks!
13:19 < andytoshi> hehe, no panicking pls, and no macros
13:20 < andytoshi> sanket1729: returning results kinda blows, we'd have to change a ton of liquid code (for example) to unwrap .witness_script() calls that we know will never ever fail
13:22 < sanket1729> So, that means we are not touching the descriptor enum and probably doing something hacky for `addr`. 
13:22 < sanket1729> everything is ugly :) 
13:46 < sgeisler> afilini's solution with another type parameter (which can probably default to the current behavior) seems quite neat
13:46 < sgeisler> the only problem I can see is that the descriptor enum is afaik public
13:47 < sgeisler> so we can't actually guarantee that a "solvable descriptor" is not of type `addr(…)`
13:47 < sgeisler> just that we won't parse it
13:47 < sgeisler> maybe wrapping the enum will help if we don't actually depend on matching the enum?
13:49 < afilini> wait, maybe you can just define the enum twice
13:50 < andytoshi> yeah i'm not entirely opposed to that
13:50 < afilini> once for every "solvable" variants. in the "non-solvable" variant you just omit the `Addr()` variant
13:50 < andytoshi> though it feels a bit wasteful API-wise and might be confusing
13:50 < andytoshi> and it will be hard to share code
13:55 < sgeisler> In the end an enume with a `Descriptor` and a `Address` variant might be the best solution. That would basically be a limited descriptor that only knows its script pub key.
13:55 < sgeisler> When parsing it just special-cases the addr variant and otherwise uses the descriptor parser
13:56 < afilini> one more idea: instead of `PhantomData<K>` we take a custom struct there (that probably just wrap PhantomData<K>) but that is not public
13:56 < afilini> sorry, i mean the type is public but the constructor isn't
13:58 < sipa> sgeisler: might as well generalize it to an enum with ScriptPubKey or Address, and a parser for the first that accepts either addr() or raw()
13:58 < afilini> so you can't instantiate it from outside the library, which means that you can't manually build that variant of the enum
13:58 < sipa> eh, ScriptPubKey or Descriptor
14:02 < sanket1729> sgeisler, afilini: the commit is neat, but requires the user to specify Full, Limited when parsing a descriptor that makes it a bit awkward. 
14:03 < sanket1729> I mean that is consequence of type
14:06 < sanket1729> But i guess the API is not very intuitive. descriptor<_, Full> = from_str(..) 
14:06 < sgeisler> sanked1729: I think that particular concern could be addressed wit ha default parameter
14:07 < sgeisler> but I kinda like sipa's idea best as it seems the most general solution, also to the `raw(…)` problem
14:13 < sanket1729> Agreed. 
14:18 < sanket1729> We already have a `Bare` variant that is basically raw, but we don't parse it as `raw()`
14:23 < sipa> sanket1729: raw is just specifying a hex scriptpubkey
14:26 < sanket1729> mabye I am not following it. w.r.t to miniscript, It is just miniscript encoded directly in scriptpubkey
14:27 < sipa> no     
14:27 < sipa> raw is "i have a scriptpubkey"
14:27 < sipa> there is no miniscript or anything else known about it
14:28 < sipa> https://github.com/bitcoin/bitcoin/blob/master/doc/descriptors.md#reference
14:55 < andytoshi> yeah +1 to putting the existing Descriptor into an enum with a ScriptPubkey and Address
14:55 < andytoshi> and calling this `Scannable` or something
14:56 < andytoshi> though it sucks that then the word `Descriptor` is used in rust-miniscript in a different way than it's used in the descriptor doc..
14:59 -!- afilini [~user@gateway/tor-sasl/afilini] has quit [Remote host closed the connection]     
15:00 -!- afilini [~user@gateway/tor-sasl/afilini] has joined ##miniscript     
15:00 -!- afilini [~user@gateway/tor-sasl/afilini] has quit [Remote host closed the connection]     
15:01 -!- afilini [~user@gateway/tor-sasl/afilini] has joined ##miniscript     
18:24 -!- Davterra [~Davterra@2601:603:4f00:63d0::1] has joined ##miniscript     
18:27 -!- Tralfaz [~Davterra@37.120.215.173] has joined ##miniscript     
18:28 -!- Tralfaz [~Davterra@37.120.215.173] has left ##miniscript []     
18:29 -!- Davterra [~Davterra@2601:603:4f00:63d0::1] has quit [Ping timeout: 244 seconds]     
18:39 -!- shesek [~shesek@unaffiliated/shesek] has quit [Remote host closed the connection]     
22:52 -!- shesek [~shesek@164.90.217.137] has joined ##miniscript     
22:52 -!- shesek [~shesek@164.90.217.137] has quit [Changing host]     
22:52 -!- shesek [~shesek@unaffiliated/shesek] has joined ##miniscript     
23:17 -!- jonatack [~jon@2a01:e0a:53c:a200:bb54:3be5:c3d0:9ce5] has quit [Ping timeout: 240 seconds]     
23:23 -!- afilini [~user@gateway/tor-sasl/afilini] has quit [Ping timeout: 240 seconds]     
23:25 -!- afilini [~user@gateway/tor-sasl/afilini] has joined ##miniscript