--- Day changed Fri Sep 28 2018
00:43 -!- elichai2 [uid212594@gateway/web/irccloud.com/x-xbcuhzcyamfocbun] has quit [Quit: Connection closed for inactivity]
09:22 -!- TamasBlummer [~Thunderbi@p200300E38F0E6094A91AEC94B319A8C2.dip0.t-ipconnect.de] has joined #rust-bitcoin
09:23 -!- TamasBlummer [~Thunderbi@p200300E38F0E6094A91AEC94B319A8C2.dip0.t-ipconnect.de] has quit [Client Quit]
10:40 -!- TamasBlummer [~Thunderbi@p200300E38F0E6094A91AEC94B319A8C2.dip0.t-ipconnect.de] has joined #rust-bitcoin
14:45 -!- grubles [~grubles@unaffiliated/grubles] has joined #rust-bitcoin
14:55 < stevenroose> It's just for HW stuff
14:56 < stevenroose> dongcarl: you know what would be awesome? :D
14:56 < stevenroose> A psbt::Input.value() getter than calculates the value either from the TxOut (trivial) or from the witness data (non-trivial) :)
14:57 < stevenroose> srry from the transaction* not the witness data
15:02 < dongcarl> stevenroose: do the PubKey wrapper, and I'll do this ;-)
15:03 < dongcarl> jk, I'll do the PubKey wrapper... oy, feel free to submit patches to psbt
15:05 < stevenroose> yeah I will, just implemented it here locally
15:06 < stevenroose> I will once it's merged I guess
15:06 < dongcarl> cool, send me the diff somewhere and I'll credit you in the commit if andytoshi thinks this is useful
15:07 < stevenroose> and it might even be too trivial to have in the codebase though
15:07 < andytoshi> it sounds useful
15:09 < dongcarl> yup, once my serialization/deserialization stuff gets merged we can start thinking about convenience methods for PSBT structs
15:09 < dongcarl> andytoshi: what do the `#[inline]` stuff in all files do?
15:10 < dongcarl> it is like what C inline does?
15:10 < andytoshi> yeah, that's a historical wart
15:10 < andytoshi> i'm pretty sure the compiler just ignores them
15:11 < andytoshi> #[inline(always)] and #[inline(never)] do stuff, but #[inline] is just a hint
15:11 < dongcarl> I see... I think we should remove them then, if the compiler has gotten smart enough
15:12 < andytoshi> sure
15:12 < dongcarl> also: going to `mv util/privkey.rs util/key.rs` and implement `PublicKey` there
15:13 < andytoshi> sure
15:15 < dongcarl> stevenroose: `Privkey` will be renamed to `PrivKey` as we already have precedence from `ExtendedPrivKey`
15:16 < dongcarl> At least it'll be capitalized right lol
15:16 < andytoshi> if you're gonna rename it we should just do PrivateKey
15:17 < dongcarl> Should we rename `ExtendedPrivKey` to `ExtendedPrivateKey` as well?
15:17 < andytoshi> i think extendedprivkey is a bip32 term
15:17 < dongcarl> Okay cool
15:18 < andytoshi> oh, no, it's not
15:18 < andytoshi> yeah .. i guess you could rename that .. it's a bit of API churn
15:18 < andytoshi> and it's already a very long type
15:18 < dongcarl> Eh, most editors have autocomplete...
15:20 < dongcarl> andytoshi: Is there a document with how to distinguish between secp key types?
15:20 < dongcarl> byte prefixes and stuff
15:21 < dongcarl> `Privkey` seems to store this info in a field...
15:23 < andytoshi> dongcarl: yes, it's http://www.secg.org/SEC1-Ver-1.0.pdf
15:23 < andytoshi> i don't ever use autocomplete
15:26 < andytoshi> specifically section 2.3.3 of that document
15:31 < dongcarl> Ah
15:31 < dongcarl> That makes sense
15:31 < dongcarl> Wondering if the `PrivateKey` struct should also be made into an enum of Compressed and Uncompressed
15:36 < stevenroose> haha, I just wanted to mention the Privkey thing since I noticed that the whole ConsensusEncodable API is changed :o
15:38 < stevenroose> ;w
15:49 < dongcarl> andytoshi: the Privkey implementation does WIF base58 conversion from string I'm guessing?
16:00 < stevenroose> Regular Rust question: if a type A has Into<B> implemented, shouldn't I be able to do Vec<a>.into_iter().collect() to get a Vec<B>? I've been struggling with this for quite a while.
16:01 < stevenroose> It also seems that having From<A> implemented for B is not equivalent as having Into<B> implemented for A
16:01 < stevenroose> While it should be because .into() is mostly based on From<> impls afaik
16:02 < dongcarl> It is equivalent: https://doc.rust-lang.org/src/core/convert.rs.html#393-398
16:02 < dongcarl> I don't think collect does conversions
16:03 < stevenroose> Yeah I also read it, but the collect() doesn't seem to do the trick..
16:03 < stevenroose> Oh, you're saying it can just do Iterable<A> to Vec<A>?
16:04 < dongcarl> Right
16:04 < dongcarl> You want map
16:04 < dongcarl> `map()`
16:04 < stevenroose> That makes sense, then I need a method on Iterable that automatically into()'s all the stuff. Does that exist? Like an auto map() based on .into()
16:04 < stevenroose> It's silly to do map(|s| s.into()), no?
16:04 < dongcarl> Lol, maybe, perhaps there a method
16:05 < dongcarl> Oh wait
16:05 < dongcarl> Gimme a sec
16:06 < stevenroose> Maybe just into() can do it
16:06 < dongcarl> That's what I'm thinking but I'm not seeing it :-/
16:06 < stevenroose> With just .into() I get "type annotation needed :))
16:06 < dongcarl> Use `from` when you can
16:07 < stevenroose> impl From<Iterable<T>> for Iterable<U: From<T>> should do th e trick
16:07 < stevenroose> Ah the plain into() works for one of my cases!
16:07 < dongcarl> Great
16:08 < stevenroose> The other needs type anotation. Ok awesome, so .into() is short for .map(|i| i.into()), which makes sense actually
16:09 < andytoshi> dongcarl: yes, Privkey does WIF base58 encoding/decoding when converting to/from a string
16:09 < andytoshi> it can be an enum of compressed/uncompressed
16:11 < dongcarl> andytoshi: it seems that we only need to store the `Network` information because of WIF right?
16:11 < dongcarl> and I'm guessing we don't need to store `Network` for PublicKey?
16:11 < andytoshi> well, we might as well
16:12 < andytoshi> a public key is still tied to a network, conceptually
16:12 < dongcarl> Is there WIF for PublicKey as well??
16:12 < dongcarl> What encoding format would encode that?
16:12 < andytoshi> no
16:13 < andytoshi> public keys are encoded in hex where they appear
16:13 < andytoshi> which is a couple places in the RPC, and also they get hashed
16:13 < andytoshi> when converting from a ec key to an address
16:14 < andytoshi> dongcarl: and in the pubkey->address conversion there is a network byte
16:15 < andytoshi> so i think we need to keep the network field for PublicKey
16:16 < dongcarl> Hmmm... Okay so when I'm deserializing a PublicKey from PSBT what network do I put for it? I don't think there's a sane default...
16:16 < andytoshi> hmm, good question
16:18 < dongcarl> I think we should separate out `Network` information... As in:
16:18 < dongcarl> The diff between secp::PubKey and PublicKey is recording compressedness
16:18 < dongcarl> The diff between PublicKey and Address is recording Network and P2*
16:18 < andytoshi> i think we should add a global network field to the PSBT if we ever propose an extension .. which will be annoying, serde has a pretty different API to do contextual deserialization, vs context-free deserialization
16:19 < andytoshi> it's pretty weird that private keys and addresses have a network associated to them but public keys don't
16:19 < dongcarl> Well we can do the same as above for PrivateKey... but rethink what we decode WIF to
16:20 < andytoshi> and this business of treating public keys as network-independent makes me uncomfortable, we're gonna wind up with weird Core-like behaviour where 3rd parties can translate addresses into other types of addresses and not be noticed
16:20 < dongcarl> Okay maybe I'm being ignorant but that's bad because....?
16:22 < andytoshi> because it's sloppy and confusing
16:23 < andytoshi> it results in a user model that depends on memorizing random collections of satoshi's sloppy thinking
16:23 < dongcarl> okay cool
16:24 < andytoshi> but i do need to think a bit more about decoding/encoding public keys
16:24 < andytoshi> i guess we can't require the Address object to always know the underlying public key
16:24 < dongcarl> urgh i wish the enum common field RFC gets implemented soon
16:27 < andytoshi> ok, i guess Address should have an Option<PublicKey> in it
16:27 < andytoshi> and we should have mappings from PrivateKey -> Address and from PublicKey -> Address
16:28 < andytoshi> oh, no, we can't do the latter
16:28 < andytoshi> PrivateKey -> Address and PrivateKey -> PublicKey
16:28 < andytoshi> and i guess for completeness we'll add a PublicKey -> Address which requires a network, even though i don't like that
16:31 < dongcarl> Okay so PublicKey won't have `Network` but PrivateKey will?
16:32 < andytoshi> correct
16:33 < dongcarl> Why does `Address` have an `Option<PublicKey>`?
16:33 < andytoshi> so that you can get the EC key out of it
16:33 < andytoshi> eg if you were implementing the `validateaddress` rpc
16:35 < andytoshi> and so that the PrivateKey -> Address map won't lose information
16:36 < dongcarl> When would we need None?
16:36 < dongcarl> Also, `Address` currently has a `Payload`
16:37 < andytoshi> when a 3rd party gives an address
16:37 < andytoshi> yes, payload only contains a hash
16:38 < dongcarl> I think I see what you're saying
16:38 < dongcarl> But how about we just make `Payload::*Hash(PublicKey)`?
16:38 < dongcarl> and when serialized, it will hash?
16:39 < andytoshi> how would you deserialize that?
16:39  * dongcarl is being dumb
16:40 < dongcarl> Hmmm... why is it bad that we lose information when we convert to an Address?
16:40 < dongcarl> Which is what we have right now I think
16:41 < andytoshi> yes, we have that right now
16:41 < andytoshi> and because otherwise people who need an EC key will be forced to do PrivateKey -> PublicKey which has to lose information
16:42 < andytoshi> so they will have no way to get public data from a private key without losing stuff
16:42 < andytoshi> which is an annoying API
16:42 < andytoshi> i'd prefer an API where users basically never need to have any PublicKeys laying around except as part of addresses
16:47 < dongcarl> Perhaps I'm still not understanding correctly, but are you saying that PrivateKey -> PublicKey will lose the `Network` information? They'll still have the original `PrivateKey` with that info no? Since `public_key` on a `PrivateKey` takes in a `&self`?
16:48 < andytoshi> it's a very footgunny API to require users be handling secret key data just to keep track of what network they're on
16:51 < dongcarl> Okay, so if I'm understanding you correctly:
16:51 < dongcarl> 1. Address has its `Option<PublicKey>` field as `Some` only when it's the result of converting from a `PrivateKey`
16:51 < dongcarl> 2. Address has its `Option<PublicKey>` field as `Some` only when it's the result of deserializing from an address where we cannot determine the Public Key
16:51 < dongcarl> 2. is `None`
16:51 < andytoshi> yes
16:51 < andytoshi> also it will be `Some` when converting from a `PublicKey`
16:52 < dongcarl> Right right
16:52 < andytoshi> maybe we already have code that creates a pay-to-pubkey address :/
16:52 < andytoshi> p2pk and p2pkh are ambiguous because this whole thing is satoshi bullshit
16:53 < dongcarl> how so?
16:53 < dongcarl> (are they ambiguous)
16:53 < andytoshi> if you serialize a p2pk address it will hash160 the EC key and serialize it as though it were a p2pkh address
16:53 < andytoshi> there is no way to deserialize
16:53 < dongcarl> wtf
16:54 < dongcarl> that's pretty confusing...
16:54 < dongcarl> Eh, I feel a little weird about not having `Network` for PublicKey but having it for `PrivateKey`...
16:54 < andytoshi> everything about addresses is bullshit basically
16:55 < andytoshi> yeah, me too
16:55 < dongcarl> Makes me uncomfortable..
16:55 < andytoshi> the issue is that public keys appear serialized in hex form in PSBT with no associated network
16:55 < dongcarl> Not sure there's a better solution tho
16:55 < andytoshi> yeah
16:56 < andytoshi> basically i want to make PublicKey a second-class citizen
16:56 < andytoshi> which means making Address a bit more powerful
16:57 < andytoshi> well, OTOH PublicKey will need to play a role in script descriptors
16:58 < dongcarl> Yeah true...
16:58 < andytoshi> i would like script descriptors to have a network associated with them .. though that's incompatible with the current implementation in Core
16:58 < dongcarl> Wait, how about we give `PublicKey` an `Option<Network>`?
16:58 < andytoshi> hmmm
16:59 < andytoshi> then the conversion from PublicKey to Address would sometimes fail
17:02 < dongcarl> :-/
17:03 < dongcarl> Life sucks when specs don't allow for clean abstractions
17:03 < andytoshi> i don't think Option<Network> can be made to be ergonomic
17:03 < dongcarl> Yeah you're right...
17:05 < dongcarl> Does this look right: https://www.irccloud.com/pastebin/oBsIh1Xe/
17:07 < andytoshi> i really think network should be pulled out of the enum, the way it is currently
17:08 < dongcarl> The way it is currently has `Network` as a field of `PrivateKey` no?
17:08 < andytoshi> yeah
17:08 < andytoshi> and a flag for compressedness
17:08 < andytoshi> i think that's fine
17:09 < andytoshi> i think we should do the same for PublicKey, for consistency
17:09 < dongcarl> Okay so don't change the `PrivateKey` from a struct to an enum is what you're saying?
17:09 < andytoshi> correct
17:09 < dongcarl> cool
17:26 < dongcarl> andytoshi: a little weird in the case of `create_address` for p2ch
17:26 < dongcarl> we have context as to which `PublicKey`s is relevant for the address
17:26 < dongcarl> But it's multiple
17:27 < andytoshi> i don't understand
17:28 < andytoshi> i'd rather not delete the p2ch code until we have script descriptors .. but if we have to, we can
17:29 < dongcarl> Well... I was envisioning `Address.public_key` as "The public key corresponding to this address, if known“
17:30 < dongcarl> in `create_address` we have multiple `PublicKey`s corresponding to the address
17:30 < andytoshi> there only types of multisig we support are p2sh and p2wsh
17:30 < andytoshi> those do not have associated PublicKeys
17:30 < andytoshi> i mean, they do, but semantically that gets very complicated and you basically need script descriptors to deal with them
17:31 < dongcarl> Yeah, sound good
17:31 < andytoshi> this is another thing were Core has insane behaviour that we don't want to emulate
17:32 < andytoshi> for some specific script templates it will find public keys, convert them to p2pkh/p2pk addresses, and display them in the RPC
17:32 < dongcarl> Haha you remind me of Roasbeef saying "Core is deprecated software"
17:32 < dongcarl> > for some specific script templates it will find public keys, convert them to p2pkh/p2pk addresses, and display them in the RPC
17:32 < dongcarl> what the actual fuck?
17:33 < andytoshi> well, i know some pretty prominent bitcoin developers who use Core wallet as an actual wallet, so i guess some people like it
17:33 < andytoshi> but for me there is too much bizarre behaviour to memorize
17:34  * dongcarl gets more pumped for when he has time for `rust-bitcoin-node`
18:15 < andytoshi> unrelated to anything we've been discussing, do you think we can get deny(unsafe) on rust-bitcoin?
18:16  * dongcarl looking
18:16 < andytoshi> looks like our use of unsafe is in Sha256dHash doing byte-munging ... that could be done by `byteorder` i'm pretty sure, or just by doing things manually
18:17 < dongcarl> will Sha256dHash be moved to bitcoin_hashes?
18:17 < andytoshi> and also there is a cast from opcodes::All to u8 that we do by transmute() .. but i'm pretty sure we can do that in safe code now, since 1.10 or something
18:17 < andytoshi> dongcarl: yeah
18:17 < andytoshi> so maybe we don't care about that
18:17 < dongcarl> yeah I'm looking at the other one
18:17 < andytoshi> oh, actually, the unsafety is in the conversion from Sha256dHash to Uint256, which is something we'll have to implement one way or the other
18:18 < andytoshi> but we can eliminate that unsafety in the transition to bitcoin_hashes
18:18 < andytoshi> rust-secp can't be deny(unsafe) of course, because of all the FFI .. but i think we should try to target that for everything else
18:20 < dongcarl> I think the opcodes one is easily fixable
18:21 < andytoshi> i definitely had to use unsafe code back when i wrote that
18:21 < dongcarl> thank god for progress
18:21 < andytoshi> there used to be a magic trait called CLike that described enums like this, but it wasn't stable
18:21 < andytoshi> for enums that were secretly just u8s
18:22 < andytoshi> yeah...it's been a long time since i used unsafe in new code (except ffi)
18:23 < dongcarl> You should open a ticket for this, juggling a few things and I might forget
18:23 < andytoshi> will do
18:24 < andytoshi> https://github.com/rust-bitcoin/rust-bitcoin/issues/170
18:25 < dongcarl> Btw, the other thing I'm working on is a deterministic toolchain with cfields... if that pans out we might be able to deterministically bootstrap `rustc` and friends from `g++`
18:25 < dongcarl> And not rely on the debian folks
18:26 < andytoshi> o.O
18:26 < andytoshi> that would be a hell of a job. i think the rustc folks wolud be very interested
18:27 < dongcarl> Yeah... it has been... we've built a toolchain and I'm writing tests to make sure it's deterministic now... fingers crossed
18:28 < andytoshi> nice
18:56 -!- TamasBlummer [~Thunderbi@p200300E38F0E6094A91AEC94B319A8C2.dip0.t-ipconnect.de] has quit [Quit: TamasBlummer]
23:56 -!- gribble [~gribble@unaffiliated/nanotube/bot/gribble] has quit [Remote host closed the connection]