btcjson+rpcclient: add compatibility for bitcoind v0.19.0

[wpaulino]

This PR aims to remedy the breaking RPC changes introduced in bitcoind v0.19.0:

• wallet/rpc: sendrawtransaction maxfeerate bitcoin/bitcoin#13541
• Bury bip9 deployments bitcoin/bitcoin#16060

This required being able to interpret the backend version the client connects to, which wasn't previously supported. btcd and bitcoind have different methods to expose this, GetInfo and GetNetworkInfo respectively.

[halseth]

This is quite an elegant solution to the problem, looks pretty good 👍

[cfromknecht]

does this break compat with older nodes? or was it always incorrect?

[Roasbeef]

Good question...it would appears that it does, unless the json library falls back to a fuzzy match on the field name within the struct itself?

[wpaulino]

does this break compat with older nodes?

It does...

unless the json library falls back to a fuzzy match on the field name within the struct itself?

Doesn't seem to be the case unfortunately due to the _. How should we address this? This same way we did with SoftForks and UnifiedSoftForks in GetBlockChainInfoResult?

[halseth]

Can we attempt to unmarshal the result into this struct, if start_time is not set, unmarshal into a new struct having the startTime field, and override?

[wpaulino]

We can. I'm wondering if this is even worth the hassle though, as it would only affect bitcoind < 0.19.0 and it wouldn't affect btcd at all. Also there's no failure from unmarshaling in this case, the StartTime field just wouldn't be set.

[Roasbeef]

Here's an option: remove this field, and add two new fields StartTime1 and StartTime2, then create a new method StartTime that returns the value. This breaks old clients, but gracefully (imo) as they'll likely see a compile error that they're passing in a method, when it's expecting an int64.

[wpaulino]

Done.

[Roasbeef]

First pass through, haven't yet run it yet to verify the open question w.r.t backwards compatibility.

[Roasbeef]

Good question...it would appears that it does, unless the json library falls back to a fuzzy match on the field name within the struct itself?

[halseth]

Looks more or less good to me now, pending start_time compat

[halseth]

Can we attempt to unmarshal the result into this struct, if start_time is not set, unmarshal into a new struct having the startTime field, and override?

[wpaulino]

rpcclient/chain_test.go:12:20: undefined: unmarshalPartialGetBlockChainInfoResult

Travis failure seems related to some weird caching issue. The test passes locally.

[Roasbeef]

Cleared the cache, the build passes now.

[Roasbeef]

Here's an option: remove this field, and add two new fields StartTime1 and StartTime2, then create a new method StartTime that returns the value. This breaks old clients, but gracefully (imo) as they'll likely see a compile error that they're passing in a method, when it's expecting an int64.

[Roasbeef]

LGTM 👑