Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rpc: fix/add missing RPCExamples for "Util" RPCs #18448

Conversation

theStack
Copy link
Contributor

Similar to #18398, this PR gives the RPCExamples in the RPC category "Util" (that currently contains createmultisig, deriveaddresses, estimatesmartfee, getdescriptorinfo, signmessagewithprivkey, validateaddress, verifymessage) some love by fixing one broken and adding three missing examples:

  • fixed HelpExampleRpc for createmultisig (disturbing escape characters and quotation marks)
  • added missing HelpExampleRpc for
    • deriveaddresses (also put descriptor in a new string constant)
    • estimatesmartfee
    • getdescriptorinfo (also put descriptor in a new string constant)

Output for createmultisig example on the master branch:

$ curl --user __cookie__ --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "createmultisig", "params": [2, "[\"03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd\",\"03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626\"]"]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
Enter host password for user '__cookie__':
{"result":null,"error":{"code":-1,"message":"JSON value is not an array as expected"},"id":"curltest"}

Output for createmultisig example on the PR branch:

$ curl --user __cookie__ --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "createmultisig", "params": [2, ["03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd","03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626"]]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
Enter host password for user '__cookie__':
{"result":{"address":"3QsFXpFJf2ZY6GLWVoNFFd2xSDwdS713qX","redeemScript":"522103789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd2103dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a6162652ae","descriptor":"sh(multi(2,03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd,03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626))#4djp057k"},"error":null,"id":"curltest"}

src/rpc/misc.cpp Outdated Show resolved Hide resolved
@theStack theStack force-pushed the 20200326-rpc-add-fix-rpcexamples-in-misc-category branch from d99a24d to 55d30ac Compare March 27, 2020 15:41
@laanwj
Copy link
Member

laanwj commented Apr 1, 2020

I sometimes wonder if these couldn't be generated from the same source data. This is a lot of repetition.
In any case, LGTM
ACK 55d30ac

@maflcko
Copy link
Member

maflcko commented Apr 1, 2020

Two observations:

  • The examples should be auto-generated. In fact I have code for this, but not yet submitted.
  • The fix seems to change the format of two calls, but leaves all other calls at the same (broken?) syntax. I'd say to fix all of them, but maybe it should just wait until they are auto-generated?

@theStack
Copy link
Contributor Author

theStack commented Apr 1, 2020

Automatic generation of RPCExamples would definitely be nice. It should also be possible to automatically test the provided examples to some extent. Maybe I find time to come up with a script the next days to do this.

@MarcoFalke: My rough plan was to divide it up to one PR per RPC category (with the categories "Blockchain", "Control", "Generating", "Mining", "Control", "Generating", "Mining", "Network", "Rawtransactions", "Util", "Wallet"), where in each PR all RPCExamples of one (or two, if they are small ones) category are thoroughly reviewed, tested and fixed or extended when needed. So it could also be done by different people (maybe a good first issue also?). I assumed that creating a mega-PR with changing all broken RPCExamples would be too high of a review burden, since that would likely contain dozens of fixes/additions.

Whether it's auto-generated or not, I think for potential users it's annoying when provided examples just don't work. Call me pedantic, but in doubt I'd even rather prefer to have no example than a broken one.

@DrahtBot
Copy link
Contributor

DrahtBot commented Apr 5, 2020

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

fixes HelpExampleRpc for
    - createmultisig

adds missing HelpExampleRpc for
    - deriveaddresses
    - estimatesmartfee
    - getdescriptorinfo
@theStack theStack force-pushed the 20200326-rpc-add-fix-rpcexamples-in-misc-category branch from 55d30ac to ea98d9c Compare August 14, 2020 10:23
@theStack
Copy link
Contributor Author

Rebased.

Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK ea98d9c looked at the code, rebased to master, ran the helps, did not try running the added json-rpc examples

before / after

Examples:
> bitcoin-cli estimatesmartfee 6

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

