core: ensure the canonical block is written before the canonical hash is set

4 files changed, 63 insertions, 17 deletions

diff --git a/core/blockchain.go b/core/blockchain.go
index 950804d40..888c98dce 100644
--- a/core/blockchain.go
+++ b/core/blockchain.go
@@ -778,6 +778,14 @@ func (self *BlockChain) WriteBlock(block *types.Block) (status WriteStatus, err
 	localTd := self.GetTd(self.currentBlock.Hash(), self.currentBlock.NumberU64())
 	externTd := new(big.Int).Add(block.Difficulty(), ptd)
 
+	// Irrelevant of the canonical status, write the block itself to the database
+	if err := self.hc.WriteTd(block.Hash(), block.NumberU64(), externTd); err != nil {
+		glog.Fatalf("failed to write block total difficulty: %v", err)
+	}
+	if err := WriteBlock(self.chainDb, block); err != nil {
+		glog.Fatalf("failed to write block contents: %v", err)
+	}
+
 	// If the total difficulty is higher than our known, add it to the canonical chain
 	// Second clause in the if statement reduces the vulnerability to selfish mining.
 	// Please refer to http://www.cs.cornell.edu/~ie53/publications/btcProcFC.pdf
@@ -788,19 +796,11 @@ func (self *BlockChain) WriteBlock(block *types.Block) (status WriteStatus, err
 				return NonStatTy, err
 			}
 		}
-		// Insert the block as the new head of the chain
-		self.insert(block)
+		self.insert(block) // Insert the block as the new head of the chain
 		status = CanonStatTy
 	} else {
 		status = SideStatTy
 	}
-	// Irrelevant of the canonical status, write the block itself to the database
-	if err := self.hc.WriteTd(block.Hash(), block.NumberU64(), externTd); err != nil {
-		glog.Fatalf("failed to write block total difficulty: %v", err)
-	}
-	if err := WriteBlock(self.chainDb, block); err != nil {
-		glog.Fatalf("failed to write block contents: %v", err)
-	}
 
 	self.futureBlocks.Remove(block.Hash())
 
diff --git a/core/blockchain_test.go b/core/blockchain_test.go
index c3e4d352d..de3ef0a9c 100644
--- a/core/blockchain_test.go
+++ b/core/blockchain_test.go
@@ -1090,3 +1090,41 @@ done:
 	}
 
 }
+
+// Tests if the canonical block can be fetched from the database during chain insertion.
+func TestCanonicalBlockRetrieval(t *testing.T) {
+	var (
+		db, _   = ethdb.NewMemDatabase()
+		genesis = WriteGenesisBlockForTesting(db)
+	)
+
+	evmux := &event.TypeMux{}
+	blockchain, _ := NewBlockChain(db, testChainConfig(), FakePow{}, evmux)
+
+	chain, _ := GenerateChain(nil, genesis, db, 10, func(i int, gen *BlockGen) {})
+
+	for i, _ := range chain {
+		go func(block *types.Block) {
+			// try to retrieve a block by its canonical hash and see if the block data can be retrieved.
+			for {
+				ch := GetCanonicalHash(db, block.NumberU64())
+				if ch == (common.Hash{}) {
+					continue // busy wait for canonical hash to be written
+				}
+				if ch != block.Hash() {
+					t.Fatalf("unknown canonical hash, want %s, got %s", block.Hash().Hex(), ch.Hex())
+				}
+				fb := GetBlock(db, ch, block.NumberU64())
+				if fb == nil {
+					t.Fatalf("unable to retrieve block %d for canonical hash: %s", block.NumberU64(), ch.Hex())
+				}
+				if fb.Hash() != block.Hash() {
+					t.Fatalf("invalid block hash for block %d, want %s, got %s", block.NumberU64(), block.Hash().Hex(), fb.Hash().Hex())
+				}
+				return
+			}
+		}(chain[i])
+
+		blockchain.InsertChain(types.Blocks{chain[i]})
+	}
+}
diff --git a/core/database_util.go b/core/database_util.go
index 1529d7369..5f9afe6ba 100644
--- a/core/database_util.go
+++ b/core/database_util.go
@@ -204,7 +204,11 @@ func GetTd(db ethdb.Database, hash common.Hash, number uint64) *big.Int {
 }
 
 // GetBlock retrieves an entire block corresponding to the hash, assembling it
-// back from the stored header and body.
+// back from the stored header and body. If either the header or body could not
+// be retrieved nil is returned.
+//
+// Note, due to concurrent download of header and block body the header and thus
+// canonical hash can be stored in the database but the body data not (yet).
 func GetBlock(db ethdb.Database, hash common.Hash, number uint64) *types.Block {
 	// Retrieve the block header and body contents
 	header := GetHeader(db, hash, number)
diff --git a/core/headerchain.go b/core/headerchain.go
index f856333a0..0f9dd7208 100644
--- a/core/headerchain.go
+++ b/core/headerchain.go
@@ -151,6 +151,14 @@ func (hc *HeaderChain) WriteHeader(header *types.Header) (status WriteStatus, er
 	localTd := hc.GetTd(hc.currentHeaderHash, hc.currentHeader.Number.Uint64())
 	externTd := new(big.Int).Add(header.Difficulty, ptd)
 
+	// Irrelevant of the canonical status, write the td and header to the database
+	if err := hc.WriteTd(hash, number, externTd); err != nil {
+		glog.Fatalf("failed to write header total difficulty: %v", err)
+	}
+	if err := WriteHeader(hc.chainDb, header); err != nil {
+		glog.Fatalf("failed to write header contents: %v", err)
+	}
+
 	// If the total difficulty is higher than our known, add it to the canonical chain
 	// Second clause in the if statement reduces the vulnerability to selfish mining.
 	// Please refer to http://www.cs.cornell.edu/~ie53/publications/btcProcFC.pdf
@@ -176,6 +184,7 @@ func (hc *HeaderChain) WriteHeader(header *types.Header) (status WriteStatus, er
 			headNumber = headHeader.Number.Uint64() - 1
 			headHeader = hc.GetHeader(headHash, headNumber)
 		}
+
 		// Extend the canonical chain with the new header
 		if err := WriteCanonicalHash(hc.chainDb, hash, number); err != nil {
 			glog.Fatalf("failed to insert header number: %v", err)
@@ -183,19 +192,14 @@ func (hc *HeaderChain) WriteHeader(header *types.Header) (status WriteStatus, er
 		if err := WriteHeadHeaderHash(hc.chainDb, hash); err != nil {
 			glog.Fatalf("failed to insert head header hash: %v", err)
 		}
+
 		hc.currentHeaderHash, hc.currentHeader = hash, types.CopyHeader(header)
 
 		status = CanonStatTy
 	} else {
 		status = SideStatTy
 	}
-	// Irrelevant of the canonical status, write the header itself to the database
-	if err := hc.WriteTd(hash, number, externTd); err != nil {
-		glog.Fatalf("failed to write header total difficulty: %v", err)
-	}
-	if err := WriteHeader(hc.chainDb, header); err != nil {
-		glog.Fatalf("failed to write header contents: %v", err)
-	}
+
 	hc.headerCache.Add(hash, header)
 	hc.numberCache.Add(hash, number)