[Main] Automate database corruption fix caused by out of sync txdb. #405

presstab · 2017-11-21T22:44:12Z

Summary:

Refactor the automated db corruption repair code to be based on comparing pcoinsTip's best block, with the best block loaded from block index db. If the wallet client is closed forcefully before it completes CheckBlock() it causes a state where the block index is stored to disk but the transaction data is not. The automated repair code will detect on initialization when there is a mismatch between the two databases. If the transaction database is behind the block index database, each block from the best height in the tx database up until the best block of the block index database will be parsed and all transactions recorded and flushed to disk.
On block 908000 a block hash checkpoint is added. If the validation process fails to validate block 908000 because it was previously marked invalid, the user will not need to use the reconsiderblock RPC command to clear the status. The code will now see that they are on block 908000, check if the blockhash matches the checkpoint, and reprocess the block.
On initial block download (IDB), the wallet client is not triggered to calculate the invalid outpoint map. The invalid outpoint map is used to calculate the accumulator checkpoint for block 908000, so if it never gets populated, then a checkpoint will be generated that does not match the blockheader's accumulator checkpoint, which will result in the block being rejected. Before calculating the checkpoint for 908000 the code now forces the population of the invalid outpoint map, avoiding this scenario.
Around 908000 many nodes have a zerocoinDB that is not in its proper state with accumulators properly calculated. If an accumulator checkpoint is rejected in close proximity to block 908000, then a reindexing of the accumulators will automatically be done, and the checkpoint recalculated in an attempt to fix this issue.
Deprecate spork_11 and replace it with a value called by Params()

Tested Scenarios:

Sync from scratch - works.
Reindex - works.
Blockchain snapshot that synced from scratch, but got stuck at block 907999 - fixed by running the new wallet.
Blockchain snapshot that was experiencing assert(hashPrevBlock == view.GetBestBlock()); from ConnectBlock() due to having a txdb that was behind the blockchain db : fixed by running the new wallet. Note: This is the issue that the majority of nodes that are stuck on block 907999 will have because the wallet froze in the middle of ConnectBlock() causing the txdb to not be flushed to disk.
Blockchain snapshot that was experiencing assert(hashPrevBlock == view.GetBestBlock()); from ConnectBlock() due to having a txdb that was ahead of the blockchain db : fixed by restarting wallet with -reindex.

Mrs-X

