-
Notifications
You must be signed in to change notification settings - Fork 38.7k
Avoid unused call to GuessVerificationProgress in NotifyHeaderTip #17250
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
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.
Anyway, this result was never used
Isn't progress used here even for an header notification
Line 991 in be50469
| progressBar->setValue(nVerificationProgress * 1000000000.0 + 0.5); |
is always 0 because the number of txs in the block can not be determined from the header alone
Right, nChainTx is not known, but I fail to see how it's zero:
Lines 5061 to 5076 in be50469
| double GuessVerificationProgress(const ChainTxData& data, const CBlockIndex *pindex) { | |
| if (pindex == nullptr) | |
| return 0.0; | |
| int64_t nNow = time(nullptr); | |
| double fTxTotal; | |
| if (pindex->nChainTx <= data.nTxCount) { | |
| fTxTotal = data.nTxCount + (nNow - data.nTime) * data.dTxRate; | |
| } else { | |
| fTxTotal = pindex->nChainTx + (nNow - pindex->GetBlockTime()) * data.dTxRate; | |
| } | |
| return pindex->nChainTx / fTxTotal; | |
| } |
It is initialized to zero: https://dev.visucore.com/bitcoin/doxygen/class_c_block_index.html#af3c6d6dd8a7579e5ce516d94b98d2db5 and only set when we have the block downloaded. Without having the transactions downloaded, it is currently impossible to know how many txs there are in the block. |
|
Then |
Code review ACK fa39809, missed that. |
|
ACK fa39809 |
…yHeaderTip fa39809 Avoid unused call to GuessVerificationProgress in NotifyHeaderTip (MarcoFalke) Pull request description: `GuessVerificationProgress` for a header (not a block) is always 0 because the number of txs in the block can not be determined from the header alone. Anyway, this result was never used, so we can optimize this call by hardcoding 0. This is the next commit in a series of changes toward removing nChainTx (see #14863, #13875) ACKs for top commit: promag: Code review ACK fa39809, missed that. laanwj: ACK fa39809 Tree-SHA512: 11016f8dbb1af1cf75241948d1ad35eac0c79d1311cd0db8c6ec806df2a9e3dc5f998dbd66ccbad5d84564e6cec7fe21ce7a2a13c2b34c746e2d3b31aa1db53a
Summary: > GuessVerificationProgress for a header (not a block) is always 0 because the number of txs in the block can not be determined from the header alone. Anyway, this result was never used, so we can optimize this call by hardcoding 0. > > This is the next commit in a series of changes toward removing nChainTx (see #14863, #13875) This is a backport of Core [[bitcoin/bitcoin#17250 | PR17250]] Test Plan: `ninja && ninja check-all` Reviewers: O1 Bitcoin ABC, #bitcoin_abc, jasonbcox Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, jasonbcox Differential Revision: https://reviews.bitcoinabc.org/D8201
GuessVerificationProgressfor a header (not a block) is always 0 because the number of txs in the block can not be determined from the header alone. Anyway, this result was never used, so we can optimize this call by hardcoding 0.This is the next commit in a series of changes toward removing nChainTx (see #14863, #13875)