[release/1.4.11] core: ensure the canonical block is written before the canonical hash is set

(cherry picked from commit bb8059f)

Conflicts:
	core/blockchain.go
	core/database_util.go
	core/headerchain.go
	eth/filters/filter.go

Author: Bas van Kervel

core/blockchain.go

@@ -770,6 +770,14 @@ func (self *BlockChain) WriteBlock(block *types.Block) (status WriteStatus, err
 	localTd := self.GetTd(self.currentBlock.Hash())
 	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(), 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
@@ -780,20 +788,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(), 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())
 
 	return

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)
+				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]})
+	}
+}

core/database_util.go

@@ -157,7 +157,11 @@ func GetTd(db ethdb.Database, hash common.Hash) *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) *types.Block {
 	// Retrieve the block header and body contents
 	header := GetHeader(db, hash)

core/headerchain.go

@@ -129,6 +129,14 @@ func (hc *HeaderChain) WriteHeader(header *types.Header) (status WriteStatus, er
 	localTd := hc.GetTd(hc.currentHeaderHash)
 	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, 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
@@ -150,26 +158,21 @@ func (hc *HeaderChain) WriteHeader(header *types.Header) (status WriteStatus, er
 			headHeader = hc.GetHeader(headHash)
 			headNumber = headHeader.Number.Uint64()
 		}
+
 		// 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)
 		}
 		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, 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)
 
 	return

eth/filters/filter.go

@@ -72,7 +72,11 @@ func (self *Filter) SetTopics(topics [][]common.Hash) {
 
 // Run filters logs with the current parameters set
 func (self *Filter) Find() vm.Logs {
-	latestBlock := core.GetBlock(self.db, core.GetHeadBlockHash(self.db))
+	latestHash := core.GetHeadBlockHash(self.db)
+	latestBlock := core.GetBlock(self.db, latestHash)
+	if latestBlock == nil {
+		return vm.Logs{}
+	}
 	var beginBlockNo uint64 = uint64(self.begin)
 	if self.begin == -1 {
 		beginBlockNo = latestBlock.NumberU64()
@@ -122,13 +126,13 @@ func (self *Filter) mipFind(start, end uint64, depth int) (logs vm.Logs) {
 }
 
 func (self *Filter) getLogs(start, end uint64) (logs vm.Logs) {
-	var block *types.Block
-
 	for i := start; i <= end; i++ {
+		var block *types.Block
 		hash := core.GetCanonicalHash(self.db, i)
 		if hash != (common.Hash{}) {
 			block = core.GetBlock(self.db, hash)
-		} else { // block not found
+		}
+		if block == nil { // block not found/written
 			return logs
 		}