if (Checkpoints::CheckBlock(pindex->nHeight - 1, block.hashPrevBlock, true)) {

pindex might be NULL if the block header is not yet known

presstab · 2017-11-21T23:44:53Z

@Mrs-X good find. Changed to:
if (pindex && Checkpoints::CheckBlock(pindex->nHeight - 1, block.hashPrevBlock, true)) {

Fuzzbawls · 2017-11-22T08:08:55Z

confirmed that a fresh sync using a bootstrap file now properly handles the accumulator/supply recalculations at block 908000

jonspock · 2017-11-22T17:12:46Z

src/init.cpp

-                        if (pindex->GetBlockHeader().nVersion >= Params().Zerocoin_HeaderVersion())
-                            nZerocoinStart = pindex->nHeight;
-                        else if (nZerocoinStart)
-                            break;


Would have helped to know you were looping backwards here. (comment). Also would a "==" have worked?

jonspock · 2017-11-22T17:15:50Z

src/init.cpp

-                                    // GetCheckpoint could have terminated due to a shutdown request. Check this here.
-                                    if (ShutdownRequested())
-                                        break;
-                                    return InitError(_("Failed to calculate accumulator checkpoint"));


what would the reason be for failure here?

jonspock · 2017-11-22T17:18:19Z

src/main.cpp

@@ -3196,6 +3198,59 @@ bool RecalculatePIVSupply(int nHeightStart)
    return true;
 }

+bool ReindexAccumulators(list<uint256> listMissingCheckpoints, string& strError)


Shouldn't list be a Const Reference here? Such as "const list<uint256> & listMissingCheckpoints" ? Otherwise the whole list is needlessly copied. If not a "const" input, then it would need to be a reference if it's an output, otherwise only modified internal to this routine

The way I have it coded, it erases a checkpoint from listMissingCheckpoints once it finds/processes it. Unable to do that with const ref.

OK, then why not just regular reference?

jonspock · 2017-11-22T17:22:48Z

src/main.cpp

+bool ReindexAccumulators(list<uint256> listMissingCheckpoints, string& strError)
+{
+    // PIVX: recalculate Accumulator Checkpoints that failed to database properly
+    if (!listMissingCheckpoints.empty() && chainActive.Height() >= Params().Zerocoin_StartHeight()) {


Below looks like a cut-and-paste section from earlier code? Is it worthwhile making a function?

That is exactly what this code is. You were commenting on the removed code above.

It was formerly not a function and just a one time procedure ran in init.cpp. Now it is accessible anytime with ReindexAccumulators()

jonspock · 2017-11-22T17:26:23Z

src/main.cpp

+                }
+
+                string strError;
+                if (!ReindexAccumulators(listCheckpoints, strError) || !CalculateAccumulatorCheckpoint(pindex->nHeight, nCheckpointCalculated))


Is it OK to pass a potentially empty "listCheckpoints" to ReindexAccumulators?

Empty list will skip the procedure within the function and return true.

jonspock · 2017-11-22T17:28:31Z

src/main.cpp

+            } else {
+                return state.DoS(100, error("ConnectBlock() : failed to calculate accumulator checkpoint"));
+            }
+        }

        if (nCheckpointCalculated != block.nAccumulatorCheckpoint) {
            LogPrintf("%s: block=%d calculated: %s\n block: %s\n", __func__, pindex->nHeight, nCheckpointCalculated.GetHex(), block.nAccumulatorCheckpoint.GetHex());
            return state.DoS(100, error("ConnectBlock() : accumulator does not match calculated value"));


if possible more text plausing possible reasons for errors would be quite useful to other people looking at this code/error message, as it's difficult to reason through the various levels as to why something is happening

Good suggestion. Adding an error message that will feed into the reindex popup.

jonspock · 2017-11-22T17:30:31Z

src/main.cpp

@@ -4665,9 +4758,20 @@ bool AcceptBlock(CBlock& block, CValidationState& state, CBlockIndex** ppindex,
        if (mi == mapBlockIndex.end())
            return state.DoS(0, error("%s : prev block %s not found", __func__, block.hashPrevBlock.ToString().c_str()), 0, "bad-prevblk");
        pindexPrev = (*mi).second;
-        if (pindexPrev->nStatus & BLOCK_FAILED_MASK)


Repeated code below?

jonspock · 2017-11-22T17:39:00Z

src/main.cpp

+        while (nSortedPos < vSortedByHeight.size()) {
+            CBlock block;
+            assert(ReadBlockFromDisk(block, pindex));
+


Would be good to have a pop-up or some explanation for user rather than just asserting and crashing

… list.

Mrs-X · 2017-11-24T21:36:58Z

utACK and merging...

…sync txdb. 8469261 Deprecate spork_11. (presstab) 15c355b Ask for sporks from 1st peer. Consider Verifying Blocks as IDB. (presstab) 43af66b Add message popup when db corrupt. Make ReindexAccumulators() use ref list. (presstab) 3eabaa2 Remove unused code and add null check. (presstab) 5654732 Automate database corruption fix caused by out of sync txdb. (presstab) Tree-SHA512: 6c612d7a8e6a80c456514c402afb161932d947fa9761544ab6f53df6932969095bfd4022a2f426913bcf1456fc5369b654c58cbfd5065081ee29634843dfe7c4

Github-Merge: PIVX-Project#405 Rebased-From: 5654732

Github-Merge: PIVX-Project#405 Rebased-From: 3eabaa2

… list. Github-Merge: PIVX-Project#405 Rebased-From: 43af66b

Github-Merge: PIVX-Project#405 Rebased-From: 15c355b

Github-Merge: PIVX-Project#405 Rebased-From: 8469261

… list. Github-Merge: PIVX-Project#405 Rebased-From: 43af66b

Github-Merge: PIVX-Project#405 Rebased-From: 15c355b

Remove invalid old invalid output logic from Dash fork. Github-Merge: PIVX-Project#405 Rebased-From: 8469261

… list. Github-Merge: PIVX-Project#405 Rebased-From: 43af66b

Github-Merge: PIVX-Project#405 Rebased-From: 15c355b

Remove invalid old invalid output logic from Dash fork. Github-Merge: PIVX-Project#405 Rebased-From: 8469261

Automate database corruption fix caused by out of sync txdb.

5654732

ghost assigned presstab Nov 21, 2017

ghost added the review label Nov 21, 2017

presstab changed the title ~~Automate database corruption fix caused by out of sync txdb.~~ [Main] Automate database corruption fix caused by out of sync txdb. Nov 21, 2017

presstab assigned presstab and unassigned presstab Nov 21, 2017

presstab added Block Storage Bug Data Corruption Usability Validation labels Nov 21, 2017

Mrs-X suggested changes Nov 21, 2017

View reviewed changes

Remove unused code and add null check.

3eabaa2

presstab removed the Bug label Nov 22, 2017

Mrs-X approved these changes Nov 22, 2017

View reviewed changes

jonspock reviewed Nov 22, 2017

View reviewed changes

presstab added 3 commits November 22, 2017 15:24

Add message popup when db corrupt. Make ReindexAccumulators() use ref…

43af66b

… list.

Ask for sporks from 1st peer. Consider Verifying Blocks as IDB.

15c355b

Deprecate spork_11.

8469261

presstab mentioned this pull request Nov 23, 2017

Failed reading from database. pcoinsTip best block is not correct. The block database is in an inconsistent state and may cause issues in the future.To force start use -forcestart #407

Closed

presstab added this to the 3.0.6 milestone Nov 24, 2017

Mrs-X merged commit 8469261 into PIVX-Project:master Nov 24, 2017

ghost removed the review label Nov 24, 2017

presstab mentioned this pull request Nov 28, 2017

Fix edge case segfault. #418

Merged

Fuzzbawls pushed a commit to Fuzzbawls/PIVX that referenced this pull request Nov 29, 2017

Automate database corruption fix caused by out of sync txdb.

76a7e0f

Github-Merge: PIVX-Project#405 Rebased-From: 5654732

Fuzzbawls pushed a commit to Fuzzbawls/PIVX that referenced this pull request Nov 29, 2017

Remove unused code and add null check.

fdaea40

Github-Merge: PIVX-Project#405 Rebased-From: 3eabaa2

Fuzzbawls pushed a commit to Fuzzbawls/PIVX that referenced this pull request Nov 29, 2017

Add message popup when db corrupt. Make ReindexAccumulators() use ref…

ff24d3a

… list. Github-Merge: PIVX-Project#405 Rebased-From: 43af66b

Fuzzbawls pushed a commit to Fuzzbawls/PIVX that referenced this pull request Nov 29, 2017

Ask for sporks from 1st peer. Consider Verifying Blocks as IDB.

77583c4

Github-Merge: PIVX-Project#405 Rebased-From: 15c355b

Fuzzbawls pushed a commit to Fuzzbawls/PIVX that referenced this pull request Nov 29, 2017

Deprecate spork_11.

37a39fc

Github-Merge: PIVX-Project#405 Rebased-From: 8469261

meyer9 pushed a commit to phoreproject/Phore that referenced this pull request Feb 15, 2018

Add message popup when db corrupt. Make ReindexAccumulators() use ref…

bcd6d12

… list. Github-Merge: PIVX-Project#405 Rebased-From: 43af66b

meyer9 pushed a commit to phoreproject/Phore that referenced this pull request Feb 15, 2018

Ask for sporks from 1st peer. Consider Verifying Blocks as IDB.

bace928

Github-Merge: PIVX-Project#405 Rebased-From: 15c355b

meyer9 pushed a commit to phoreproject/Phore that referenced this pull request Feb 15, 2018

Deprecate spork_11.

cd20783

Remove invalid old invalid output logic from Dash fork. Github-Merge: PIVX-Project#405 Rebased-From: 8469261

meyer9 pushed a commit to phoreproject/Phore that referenced this pull request Feb 20, 2018

Add message popup when db corrupt. Make ReindexAccumulators() use ref…

5447e11

… list. Github-Merge: PIVX-Project#405 Rebased-From: 43af66b

meyer9 pushed a commit to phoreproject/Phore that referenced this pull request Feb 20, 2018

Ask for sporks from 1st peer. Consider Verifying Blocks as IDB.

ca7826c

Github-Merge: PIVX-Project#405 Rebased-From: 15c355b

meyer9 pushed a commit to phoreproject/Phore that referenced this pull request Feb 20, 2018

Deprecate spork_11.

a89bb6a

Remove invalid old invalid output logic from Dash fork. Github-Merge: PIVX-Project#405 Rebased-From: 8469261

akshaynexus mentioned this pull request Feb 5, 2019

Fix bestblock error assertion dogecash/dogecash-oldchain#12

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Main] Automate database corruption fix caused by out of sync txdb. #405

[Main] Automate database corruption fix caused by out of sync txdb. #405

presstab commented Nov 21, 2017 •

edited

Loading

Mrs-X left a comment

presstab commented Nov 21, 2017

Fuzzbawls commented Nov 22, 2017

jonspock Nov 22, 2017

jonspock Nov 22, 2017

jonspock Nov 22, 2017

presstab Nov 22, 2017

jonspock Nov 22, 2017

presstab Nov 22, 2017

jonspock Nov 22, 2017

presstab Nov 22, 2017

jonspock Nov 22, 2017

presstab Nov 22, 2017

jonspock Nov 22, 2017

presstab Nov 22, 2017

jonspock Nov 22, 2017

jonspock Nov 22, 2017

Mrs-X commented Nov 24, 2017

[Main] Automate database corruption fix caused by out of sync txdb. #405

[Main] Automate database corruption fix caused by out of sync txdb. #405

Conversation

presstab commented Nov 21, 2017 • edited Loading

Mrs-X left a comment

Choose a reason for hiding this comment

presstab commented Nov 21, 2017

Fuzzbawls commented Nov 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mrs-X commented Nov 24, 2017

presstab commented Nov 21, 2017 •

edited

Loading