rpc: Properly document return values (submitblock, gettxout, getblocktemplate, scantxoutset) #20556

pull MarcoFalke wants to merge 4 commits into bitcoin:master from MarcoFalke:2012-rpcDoc changing 2 files +160 −152
  1. MarcoFalke commented at 9:19 AM on December 3, 2020: member

    Currently a few return values are undocumented. This is causing confusion at the least. See for example #18476

  2. MarcoFalke added the label Docs on Dec 3, 2020
  3. MarcoFalke added the label RPC/REST/ZMQ on Dec 3, 2020
  4. DrahtBot commented at 1:30 PM on December 3, 2020: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #21426 (rpc: remove scantxoutset EXPERIMENTAL warning by jonatack)
    • #21401 (Refactor versionbits deployments to avoid potential uninitialized variables by achow101)
    • #21399 (Genericide BIP9 in variable/type names and comments by luke-jr)
    • #21393 (BIP 341: Add Speedy Trial activation parameters by achow101)
    • #21392 (Implement BIP 8 based Speedy Trial activation by achow101)
    • #20286 (rpc: deprecate addresses and reqSigs from rpc outputs by mjdietzx)
    • #16795 (rpc: have raw transaction decoding infer output descriptors by instagibbs)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  5. DrahtBot cross-referenced this on Dec 3, 2020 from issue rpc: Fail to return undocumented return values by MarcoFalke
  6. DrahtBot cross-referenced this on Dec 3, 2020 from issue rpc: deprecate `addresses` and `reqSigs` from rpc outputs by mjdietzx
  7. DrahtBot cross-referenced this on Dec 3, 2020 from issue rpc: Add RPCContext by promag
  8. DrahtBot cross-referenced this on Dec 3, 2020 from issue Coinstats Index by fjahr
  9. DrahtBot cross-referenced this on Dec 3, 2020 from issue docs: Document that 'fee' is unavailable when there are non-wallet inputs by shesek
  10. DrahtBot cross-referenced this on Dec 3, 2020 from issue rpc: have raw transaction decoding infer output descriptors by instagibbs
  11. jarolrod commented at 4:25 PM on December 4, 2020: member

    Testing fa3cbac Calling gettxout with an invalid utxo returns no output using bitcoin-cli, but returns null in the gui. I think the better behavior would be to return an empty json block like we do with other rpc commands such as listtransactions

  12. MarcoFalke commented at 4:39 PM on December 4, 2020: member

    This can be discussed in #18476 further. The goal of this pull is to simply document the return values as they are and always have been.

  13. jonasschnelli commented at 8:09 AM on December 7, 2020: contributor

    Looks good. What is the rational in renaming STR_AMOUNT to NUM_AMOUNT? It creates a fairly large diff and IMO it still is represented as a string on the JSON layer.

  14. MarcoFalke commented at 8:18 AM on December 7, 2020: member

    The (scripted) diff is only 50 lines. If there are any conflicts, it should be trivial to resolve. That the internal representation is a string could be a coincidence. The type is really VNUM (numeric) and not VSTR (string).

    I checked that the only conflict due to this scripted diff is #19002.

  15. laanwj commented at 1:06 PM on December 18, 2020: member

    Just an aside: there used to be talk of, at some point, of making ValueFromAmount return a decimal string instead of a number, or at least having an option to do so, because it's easier to parse exactly in some languages that interpret javascript numbers as floating point number (which are unwise to use for monetary amounts). See e.g. #3759.

    This bled out but seeing them as separate types an abstraction level above JSON string/value distinction makes sense.

  16. MarcoFalke force-pushed on Dec 21, 2020
  17. MarcoFalke commented at 2:11 PM on December 21, 2020: member

    Ok, dropped the scripted diff because it seemed controversial

  18. DrahtBot cross-referenced this on Dec 26, 2020 from issue [POC/DRAFT] - Finalize remove reqsigs deprecation from rpcs by mjdietzx
  19. MarcoFalke force-pushed on Jan 29, 2021
  20. MarcoFalke commented at 9:12 AM on January 29, 2021: member

    Rebased

  21. DrahtBot cross-referenced this on Feb 3, 2021 from issue Replace unused BIP 9 logic with draft BIP 8 by luke-jr
  22. laanwj added this to the "Blockers" column in a project
  23. in src/rpc/blockchain.cpp:1114 in fa84858cc0 outdated 
    1133 | +            {"txid", RPCArg::Type::STR, RPCArg::Optional::NO, "The transaction id"},
1134 | +            {"n", RPCArg::Type::NUM, RPCArg::Optional::NO, "vout number"},
1135 | +            {"include_mempool", RPCArg::Type::BOOL, /* default */ "true", "Whether to include the mempool. Note that an unspent output that is spent in the mempool won't appear."},
1136 | +        },
1137 | +        {
1138 | +            RPCResult{"When the UTXO couldn't be found", RPCResult::Type::NONE, "", ""},
    amitiuttarwar commented at 9:58 PM on February 24, 2021:

    I'd find it clearer to say:

                RPCResult{"If the UTXO was not found", RPCResult::Type::NONE, "", ""},
  24. in src/rpc/mining.cpp:533 in fa84858cc0 outdated 
     553 | -                RPCResult{
 554 | -                    RPCResult::Type::OBJ, "", "",
 555 | +        },
 556 | +        {
 557 | +            RPCResult{"When mode=='proposal'", RPCResult::Type::STR, "", ""},
 558 | +            RPCResult{"When mode=='proposal'", RPCResult::Type::NONE, "", ""},
    amitiuttarwar commented at 10:11 PM on February 24, 2021:

    as an RPC user, I'm confused about how to interpret this output.. when the mode is proposal, there might be a string returned (of what?) or a json null object? what conditions lead to one or the other?

    don't need to go into detail, but since the goal here is to document return types & help usability, I think this is worth clarifying.

    MarcoFalke commented at 7:21 AM on February 25, 2021:

    Thanks, fixed

  25. amitiuttarwar commented at 10:20 PM on February 24, 2021: contributor

    concept ACK. one improvement requested, otherwise generally looks good. I checked most of the documented results actually match up.

  26. rpc: Properly document gettxout return value
    Can be reviewed with --ignore-all-space
    faa2059547
  27. rpc: Properly document scantxoutset return value
    Can be reviewed with --ignore-all-space
    fabaccf031
  28. MarcoFalke force-pushed on Feb 25, 2021
  29. rpc: Properly document getblocktemplate return value
    Can be reviewed with --ignore-all-space
    fae542c28b
  30. rpc: Properly document submitblock return value
    Can be reviewed with --ignore-all-space
    fa7ff0790e
  31. MarcoFalke force-pushed on Feb 25, 2021
  32. MarcoFalke commented at 7:22 AM on February 25, 2021: member

    Force pushed to address feedback. Should be easy to re-ACK with git range-diff

  33. in src/rpc/mining.cpp:519 in fa7ff0790e 
     521 | +        "    https://github.com/bitcoin/bips/blob/master/bip-0145.mediawiki\n",
 522 | +        {
 523 | +            {"template_request", RPCArg::Type::OBJ, "{}", "Format of the template",
 524 | +            {
 525 | +                {"mode", RPCArg::Type::STR, /* treat as named arg */ RPCArg::Optional::OMITTED_NAMED_ARG, "This must be set to \"template\", \"proposal\" (see BIP 23), or omitted"},
 526 | +                {"capabilities", RPCArg::Type::ARR, /* treat as named arg */ RPCArg::Optional::OMITTED_NAMED_ARG, "A list of strings",
    amitiuttarwar commented at 7:57 PM on February 25, 2021:

    tangential since this PR just aims to document return types, but I think a data param is missing from these help docs?

    MarcoFalke commented at 9:04 AM on February 26, 2021:

    I think this is intentionally omitted (via template_request, which is described in the BIPs)

  34. amitiuttarwar approved
  35. amitiuttarwar commented at 10:01 PM on February 25, 2021: contributor

    tACK fa7ff0790e

    • gettxout -> checked we get an empty result if UTXO is not found
    • scantxoutset -> checked abort returns bool, status returns empty or progress based on if scan is occurring, the existing docs apply to results from start command.
    • getblocktemplate -> checked we get string result if proposal is rejected, didn't test an accepted proposal, but saw in the code that we would return null, saw the existing docs apply to other modes.
    • submitblock -> same, saw string for failure, looked at code for success

    made sure all the help docs make sense & look good when called from command line

    the changes from }, \n }, -> }}, kind of confuse me, but I assume its fine considering code compiles & help docs are showing up correctly.

  36. DrahtBot cross-referenced this on Mar 9, 2021 from issue BIP 341: Add Speedy Trial activation parameters by achow101
  37. DrahtBot cross-referenced this on Mar 9, 2021 from issue Implement BIP 8 based Speedy Trial activation by achow101
  38. DrahtBot cross-referenced this on Mar 10, 2021 from issue Refactor versionbits deployments to avoid potential uninitialized variables by achow101
  39. DrahtBot cross-referenced this on Mar 10, 2021 from issue Genericide BIP9 in variable/type names and comments by luke-jr
  40. DrahtBot cross-referenced this on Mar 13, 2021 from issue rpc: remove scantxoutset EXPERIMENTAL warning by jonatack
  41. fjahr commented at 8:55 PM on March 14, 2021: contributor

    utACK fa7ff0790ec21d187f1a32310918b0c8d66e5266

    Reviewed changes ignoring whitespace.

  42. fanquake merged this on Mar 15, 2021
  43. fanquake closed this on Mar 15, 2021
  44. MarcoFalke deleted the branch on Mar 15, 2021
  45. sidhujag referenced this in commit 08592b0bba on Mar 15, 2021
  46. jnewbery removed this from the "Blockers" column in a project
  47. luke-jr referenced this in commit 1efe3b8b8d on Dec 14, 2021
  48. luke-jr referenced this in commit e9aca6156e on Dec 14, 2021
  49. luke-jr referenced this in commit 175336e44e on Dec 14, 2021
  50. luke-jr referenced this in commit b83129bb8a on Dec 14, 2021
  51. Fabcien referenced this in commit ae11b360f0 on Apr 13, 2022
  52. bitcoin locked this on Aug 16, 2022
Contributors
MarcoFalkeDrahtBotjarolrodjonasschnellilaanwjamitiuttarwarfjahr
Labels
DocsRPC/REST/ZMQ
Linked (view graph)
#3759 Add option `-rpcamount` for monetary amounts as strings/satoshis in RPC#18476 bitcoin-cli returns no error on unsuccessful gettxout call
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-19 06:53 UTC