From 9e45f60e9e1f46f8a9660dc00e2d7b49e44cffca Mon Sep 17 00:00:00 2001 From: Calvin Kim Date: Mon, 22 Sep 2025 15:42:35 +0900 Subject: [PATCH] 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 | 29 +++++++ blockchain/blockindex_test.go | 140 +++++++++++++++++++++++++++++++++- 2 files changed, 168 insertions(+), 1 deletion(-) diff --git a/blockchain/blockindex.go b/blockchain/blockindex.go index 3e1606f7..8e330c6b 100644 --- a/blockchain/blockindex.go +++ b/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 diff --git a/blockchain/blockindex_test.go b/blockchain/blockindex_test.go index cd08969f..47a47e92 100644 --- a/blockchain/blockindex_test.go +++ b/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)