From bb8059f6aa86d1052d7c2dd75a6985982cb278f4 Mon Sep 17 00:00:00 2001 From: Bas van Kervel Date: Tue, 26 Jul 2016 16:37:04 +0200 Subject: [PATCH] core: ensure the canonical block is written before the canonical hash is set --- core/blockchain.go | 18 +++++++++--------- core/blockchain_test.go | 38 ++++++++++++++++++++++++++++++++++++++ core/database_util.go | 6 +++++- core/headerchain.go | 18 +++++++++++------- eth/filters/filter.go | 9 ++++++--- 5 files changed, 69 insertions(+), 20 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) diff --git a/eth/filters/filter.go b/eth/filters/filter.go index 995b588fb..fd739bf0e 100644 --- a/eth/filters/filter.go +++ b/eth/filters/filter.go @@ -74,6 +74,9 @@ func (self *Filter) SetTopics(topics [][]common.Hash) { func (self *Filter) Find() vm.Logs { latestHash := core.GetHeadBlockHash(self.db) latestBlock := core.GetBlock(self.db, latestHash, core.GetBlockNumber(self.db, latestHash)) + if latestBlock == nil { + return vm.Logs{} + } var beginBlockNo uint64 = uint64(self.begin) if self.begin == -1 { beginBlockNo = latestBlock.NumberU64() @@ -123,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, i) - } else { // block not found + } + if block == nil { // block not found/written return logs }