[wallet] Backport(0.17): Restore ability to list incoming transactions by label #14441

1. 
   jnewbery commented at 8:02 am on October 9, 2018: member

   Backport of PR #14411 to v0.17.

   This change partially reverts #13075 and #14023.

   Fixes #14382

2. jnewbery renamed this:
   [wallet] Restore ability to list incoming transactions by label
   [wallet] Backport(0.17): Restore ability to list incoming transactions by label
   on Oct 9, 2018
3. fanquake added the label Wallet on Oct 9, 2018
4. fanquake added the label Backport on Oct 9, 2018
5. fanquake added this to the milestone 0.17.1 on Oct 9, 2018
6. in doc/release-notes.md:74 in c8d1f1513f outdated

   70@@ -71,7 +71,17 @@ Notable changes
   71 0.17.x change log
   72 =================
   73 
   74-...
   75+`listtransaction` label support
   

   ---

   ---

   

   ryanofsky commented at 2:50 am on October 10, 2018:

   Should be listtransactions with s.

   ---

   

   jnewbery commented at 7:32 am on October 10, 2018:

   fixed
7. in doc/release-notes.md:80 in c8d1f1513f outdated

   76+-------------------------------
   77+
   78+The `listtransactions` RPC `account` parameter which was deprecated in 0.17.0
   79+and renamed to `dummy` has been un-deprecated and renamed again to `label`.
   80+
   81+The behavior of the `label` argument is identical to previous behavior of the
   

   ---

   ---

   

   ryanofsky commented at 3:08 am on October 10, 2018:

   This paragraph doesn’t really make sense when both behaviors are present in the same release. I’d maybe change it to:

   When bitcoin is configured with the -deprecatedrpc=accounts setting, specifying a label/account/dummy argument will return both outgoing and incoming transactions. Without the -deprecatedrpc=accounts setting, it will only return incoming transactions (because it used to be possible to create transactions spending from specific accounts, but this is no longer possible with labels).

   ---

   

   ryanofsky commented at 3:25 am on October 10, 2018:

   I think it’d be good to document the empty string change. Maybe:

   Also for backwards compatibility when -deprecatedrpc=accounts is set, it’s possible to pass an empty string label "" to list transactions that aren’t labeled. Without -deprecatedrpc=accounts, passing the empty string is an error because returning only non-labeled transactions isn’t actually very useful, and can be confusing.

   ---

   

   jnewbery commented at 7:32 am on October 10, 2018:

   Both are good changes. Thanks!
8. in src/wallet/rpcwallet.cpp:2020 in c8d1f1513f outdated

   2016@@ -2010,10 +2017,10 @@ UniValue listtransactions(const JSONRPCRequest& request)
   2017     pwallet->BlockUntilSyncedToCurrentChain();
   2018 
   2019     std::string strAccount = "*";
   2020-    if (!request.params[0].isNull()) {
   2021+    if (!request.params[0].isNull() && request.params[0].get_str() != "*") {
   

   ---

   ---

   

   ryanofsky commented at 3:17 am on October 10, 2018:

   I don’t see how the new check for “*” here changes anything. Maybe drop for clarity (unless I’m missing something).

   ---

   

   jnewbery commented at 7:39 am on October 10, 2018:

   No, you’re right. It’s better without. The behaviour would be identical with or without the check for “*”. I’ve removed it.
9. ryanofsky approved
10. 
    ryanofsky commented at 5:54 am on October 10, 2018: member

    utACK c8d1f1513fb39d80cebbc32e1b5327209485e085
11. [wallet] Restore ability to list incoming transactions by label

    Backport of PR 14411 to v0.17.
    
    This change partially reverts #13075 and #14023.
    
    Fixes #14382

    89306ab0df
12. jnewbery force-pushed on Oct 10, 2018
13. 
    jnewbery commented at 7:40 am on October 10, 2018: member

    Thanks for the review @ryanofsky! I’ve updated the PR with your changes.
14. 
    ryanofsky commented at 7:57 am on October 10, 2018: member

    utACK 89306ab0df93bfdf5630910bc20b1eccb7379172. Only changes since last review were code simplification and release notes updates suggested above.
15. 
    promag commented at 12:02 pm on October 10, 2018: member

    utACK 89306ab.
16. DrahtBot added the label Needs rebase on Oct 26, 2018
17. DrahtBot removed the label Needs rebase on Nov 6, 2018
18. 
    meshcollider commented at 3:52 am on November 9, 2018: contributor

    utACK https://github.com/bitcoin/bitcoin/pull/14441/commits/89306ab0df93bfdf5630910bc20b1eccb7379172

    This is slightly different from other backport PRs because the changes to src/wallet/rpcwallet.cpp are significantly different from the original PR to achieve the same functionality. The tests are basically a clean backport though.

19. laanwj merged this on Nov 10, 2018
20. laanwj closed this on Nov 10, 2018

    ---

21. laanwj referenced this in commit 5150accdd2 on Nov 10, 2018
22. jnewbery deleted the branch on Oct 9, 2019
23. xdustinface referenced this in commit de7c6514a3 on Dec 22, 2020
24. xdustinface referenced this in commit 96d306324f on Dec 22, 2020
25. UdjinM6 referenced this in commit fbfc4acaeb on Aug 7, 2021
26. UdjinM6 referenced this in commit 5afaeb24cb on Sep 17, 2021
27. UdjinM6 referenced this in commit b6640644eb on Sep 24, 2021
28. kittywhiskers referenced this in commit c3d897f094 on Oct 12, 2021
29. DrahtBot locked this on Dec 16, 2021

Contributors
jnewbery ryanofsky promag meshcollider

Labels
Wallet Backport

Milestone
0.17.1