Fix rescanblockchain rpc to properly report progress #13079
Merged
Conversation
This file contains 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
skeees
reviewed
Apr 25, 2018
src/wallet/wallet.cpp
Outdated
| RescanVerificationProgress(const CWallet& wallet, const CBlockIndex* const stop_block) | ||
| : m_wallet(wallet), m_time(GetTime()), m_stop_block(stop_block) {} | ||
|
|
||
| void Start(const CBlockIndex* const start_block) |
There was a problem hiding this comment.
simpler to RAII this instead of a separate Constructor/Start/AnnounceStart methods?
There was a problem hiding this comment.
The main reason I kept this separate is that GuessVerificationProgress needs cs_main held, and I wanted to minimize the amount of code in the lock-block.
Empact
force-pushed
the
scan-for-wallet-stopblock-progress
branch
from
b5fe7a2
to
409db40
Compare
Empact
changed the title
|
Removed the refactoring for easier review. |
|
Makes sense. Reporting progress for partial rescans based on the given range seems to be more logical. |
maflcko
reviewed
Apr 30, 2018
src/wallet/wallet.cpp
Outdated
| @@ -1742,9 +1742,13 @@ CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, CBlock | |||
| double dProgressTip; | |||
There was a problem hiding this comment.
should probably be called progress_end
maflcko
reviewed
Apr 30, 2018
src/wallet/wallet.cpp
Outdated
| dProgressStart = GuessVerificationProgress(chainParams.TxData(), pindex); | ||
| dProgressTip = GuessVerificationProgress(chainParams.TxData(), tip); | ||
| if (pindexStop == nullptr) { | ||
| tip = chainActive.Tip() |
There was a problem hiding this comment.
missing semicolon, also wrong indentation.
|
utACK 409db40 |
|
From travis: |
Empact
force-pushed
the
scan-for-wallet-stopblock-progress
branch
from
409db40
to
d700433
Compare
|
Added semicolon, renamed progress vars to |
Empact
changed the title
CWallet::ScanForWalletTransactions did not previously take into account pindexStop when calculating progress. Renamed progress vars to progress_*. rescanblockchain is the only rpc that uses this parameter.
Empact
force-pushed
the
scan-for-wallet-stopblock-progress
branch
from
d700433
to
16be133
Compare
|
Rebased |
|
utACK 16be133 |
1 similar comment
|
utACK 16be133 |
|
Tested ACK 16be133 |
jonasschnelli
added a commit
that referenced
this pull request
May 7, 2018
16be133 Fix rescanblockchain rpc to property report progress (Ben Woosley) Pull request description: Previously it assumed tip in all cases. This also extracts a RescanVerificationProgress helper object whose role is to manage reporting, in order to simplify ScanForWalletTransactions - more lines in total, but much simpler to follow the core logic. Tree-SHA512: 5ebed0c56fae4ccfe613ff1d7082cb6da5a86635a8993ed3af70b500a4ea43074121aea9219b2f0321fbfeb7efcb964bdc2199297a64ca0fa85d9d07aa637d40
|
utACK 16be133. |
jasonbcox
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Sep 27, 2019
Summary: 16be133 Fix rescanblockchain rpc to property report progress (Ben Woosley) Pull request description: Previously it assumed tip in all cases. This also extracts a RescanVerificationProgress helper object whose role is to manage reporting, in order to simplify ScanForWalletTransactions - more lines in total, but much simpler to follow the core logic. Tree-SHA512: 5ebed0c56fae4ccfe613ff1d7082cb6da5a86635a8993ed3af70b500a4ea43074121aea9219b2f0321fbfeb7efcb964bdc2199297a64ca0fa85d9d07aa637d40 Backport of Core PR13079 bitcoin/bitcoin#13079 Test Plan: make check ./bitcoind --printtoconsole ./bitcoin-cli rescanblockchain 0 1300000 Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc Reviewed By: deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc Differential Revision: https://reviews.bitcoinabc.org/D4112
jonspock
pushed a commit
to jonspock/devault
that referenced
this pull request
Dec 24, 2019
Summary: 16be13345 Fix rescanblockchain rpc to property report progress (Ben Woosley) Pull request description: Previously it assumed tip in all cases. This also extracts a RescanVerificationProgress helper object whose role is to manage reporting, in order to simplify ScanForWalletTransactions - more lines in total, but much simpler to follow the core logic. Tree-SHA512: 5ebed0c56fae4ccfe613ff1d7082cb6da5a86635a8993ed3af70b500a4ea43074121aea9219b2f0321fbfeb7efcb964bdc2199297a64ca0fa85d9d07aa637d40 Backport of Core PR13079 bitcoin/bitcoin#13079 Test Plan: make check ./bitcoind --printtoconsole ./bitcoin-cli rescanblockchain 0 1300000 Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc Reviewed By: deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc Differential Revision: https://reviews.bitcoinabc.org/D4112
jonspock
pushed a commit
to jonspock/devault
that referenced
this pull request
Dec 26, 2019
Summary: 16be13345 Fix rescanblockchain rpc to property report progress (Ben Woosley) Pull request description: Previously it assumed tip in all cases. This also extracts a RescanVerificationProgress helper object whose role is to manage reporting, in order to simplify ScanForWalletTransactions - more lines in total, but much simpler to follow the core logic. Tree-SHA512: 5ebed0c56fae4ccfe613ff1d7082cb6da5a86635a8993ed3af70b500a4ea43074121aea9219b2f0321fbfeb7efcb964bdc2199297a64ca0fa85d9d07aa637d40 Backport of Core PR13079 bitcoin/bitcoin#13079 Test Plan: make check ./bitcoind --printtoconsole ./bitcoin-cli rescanblockchain 0 1300000 Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc Reviewed By: deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc Differential Revision: https://reviews.bitcoinabc.org/D4112
jonspock
pushed a commit
to devaultcrypto/devault
that referenced
this pull request
Dec 27, 2019
Summary: 16be13345 Fix rescanblockchain rpc to property report progress (Ben Woosley) Pull request description: Previously it assumed tip in all cases. This also extracts a RescanVerificationProgress helper object whose role is to manage reporting, in order to simplify ScanForWalletTransactions - more lines in total, but much simpler to follow the core logic. Tree-SHA512: 5ebed0c56fae4ccfe613ff1d7082cb6da5a86635a8993ed3af70b500a4ea43074121aea9219b2f0321fbfeb7efcb964bdc2199297a64ca0fa85d9d07aa637d40 Backport of Core PR13079 bitcoin/bitcoin#13079 Test Plan: make check ./bitcoind --printtoconsole ./bitcoin-cli rescanblockchain 0 1300000 Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc Reviewed By: deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc Differential Revision: https://reviews.bitcoinabc.org/D4112
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jun 17, 2020
…ress 16be133 Fix rescanblockchain rpc to property report progress (Ben Woosley) Pull request description: Previously it assumed tip in all cases. This also extracts a RescanVerificationProgress helper object whose role is to manage reporting, in order to simplify ScanForWalletTransactions - more lines in total, but much simpler to follow the core logic. Tree-SHA512: 5ebed0c56fae4ccfe613ff1d7082cb6da5a86635a8993ed3af70b500a4ea43074121aea9219b2f0321fbfeb7efcb964bdc2199297a64ca0fa85d9d07aa637d40
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jun 27, 2020
…ress 16be133 Fix rescanblockchain rpc to property report progress (Ben Woosley) Pull request description: Previously it assumed tip in all cases. This also extracts a RescanVerificationProgress helper object whose role is to manage reporting, in order to simplify ScanForWalletTransactions - more lines in total, but much simpler to follow the core logic. Tree-SHA512: 5ebed0c56fae4ccfe613ff1d7082cb6da5a86635a8993ed3af70b500a4ea43074121aea9219b2f0321fbfeb7efcb964bdc2199297a64ca0fa85d9d07aa637d40
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jun 28, 2020
…ress 16be133 Fix rescanblockchain rpc to property report progress (Ben Woosley) Pull request description: Previously it assumed tip in all cases. This also extracts a RescanVerificationProgress helper object whose role is to manage reporting, in order to simplify ScanForWalletTransactions - more lines in total, but much simpler to follow the core logic. Tree-SHA512: 5ebed0c56fae4ccfe613ff1d7082cb6da5a86635a8993ed3af70b500a4ea43074121aea9219b2f0321fbfeb7efcb964bdc2199297a64ca0fa85d9d07aa637d40
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Previously it assumed tip in all cases. This also extracts a RescanVerificationProgress helper object whose role is to manage reporting, in order to simplify ScanForWalletTransactions - more lines in total, but much simpler to follow the core logic.