--- Day changed Sun Nov 11 2018
00:49 < stevenroose> sgeisler: would be nice to have CheckedXxx on Amount.. any idea if those traits are going to be landing in std::ops?
00:50 < stevenroose> andytoshi: rust 1.14 doesn't seem to support PhantomData: https://travis-ci.org/rust-bitcoin/rust-bitcoin/jobs/453433516#L850
00:55 < sgeisler> stevenroose: I'm pretty sure I'm using phantom data in lightnng-invoice which compiles on 1.14.0. Did you import it?
00:55 < sgeisler> s/import/use/
00:57 < stevenroose> I did
01:00 <+andytoshi> PhantomData has been around since before 1.0
01:01 < sgeisler> you are using it in tests and the semantics `use super::*;` have changed since 1.14
01:01 < sgeisler> or the semantics of use and modules, idk
01:01 <+andytoshi> i think the semantics of super::*. it's pretty annoying
01:01 < sgeisler> just import it in your tests module agian
01:03 < stevenroose> ooh, hmm k, weird way to complain about that, but that makes sense
01:03 < stevenroose> like FromStr
01:03 <+andytoshi> yeah, that's a bizarre error
04:06 < dongcarl> Have people taken a look at: https://github.com/Geal/nom
04:06 < dongcarl> Looks like a good crate with minimal dependencies that's very good at parsing
04:18 < sgeisler> Yes, that's what I used for lightning-invoice before matt told me about the minial dependency policy.
04:20 < sgeisler> Back then it was quite a pita if things failed, I don't know if the error messages got any better
04:22 < sgeisler> if you have a nice binary protocol basically consisting of (len, data) tuples it feels too complex for that simple task
04:23 < sgeisler> but maybe I was just using it wrong
13:30 < stevenroose> I got a failure from a fuzz test in Travis while not touching anything that's fuzzed: https://travis-ci.org/rust-bitcoin/rust-bitcoin/jobs/453538241
14:15 < stevenroose> BlueMatt, ariard, I tried upgrading rust-lightning to rust-bitcoin v0.15, but somehow after everything compiles, all tests for `chain.does_match_tx(tx)` in the channelmanager tests fail. I see that you have a different implementation there for tests and outside of tests, but I can't really see how they could behave differently
14:21 < stevenroose> full log: https://gist.github.com/stevenroose/8a1ad0a1b4eaf29f35d41c8aa0d7bd96
14:51 <+andytoshi> stevenroose: heh, that's a stack overflow
14:54 <+andytoshi> in strason
14:57 <+andytoshi> it's a very deeply nested array and strason uses a naive recursive descent parser with no limits .. which is a bug certainly, but the fix is probably just to move away from strason
14:57 <+andytoshi> for now i'm going to just restart the rust-bitcoin job and see if the fuzzer passes
18:30 < stevenroose> andytoshi: what prevents us from ditching strason alltogether right now?
18:31 < stevenroose> hmm, to get rid of it in 0.16, we probably should have somehow deprecated it in 0.15, but I don't know the practice of deprecating foreign trait implementations..
18:31 < stevenroose> how does that work?
18:48 <+andytoshi> stevenroose: the rustc 1.14.0 requirement prevents us going to serde_json
18:48 <+andytoshi> but i think if we pull the Decimal types out of rust-bitcoin we can just drop json altogether (thank god)
18:49 <+andytoshi> at this point i should probably either fix the parser or label the project deprecated/out of maintenance..
18:49 <+andytoshi> strason that i
18:49 <+andytoshi> is
18:51 < stevenroose> "drop json"
18:51 < stevenroose> what do you mean?
18:51 < stevenroose> ah you mean keep serde, but not serde_json stuff?
18:52 < stevenroose> The Amount thing that I PR'd uses serde_json::Number
18:53 <+andytoshi> yes, that's what i meant
18:53 <+andytoshi> serde_json::Number can't be used with rust 1.14
18:54 <+andytoshi> ah, yeah, you added a feature but didn't add a CI target that enables it
18:56 < stevenroose> didn't add a feature
18:57 < stevenroose> I just used the existing serde feature I think.. Hmm, might be wrong
18:58 < stevenroose> oh you're right, I used serde_json feature instead of serde
18:58 < stevenroose> well, fwiw, I took that impl from rust-bitcoin-amount, but I never used it myself
18:59 < stevenroose> when is it useful to have both a From<serde_json::Number> and serde::Deserialize?
18:59 <+andytoshi> i don't know, those are very different things
19:04 <+andytoshi> the reason to go through serde_json::Number is that it is the only type that can be deserialized from a json number without loss
19:04 <+andytoshi> well, this isn't quite true, you can copy the struct-name hack out of the serde_json source..
19:04 <+andytoshi> and define your own type that works similarly
19:54 < TamasBlummer> Published first cut of Hammersbald, a fast embedded blockchain database written in safe rust. See design here: https://medium.com/@tamas.blummer/hammersbald-7c0bda14da1e
19:55 < TamasBlummer> https://github.com/rust-bitcoin/hammersbald
20:38 < stevenroose> TamasBlummer: love the name
20:43 < dongcarl> TamasBlummer: So potentially this can be paired with a p2p crate to create a full rust bitcoin node?
20:47 < stevenroose> that seems like the last missing piece, doesn't it? :)
20:48 < stevenroose> I'd be very interested in a neutrino-like client in Rust, could be super lightweight. But also needs the p2p part.
20:49 < stevenroose> Would be interested to work on it.  I think we should be able to learn quite a bit from grin, no? Or Parity fwiw.
20:53 < stevenroose> BlueMatt, ariard: rust-lightning doesn't have p2p yet ,right?
20:58 < ariard> what do you mean by p2p, BOLT 10 on dns bootstrap ?
20:58 < ariard> or node_announcement stuff in bolt 7
21:02 < TamasBlummer> @dongcarl just used rust-bitcoin-spv to download the entire chain into hammersbald.
21:04 < TamasBlummer> This is not the target use (for me) but is doable. I want to use hammersbald to store headers and BIP158 filters for spv name
21:04 < TamasBlummer> *node
21:05 < dongcarl> TamasBlummer: so you’re saying its target use is not for full nodes?
21:05 < TamasBlummer> @stevenroose p2p is in rust-bitcoin-spv
21:06 < TamasBlummer> it can be used for that
21:07 < TamasBlummer> hammersbald target use is embedded blockchain store, full or partial or whatever. it does not define any property of the data stored othe rthan that it might be a graph and huge
21:15 < stevenroose> ariard: I was just thinking the most basic p2p thing, like having a notion of your peers and sending messages over tcp (like bolt 2 and 3)
21:15 < stevenroose> TamasBlummer: oh, it does? didn't know that :o
21:16 < stevenroose> bitcoin-spv also implements the BIP158 filters?
21:31 < ariard> stevenroose: well I think the design goal of rust-lightning is letting this kind of component up to the user, so not a rust-lightning repo thing per se but maybe nice to have one a default, minimalistic one, at least for testing
21:34 < dongcarl> andytoshi: sgeisler: w/re https://github.com/rust-bitcoin/rust-secp256k1/issues/76, shall we replace the existing `serialize` and `serialize_uncompressed` with ones that take in an `io::Write`? Or name them differently?
21:35 < sgeisler> I'd like to keep the serialize to Vec versions and make them use the serialize to Write internally (Vecs ipl Write)
21:37 < TamasBlummer> @stevenroose: BIP158: https://github.com/rust-bitcoin/rust-bitcoin-spv/blob/master/src/blockfilter.rs
21:37 < TamasBlummer> p2p: https://github.com/rust-bitcoin/rust-bitcoin-spv/blob/master/src/p2p.rs
21:38 <+andytoshi> dongcarl: i agree with sgeisler
21:38 < dongcarl> sgeisler: Makes sense...
21:38 < TamasBlummer> rust-bitcoin spv IS meant to be neutrino like support for rust-lightning
21:39 <+andytoshi> i've been trying to figure out why sgeisler can't request reviewers on rust-bitcoin ... he's a member of the org .. i don't have to grant write/merge access to let people do basic issue management do i?
21:46 < sgeisler> andytoshi: after merging a bunch of stuff https://github.com/rust-bitcoin/bitcoin_hashes/pull/1 needs a rebase. After that dongcarl and me should review it again since I don't think we had a thorough look at it.
21:47 <+andytoshi> sgeisler: yep, rebasing now .. i also have to add a serde impl for the new Hmac type
21:47 <+andytoshi> then i'll try to do a PR that does the "rename every type to Hash so users have to write sha256d::Hash" and see what people think
21:48 <+andytoshi> and then we'll be 0.1 ready
21:48 <+andytoshi> also i'd like to merge the truncated-hex thing but that can wait for 0.1.1
21:57 < ariard> BlueMatt : what's your take on StaticOutputDescriptor, do I let or cut them ?
21:57 <+andytoshi> sgeisler: i think i'll add a fuzz test to the bitcoin_hashes serde thing that uses serde_json
21:58 < ariard> maybe they can be useful to a cold wallet which doesn't have chain access
21:58 < sgeisler> andytoshi: ok, that would have been my first questiong after rebasing: do we actually test the serde stuff since it's feature gated?
21:59 < sgeisler> but fuzzing is always good
21:59 <+andytoshi> yeah, it's hard, we've seen with `strason` that actually people run into weird problems because there are a trillion ways to use serde
21:59 <+andytoshi> i think i'll add a json test and a cbor test that simply round-trip some hashes and see how that goes
22:00 <+andytoshi> although hm, maybe that will prevent CI from running on 1.14
22:00 < sgeisler> then just run this test on stable or whatever, you can use stages for that (or how travis calls it)
22:01 <+andytoshi> heh, it looks like we're doing something sillier now to ensure the fuzztests run on stable .. `if [ "$(rustup show | grep default | grep stable)" != "" ];`
22:01 <+andytoshi> i'll just change that to nightly
22:01 <+andytoshi> err, no, actually it should be fine as-is
22:02 < sgeisler> :D but you could use travis for that: https://github.com/rust-bitcoin/rust-lightning-invoice/blob/master/.travis.yml
22:02 < sgeisler> this only runs coverage reports for stable for example
22:02 <+andytoshi> oh, nice, ok, i'll steal all this stuff in a separate PR then
22:03 < ariard> stevenroose: do you have a local branch of rust-lightning upgraded to 0.15, I can try to have a look on does_match_tx failure ?
22:03 < ariard> because I don't have any idean what can fail looking at the changelog
22:03 < sgeisler> I wouldn't, otherwise I would already have proposed that. At the moment it's still a bit hacky (especially the uploading of the coverage results).
22:04 <+andytoshi> it seems like a lot of people have coverage data on their github now
22:04 <+andytoshi> isn't there a fairly standard way to do it?
22:05 < sgeisler> typically third party services I guess
22:05 < sgeisler> But I just upload the report to the github pages branch
22:06 < sgeisler> https://rust-bitcoin.github.io/rust-lightning-invoice/target/kcov/merged/kcov-merged/index.html
22:08 <+andytoshi> heh, ok, i'll just go back to what i was originally going to do
22:08 <+andytoshi> this seems like too much thinking
22:23 <+andytoshi> sgeisler: pushed serde tests
22:39 < stevenroose> ariard: https://github.com/rust-bitcoin/rust-lightning/pull/249
22:41 < stevenroose> ariard: diff is fairly minimal tbh
22:45 < stevenroose> TamasBlummer: how hard would it be to take the p2p part out of bitcoin-spv into -say- bitcoin-p2p? I saw f.e. that one of the p2p structs keeps a database reference.