-
Notifications
You must be signed in to change notification settings - Fork 38.7k
[wallet] Backport(0.17): Restore ability to list incoming transactions by label #14441
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
Conversation
ryanofsky
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK c8d1f1513fb39d80cebbc32e1b5327209485e085
doc/release-notes.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be listtransactions with s.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
doc/release-notes.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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=accountssetting,
specifying alabel/account/dummyargument will return both outgoing and
incoming transactions. Without the-deprecatedrpc=accountssetting, 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).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it'd be good to document the empty string change. Maybe:
Also for backwards compatibility when
-deprecatedrpc=accountsis 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both are good changes. Thanks!
src/wallet/rpcwallet.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see how the new check for "*" here changes anything. Maybe drop for clarity (unless I'm missing something).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, you're right. It's better without. The behaviour would be identical with or without the check for "*". I've removed it.
Backport of PR 14411 to v0.17. This change partially reverts bitcoin#13075 and bitcoin#14023. Fixes bitcoin#14382
c8d1f15 to
89306ab
Compare
|
Thanks for the review @ryanofsky! I've updated the PR with your changes. |
ryanofsky
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 89306ab. Only changes since last review were code simplification and release notes updates suggested above.
|
utACK 89306ab. |
|
utACK 89306ab This is slightly different from other backport PRs because the changes to |
…ng transactions by label 89306ab [wallet] Restore ability to list incoming transactions by label (Russell Yanofsky) Pull request description: Backport of PR #14411 to v0.17. This change partially reverts #13075 and #14023. Fixes #14382 Tree-SHA512: 1f8300e1a79e826cd706561265b8788deef505fa510be1a76ed9a62e5fca37cf6a741423ac0e5de2a36d6e8b9f25f141885455aacacbbf6474814e6eae406a27
… incoming transactions by label 89306ab [wallet] Restore ability to list incoming transactions by label (Russell Yanofsky) Pull request description: Backport of PR bitcoin#14411 to v0.17. This change partially reverts bitcoin#13075 and bitcoin#14023. Fixes bitcoin#14382 Tree-SHA512: 1f8300e1a79e826cd706561265b8788deef505fa510be1a76ed9a62e5fca37cf6a741423ac0e5de2a36d6e8b9f25f141885455aacacbbf6474814e6eae406a27
… to list incoming transactions by label" This reverts commit 96d3063.
… to list incoming transactions by label" This reverts commit 96d3063.
Backport bitcoin#14023, bitcoin#13825, bitcoin#14411 (and revert bitcoin#14441)
Backport of PR #14411 to v0.17.
This change partially reverts #13075 and #14023.
Fixes #14382