Merge bitcoin/bitcoin#24851: init: ignore BIP-30 verification in DisconnectBlock for problematic blocks

e899d4ca6f44785b6e5481e200a6a9a8d2448612 init: limit bip30 exceptions to coinbase txs (Chris Geihsler)
511eb7fdeac36da9d6576c878a1c8d390b38f1bd Ignore problematic blocks in DisconnectBlock (Chris Geihsler)

Pull request description:

  Fixes https://github.com/bitcoin/bitcoin/issues/22596

  When using checklevel=4, block verification fails because of duplicate coinbase transactions involving blocks 91812 and 91722. There was already a check in place within `ConnectBlock` to ignore the problematic blocks, but `DisconnectBlock` did not contain a similar check to ignore these blocks when called from `VerifyDB`.

  By ignoring these two blocks in `DisconnectBlock`, the block verification process succeeds at checklevel=4.

  (Note to reviewers: this is my first contribution to Bitcoin Core, so any feedback is most welcome. Thanks in advance for reviewing!)

  ## Steps to reproduce:

  Use the following bitcoin.conf file and start bitcoind. I only used block data through block ~100000 so that the verification process was much faster.

  ```
  assumevalid=0
  checkblocks=0
  checklevel=4
  ```

  Without this change, you will see the following error when the blocks are verified:

  ```
  2022-04-14T02:56:44Z init message: Verifying blocks…
  2022-04-14T02:56:44Z Verifying last 101881 blocks at level 4
  2022-04-14T02:56:44Z [0%]...[10%]...[20%]...[30%]...[40%]...ERROR: VerifyDB(): *** coin database inconsistencies found (last 10160 blocks, 142571 good transactions before that)

  2022-04-14T02:57:01Z : Corrupted block database detected.
  Please restart with -reindex or -reindex-chainstate to recover.
  : Corrupted block database detected.
  Please restart with -reindex or -reindex-chainstate to recover.
  ```

  With this change, you will see this instead:

  ```
  2022-04-14T02:32:29Z init message: Verifying blocks…
  2022-04-14T02:32:29Z Verifying last 101746 blocks at level 4
  2022-04-14T02:32:29Z [0%]...[10%]...[20%]...[30%]...[40%]...[50%]...[60%]...[70%]...[80%]...[90%]...[DONE].
  2022-04-14T02:32:48Z No coin database inconsistencies in last 101746 blocks (226126 transactions)
  ```

ACKs for top commit:
  laanwj:
    Code review ACK e899d4ca6f44785b6e5481e200a6a9a8d2448612
  achow101:
    ACK e899d4ca6f44785b6e5481e200a6a9a8d2448612
  jamesob:
    (Biased) ACK e899d4ca6f44785b6e5481e200a6a9a8d2448612 ([`jamesob/ackr/24851.2.seejee.init_ignore_bip_30_verif`](https://github.com/jamesob/bitcoin/tree/ackr/24851.2.seejee.init_ignore_bip_30_verif))

Tree-SHA512: d2f6d25e9619aee32c1a73fe846b1b587698eaa5a4994fa6424f1038f45654f9fd52b74a69843cc84d90168d74827130ccf8e9201502f5d52281acdb20429291

1 files changed, 13 insertions, 1 deletions

diff --git a/src/validation.cpp b/src/validation.cpp
index 4941d2bcb6..c09d442b85 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1838,11 +1838,21 @@ DisconnectResult Chainstate::DisconnectBlock(const CBlock& block, const CBlockIn
         return DISCONNECT_FAILED;
     }
 
+    // Ignore blocks that contain transactions which are 'overwritten' by later transactions,
+    // unless those are already completely spent.
+    // See https://github.com/bitcoin/bitcoin/issues/22596 for additional information.
+    // Note: the blocks specified here are different than the ones used in ConnectBlock because DisconnectBlock
+    // unwinds the blocks in reverse. As a result, the inconsistency is not discovered until the earlier
+    // blocks with the duplicate coinbase transactions are disconnected.
+    bool fEnforceBIP30 = !((pindex->nHeight==91722 && pindex->GetBlockHash() == uint256S("0x00000000000271a2dc26e7667f8419f2e15416dc6955e5a6c6cdf3f2574dd08e")) ||
+                           (pindex->nHeight==91812 && pindex->GetBlockHash() == uint256S("0x00000000000af0aed4792b1acee3d966af36cf5def14935db8de83d6f9306f2f")));
+
     // undo transactions in reverse order
     for (int i = block.vtx.size() - 1; i >= 0; i--) {
         const CTransaction &tx = *(block.vtx[i]);
         uint256 hash = tx.GetHash();
         bool is_coinbase = tx.IsCoinBase();
+        bool is_bip30_exception = (is_coinbase && !fEnforceBIP30);
 
         // Check that all outputs are available and match the outputs in the block itself
         // exactly.
@@ -1852,7 +1862,9 @@ DisconnectResult Chainstate::DisconnectBlock(const CBlock& block, const CBlockIn
                 Coin coin;
                 bool is_spent = view.SpendCoin(out, &coin);
                 if (!is_spent || tx.vout[o] != coin.out || pindex->nHeight != coin.nHeight || is_coinbase != coin.fCoinBase) {
-                    fClean = false; // transaction output mismatch
+                    if (!is_bip30_exception) {
+                        fClean = false; // transaction output mismatch
+                    }
                 }
             }
         }