--- Log opened Sat Dec 19 00:00:51 2020
03:21 -!- Sandra76Hilpert [~Sandra76H@static.57.1.216.95.clients.your-server.de] has joined #rust-bitcoin
04:22 -!- Sandra76Hilpert [~Sandra76H@static.57.1.216.95.clients.your-server.de] has quit [Ping timeout: 268 seconds]
07:31 < stevenroose> andytoshi: let me read your feedback, so I know what's the problem with the args
07:32 < andytoshi> stevenroose: it's just that if you want to provide a boolean, or a number, or whatever, you have to convert that to a RawValue
07:33 < andytoshi> which is secretly converting to a String except that there are no methods to do it directly
07:36 < stevenroose> oh you're just talking about ergonomics? yeah taking any collection of a trait type will force all of them to be the same, unless you box them. buy serde::Serialize can't be boxed :| such a stupid restriction, I wonder if the serde folks or Rust folks are trying to resolve that
07:37 < stevenroose> actually at some point I thought about this. I think I considered writing a `pub fn jsonrpc::arg<T: serde::Serialize>(arg: &T) -> RawValue;` just for that :/
07:37 < stevenroose> it's not ideal
07:38 < stevenroose> Using serde_json::Value is not too bad as long as arguments are not objects or lists, though...
07:38 < stevenroose> but tbf wrapping `arg(true)` or `true.into()` is kinda the same
07:42 < stevenroose> but wait somehow for bitcoinore-rpc I didn't have to make any changes, maybe I'm still using a temporary method, le me check
07:54 < andytoshi> yes serde has solved it with erased_serde crate, but it doesn't work with rust 1.29
07:55 < andytoshi> wdym mean `arg(true)`, where is arg defined?
07:55 < andytoshi> and converting to a bool to a RawValue will be much more expensive than converting it to a Value. the former requires allocating and writing a string
07:57 -!- belcher_ [~belcher@unaffiliated/belcher] has joined #rust-bitcoin
08:00 < stevenroose> Yeah my initial reasoning was that the strings would have to be creates/allocated anyway, whether at that level or more upstream when they are serialized. But it's true that Value's can be directly serialized into the stream or at least into the entire body. K I think I agree in this case arguments are better off as Values. At least for Core or for modal "arguments"
08:00 -!- belcher [~belcher@unaffiliated/belcher] has quit [Ping timeout: 268 seconds]
08:00 < andytoshi> i think in literally all cases
08:00 < andytoshi> except if your arguments are already json-serialized Strings i guess
08:00 < stevenroose> F.e. all the hashes we insert will also just become Value::String and it's also an allocation
08:01 < stevenroose> any argument that's an object is even a lot worse
08:01 < andytoshi> yeah that's true
08:01 < stevenroose> then it's a hashmap with a bunch of strings all over the place
08:01 < andytoshi> mm yeah that's true
08:02 < stevenroose> yeah I had some thought about this, didn't settle 100% for anything, I just really dislike using Value as a proxy
08:02 < stevenroose> But perhaps the overhead it causes for responses is a lot worse than for arguments
08:02 < andytoshi> i mean, Value does not involve any allocations or conversions for bools or numbers
08:02 < stevenroose> but that's quite specific to bitcoin core (even though we also pass hashes almost in every call)
08:03 < stevenroose> sendraw, signraw, all those will also allocate a Value::String
08:03 < andytoshi> the alternate is to replace the whole list with T:Serialize ... but this also has bad ergonomics, in that it's hard to write mock RPC handlers that pull the arguments apart
08:03 < andytoshi> and you can't make Transport object-safe
08:03 < stevenroose> yeah no that doesnt really work sadly
08:03 < stevenroose> perhaps soon(TM) with that erased_serde :p
08:03 < andytoshi> ok, for hashes etc i think Value and RawValue are basically the same
08:04 < stevenroose> yeah so the big pro for RawValue is objects and the pro for Value is bools and numbers
08:04 < andytoshi> we could maybe backport the parts of erased_serde that we need
08:04 < stevenroose> :D :D :D pfff
08:04 < andytoshi> though it might wind up being "a shitload of unsafe code". not sure
08:04 < andytoshi> 16:04 < stevenroose> yeah so the big pro for RawValue is objects and the pro for Value is bools and numbers
08:04 < andytoshi> yeah
08:05 < andytoshi> heh idk what to think then, Core RPC is moving more and more toward objects
08:05 < andytoshi> because positional arguments are super annoying
08:05 < stevenroose> The thing with optimizations for an HTTP client is that the HTTP roundtrip usually makes every optimization quite pointless :D
08:05 < stevenroose> even though daemons are of tne on the same machine, though
08:05 < andytoshi> lol true
08:05 < andytoshi> though with batching we could maybe amortize that and then serialization would matter
08:06 < stevenroose> what I said erlier about `arg(true)` would be a utility method we provide ourselves
08:06 < andytoshi> ok if you implement that i'll stop complaining :P
08:06 < stevenroose> an ergonomic batching interface is super hard to write though
08:06 < stevenroose> because you have a set of Response objects that need some way to remember what their result will be, using phantomdata or so
08:07 < andytoshi> yeah that's fine, we can punt on batching for now (though we do actually have a batching interface don't we?)
08:07 < stevenroose> yeah but it's raw
08:07 < stevenroose> you give some requests and it gives responses
08:07 < stevenroose> bitcoincore-rpc doesn't have a way to use its methods in batches, is what I mean
08:08 < andytoshi> yeah i see
08:11 < andytoshi> it may be that you want to implement special purpose methods for this
08:11 < andytoshi> like "get every block"
08:11 < andytoshi> which don't expose a batch interface to the user
08:26 < stevenroose> "every block" :D :D
08:27 < stevenroose> well I'd feel a lot more comfortable implementing that as an iterator and lazily make requests than to allocate the entire blockchain :D :D :D
08:27 < andytoshi> every block from height X maybe :P
08:27 < andytoshi> yeah, for sure
08:27 < andytoshi> but you might still allocate 100 blocks at a time or something inside the iterator
08:28 < stevenroose> ah yeah that makes sense
08:28 < andytoshi> i kinda wish Core exposed a way to get raw blocks and also the next blockheader, so you didn't have to call `getblockhash`
08:29 < stevenroose> k, I pushed my changes, dpc requested some more unit tests for the URL parsing method
08:29 < andytoshi> `getblockhash` is exactly the kinda call where you'd wanna call it 1000 times in a botch
08:29 < andytoshi> ok cool, i'll check it out now
09:00 < andytoshi> stevenroose: nice :) i nitted the naming of safe_arg but otherwise lgtm
09:39 -!- belcher_ is now known as belcher
11:43 < stevenroose> andytoshi: changed those names, and added a bunch of tests
13:08 < andytoshi> stevenroose: dope
13:27 < andytoshi> unit tests fail on 6c2fc02600324c8d3907e5a785ebbfe7b527caea
13:27 < andytoshi> thread 'simple_http::tests::test_urls' panicked at 'expected error for url localhast, got SimpleHttpTransport { addr: 198.105.254.23:8332, path: "/", timeout: 15s, basic_auth: None }', src/simple_http.rs:395:17
13:27 < andytoshi> o.O "localhast" goes somewhere on my system
13:27 < andytoshi> lol what the hell
13:28 < andytoshi> PING localhast.Home (198.105.254.23) ... it's a local IP
13:32 < andytoshi> lol and if i turn off my network the tests fail becasue it can't resolve google
13:33 < andytoshi> stevenroose: i think you need to drop the google test vector because it fails without network connectivity
13:33 < andytoshi> and i guess drop the localhast one, although my guess is that it's only failing at this one place that i'm leaving in 5 days..
13:42 < stevenroose> andytoshi: lol about localhast. hmm yeah the auto resolving of Rust is a bit annoying here , I also had doubts about the Google one. but I kinda did want one that had a domain name in it..
13:51 < andytoshi> yeah :/ not sure what to do there, i don't know of any hostnames that are guaranteed to resolve
13:52 < andytoshi> that have a . in them
18:38 -!- DeanGuss [~dean@gateway/tor-sasl/deanguss] has quit [Remote host closed the connection]
18:38 -!- DeanWeen [~dean@gateway/tor-sasl/deanguss] has joined #rust-bitcoin
22:12 < gwillen> andytoshi: uh, I think there is something wrong with your system
22:12 < gwillen> localhost should only ever resolve to 127.0.0.1
22:12 < gwillen> having it resolve to anything else breaks a TON of stuff, as you are observing
22:12 < gwillen> oh, you're at an AirBnB or something? They have a very fucked-up network, lol.
22:13 < gwillen> ... also wait, "localhast", delightful. I see, they're doing NXDOMAIN interception or something.
23:17 -!- shesek [~shesek@unaffiliated/shesek] has quit [Remote host closed the connection]
--- Log closed Sun Dec 20 00:00:51 2020