rpc: update validateaddress RPCExamples to bech32 #18122

pull theStack wants to merge 1 commits into bitcoin:master from theStack:20200211-rpc-update-validateaddress-rpcexamples-to-bech32 changing 5 files +13 −7

theStack commented at 10:20 PM on February 11, 2020: member

Another small step to get rid of legacy addresses in the RPC help texts and by that encourage the use of bech32 addresses by default. The (invalid) address is the same as in the getaddressinfo RPC (see 2ee0cb3330ccf70f0540cb42370796e32eff1569, kudos to jonatack!), I don't think it adds any value to have a different example address per RPC.
fanquake added the label RPC/REST/ZMQ on Feb 11, 2020
in src/rpc/misc.cpp:28 in ca3effc0c0 outdated
```
  24 | @@ -25,6 +25,8 @@
  25 |  
  26 |  static UniValue validateaddress(const JSONRPCRequest& request)
  27 |  {
  28 | +    const std::string example_address = "\"bc1q09vm5lfy0j5reeulh4x5752q25uqqvz34hufdl\"";
```
MarcoFalke commented at 10:26 PM on February 11, 2020:

Compile-time constants are usually all-upper-case. Also, I wouldn't mind to have a single place where this is defined/declared instead of one for each RPC that uses it.

jonatack commented at 10:41 PM on February 11, 2020:

Good idea. Perhaps like UNIX_EPOCH_TIME in src/rpc/util.h and src/rpc/util.cpp in e2f32cb

theStack commented at 11:00 PM on February 11, 2020:

@MarcoFalke @jonatack: Thanks for reviewing! I pushed a commit that factors out EXAMPLE_ADDRESS into src/rpc/util.{h,cpp}. Will update the developer notes as well.
jonatack commented at 10:52 PM on February 11, 2020: member

Concept ACK, would suggest covering all the cases and updating doc/developer-notes.md; see the last two entries about bech32 addresses and UNIX epoch time.
theStack commented at 11:20 PM on February 11, 2020: member

Concept ACK, would suggest covering all the cases and updating doc/developer-notes.md; see the last two entries about bech32 addresses and UNIX epoch time.

For the developer notes, I cautiously just added EXAMPLE_ADDRESS as an example for an invalid bech32 address in parantheses. A guideline like "Use EXAMPLE_ADDRESS for RPCExamples" seemed to strict on me (what if, for example, one needs two different addresses in an RPCExample etc.). Thoughts on this?
theStack commented at 11:28 PM on February 11, 2020: member

@jonatack: Oh, and for "covering all the cases", this would probably make good follow-up PR candidates? Will probably create a "good-first-issue" issue tomorrow containing a check-box list of all RPCs that still contain legacy addresses in their RPCExamples.
jonatack commented at 11:31 AM on February 12, 2020: member

@jonatack: Oh, and for "covering all the cases", this would probably make good follow-up PR candidates? Will probably create a "good-first-issue" issue tomorrow containing a check-box list of all RPCs that still contain legacy addresses in their RPCExamples.

I thought that was a good initiative on your part with the test issue. In this case, I would change all the cases in one commit like with e2f32cb5 (unless it's much less trivial to do than here), to optimise for less review resource use.
MarcoFalke commented at 8:17 PM on February 12, 2020: member

ACK, could squash?

rpc: update validateaddress RPCExamples to bech32

also contains the following changes:
- rpc: factor out example bech32 address for RPCExamples
- doc: update developer notes wrt RPCExamples addresses
 (mention the EXAMPLE_ADDRESS constant as an example for an invalid bech32
  address suitable for RPCExamples help documentation)

7f1475c711

theStack force-pushed on Feb 13, 2020
theStack commented at 12:02 PM on February 13, 2020: member

ACK, could squash?

Sure, done.
MarcoFalke commented at 12:57 PM on February 13, 2020: member

ACK 7f1475c7119e8c72bce39a63386a6ca859066b80
fanquake approved

fanquake commented at 12:10 PM on February 20, 2020: member

ACK 7f1475c7119e8c72bce39a63386a6ca859066b80

src/bitcoin-cli help validateaddress
...
Examples:
> bitcoin-cli validateaddress "bc1q09vm5lfy0j5reeulh4x5752q25uqqvz34hufdl"
> curl --user myusername --data-binary '{"jsonrpc": "1.0", "id":"curltest", "method": "validateaddress", "params": ["bc1q09vm5lfy0j5reeulh4x5752q25uqqvz34hufdl"] }' -H 'content-type: text/plain;' http://127.0.0.1:8332/

src/bitcoin-cli help getaddressinfo
...
Examples:
> bitcoin-cli getaddressinfo "bc1q09vm5lfy0j5reeulh4x5752q25uqqvz34hufdl"
> curl --user myusername --data-binary '{"jsonrpc": "1.0", "id":"curltest", "method": "getaddressinfo", "params": ["bc1q09vm5lfy0j5reeulh4x5752q25uqqvz34hufdl"] }' -H 'content-type: text/plain;' http://127.0.0.1:8332/

fanquake merged this on Feb 20, 2020
fanquake closed this on Feb 20, 2020
sidhujag referenced this in commit 022e6c00d4 on Feb 22, 2020
jasonbcox referenced this in commit 7221978100 on Oct 19, 2020
sidhujag referenced this in commit c5bf71b9d0 on Nov 10, 2020
theStack deleted the branch on Dec 1, 2020
DrahtBot locked this on Feb 15, 2022

Contributors

Labels

Linked (view graph)

#18185 Change address format in RPCExamples from legacy to bech32