From 84be00915416872b6136b06a6f7b12b095585e36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Mon, 22 Jan 2018 14:07:47 +0200 Subject: [PATCH] core: sorted reorg insertion order for proper head header updating --- core/blockchain.go | 13 ++++++----- core/blockchain_test.go | 48 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 7 deletions(-) diff --git a/core/blockchain.go b/core/blockchain.go index 737fbe3eee..f886ffe4ed 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -465,7 +465,7 @@ func (bc *BlockChain) insert(block *types.Block) { } bc.currentBlock = block - // If the block is better than out head or is on a different chain, force update heads + // If the block is better than our head or is on a different chain, force update heads if updateHeads { bc.hc.SetCurrentHeader(block.Header()) @@ -1140,18 +1140,17 @@ func (bc *BlockChain) reorg(oldBlock, newBlock *types.Block) error { } else { log.Error("Impossible reorg, please file an issue", "oldnum", oldBlock.Number(), "oldhash", oldBlock.Hash(), "newnum", newBlock.Number(), "newhash", newBlock.Hash()) } + // Insert the new chain, taking care of the proper incremental order var addedTxs types.Transactions - // insert blocks. Order does not matter. Last block will be written in ImportChain itself which creates the new head properly - for _, block := range newChain { + for i := len(newChain) - 1; i >= 0; i-- { // insert the block in the canonical way, re-writing history - bc.insert(block) + bc.insert(newChain[i]) // write lookup entries for hash based transaction/receipt searches - if err := WriteTxLookupEntries(bc.chainDb, block); err != nil { + if err := WriteTxLookupEntries(bc.chainDb, newChain[i]); err != nil { return err } - addedTxs = append(addedTxs, block.Transactions()...) + addedTxs = append(addedTxs, newChain[i].Transactions()...) } - // calculate the difference between deleted and added transactions diff := types.TxDifference(deletedTxs, addedTxs) // When transactions get deleted from the database that means the diff --git a/core/blockchain_test.go b/core/blockchain_test.go index 26c816027f..cbde3bcd2d 100644 --- a/core/blockchain_test.go +++ b/core/blockchain_test.go @@ -1197,3 +1197,51 @@ func TestEIP161AccountRemoval(t *testing.T) { t.Error("account should not exist") } } + +// This is a regression test (i.e. as weird as it is, don't delete it ever), which +// tests that under weird reorg conditions the blockchain and its internal header- +// chain return the same latest block/header. +// +// https://github.com/ethereum/go-ethereum/pull/15941 +func TestBlockchainHeaderchainReorgConsistency(t *testing.T) { + // Generate a canonical chain to act as the main dataset + engine := ethash.NewFaker() + + db, _ := ethdb.NewMemDatabase() + genesis := new(Genesis).MustCommit(db) + blocks, _ := GenerateChain(params.TestChainConfig, genesis, engine, db, 64, func(i int, b *BlockGen) { b.SetCoinbase(common.Address{1}) }) + + // Generate a bunch of fork blocks, each side forking from the canonical chain + forks := make([]*types.Block, len(blocks)) + for i := 0; i < len(forks); i++ { + parent := genesis + if i > 0 { + parent = blocks[i-1] + } + fork, _ := GenerateChain(params.TestChainConfig, parent, engine, db, 1, func(i int, b *BlockGen) { b.SetCoinbase(common.Address{2}) }) + forks[i] = fork[0] + } + // Import the canonical and fork chain side by side, verifying the current block + // and current header consistency + diskdb, _ := ethdb.NewMemDatabase() + new(Genesis).MustCommit(diskdb) + + chain, err := NewBlockChain(diskdb, params.TestChainConfig, engine, vm.Config{}) + if err != nil { + t.Fatalf("failed to create tester chain: %v", err) + } + for i := 0; i < len(blocks); i++ { + if _, err := chain.InsertChain(blocks[i : i+1]); err != nil { + t.Fatalf("block %d: failed to insert into chain: %v", i, err) + } + if chain.CurrentBlock().Hash() != chain.CurrentHeader().Hash() { + t.Errorf("block %d: current block/header mismatch: block #%d [%x…], header #%d [%x…]", i, chain.CurrentBlock().Number(), chain.CurrentBlock().Hash().Bytes()[:4], chain.CurrentHeader().Number, chain.CurrentHeader().Hash().Bytes()[:4]) + } + if _, err := chain.InsertChain(forks[i : i+1]); err != nil { + t.Fatalf(" fork %d: failed to insert into chain: %v", i, err) + } + if chain.CurrentBlock().Hash() != chain.CurrentHeader().Hash() { + t.Errorf(" fork %d: current block/header mismatch: block #%d [%x…], header #%d [%x…]", i, chain.CurrentBlock().Number(), chain.CurrentBlock().Hash().Bytes()[:4], chain.CurrentHeader().Number, chain.CurrentHeader().Hash().Bytes()[:4]) + } + } +}