blockchain: don't flush blockNodes that we don't have the data for

On flushes to the database, we check that the blockNodes we have for
the downloaded block headers are not flushed to the disk unless the
block data is stored as well for backwards compatibility.

With older btcd clients, they rely on the fact that the blockNode is
present to check if the block data is also present. Since we now
store blockNodes for just the block headers, this is no longer true.

Because of this, we don't flush the blockNodes if there's no
accompanying block data for it. This results in downloading and
verifying the headers again if the node were to restart but since the
header data is small and the verification is quick, it's not a big
downside.

As an optimization, flushToDB now skips opening a write transaction
entirely when every dirty node is header-only. This avoids a no-op
write transaction on every ProcessBlockHeader call during header sync.

blockchain/blockindex.go

@@ -505,8 +505,37 @@ func (bi *blockIndex) flushToDB() error {
 		return nil
 	}
 
+	// Check if any dirty node actually needs to be written.  Header-only
+	// nodes are skipped for backwards compatibility (see NOTE below), so
+	// if every dirty node is header-only, we can avoid opening a write
+	// transaction entirely.  This matters during header sync where every
+	// ProcessBlockHeader call would otherwise open a no-op write txn.
+	needsWrite := false
+	for node := range bi.dirty {
+		if node.status.HaveData() {
+			needsWrite = true
+			break
+		}
+	}
+	if !needsWrite {
+		bi.dirty = make(map[*blockNode]struct{})
+		bi.Unlock()
+		return nil
+	}
+
 	err := bi.db.Update(func(dbTx database.Tx) error {
 		for node := range bi.dirty {
+			// NOTE: we specifically don't flush the block indexes that
+			// we don't have the data for backwards compatibility.
+			// While flushing would save us the work of re-downloading
+			// the block headers upon restart, if the user were to start
+			// up a btcd node with an older version, it would result in
+			// an unrecoverable error as older versions would consider a
+			// blockNode being present as having the block data as well.
+			if node.status.HaveHeader() &&
+				!node.status.HaveData() {
+				continue
+			}
 			err := dbStoreBlockNode(dbTx, node)
 			if err != nil {
 				return err

blockchain/blockindex_test.go

@@ -1,4 +1,4 @@
-// Copyright (c) 2023 The utreexo developers
+// Copyright (c) 2015-2026 The btcsuite developers
 // Use of this source code is governed by an ISC
 // license that can be found in the LICENSE file.
 
@@ -7,8 +7,146 @@ package blockchain
 import (
 	"math/rand"
 	"testing"
+
+	"github.com/btcsuite/btcd/chaincfg"
+	"github.com/btcsuite/btcd/database"
+	"github.com/btcsuite/btcd/wire"
 )
 
+// countingDB wraps a database.DB and counts the number of Update calls.
+type countingDB struct {
+	database.DB
+	updates int
+}
+
+// Update increments the updates counter on a call.
+func (c *countingDB) Update(fn func(tx database.Tx) error) error {
+	c.updates++
+	return c.DB.Update(fn)
+}
+
+// TestFlushToDB tests that flushToDB only opens a write transaction when at
+// least one dirty node has block data and skips the transaction when all dirty
+// nodes are header-only.
+func TestFlushToDB(t *testing.T) {
+	tests := []struct {
+		name string
+
+		// statuses defines the dirty nodes to create for this test
+		// case. Each entry's status determines whether the node is
+		// header-only or has block data. A nil slice means no nodes
+		// are added (empty dirty set).
+		statuses []blockStatus
+
+		// wantUpdates is the expected number of DB Update calls.
+		wantUpdates int
+	}{
+		{
+			name:        "empty dirty set",
+			statuses:    nil,
+			wantUpdates: 0,
+		},
+		{
+			name:        "single header-only node",
+			statuses:    []blockStatus{statusHeaderStored},
+			wantUpdates: 0,
+		},
+		{
+			name: "multiple header-only nodes",
+			statuses: []blockStatus{
+				statusHeaderStored,
+				statusHeaderStored,
+				statusHeaderStored,
+			},
+			wantUpdates: 0,
+		},
+		{
+			name:        "single data node",
+			statuses:    []blockStatus{statusDataStored | statusHeaderStored},
+			wantUpdates: 1,
+		},
+		{
+			name: "header-only and data nodes mixed",
+			statuses: []blockStatus{
+				statusHeaderStored,
+				statusDataStored | statusHeaderStored,
+			},
+			wantUpdates: 1,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			chain, teardown, err := chainSetup(
+				"flushtodbtest", &chaincfg.SimNetParams,
+			)
+			if err != nil {
+				t.Fatalf("failed to setup chain: %v", err)
+			}
+			defer teardown()
+
+			bi := chain.index
+			cdb := &countingDB{DB: bi.db}
+			bi.db = cdb
+
+			// Create the dirty nodes for this test case, chaining
+			// each off the genesis tip.
+			tip := chain.bestChain.Tip()
+			var nodes []*blockNode
+			for i, status := range test.statuses {
+				node := newBlockNode(&wire.BlockHeader{
+					PrevBlock: tip.hash,
+					Nonce:     uint32(i),
+				}, tip)
+				node.status = status
+				bi.AddNode(node)
+				nodes = append(nodes, node)
+				tip = node
+			}
+
+			err = bi.flushToDB()
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+
+			if cdb.updates != test.wantUpdates {
+				t.Fatalf("expected %d Update calls, got %d",
+					test.wantUpdates, cdb.updates)
+			}
+
+			bi.RLock()
+			dirtyLen := len(bi.dirty)
+			bi.RUnlock()
+
+			if dirtyLen != 0 {
+				t.Fatalf("expected dirty set to be empty, got %d",
+					dirtyLen)
+			}
+
+			// Nodes with block data should be in the DB;
+			// header-only nodes should not.
+			for i, node := range nodes {
+				var found bool
+				err := bi.db.View(func(dbTx database.Tx) error {
+					bucket := dbTx.Metadata().Bucket(blockIndexBucketName)
+					key := blockIndexKey(&node.hash, uint32(node.height))
+					found = bucket.Get(key) != nil
+					return nil
+				})
+				if err != nil {
+					t.Fatalf("node %d: View failed: %v", i, err)
+				}
+
+				wantInDB := node.status.HaveData()
+				if found != wantInDB {
+					t.Fatalf("node %d: in database = %v, want %v",
+						i, found, wantInDB)
+				}
+			}
+		})
+	}
+}
+
 func TestAncestor(t *testing.T) {
 	height := 500_000
 	blockNodes := chainedNodes(nil, height)