rpcwallet: Add missing transaction categories to rpc helptexts #14653

pull andrewtoth wants to merge 2 commits into bitcoin:master from andrewtoth:listtransactions-help changing 3 files +87 −12
  1. andrewtoth commented at 1:54 AM on November 5, 2018: contributor

    The current helptext for listtransactions, listsinceblock and gettransaction only list two of the five possible options for category. This incorrectly implies that these are the only two options, and can cause problems if the other three options aren't accounted for. Also, some of the documentation is incorrect when specifying which options are returned for which categories.

    This PR updates the helptext for these RPCs and adds a functional regression test for the cases when the other three categories are returned.

  2. fanquake added the label Docs on Nov 5, 2018
  3. fanquake added the label RPC/REST/ZMQ on Nov 5, 2018
  4. DrahtBot commented at 2:09 AM on November 5, 2018: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    No conflicts as of last run.<!--2502f1a698b3751726fa55edcda76cd3-->

    Coverage

    Coverage Change (pull 14653) Reference (master)
    Lines +0.0176 % 87.0960 %
    Functions -0.0612 % 84.3822 %
    Branches +0.0122 % 51.5722 %

    <sup>Updated at: 2018-11-05T21:23:44.257742.</sup>

  5. in src/wallet/rpcwallet.cpp:1349 in 8f06eb9359 outdated 
    1345 | @@ -1346,7 +1346,7 @@ UniValue listtransactions(const JSONRPCRequest& request)
1346 |              "[\n"
1347 |              "  {\n"
1348 |              "    \"address\":\"address\",    (string) The bitcoin address of the transaction.\n"
1349 | -            "    \"category\":\"send|receive\", (string) The transaction category.\n"
1350 | +            "    \"category\":\"send|receive|generate|immature|orphan\", (string) The transaction category.\n"
    promag commented at 11:14 AM on November 5, 2018:

    Could improve test/functional/wallet_listtransactions.py with these categories.

    andrewtoth commented at 3:00 AM on November 6, 2018:

    Done

  6. andrewtoth cross-referenced this on Nov 6, 2018 from issue Track coinbase transactions by andrewtoth
  7. meshcollider commented at 7:01 AM on November 6, 2018: contributor

    Thanks, utACK https://github.com/bitcoin/bitcoin/pull/14653/commits/31066f523fc11ef8ba7943693180e56e04fd0bac The listsinceblock RPC helptext also still refers to the 'move' category which was removed when accounts were, you could fix that up too in the same commit if you want :)

  8. andrewtoth force-pushed on Nov 7, 2018
  9. andrewtoth commented at 1:06 AM on November 7, 2018: contributor

    @MeshCollider done. Also updated the gettransaction help.

  11. in src/wallet/rpcwallet.cpp:1349 in 4a78c4888a outdated 
    1345 | @@ -1346,9 +1346,13 @@ UniValue listtransactions(const JSONRPCRequest& request)
1346 |              "[\n"
1347 |              "  {\n"
1348 |              "    \"address\":\"address\",    (string) The bitcoin address of the transaction.\n"
1349 | -            "    \"category\":\"send|receive\", (string) The transaction category.\n"
1350 | -            "    \"amount\": x.xxx,          (numeric) The amount in " + CURRENCY_UNIT + ". This is negative for the 'send' category, and is positive\n"
1351 | -            "                                        for the 'receive' category,\n"
1352 | +            "    \"category\":\"send|receive|generate|immature|orphan\",     (string) The transaction category.\n"
    promag commented at 1:38 PM on November 12, 2018:

    You could improve formatting by, for instance, following: https://github.com/bitcoin/bitcoin/blob/ae32806ea20235c4542b1a5d4011df5ae267b58a/src/wallet/rpcwallet.cpp#L3017-L3023

  12. andrewtoth force-pushed on Nov 13, 2018
  13. andrewtoth commented at 2:17 AM on November 13, 2018: contributor

    @promag Reformatted help docs, added tests for all rpcs that have transaction category.

  14. andrewtoth force-pushed on Nov 14, 2018
  15. andrewtoth renamed this:
    Add all category options to listtransactions help
    Add all transaction category options to rpc helptexts
    on Nov 17, 2018
  16. andrewtoth renamed this:
    Add all transaction category options to rpc helptexts
    Add missing transaction category options to rpc helptexts
    on Nov 17, 2018
  17. andrewtoth renamed this:
    Add missing transaction category options to rpc helptexts
    Add missing transaction categories to rpc helptexts
    on Nov 17, 2018
  18. andrewtoth cross-referenced this on Nov 18, 2018 from issue Test for coinbase transactions by andrewtoth
  20. andrewtoth commented at 12:50 AM on November 20, 2018: contributor

    @promag Are the formatting and tests ok? @MeshCollider Sorry to keep invalidating your utACKs. I updated to include category tests for all 3 rpcs.

  21. in src/wallet/rpcwallet.cpp:1367 in a2c555d23f outdated 
    1362 | @@ -1363,9 +1363,13 @@ UniValue listtransactions(const JSONRPCRequest& request)
