Merge pull request #194 from kcalvinalvin/2024-07-19-pruneblocks-bug-fix

wire, blockchain: don't use blockheightbyhash for height fetching

blockchain/utxocache.go

@@ -726,10 +726,18 @@ func (b *BlockChain) flushNeededAfterPrune(earliestHeight int32) (bool, error) {
 	if earliestHeight < 0 {
 		return false, nil
 	}
-	lastFlushHeight, err := b.BlockHeightByHash(&b.utxoCache.lastFlushHash)
-	if err != nil {
-		return false, err
-	}
-
+	node := b.index.LookupNode(&b.utxoCache.lastFlushHash)
+	if node == nil {
+		// If we couldn't find the node where we last flushed at, have the utxo cache
+		// flush to be safe and that will set the last flush hash again.
+		//
+		// This realistically should never happen as nodes are never deleted from
+		// the block index.  This happening likely means that there's a hardware
+		// error which is something we can't recover from.  The best that we can
+		// do here is to just force a flush and hope that the newly set
+		// lastFlushHash doesn't error.
+		return true, nil
+	}
+	lastFlushHeight := node.height
 	return earliestHeight > lastFlushHeight, nil
 }

blockchain/utxocache_test.go

@@ -717,18 +717,33 @@ func TestFlushOnPrune(t *testing.T) {
 	}
 
 	syncBlocks := func() {
+		// Modify block 1 to be a different hash.  This is to artificially
+		// create a stale branch in the chain.
+		staleMsgBlock := blocks[1].MsgBlock().Copy()
+		staleMsgBlock.Header.Nonce = 0
+		staleBlock := btcutil.NewBlock(staleMsgBlock)
+
+		// Add the stale block here to create a chain view like so. The
+		// block will be the main chain at first but become stale as we
+		// keep adding blocks.  BFNoPoWCheck is given as the pow check will
+		// fail.
+		//
+		// (genesis block) -> 1 -> 2 -> 3 -> ...
+		//                \-> 1a
+		_, _, err = chain.ProcessBlock(staleBlock, BFNoPoWCheck)
+		if err != nil {
+			t.Fatal(err)
+		}
+
 		for i, block := range blocks {
 			if i == 0 {
 				// Skip the genesis block.
 				continue
 			}
-			isMainChain, _, err := chain.ProcessBlock(block, BFNone)
+			_, _, err = chain.ProcessBlock(block, BFNone)
 			if err != nil {
-				t.Fatal(err)
-			}
-
-			if !isMainChain {
-				t.Fatalf("expected block %s to be on the main chain", block.Hash())
+				t.Fatalf("Failed to process block %v(%v). %v",
+					block.Hash().String(), block.Height(), err)
 			}
 		}
 	}
@@ -737,36 +752,40 @@ func TestFlushOnPrune(t *testing.T) {
 	ffldb.TstRunWithMaxBlockFileSize(chain.db, maxBlockFileSize, syncBlocks)
 
 	// Function that errors out if the block that should exist doesn't exist.
-	shouldExist := func(dbTx database.Tx, blockHash *chainhash.Hash) {
+	shouldExist := func(dbTx database.Tx, blockHash *chainhash.Hash) error {
 		bytes, err := dbTx.FetchBlock(blockHash)
 		if err != nil {
-			t.Fatal(err)
+			return err
 		}
 		block, err := btcutil.NewBlockFromBytes(bytes)
 		if err != nil {
-			t.Fatalf("didn't find block %v. %v", blockHash, err)
+			return fmt.Errorf("didn't find block %v. %v", blockHash, err)
 		}
 
 		if !block.Hash().IsEqual(blockHash) {
-			t.Fatalf("expected to find block %v but got %v",
+			return fmt.Errorf("expected to find block %v but got %v",
 				blockHash, block.Hash())
 		}
+
+		return nil
 	}
 
 	// Function that errors out if the block that shouldn't exist exists.
-	shouldNotExist := func(dbTx database.Tx, blockHash *chainhash.Hash) {
+	shouldNotExist := func(dbTx database.Tx, blockHash *chainhash.Hash) error {
 		bytes, err := dbTx.FetchBlock(chaincfg.MainNetParams.GenesisHash)
 		if err == nil {
-			t.Fatalf("expected block %s to be pruned", blockHash)
+			return fmt.Errorf("expected block %s to be pruned", blockHash.String())
 		}
 		if len(bytes) != 0 {
-			t.Fatalf("expected block %s to be pruned but got %v",
+			return fmt.Errorf("expected block %s to be pruned but got %v",
 				blockHash, bytes)
 		}
+
+		return nil
 	}
 
 	// The below code checks that the correct blocks were pruned.
-	chain.db.View(func(dbTx database.Tx) error {
+	err = chain.db.View(func(dbTx database.Tx) error {
 		exist := false
 		for _, block := range blocks {
 			// Blocks up to the last flush hash should not exist.
@@ -777,15 +796,23 @@ func TestFlushOnPrune(t *testing.T) {
 			}
 
 			if exist {
-				shouldExist(dbTx, block.Hash())
+				err = shouldExist(dbTx, block.Hash())
+				if err != nil {
+					return err
+				}
 			} else {
-				shouldNotExist(dbTx, block.Hash())
+				err = shouldNotExist(dbTx, block.Hash())
+				if err != nil {
+					return err
+				}
 			}
-
 		}
 
 		return nil
 	})
+	if err != nil {
+		t.Fatal(err)
+	}
 }
 
 func TestInitConsistentState(t *testing.T) {

wire/msgblock.go

@@ -54,6 +54,20 @@ type MsgBlock struct {
 	UData *UData
 }
 
+// Copy creates a deep copy of MsgBlock.
+func (msg *MsgBlock) Copy() *MsgBlock {
+	block := &MsgBlock{
+		Header:       msg.Header,
+		Transactions: make([]*MsgTx, len(msg.Transactions)),
+	}
+
+	for i, tx := range msg.Transactions {
+		block.Transactions[i] = tx.Copy()
+	}
+
+	return block
+}
+
 // AddTransaction adds a transaction to the message.
 func (msg *MsgBlock) AddTransaction(tx *MsgTx) error {
 	msg.Transactions = append(msg.Transactions, tx)