-
Notifications
You must be signed in to change notification settings - Fork 174
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
Tally after reorganize #756
Tally after reorganize #756
Conversation
Remove the incremental steps and tally after reorganize. Also tally every N block as in GridcoinServices.
Also remove the tallying while connecting blocks after reorganize. The PoR/PoS checks are skipped anyway.
// Perform Gridcoin services now that w have a new head. | ||
// Remove V9 checks after the V9 switch. | ||
if(IsV9Enabled(nBestHeight)) | ||
GridcoinServices(); |
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.
It looks like this is not necessary as ProcessBlock exits early if the block is not the next block in the main change. I will probably remove this commit.
Edit: Actually, when done here it also covers orphan attachments.
//10-6-2015 Make Reorganize work more gracefully - try up to 5 times to reorganize, each with an intermediate further back | ||
for (int iRegression = 0; iRegression < 5; iRegression++) | ||
// Reorganize is costly in terms of db load, as it works in a single db transaction. | ||
// Try to limit how much needs to be done inside |
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 is not entirely true. Less transactions makes for less database loads. Agreed that big txes are slower, but there are less of them.
|
||
// Retally after reorganize to sync up amounts owed. | ||
BusyWaitForTally_retired(); | ||
TallyNetworkAverages_v9(); |
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.
Please also have a look at main.cpp#L3431 GetLifetimeCPID()
call, because it does not work properly. It needs to be here, where memory index is consistent, an not where it is currently.
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.
Will do!
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.
Started adding this but we either need to remove the disconnected payments from StructCPID::PaymentAmountsResearch
(and its siblings) or we remove the variables entirely. They are only used in the CSV magnitude report and serve no other purpose.
Please notify me when it's final, I will have a look. More eyes, more attention. |
It's ready for review with the exception of the changes needed from #759 (or a more elaborate erasing when disconnecting blocks). |
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.
It is an improvement. But do not close the cpid/attach-detach issue.
src/main.cpp
Outdated
@@ -3388,7 +3388,7 @@ bool static Reorganize(CTxDB& txdb, CBlockIndex* pindexNew) | |||
printf("REORGANIZE: Disconnect %" PRIszu " blocks; %s..%s\n", vDisconnect.size(), pfork->GetBlockHash().ToString().substr(0,20).c_str(), pindexBest->GetBlockHash().ToString().substr(0,20).c_str()); | |||
printf("REORGANIZE: Connect %" PRIszu " blocks; %s..%s\n", vConnect.size(), pfork->GetBlockHash().ToString().substr(0,20).c_str(), pindexNew->GetBlockHash().ToString().substr(0,20).c_str()); | |||
|
|||
if (vDisconnect.size() > 0) | |||
if (vDisconnect.empty() == false) |
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.
One better than the other. Can you !empty?
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.
Done
Fixup for adding the third_party/googletest submodule.
This simplifies the reorganize procedure and ensures that we tally after reorganize. Previously it only tallied if it had to do a very large reorganize.
This also moves GridcoinServices from ProcessBlock to SetBestChain with v9. Previously it kept triggering while receiving blocks even though the best height didn't move.