--- Log opened Tue Dec 22 00:00:52 2020
01:12 -!- jonatack [~jon@213.152.162.154] has quit [Quit: jonatack]
01:16 -!- jonatack [~jon@88.124.242.136] has joined #rust-bitcoin
01:21 -!- jonatack [~jon@88.124.242.136] has quit [Ping timeout: 256 seconds]
01:21 -!- jonatack [~jon@213.152.161.30] has joined #rust-bitcoin
02:21 < elichai2> andytoshi: doesn't fuzztarget also breaks --all-features?
02:21 < elichai2> and I think BlueMatt uses external-symbols
02:30 -!- valwal_ [sid334773@gateway/web/irccloud.com/x-kcpafitmewgluevi] has joined #rust-bitcoin
02:54 -!- jeremyrubin [~jr@2601:645:c200:14:d16a:4949:3622:4cc0] has quit [Ping timeout: 268 seconds]
02:56 < real_or_random> andytoshi: ah I see you merged #233. I think elichai was going to update #235. :D Well, they all do the job, let's see if people like it
02:56 < real_or_random> and we could revisit in the future
02:56 < real_or_random> if necessary
03:04 -!- warren [~warren@fedora/wombat/warren] has quit [Ping timeout: 240 seconds]
03:07 -!- warren [~warren@fedora/wombat/warren] has joined #rust-bitcoin
03:36 < stevenroose> andytoshi: I'm ok dropping it. I think someone (perhaps BlueMatt?) also argued that one could use that to avoid double compilation (not just to avoid collision)
04:19 < stevenroose> andytoshi: dr-orlovsky: sgeisler: updated 508, tested all commits against MSRV (finally made a script to do that) with all features :)
04:20 -!- tibo [~tibo@2400:4050:2a83:7000:bc4e:d063:9de0:75af] has quit [Remote host closed the connection]
04:21 -!- tibo [~tibo@2400:4050:2a83:7000:bc4e:d063:9de0:75af] has joined #rust-bitcoin
04:25 -!- tibo [~tibo@2400:4050:2a83:7000:bc4e:d063:9de0:75af] has quit [Ping timeout: 264 seconds]
04:57 < sgeisler> stevenroose: I messed around with the map serialization in #508 a bit, I understand now why you built it that way. Only one nit: the values aren't serialized as hex, but as a byte array.
04:58 < sgeisler> Apart from that I think it's good, I'm happy we are slowly getting rid of some of the macros.
05:06 < stevenroose> sgeisler: oooooh yeah, ugh, now sure how we fix that hex thing. ugh, yeah I don't think that's gonna be possible even
05:06 < stevenroose> s/now/not/
05:07 < sgeisler> Wrapper types? You'd need two imo: one borrowing for serializing with the right serde derive macros and one owning to destructure when parsing
06:20 -!- shesek [~shesek@164.90.217.137] has joined #rust-bitcoin
06:20 -!- shesek [~shesek@164.90.217.137] has quit [Changing host]
06:20 -!- shesek [~shesek@unaffiliated/shesek] has joined #rust-bitcoin
06:58 < andytoshi> elichai2: where does Blackwolfsa4 use external-symbols?
06:58 < elichai2> in his bitcoin core integration I think, because he doesn't use cargo
06:58 < andytoshi> if he doesn't use cargo then why would he need a cargo feature
06:59 < andytoshi> real_or_random: i mean, we could go back to #235, i think it'd be straightforward now, just replace AlignType with [u8] in the APIs, remove the division from _size, and stick unsafe on some things
06:59 < andytoshi> sgeisler: the problem with wrapper types is that you need them in the map itself which makes the map less usable
07:00 < andytoshi> i think the right solution here is to just duplicate the btreemap serde code
07:00 < andytoshi> stevenroose: elichai2: if you want to avoid double-copmiling libsecp then you can't use cargo
07:00 < andytoshi> end of story
07:01 < andytoshi> having a have-baked untestable cargo feature isn't really maintainable
07:01 < elichai2> agreed, just wanted to ping him on this
07:01 < andytoshi> yep good call
07:04 < andytoshi> we can expose a direct --cfg flag to disable the renaming in #[link]
07:04 < andytoshi> but a cargo feature will just cause breakage for cargo users
07:06 < sgeisler> andytoshi: I don't think so, when serializing just take the key-value-tuples and put them into a wrapper for serializing, when deserializing deserialize the wrapper, destructure it and put the parts into the map. But you are right that this doesn't work with any `U: Ser + De` but needs more specific bounds that tell us it can be treated as bytes.
07:06 < andytoshi> ok, going to PR this, just chasing down a really weird error... i have a "unnecessary unsafe" warning on src/schnorrsig.rs:256
07:06 < andytoshi> when i'm certain the unsafe is necessary. may be a rustc bug that somehow affects every version from 1.29 onward
07:06 < andytoshi> sgeisler: right ... and many of our hashmaps have types that don't support this
07:06 < andytoshi> btreemaps*
07:07 < andytoshi> lol actually maybe we could use ConsensusEncodable
07:07 < andytoshi> no, that would length-prefix things
07:07 < sgeisler> I see, that's annoying, but do most of these need this special treatment anyway? Afaik the module only exits to allow non-string, object keys.
07:08 < andytoshi> ah good question, let me re-grep the set of BTreeMaps
07:08 < sgeisler> So it shouldn't be used for _all_ our maps
07:08 < andytoshi> src/util/psbt/map/global.rs:    pub xpub: BTreeMap<ExtendedPubKey, KeySource>,
07:08 < andytoshi> yep, sorry
07:09 < andytoshi> would've been nice if all our non-string-key maps had Vec<u8> values..
07:09 < andytoshi> having said this, KeySource and Vec<u8> are actually the only value types we use anywhere in the library
07:09 < andytoshi> so surely we could do _something_..
07:10 < sgeisler> What do you think about the approach I posted in the PR?
07:10 < sgeisler> Could we get away with having a `Preferred(De)Serialize` trait which is identical to the `(De)Serialize` traits and has a default impl just passing through to the serde version? But for certain types we can implement default (de)serialization methods.
07:11 < andytoshi> does specialization actually work?
07:11 < sgeisler> it wouldn't be specialization afaik
07:11 < andytoshi> oh, an actual default impl
07:11 < andytoshi> innnteresting
07:11 < sgeisler> yes, exactly
07:11 < andytoshi> so, the short answer is yes, i like that approach
07:12 < andytoshi> but the long answer is that maaybe we can use bitcoin::hashes::hex::{From,To}Hex
07:12 < andytoshi> as our "extra bound"
07:12 < andytoshi> and that'd be even simpler
07:12 < sgeisler> would that work with KeySource?
07:12 < sgeisler> isn't that a fingerprint and a path?
07:12 < andytoshi> no. we'd need to impl those on KeySource, but it'd let us factor out some keysource parsing logic that we have in the psbt module
07:13 < andytoshi> oh, i guess we don't actually want to hex-serialize is
07:13 < andytoshi> it
07:13 < andytoshi> it has its own string serialization
07:13 < sgeisler> yeah, that's what I meant
07:13 < andytoshi> lol sorry, i just assume that hex serialization is always the rigth serialization
07:13 < andytoshi> because everything else has randomly-varying length and is hard to read
07:13 < sgeisler> I'll try if my idea works with a minimal example, will let you know
07:14 < andytoshi> cool, thanks
07:14 < andytoshi> i think it's the right approach
07:26 < andytoshi> elichai2: i'm gonna redo the no-external-symbols PR to just make it a --cfg flag rather than a Cargo feature
07:26 < andytoshi> and ditto with fuzztarget
07:27 < elichai2> +1
07:27 < andytoshi> then you can enable fuzztarget (if this works) using `RUSTFLAGS='--cfg rust-secp-fuzztarget'` which is waay harder to do by accident than enabling a cargo feature
07:28 < sgeisler> andytoshi: one thing I forgot but probably isn't too bad: you still need to impl PreferredSerialize for every type you want to use in the map https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=b885eabb28155aa553a944a2b38083ea
07:29 < andytoshi> sgeisler: ah right
07:30 < andytoshi> but ok that's not bad since we only have two types :P
07:30 < andytoshi> so far
07:30 < sgeisler> Better than copying the whole module
07:30 < andytoshi> this is also nice cuz it'll work the same way with HashMap
07:30 < andytoshi> right exactly
07:32 < sgeisler> How time critical is the PR? I don't think I'll have enough uninterrupted time today to properly implement this.
07:33 < sgeisler> stevenroose: if you want to include it in your PR that would be great, otherwise I might find the time tomorrow.
07:33 < andytoshi> sgeisler: i was hoping to get a release out by christmas
07:33 < andytoshi> but we're blocked on rust-secp rather than this
07:33 < andytoshi> so no rush
07:34 < sgeisler> nice present for all rustacean bitcoiners :P
07:45 < andytoshi> hehehe
07:49 < andytoshi> ok elichai2 https://github.com/rust-bitcoin/rust-secp256k1/pull/263
07:51 < elichai2> andytoshi: reviewed, one comment
07:58 -!- belcher_ [~belcher@unaffiliated/belcher] has joined #rust-bitcoin
08:00 -!- belcher [~belcher@unaffiliated/belcher] has quit [Ping timeout: 260 seconds]
08:02 -!- belcher_ is now known as belcher
08:53 -!- ghost43_ [~daer@gateway/tor-sasl/daer] has joined #rust-bitcoin
08:54 -!- ghost43 [~daer@gateway/tor-sasl/daer] has quit [Ping timeout: 240 seconds]
09:36 < stevenroose> sgeisler: uh, I kinda lost what your idea is here, I'll try read it on the PR. It's not time-critical. Just quite handy as currently our PSBT types can't be JSONized.
09:36 < stevenroose> But I'm not directly waiting for them IIRC. (In hal f.e. I always base64 serialize psbt values)
09:44 -!- jeremyrubin [~jr@2601:645:c200:14:d16a:4949:3622:4cc0] has joined #rust-bitcoin
09:54 -!- shesek [~shesek@unaffiliated/shesek] has quit [Ping timeout: 272 seconds]
12:05 < sgeisler> stevenroose: the basic ideas is to use a proxy trait for (de)serialization of the value type in the BTreeMap. This proxy trait allows to change the (de)serialization behavior for certain types (Vec<u8> as hex) inside our library. See the example at https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=351c102fdb198bc3d8a4c179c8b59e9e
12:07 < sgeisler> So instead of `U: Serialze` you'd require `U: PreferredSerialize` and impl `PreferredSerialize` for all types to be used as values. For most the impl can be empty due to the default, for `Vec<u8>` it would use some hex impl.
15:25 -!- tibo [~tibo@2400:4050:2a83:7000:7549:8143:68ed:2722] has joined #rust-bitcoin
15:29 < andytoshi> elichai2: i'd like to get https://github.com/rust-bitcoin/rust-secp256k1/pull/264 in and then i think we'll be ready for release on rust-secp
15:29 < andytoshi> BlueMatt: ^
16:14 -!- ghost43 [~daer@gateway/tor-sasl/daer] has joined #rust-bitcoin
16:15 < cloudhead> bip158 support in the p2p layer is only landing in 0.21 correct?
16:15 -!- ghost43_ [~daer@gateway/tor-sasl/daer] has quit [Ping timeout: 240 seconds]
16:51 -!- jonatack [~jon@213.152.161.30] has quit [Ping timeout: 264 seconds]
16:53 -!- jonatack [~jon@88.124.242.136] has joined #rust-bitcoin
22:29 -!- fiatjaf1 [~fiatjaf@2804:7f2:2a8a:f273:ea40:f2ff:fe85:d2dc] has joined #rust-bitcoin
22:33 -!- fiatjaf [~fiatjaf@186.213.91.147] has quit [Ping timeout: 260 seconds]
--- Log closed Wed Dec 23 00:00:55 2020