From 65df573049cf8d86b1d93634083ce15ea4915931 Mon Sep 17 00:00:00 2001 From: NganSM Date: Tue, 9 Jan 2024 17:14:04 +0700 Subject: [PATCH] blockchain: fix inconsistent reorg behavior If the chain is reorganized because the newly submit block has higher justified block number, it can be reverted back to the old chain prior to reorg because the old blocks cause `ErrKnownBlock` during insertion via the `insertChain` method, which invokes `writeKnownBlock` to set the old chain as the canonical chain again. --- core/blockchain.go | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/core/blockchain.go b/core/blockchain.go index 5335158c59..b51cef2489 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -1665,7 +1665,7 @@ func (bc *BlockChain) insertChain(chain types.Blocks, verifySeals bool) (int, er ) for block != nil && bc.skipBlock(err, it) { externTd = new(big.Int).Add(externTd, block.Difficulty()) - if localTd.Cmp(externTd) < 0 { + if bc.reorgNeeded(current, localTd, block, externTd) { break } log.Debug("Ignoring already known block", "number", block.Number(), "hash", block.Hash()) @@ -1734,7 +1734,19 @@ func (bc *BlockChain) insertChain(chain types.Blocks, verifySeals bool) (int, er } }() - for ; block != nil && err == nil || errors.Is(err, ErrKnownBlock); block, err = it.next() { + var ( + current = bc.CurrentBlock() + localTd = bc.GetTd(current.Hash(), current.NumberU64()) + externTd = common.Big0 + ) + + if block != nil { + externTd = bc.GetTd(block.ParentHash(), block.NumberU64()-1) + } + + for ; (block != nil && err == nil) || errors.Is(err, ErrKnownBlock); block, err = it.next() { + // err == ErrknownBlock means block != nil + externTd = new(big.Int).Add(externTd, block.Difficulty()) // If the chain is terminating, stop processing blocks if bc.insertStopped() { log.Debug("Abort during block processing") @@ -1751,7 +1763,7 @@ func (bc *BlockChain) insertChain(chain types.Blocks, verifySeals bool) (int, er // just skip the block (we already validated it once fully (and crashed), since // its header and body was already in the database). But if the corresponding // snapshot layer is missing, forcibly rerun the execution to build it. - if bc.skipBlock(err, it) { + if bc.skipBlock(err, it) && bc.reorgNeeded(current, localTd, block, externTd) { logger := log.Debug if bc.chainConfig.Clique == nil { logger = log.Warn