Disentangle progress estimation from checkpoints and update it #9472
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
Jan 4, 2017
| 1488, | ||
| 300 | ||
| // Data as of block 00000000c2872f8f8a8935c8e3c5862be9038c97d4de2cf37ed496991166928a (height 1063660) | ||
| 1483546230 |
There was a problem hiding this comment.
I think this might not compile due to missing ,?
|
utACK fec404013df40cfd767ae186723a94c23990c424 mod nit |
fec4040 to
f9a0275
Compare
|
Concept ACK. |
|
Just realized that this code is wrong: it assumes the SIGCHECK_VERIFICATION_FACTOR applies after the timestamp and not before (as opposed to after the last checkpoint). |
|
Suggestion: get rid of the SIGCHECK_VERIFICATION_FACTOR entirely (as it now applies to over 80% of the chain's transactions). |
|
I agree with getting rid of it. |
f9a0275 to
2ac28a1
Compare
|
Updated to remove SIGCHECK_VERIFICATION_FACTOR |
|
Concept ACK |
|
ACK 2ac28a1749ccc1e733c24b762b77ab099df48a3b |
TheBlueMatt
reviewed
Jan 7, 2017
|
|
||
| int64_t nNow = time(NULL); | ||
|
|
||
| double fTxTotal; |
There was a problem hiding this comment.
Dont we use "d" for floats to disambiguate flags?
There was a problem hiding this comment.
Post merge µNit: Seems this was not fixed.
There was a problem hiding this comment.
Bah, why are we using hungarian notation of variables in new code at all?
2ac28a1 to
df36371
Compare
|
ACK. |
laanwj
added a commit
that referenced
this pull request
Jan 12, 2017
…ate it df36371 Update estimated transaction count data (Pieter Wuille) e356d9a Shorten variable names and switch to tx/s (Pieter Wuille) 6dd8116 Remove SIGCHECK_VERIFICATION_FACTOR (Pieter Wuille) 3641141 Move tx estimation data out of CCheckPointData (Pieter Wuille) a4bac66 [MOVEONLY] Move progress estimation out of checkpoints (Pieter Wuille)
sipa
commented
Jan 12, 2017
|
|
||
| int64_t nNow = time(NULL); | ||
|
|
||
| double fTxTotal; |
There was a problem hiding this comment.
Because there is a persistent "imitate the surrounding code" suggestion. If we actually want to migrate away from using that style, I'm all for it, but it means we should start enforcing a new style in reviews.
codablock
pushed a commit
to codablock/dash
that referenced
this pull request
Jan 21, 2018
…and update it df36371 Update estimated transaction count data (Pieter Wuille) e356d9a Shorten variable names and switch to tx/s (Pieter Wuille) 6dd8116 Remove SIGCHECK_VERIFICATION_FACTOR (Pieter Wuille) 3641141 Move tx estimation data out of CCheckPointData (Pieter Wuille) a4bac66 [MOVEONLY] Move progress estimation out of checkpoints (Pieter Wuille)
andvgal
pushed a commit
to energicryptocurrency/gen2-energi
that referenced
this pull request
Jan 6, 2019
…and update it df36371 Update estimated transaction count data (Pieter Wuille) e356d9a Shorten variable names and switch to tx/s (Pieter Wuille) 6dd8116 Remove SIGCHECK_VERIFICATION_FACTOR (Pieter Wuille) 3641141 Move tx estimation data out of CCheckPointData (Pieter Wuille) a4bac66 [MOVEONLY] Move progress estimation out of checkpoints (Pieter Wuille)
CryptoCentric
pushed a commit
to absolute-community/absolute
that referenced
this pull request
Feb 27, 2019
CryptoCentric
pushed a commit
to absolute-community/absolute
that referenced
this pull request
Feb 27, 2019
…dablock committed on Jan 20, 2018 Use version 2 blocks for miner_tests … @codablock codablock committed on Jan 20, 2018 Merge bitcoin#7871: Manual block file pruning. … @laanwj @codablock laanwj authored and codablock committed on Jan 11, 2017 Merge bitcoin#9507: Fix use-after-free in CTxMemPool::removeConflicts() … @sipa @codablock sipa authored and codablock committed on Jan 11, 2017 Merge bitcoin#9297: Various RPC help outputs updated … @MarcoFalke @codablock MarcoFalke authored and codablock committed on Jan 12, 2017 Merge bitcoin#9416: travis: make distdir before make … @MarcoFalke @codablock MarcoFalke authored and codablock committed on Jan 12, 2017 Merge bitcoin#9520: Deprecate non-txindex getrawtransaction and bette… … @MarcoFalke @codablock MarcoFalke authored and codablock committed on Jan 12, 2017 Merge bitcoin#9518: Return height of last block pruned by pruneblockc… … @MarcoFalke @codablock MarcoFalke authored and codablock committed on Jan 12, 2017 Merge bitcoin#9472: Disentangle progress estimation from checkpoints … … @laanwj @codablock laanwj authored and codablock committed on Jan 12, 2017 Merge bitcoin#8883: Add all standard TXO types to bitcoin-tx … @laanwj @codablock laanwj authored and codablock committed on Jan 12, 2017 Merge bitcoin#9261: Add unstored orphans with rejected parents to rec… … @laanwj @codablock laanwj authored and codablock committed on Jan 12, 2017 Merge bitcoin#9468: [Depends] Dependency updates for 0.14.0 … @laanwj @codablock laanwj authored and codablock committed on Jan 12, 2017 Merge bitcoin#9222: Add 'subtractFeeFromAmount' option to 'fundrawtra… … @laanwj @codablock laanwj authored and codablock committed on Jan 12, 2017 Merge bitcoin#9490: Replace FindLatestBefore used by importmuti with … … @sipa @codablock sipa authored and codablock committed on Jan 13, 2017 Merge bitcoin#9469: [depends] Qt 5.7.1 … @laanwj @codablock laanwj authored and codablock committed on Jan 15, 2017 Merge bitcoin#9380: Separate different uses of minimum fees … @laanwj @codablock laanwj authored and codablock committed on Jan 16, 2017 Remove SegWit related code in dash-tx @codablock codablock committed on Sep 21, 2017 Merge bitcoin#9561: Wake message handling thread when we receive a ne… … @sipa @codablock sipa authored and codablock committed on Jan 17, 2017 Merge bitcoin#9508: Remove unused Python imports … @MarcoFalke @codablock MarcoFalke authored and codablock committed on Jan 18, 2017 Merge bitcoin#9512: Fix various things -fsanitize complains about
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.
Fixes #9448.
This moves the transaction estimation code out of the checkpoint code and data, as this information is not used for checkpoints (a consensus-relevant property). In the last commit, the estimated transaction count is updated for mainnet and testnet to blocks from a few days ago.