Assertion failure in validation.cpp:4203 (re: pindexFirstNeverProcessed)

[ajtowns]

Reported by @rustyrussell on irc:

<rusty> Latest master branch, bitcoind in regtest mode:
bitcoind: validation.cpp:4203: void CheckBlockIndex(const Consensus::Params&): Assertion `(pindexFirstNeverProcessed != nullptr) == (pindex->nChainTx == 0)' failed.
<rusty> Pretty sure that was a .bitcoind dir from an older bitcoind.

I can reproduce this error with the following simple test case:

class RustyAssertTest(BitcoinTestFramework):
    def set_test_params(self):
        self.num_nodes = 1
        self.setup_clean_chain = True

    def run_test(self):
        self.log.info("initialise chain by activating segwit")
        self.restart_node(0, ["-vbparams=segwit:0:999999999999"])
        self.nodes[0].generate(500)
        assert_equal(get_bip9_status(self.nodes[0], 'segwit')['status'], 'active')

        self.log.info("restart with segwit always active")
        self.restart_node(0)

or by hand by running:

$ rm -rf ~/.bitcoin/regtest
$ ./bitcoind -regtest -vbparams=segwit:0:999999999999 -daemon
$ ./bitcoin-cli -regtest generate 500
$ ./bitcoin-cli -regtest stop
$ ./bitcoind -regtest
bitcoind: validation.cpp:4213: void CheckBlockIndex(const Consensus::Params&): Assertion `(pindexFirstNeverProcessed != nullptr) == (pindex->nChainTx == 0)' failed.
Aborted

I believe what's happening is:

• bitcoind is invoking RewindBlockIndex at startup, and seeing that segwit is immediately active (due to #11389), but it doesn't have witness data stored (because the blocks were generated with segwit only activating following bip9)
• so the blocks are disconnected and then their validity is reduced, by setting nTx and nChainTx both to 0
• once that's done, RewindBlockIndex calls CheckBlockIndex which is where the assertion fails
• pindexFirstNeverProcessed gets set quickly, because nTx is mostly 0, and it stays set provided there's a subnode, of which there should be plenty
• the assertion then fails when it hits a block where nChainTx != 0
• and that happens as soon as it gets to a block where segwit had activated under bip9 rules: since segwit had activated, that block was stored with witness data, and the validation reduction didn't occur, leaving nChainTx as whatever it had been -- this should be block 433 on regtest i think (144 blocks of started, 144 blocks active voting, 144 blocks locked in, and 1 block active)

I don't think this bug can be hit on mainnet or testnet -- running an old client will either not recognise segwit at all and never store any witness data (so nChainTx won't be non-zero), or will see segwit started at the exact same block the current client does (so won't reduce the validation of any blocks). It also shouldn't impact most future bip 9 (or similar) deployments, as most of those presumably won't need to change the storage format.

So I think this might just warrant a cleaner error message rather than handling it properly. Perhaps RewindBlockIndex's test should change from:

if (IsWitnessEnabled(pprev) && !BLOCK_OPT_WITNESS && !chainActive) { ... }

to something like:

if (IsWitnessEnabled(pprev)) {
    if (!BLOCK_OPT_WITNESS && !chainActive) {
        ...
    } else if (BLOCK_OPT_WITNESS && pprev->nChainTx == 0) {
        LogPrintf("segwit activation height has changed, cannot reuse blockchain");
        return false;
    }
}

[TheBlueMatt]

Looks like the same issue as #11767

[TheBlueMatt]

The other option is to fix the bug ala https://github.com/TheBlueMatt/bitcoin/commit/3b0575bd40c76dcd323bb470a50533c354d1c60b though I agree its probably not worth fixing, people should just wipe their regtest chain instead.

[maflcko]

Why wipe? The commitments are not required, a reindex should do if you didn't violate consensus, no?

On Nov 28, 2017 18:44, "Matt Corallo" notifications@github.com wrote:

The other option is to fix the bug ala TheBlueMatt/bitcoin@3b0575b https://github.com/TheBlueMatt/bitcoin/commit/3b0575bd40c76dcd323bb470a50533c354d1c60b though I agree its probably not worth fixing, people should just wipe their regtest chain instead.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/issues/11782#issuecomment-347702572, or mute the thread https://github.com/notifications/unsubscribe-auth/AGGmv6l54EQOnVII1nqQtqZiGkiCBPdMks5s7JrvgaJpZM4QtBse .

[ajtowns]

A reindex isn't sufficient to set BLOCK_OPT_WITNESS so bitcoind simply won't recognise the earlier blocks without redownloading them from a peer first:

$ rm -rf ~/.bitcoin/regtest
$ ./bitcoind -regtest -vbparams=segwit:0:999999999999 -daemon
$ ./bitcoin-cli -regtest generate 144
$ ./bitcoin-cli -regtest getblockchaininfo | grep -E 'blocks|headers'
  "blocks": 144,
  "headers": 144,
$ ./bitcoin-cli -regtest stop

$ ./bitcoind -regtest -daemon   # doesn't crash because segwit hadn't activated
$ ./bitcoin-cli -regtest getblockchaininfo | grep -E 'blocks|headers'
  "blocks": 0,
  "headers": 144,
$ ./bitcoin-cli -regtest stop

$ ./bitcoind -regtest -daemon -reindex
$ ./bitcoin-cli -regtest getblockchaininfo | grep -E 'blocks|headers'
  "blocks": 0,
  "headers": 144,

[ajtowns]

I had a go at making bitcoin give a more helpful error message in https://github.com/ajtowns/bitcoin/commits/rewindblockindex ; in particular https://github.com/ajtowns/bitcoin/commit/72e54daab692d000e3c282085f628fa5cd1ef927?diff=unified&w=1 . It seems to work, but I'm not sure if pprev can be relied upon to be valid at that point or if there's other things to be careful of though.

[maflcko]

The feature request didn't seem to attract much attention in the past. Also, the issue seems not important enough right now to keep it sitting around idle in the list of open issues.

Closing due to lack of interest. Pull requests with improvements are always welcome.