Create a multisig address from 2 public keys
> bitcoin-cli createmultisig 2 "[\"03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd\",\"03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626\"]"

As a JSON-RPC call
> curl --user myusername --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "createmultisig", "params": [2, "[\"03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd\",\"03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626\"]"]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/


Examples:

Create a multisig address from 2 public keys
> bitcoin-cli createmultisig 2 "[\"03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd\",\"03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626\"]"

As a JSON-RPC call
> curl --user myusername --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "createmultisig", "params": [2, ["03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd","03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626"]]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
Examples:
Analyse a descriptor
> bitcoin-cli getdescriptorinfo "wpkh([d34db33f/84h/0h/0h]0279be667ef9dcbbac55a06295Ce870b07029Bfcdb2dce28d959f2815b16f81798)"

Examples:
Analyse a descriptor
> bitcoin-cli getdescriptorinfo "wpkh([d34db33f/84h/0h/0h]0279be667ef9dcbbac55a06295Ce870b07029Bfcdb2dce28d959f2815b16f81798)"
> curl --user myusername --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "getdescriptorinfo", "params": ["wpkh([d34db33f/84h/0h/0h]0279be667ef9dcbbac55a06295Ce870b07029Bfcdb2dce28d959f2815b16f81798)"]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
Examples:
First three native segwit receive addresses
> bitcoin-cli deriveaddresses "wpkh([d34db33f/84h/0h/0h]xpub6DJ2dNUysrn5Vt36jH2KLBT2i1auw1tTSSomg8PhqNiUtx8QX2SvC9nrHu81fT41fvDUnhMjEzQgXnQjKEu3oaqMSzhSrHMxyyoEAmUHQbY/0/*)#cjjspncu" "[0,2]"

Examples:
First three native segwit receive addresses
> bitcoin-cli deriveaddresses "wpkh([d34db33f/84h/0h/0h]xpub6DJ2dNUysrn5Vt36jH2KLBT2i1auw1tTSSomg8PhqNiUtx8QX2SvC9nrHu81fT41fvDUnhMjEzQgXnQjKEu3oaqMSzhSrHMxyyoEAmUHQbY/0/*)#cjjspncu" "[0,2]"
> curl --user myusername --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "deriveaddresses", "params": ["wpkh([d34db33f/84h/0h/0h]xpub6DJ2dNUysrn5Vt36jH2KLBT2i1auw1tTSSomg8PhqNiUtx8QX2SvC9nrHu81fT41fvDUnhMjEzQgXnQjKEu3oaqMSzhSrHMxyyoEAmUHQbY/0/*)#cjjspncu", "[0,2]"]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/

The examples are a bit inconsistent in titles and line spacing; can also be done if auto-generating them.

@fanquake
Copy link
Member

fanquake commented Sep 2, 2021

@MarcoFalke is / should this be merged now.

@maflcko
Copy link
Member

maflcko commented Sep 2, 2021

rendered diff:

diff --git a/createmultisig b/createmultisig
index aff8a24..2552278 100644
--- a/createmultisig
+++ b/createmultisig
@@ -25,4 +25,4 @@ Create a multisig address from 2 public keys
 > bitcoin-cli createmultisig 2 "[\"03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd\",\"03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626\"]"
 
 As a JSON-RPC call
-> curl --user myusername --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "createmultisig", "params": [2, "[\"03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd\",\"03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626\"]"]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
+> curl --user myusername --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "createmultisig", "params": [2, ["03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd","03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626"]]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
diff --git a/deriveaddresses b/deriveaddresses
index 1920545..eedaee4 100644
--- a/deriveaddresses
+++ b/deriveaddresses
@@ -24,3 +24,4 @@ Result:
 Examples:
 First three native segwit receive addresses
 > bitcoin-cli deriveaddresses "wpkh([d34db33f/84h/0h/0h]xpub6DJ2dNUysrn5Vt36jH2KLBT2i1auw1tTSSomg8PhqNiUtx8QX2SvC9nrHu81fT41fvDUnhMjEzQgXnQjKEu3oaqMSzhSrHMxyyoEAmUHQbY/0/*)#cjjspncu" "[0,2]"