1363 |              "[\n"
1364 |              "  {\n"
1365 |              "    \"address\":\"address\",    (string) The bitcoin address of the transaction.\n"
1366 | -            "    \"category\":\"send|receive\", (string) The transaction category.\n"
1367 | -            "    \"amount\": x.xxx,          (numeric) The amount in " + CURRENCY_UNIT + ". This is negative for the 'send' category, and is positive\n"
1368 | -            "                                        for the 'receive' category,\n"
1369 | +            "    \"category\":               (string) The transaction category.\n"
1370 | +            "                \"send\"|                 'send' specifies transactions sent.\n"
    promag commented at 10:50 AM on November 20, 2018:

    I don't see the need to repeat. Something along this?

    "    \"category\"                    (string) The transaction category. One of\n" 
"        \"send\"                    transactions sent\n"
"        \"receive\"                 non-coinbase transations received\n" 
...

    However I don't have strong opinion here and currently there is no "standard" output defined (that I know of).

    andrewtoth commented at 2:46 AM on November 21, 2018:

    Reformatted to remove the repetition.

  22. andrewtoth force-pushed on Nov 21, 2018
  23. andrewtoth force-pushed on Nov 21, 2018
  24. andrewtoth force-pushed on Nov 22, 2018
  25. andrewtoth force-pushed on Nov 22, 2018
  26. andrewtoth force-pushed on Nov 22, 2018
  27. in src/wallet/rpcwallet.cpp:1374 in 1d25ee1791 outdated 
    1368 | @@ -1369,9 +1369,14 @@ UniValue listtransactions(const JSONRPCRequest& request)
1369 |              "[\n"
1370 |              "  {\n"
1371 |              "    \"address\":\"address\",    (string) The bitcoin address of the transaction.\n"
1372 | -            "    \"category\":\"send|receive\", (string) The transaction category.\n"
1373 | +            "    \"category\":               (string) The transaction category.\n"
1374 | +            "                \"send\"                  Transactions sent.\n"
1375 | +            "                \"receive\"               Non-coinbase transations received.\n"
    practicalswift commented at 6:09 AM on November 23, 2018:

    Should be "transactions" :-)

  28. in src/wallet/rpcwallet.cpp:1507 in 1d25ee1791 outdated 
    1506 | -            "    \"amount\": x.xxx,          (numeric) The amount in " + CURRENCY_UNIT + ". This is negative for the 'send' category, and for the 'move' category for moves \n"
1507 | -            "                                          outbound. It is positive for the 'receive' category, and for the 'move' category for inbound funds.\n"
1508 | +            "    \"address\":\"address\",    (string) The bitcoin address of the transaction..\n"
1509 | +            "    \"category\":               (string) The transaction category.\n"
1510 | +            "                \"send\"                  Transactions sent.\n"
1511 | +            "                \"receive\"               Non-coinbase transations received.\n"
    practicalswift commented at 6:10 AM on November 23, 2018:

    Same typo here.

  29. in src/wallet/rpcwallet.cpp:1660 in 1d25ee1791 outdated 
    1654 | @@ -1645,7 +1655,12 @@ static UniValue gettransaction(const JSONRPCRequest& request)
1655 |              "  \"details\" : [\n"
1656 |              "    {\n"
1657 |              "      \"address\" : \"address\",          (string) The bitcoin address involved in the transaction\n"
1658 | -            "      \"category\" : \"send|receive\",    (string) The category, either 'send' or 'receive'\n"
1659 | +            "      \"category\" :                      (string) The transaction category.\n"
1660 | +            "                   \"send\"                  Transactions sent.\n"
1661 | +            "                   \"receive\"               Non-coinbase transations received.\n"
    practicalswift commented at 6:10 AM on November 23, 2018:

    And here :-)

    andrewtoth commented at 2:53 PM on November 23, 2018:

    All done

  30. andrewtoth force-pushed on Nov 23, 2018
  31. Add all category options to wallet rpc help e982f0b682
  32. Test coinbase category in wallet rpcs f3f6dde56e
  33. andrewtoth force-pushed on Dec 1, 2018
  34. andrewtoth cross-referenced this on Dec 8, 2018 from issue rpc listtransactions new argument options (paginatebypointer impl) by hosseinzoda
  35. ryanofsky approved
  36. ryanofsky commented at 5:03 PM on December 10, 2018: contributor

    utACK f3f6dde56ead45072864123ed0a2bb04ec584b65. Clearly a good change. It cleans up inaccurate documentation and adds a test.

  37. ryanofsky commented at 9:08 PM on December 18, 2018: contributor

    This looks like it could be merged.

  38. promag commented at 9:43 PM on December 18, 2018: member

    utACK f3f6dde.

  39. luke-jr commented at 5:14 AM on December 20, 2018: member

    utACK

  40. MarcoFalke renamed this:
    Add missing transaction categories to rpc helptexts
    rpcwallet: Add missing transaction categories to rpc helptexts
    on Dec 20, 2018
  41. MarcoFalke merged this on Dec 20, 2018
  42. MarcoFalke closed this on Dec 20, 2018
  43. MarcoFalke referenced this in commit 86e0a33f5c on Dec 20, 2018
  44. andrewtoth deleted the branch on Apr 29, 2019
  45. deadalnix referenced this in commit 94bf2594a2 on Sep 2, 2020
  46. deadalnix referenced this in commit 90194ebd87 on Sep 2, 2020
  47. bitcoin locked this on Dec 16, 2021
Contributors
andrewtothDrahtBotpromagmeshcolliderchris-belcherpracticalswiftryanofskyluke-jr
Labels
DocsRPC/REST/ZMQ
Linked (view graph)
#14898 rpc listtransactions new argument options (paginatebypointer impl)
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-20 06:54 UTC