Use serde_json::value::to_raw_value in Client::call()

[romanz]

Since MSRV is 1.56.1: https://github.com/rust-bitcoin/rust-bitcoincore-rpc#minimum-supported-rust-version-msrv

[apoelstra]

For context: this comment appears in #180 and the MSRV incompatibility seems to come from us pinning a version of serde_json somewhere in our dep tree.

This function seems to be introduced in serde_json 1.0.52. We should update the dependency in Cargo.toml from "1" to this value. (Furthermore, we should bisect backward to find out what our actual minimum dependency versions are. I think this crate is riding on rust-bitcoin's value for serde and probably others, but it shouldn't be.)

As a second thought, in rust-jsonrpc we have some hacky stuff related to raw values and maybe we should be using this new method as well.

[tcharding]

Is this live still @romanz? What problem is it trying to fix, does the problem still exist? Please note we are on MSRV of 1.56.1 now and also have MSRV dependency pinning fixes in #338. Thanks.

[romanz]

What problem is it trying to fix, does the problem still exist?

IIRC, it shouldn't change the functionality - only simplify the existing code a bit.
https://docs.rs/serde_json/1.0.108/src/serde_json/raw.rs.html#284-290

I have thought to suggest this change when reviewing the code at:

rust-bitcoincore-rpc/client/src/client.rs

Line 1319 in 33293a5

serde_json::value::RawValue::from_string(json_string) // we can't use to_raw_value here due to compat with Rust 1.29

If it's not relevant, I can close this PR.

[tcharding]

Ah yeah, I've spent more time in the RawValue docs since I asked that question. I believe you are correct and this is refactor only.

[tcharding]

ACK 8049ea6

[tcharding]

I took the liberty of editing the PR description for you @romanz because the MSRV has bumped since this PR was opened. The commit message doesn't include the msrv so we are good to go. Thanks man!

[romanz]

LGTM, thanks @tcharding!

[apoelstra]

ACK 8049ea6

[tcharding]

I just noticed that there is another instance of this in log_response(), I think we need

diff --git a/client/src/client.rs b/client/src/client.rs
index 745b863..80fd717 100644
--- a/client/src/client.rs
+++ b/client/src/client.rs
@@ -1342,11 +1342,7 @@ fn log_response(cmd: &str, resp: &Result<jsonrpc::Response>) {
                         debug!(target: "bitcoincore_rpc", "JSON-RPC error for {}: {:?}", cmd, e);
                     }
                 } else if log_enabled!(Trace) {
-                    // we can't use to_raw_value here due to compat with Rust 1.29
-                    let def = serde_json::value::RawValue::from_string(
-                        serde_json::Value::Null.to_string(),
-                    )
-                    .unwrap();
+                    let def = serde_json::value::to_raw_value(&serde_json::value::Value::Null);
                     let result = resp.result.as_ref().unwrap_or(&def);
                     trace!(target: "bitcoincore_rpc", "JSON-RPC response for {}: {}", cmd, result);

[apoelstra]

Good catch. And yep, I think your solution is right ... I don't see a way to directly create a raw null.

[romanz]

Thanks - fixed in 308448d.

[tcharding]

CI fail is unrelated to this PR, fixed in #351

[tcharding]

ACK 308448d