+> curl --user myusername --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "deriveaddresses", "params": ["wpkh([d34db33f/84h/0h/0h]xpub6DJ2dNUysrn5Vt36jH2KLBT2i1auw1tTSSomg8PhqNiUtx8QX2SvC9nrHu81fT41fvDUnhMjEzQgXnQjKEu3oaqMSzhSrHMxyyoEAmUHQbY/0/*)#cjjspncu", "[0,2]"]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
diff --git a/estimatesmartfee b/estimatesmartfee
index 0ac5eed..3c4d849 100644
--- a/estimatesmartfee
+++ b/estimatesmartfee
@@ -33,3 +33,4 @@ Result:
 
 Examples:
 > bitcoin-cli estimatesmartfee 6
+> curl --user myusername --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "estimatesmartfee", "params": [6]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
diff --git a/getdescriptorinfo b/getdescriptorinfo
index d04f048..09448c5 100644
--- a/getdescriptorinfo
+++ b/getdescriptorinfo
@@ -17,3 +17,4 @@ Result:
 Examples:
 Analyse a descriptor
 > bitcoin-cli getdescriptorinfo "wpkh([d34db33f/84h/0h/0h]0279be667ef9dcbbac55a06295Ce870b07029Bfcdb2dce28d959f2815b16f81798)"
+> curl --user myusername --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "getdescriptorinfo", "params": ["wpkh([d34db33f/84h/0h/0h]0279be667ef9dcbbac55a06295Ce870b07029Bfcdb2dce28d959f2815b16f81798)"]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/

@maflcko maflcko merged commit dd097c4 into bitcoin:master Sep 2, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Sep 4, 2021
ea98d9c rpc: fix/add missing RPCExamples for "Util" RPCs (Sebastian Falbesoner)

Pull request description:

  Similar to bitcoin#18398, this PR gives the RPCExamples in the RPC category "Util" (that currently contains `createmultisig`, `deriveaddresses`, `estimatesmartfee`, `getdescriptorinfo`, `signmessagewithprivkey`, `validateaddress`, `verifymessage`) some love by fixing one broken and adding three missing examples:
  - fixed `HelpExampleRpc` for `createmultisig` (disturbing escape characters and quotation marks)
  - added missing `HelpExampleRpc` for
      -  `deriveaddresses` (also put descriptor in a new string constant)
      - `estimatesmartfee`
      - `getdescriptorinfo` (also put descriptor in a new string constant)

  Output for `createmultisig` example on the master branch:
  ```
  $ curl --user __cookie__ --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "createmultisig", "params": [2, "[\"03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd\",\"03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626\"]"]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
  Enter host password for user '__cookie__':
  {"result":null,"error":{"code":-1,"message":"JSON value is not an array as expected"},"id":"curltest"}
  ```

  Output for `createmultisig` example on the PR branch:
  ```
  $ curl --user __cookie__ --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "createmultisig", "params": [2, ["03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd","03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626"]]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
  Enter host password for user '__cookie__':
  {"result":{"address":"3QsFXpFJf2ZY6GLWVoNFFd2xSDwdS713qX","redeemScript":"522103789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd2103dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a6162652ae","descriptor":"sh(multi(2,03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd,03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626))#4djp057k"},"error":null,"id":"curltest"}
  ```

ACKs for top commit:
  jonatack:
    ACK ea98d9c looked at the code, rebased to master, ran the helps, did not try running the added json-rpc examples

Tree-SHA512: d6ecb6da66f19517065453357d210102e2cc9f1f8037aeb6a9177ff036d0c21773dddf5e0acdbc71edbbde3026e4d1e7ce7c0935cd3e023c60f34e1b173b3299
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants