blockchain: change HaveBlock to also check for block data availability
What the developer wrote
Authored by Calvin Kim
blockchain: change HaveBlock to also check for block data availability
In block processing and block downloading, HaveBlock is used to check if
the block data already exists. It was ok to just check for the
existence of the blockNode but we now we also need to check if the data
exists as the blockNode may be present for just the block header.
What changed, and why it matters
This commit fixes a logic bug in btcd (a Bitcoin implementation in Go). Previously, when the software was asked 'Do you already have this block?', it answered 'yes' if it knew the block's header, even if it did not actually have the full block data stored. Now it correctly checks that both the header is known and the full block data exists on disk. This could have caused the node to skip downloading a block it thought it already had, leading to incomplete data or inconsistent behavior.
Review callers of HaveBlock to confirm they now behave correctly when only a header is present. Ensure the fix is backported if this code is in a release branch. Consider whether any network-facing code relied on the old behavior and could now trigger unnecessary downloads.
Security signals we found
Logic correctness fix in block availability check
Potential denial-of-service or data-availability issue if node skips block download
Header-only block state now distinguished from full block data state
Evidence from the diff
The change updates HaveBlock in blockchain/blockindex.go to return true only when the block index contains the hash and the associated blockNode’s status indicates that block data is present on disk (node.status.HaveData()). The BlockChain.HaveBlock comment in blockchain/chain.go is updated to clarify that it checks for block data, not just the block’s existence. A test is added to verify that processing only a block header results in HaveBlock returning false for that block.
Changed components
blockchain/blockindex.goblockchain/chain.goblockchain/chain_test.goInspect captured patch +35 / −6
diff --git a/blockchain/blockindex.go b/blockchain/blockindex.go
index 8e330c6..ff04c5b 100644
--- a/blockchain/blockindex.go
+++ b/blockchain/blockindex.go
@@ -385,14 +385,16 @@ func newBlockIndex(db database.DB, chainParams *chaincfg.Params) *blockIndex {
}
}
-// HaveBlock returns whether or not the block index contains the provided hash.
+// HaveBlock returns whether or not the block index contains the provided hash
+// and if the data exists on disk.
//
// This function is safe for concurrent access.
func (bi *blockIndex) HaveBlock(hash *chainhash.Hash) bool {
bi.RLock()
- _, hasBlock := bi.index[*hash]
+ node, hasBlock := bi.index[*hash]
+ haveData := hasBlock && node.status.HaveData()
bi.RUnlock()
- return hasBlock
+ return haveData
}
// LookupNode returns the block node identified by the provided hash. It will
diff --git a/blockchain/chain.go b/blockchain/chain.go
index 48af310..58d7256 100644
--- a/blockchain/chain.go
+++ b/blockchain/chain.go
@@ -195,9 +195,10 @@ type BlockChain struct {
notifications []NotificationCallback
}
-// HaveBlock returns whether or not the chain instance has the block represented
-// by the passed hash. This includes checking the various places a block can
-// be like part of the main chain, on a side chain, or in the orphan pool.
+// HaveBlock returns whether or not the chain instance has the block data
+// represented by the passed hash. This includes checking the various places a
+// block can be like part of the main chain, on a side chain, or in the orphan
+// pool.
//
// This function is safe for concurrent access.
func (b *BlockChain) HaveBlock(hash *chainhash.Hash) (bool, error) {
diff --git a/blockchain/chain_test.go b/blockchain/chain_test.go
index b3bccf5..b0e07cf 100644
--- a/blockchain/chain_test.go
+++ b/blockchain/chain_test.go
@@ -21,6 +21,7 @@ import (
// TestHaveBlock tests the HaveBlock API to ensure proper functionality.
func TestHaveBlock(t *testing.T) {
// Load up blocks such that there is a side chain.
+ // We'll only process the header for block 4.
// (genesis block) -> 1 -> 2 -> 3 -> 4
// \-> 3a
testFiles := []string{
@@ -51,7 +52,29 @@ func TestHaveBlock(t *testing.T) {
// maturity to 1.
chain.TstSetCoinbaseMaturity(1)
+ // We want to process just the header for block 4.
+ block4Hash := newHashFromStr("000000002f264d6504013e73b9c913de9098d4d771c1bb219af475d2a01b128e")
+
for i := 1; i < len(blocks); i++ {
+ // Add just the header for the block 4.
+ if blocks[i].Hash().IsEqual(block4Hash) {
+
+ isMainChain, err := chain.ProcessBlockHeader(
+ &blocks[i].MsgBlock().Header, BFNone, false)
+ if err != nil {
+ t.Errorf("ProcessBlockHeader fail on block %v: %v\n",
+ i, err)
+ return
+ }
+ if !isMainChain {
+ t.Errorf("ProcessBlockHeader incorrectly returned "+
+ "block %v is a side-chain\n", i)
+ return
+ }
+
+ continue
+ }
+
_, isOrphan, err := chain.ProcessBlock(blocks[i], BFNone)
if err != nil {
t.Errorf("ProcessBlock fail on block %v: %v\n", i, err)
@@ -87,6 +110,9 @@ func TestHaveBlock(t *testing.T) {
// Block 3a should be present (on a side chain).
{hash: "00000000474284d20067a4d33f6a02284e6ef70764a3a26d6a5b9df52ef663dd", want: true},
+ // Block 4 shouldn't be present as we only have its header.
+ {hash: "000000002f264d6504013e73b9c913de9098d4d771c1bb219af475d2a01b128e", want: false},
+
// Block 100000 should be present (as an orphan).
{hash: "000000000003ba27aa200b1cecaad478d2b00432346c3f1f3986da1afd33e506", want: true},
Why this scored 46/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.