[refactor] Merge getreceivedby tally into GetReceived function #17579
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maflcko
reviewed
Nov 24, 2019
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
andrewtoth
force-pushed
the
getreceivedby-refactor
branch
from
2377697 to
915a40a
Compare
andrewtoth
commented
Dec 2, 2019
src/wallet/rpcwallet.cpp
Outdated
Contributor
Author
There was a problem hiding this comment.
This uses the multiple address case, used by getreceivedbylabel, and the singular case used by getreceivedbyaddress is no longer needed.
theStack
reviewed
Jan 23, 2020
Contributor
theStack
left a comment
theStack
left a comment
There was a problem hiding this comment.
Concept ACK, left a view code review comments (mostly nits though)
andrewtoth
force-pushed
the
getreceivedby-refactor
branch
from
915a40a to
994ffd6
Compare
Contributor
Author
|
@theStack thank you for the review. I've addressed all comments. |
andrewtoth
force-pushed
the
getreceivedby-refactor
branch
2 times, most recently
from
dcd65c8 to
4111967
Compare
theStack
approved these changes
Jan 26, 2020
This was referenced Feb 11, 2020
Member
|
utACK 4111967281a37cfe0d34802a80dba0715bfe3ffb Nice cleanup, and also nice to hoist the DepthInMainChain check out of the inner loop. Optionally, since this is more than a pure code move, some of the variable names/comments could be improved as well (the "addresses" variable name, as well as the comment referring to pubkeys are outdated). |
maflcko
reviewed
Mar 27, 2020
andrewtoth
force-pushed
the
getreceivedby-refactor
branch
from
4111967 to
a1d5b12
Compare
theStack
approved these changes
Mar 29, 2020
maflcko
reviewed
Apr 20, 2020
Comment on lines
+600
to
+603
| // Minimum confirmations | ||
| int min_depth = 1; | ||
| if (!params[1].isNull()) | ||
| min_depth = params[1].get_int(); |
Member
There was a problem hiding this comment.
Suggested change
| // Minimum confirmations | |
| int min_depth = 1; | |
| if (!params[1].isNull()) | |
| min_depth = params[1].get_int(); | |
| const int min_depth{params[1].isNull() ? 1 : params[1].get_int()}; |
can be written shorter
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Apr 20, 2020
…eived function a1d5b12 Merge getreceivedby tally into GetReceived function (Andrew Toth) Pull request description: This PR merges the tally code of `getreceivedbyaddress` and `getreceivedbylabel` into a single function `GetReceived`. This reduces repeated code and makes it similar to `listreceivedbyaddress` and `listreceivedbylabel`, which use the function `ListReceived`. It will also make the change in bitcoin#14707 simpler and easier to review. ACKs for top commit: theStack: re-ACK bitcoin@a1d5b12 meshcollider: utACK a1d5b12 Tree-SHA512: 43d9cd92f7c2c6a8b9c7509aa85a9b9233a6cfec1c43a9062e3bdfb83515413d1feafa8938c828351278ba22bd31c47e62ab5341e4bddc2493103b094d73b047
jasonbcox
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Oct 15, 2020
…d function
Summary:
a1d5b12ec07d2f7aa9fa955a6dd99e8a2be5cb25 Merge getreceivedby tally into GetReceived function (Andrew Toth)
Pull request description:
This PR merges the tally code of `getreceivedbyaddress` and `getreceivedbylabel` into a single function `GetReceived`. This reduces repeated code and makes it similar to `listreceivedbyaddress` and `listreceivedbylabel`, which use the function `ListReceived`. It will also make the change in #14707 simpler and easier to review.
ACKs for top commit:
theStack:
re-ACK bitcoin/bitcoin@a1d5b12
meshcollider:
utACK a1d5b12ec07d2f7aa9fa955a6dd99e8a2be5cb25
---
Backport of Core [[bitcoin/bitcoin#17579 | PR17579]]
Test Plan:
ninja all check check-functional
Reviewers: #bitcoin_abc, jasonbcox
Reviewed By: #bitcoin_abc, jasonbcox
Differential Revision: https://reviews.bitcoinabc.org/D7939
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
7 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR merges the tally code of
getreceivedbyaddressandgetreceivedbylabelinto a single functionGetReceived. This reduces repeated code and makes it similar tolistreceivedbyaddressandlistreceivedbylabel, which use the functionListReceived. It will also make the change in #14707 simpler and easier to review.