--- Day changed Fri Aug 10 2018 00:07 < dongcarl> andytoshi: Yeah I'm trying to think this out... From my conversations with sipa, descriptors are functions from indices to scriptPubKeys 00:08 < dongcarl> and so I think that PubKeyDescriptors will be functions from indices to secp PubKeys 00:08 < dongcarl> Does that make sense? 00:09 < dongcarl> I think we should include the key origin ID stuff in the descriptor... Just so we can parse the example given: `pkh(d34db33f/44'/0'/0':xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/1/*)` 00:09 < dongcarl> and who knows, someone in the future might want to use this info 00:10 < dongcarl> (seems useful for PSBT as well) 00:10 < dongcarl> depending on what functionality we want to have for PSBT, we might want the descriptor library to be a full dependency 00:11 < dongcarl> As I'd like for the PSBT module to have a function where you throw a bunch of PubKeys, Scripts, etc at it and it fills in what makes sense in the PSBT 00:26 < andytoshi> right, that's a design goal of all this descriptor stuff 00:27 < andytoshi> to parse your example the descriptor parser should see `pkh(XXX)` and then call out to a pubkey deserializer to deal with the XXX 00:27 < andytoshi> which is why my original design had parameterized the Descriptor over public keys ... the idea would be that each wallet would implement a type with all the different pubkeys they understood 00:29 < andytoshi> my motivation for that was simply that liquid would have pubkeys that could be contract-hash tweaked, and i didn't want to force all users to support that ... but actually p2c is a pretty generic construction, maybe we should just support it 00:36 < dongcarl> I think we can make a PublicKeyDescriptor trait, but I'd like to supply implementations for at least the types specified in the gist. Current questions: 1. Do you agree that the trait should specify that a PublicKeyDescriptor is a function from index to secp256k1::Publickey? e.g. `fn index(&self, _: u32) -> secp256k1::PublicKey;` 2. I think we should parse out the Key Origin information if it is 00:36 < dongcarl> there, for PSBT and such, right? 3. Right now, in the gist there are two `impl`s of PublicKeyDescriptor: normal public key and xpub with optional path, both of these impls can be prefixed with an optional KeyOrigin, will this be the case for all things that impl PublicKeyDescriptor or will it be possible for some things that impl PublicKeyDescriptor to not make any sense with a KeyOrigin? 00:37 < dongcarl> *supply implementations for at least 1. Normal Public Key with optional KeyOrigin, 2. Xpub with optional path with optional KeyOrigin 00:37 < dongcarl> On your walk, why did you think it'd be impractical to make into a trait? 00:38 < andytoshi> because how can the descriptor parser know what function to call in order to parse a pubkey? 00:38 < andytoshi> 1. No, not ever a BIP32 path can be described by a u32 00:38 < andytoshi> 2. does PSBT have some standard way to use that information? 00:39 < andytoshi> i don't understand why PSBT needs anything more than the public keys 00:39 < dongcarl> 2. See https://gist.github.com/sipa/e3d23d498c430bb601c5bca83523fa82#key-origin-identification, grep for BIP174 00:40 < andytoshi> ah nice 00:40 < andytoshi> so yes, i do think if we go the trait route that we need standard implementations for basic pubkeys and BIP32 paths 00:41 < andytoshi> because those are described in the descriptor language gist 00:41 < dongcarl> 1. This is a long convo I had with sipa. So for paths that end in /* or /*', the index will be substituted in to determine what the path actually is... e.g. pkh(xpub.../0/*') evaluated with an index of 0 will be pkh(xpub.../0/0') 00:41 < andytoshi> hmmm 00:42 < dongcarl> So for things like pkh(xpub.../0/0'), evaluating it with anything will yield xpub.../0/0' 00:42 < dongcarl> also... the index really should be u31... but that's not a thing lol 00:42 < andytoshi> heh yeah 00:42 < andytoshi> there is a ChildNumber object in rust-bitcoin somewhere 00:43 < andytoshi> so, things like pkh(xpub.../0/0') shouldn't be able to be evaluated 00:43 < dongcarl> Yup I've seen that... the documentation is wrong and is fixed in one of my PSBT commits (which I need to finish rebasing lol) 00:43 < andytoshi> like, we should use the type system to prevent that 00:43 < andytoshi> and to let /* addresses be evaluated at u32s, and to let contracthash pubkeys be evaluated at arbitrary strings 00:43 < andytoshi> cool, thanks for the doc fix 00:44 < andytoshi> might be worth PRing separately since that can get in with little review 00:44 < dongcarl> Yeah true 00:44 * dongcarl reading 00:45 * dongcarl thinking 00:47 < dongcarl> andytoshi: pub trait PublicKey { fn evaluate(&self, input: T) -> secp256k1::PublicKey; } 00:48 < andytoshi> T should be an associated type 00:48 < andytoshi> not an input type 00:49 < andytoshi> pub trait PublicKey { type Index; fn evaluate(&self, input: Self::Index) -> secp256k1::PublicKey } 00:49 < dongcarl> Ahhhh... I like it! What'd be the difference in this case? 00:50 < andytoshi> if you have a specific type, you know how to evaluate it 00:50 < andytoshi> vs your construction where you need to parameterize anything that evaluates anything 00:50 < dongcarl> andytoshi: very true 00:50 < andytoshi> before associated types, Iterator was parameterized over its return value and it was horrible 00:50 < dongcarl> :-/ 00:52 < andytoshi> basically, if i take a bip32 path and try to do evaluate(x as i64), i want the compiler to say "got i64, expected u32" 00:52 < andytoshi> not "PublicKey not implemented for Bip32Path" 00:52 < dongcarl> love it 00:53 < dongcarl> okay cool, so... just to make sure: 1. Use the trait definition we described above 2. Include origin information where it makes sense (currently only normal keys and bip32) correct? 02:15 < andytoshi> yeah .. maybe we could abstract over the origin info somehow 02:15 < andytoshi> but i'll leave that to you 02:16 < andytoshi> like, the trait should have some sort of function for extracting info that psbt cares about 03:16 < dongcarl> andytoshi: so would including origin information make sense for functionary keys? Or just normal keys and BIP32? I guess I don't know enough about functionary keys to comment 12:42 < andytoshi> dongcarl: i would like the trait to be sufficiently expressive that instagibbs and can decide that later :P 12:42 < andytoshi> but probably not 15:00 < dongcarl> andytoshi: fo sho 15:00 < dongcarl> andytoshi: so after quite a bit of experimenting... It seems like we have to use enums... 15:02 < dongcarl> When I use something like... Pk(Box) where PublicKey is a trait we talked about above, the compiler complains that we need to specify the associated type... And I'm guessing we want to mix Pay to Contract with other things... 15:52 < andytoshi> yeah, that's what i feared 15:52 < andytoshi> no worries, i think we'll just make script descriptors and p2c one library. they're actually conceptually pretty related 15:59 < andytoshi> BTW i've totally rewritten the parser locally, just so you know .. need to add some stuff and then rewrite the compiler (which i guess will require your changes) then we'll see where we're at 15:59 < andytoshi> today i think i have other work to do, but i'll go through the weekend on this 16:18 < dongcarl> andytoshi: parser as in parser.rs right? 16:19 < andytoshi> yep 16:19 < dongcarl> Sounds good 19:47 < dongcarl> I was thinking we should have a convention for imports... Like \n\n\n\n\n\n 19:47 < dongcarl> At least that's what I've been doing 20:52 < dongcarl> all: Commits are cleaned up on my PSBT PR: https://github.com/rust-bitcoin/rust-bitcoin/pull/103 20:52 < dongcarl> First 3 commits which are non-PSBT are also available as cherry-picks at #122 #123 #124 20:53 < andytoshi> i think that's roughly my import convention too. sure, we can formalize it somewhere 20:54 < andytoshi> thanks for the cherry-pick PRs 20:55 < dongcarl> andytoshi: for #123 and #124, I'm not sure whether it'd make sense to implement ConsensusDecodable for them or just my PSBTDeserialize, do they make sense in a non-PSBT context? 20:55 < andytoshi> i don't think they make sense in a non-PSBT context 20:56 < andytoshi> and in bitcoin consensus, ecdsa keys can appear uncompressed or in hybrid mode, in the places they do (which i think is only inside script) 20:56 < dongcarl> andytoshi: Okay, I will close #123 and #124, and make those only PSBTDeserialize 20:57 < andytoshi> cool, thanks 21:00 < dongcarl> #122 still good tho 21:01 < andytoshi> i think so. waiting on travis 21:02 < andytoshi> good call on the documentation mistake 21:09 < dongcarl> ;-) 21:30 < dongcarl> btw, `git rebase -ix 'cargo test --verbose --features=bitcoinconsensus' origin/master` is a great thing to run... perhaps should integrate into Travis if we wanna make sure all commits are valid 21:30 < dongcarl> Travis seems to be failing to fetch rust-docs... 21:31 < dongcarl> Urgh... Travis is having networking issues... 21:54 < andytoshi> yeah, not a bad idea to add that to travis 22:41 < dongcarl> If someone can merge https://github.com/rust-bitcoin/rust-bitcoin/pull/122 I'll rebase over it 22:43 < andytoshi> sure 5 minutes 22:46 < andytoshi> ok merged 22:47 < dongcarl> danke schon