btcsuite / btcd

An alternative full node bitcoin implementation written in Go (golang)
https://github.com/btcsuite/btcd/blob/master/README.md
ISC License
6.29k stars 2.38k forks source link

getnetworkinfo core RPC introduced new field in json object. #2224

Closed ziggie1984 closed 2 months ago

ziggie1984 commented 3 months ago

The current master branch of bitcoin-core introduced a new field into the getnetworkinfo rpc json-response. We should adapt the btcd code to account for this however I am not sure how we should do this in a backwards compatible manner. Any suggestions from prior RPC updates of the core software ?

bitcoin-cli --regtest getnetworkinfo
{
  ...
  "warnings": [
    "This is a pre-release test build - use at your own risk - do not use for mining or merchant applications"
  ]
}
diff --git a/btcjson/chainsvrresults.go b/btcjson/chainsvrresults.go
index 11c0483d..cfb75485 100644
--- a/btcjson/chainsvrresults.go
+++ b/btcjson/chainsvrresults.go
@@ -380,7 +380,7 @@ type GetNetworkInfoResult struct {
        RelayFee        float64                `json:"relayfee"`
        IncrementalFee  float64                `json:"incrementalfee"`
        LocalAddresses  []LocalAddressesResult `json:"localaddresses"`
-       Warnings        string                 `json:"warnings"`
+       Warnings        []string               `json:"warnings"`
 }

Thanks @michael1011 for pointing this out.

stickies-v commented 3 months ago

FYI see https://github.com/bitcoin/bitcoin/pull/29845 for more context. Old behaviour can temporarily be reverted via -deprecatedrpc=warnings as per the release notes but this will be removed in a future version so best to upgrade when you can.

See also https://github.com/bitcoin/bitcoin/blob/master/doc/JSON-RPC-interface.md#versioning.

Where possible, these changes are also documented in the help RPC, see e.g.

./src/bitcoin-cli -regtest help getnetworkinfo | grep warnings
  "warnings" : [                                     (json array) any network and blockchain warnings (run with `-deprecatedrpc=warnings` to return the latest warning as a single string)
Roasbeef commented 3 months ago

. We should adapt the btcd code to account for this however I am not sure how we should do this in a backwards compatible manner. Any suggestions from prior RPC updates of the core software ?

See: https://github.com/btcsuite/btcd/issues/1934

We no longer commit to staying 1:1 in step with all the bitcoind related RPC changes. Even just on the wallet side, there've been a bunch over the past few years, and btcd doesn't integrate its own wallet, so there isn't as strong a case to mirror all the changes there.

For a change like this, one could consider adding a new field for the new value. Not sure how upstream clients that are depending (what do people even use it for today?) on the warnings field will handle the change in general through (string to slice).