--- 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, 07:08 < andytoshi> yep, sorry 07:09 < andytoshi> would've been nice if all our non-string-key maps had Vec values.. 07:09 < andytoshi> having said this, KeySource and Vec 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 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` 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