blockchain: fix inconsistent reorg behavior (axieinfinity#391)

* 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.

* consortium-v2: make the engine fully implement FastFinalityPoSA interface

When testing, we may need to create the standalone consortium-v2 engine so we
need to make it fully implement FastFinalityPoSA interface.

* chain_makers: add GenerateConsortiumChain to genernate Consortium blocks

When generating Consortium blocks, we need to modify the block after
FinalizeAndAssemble to change the block signature in header's extra data. New
function GenerateConsortiumChain adds a new parameter so the caller can pass a
function to modify block after FinalizeAndAssemble. Besides, the fake chain
reader also needs to return correct block header to be used by consensus engine
in FinalizeAndAssemble.

* consortium-v2: add unit test for known block reorg case

---------

Co-authored-by: Bui Quang Minh <minh.bui@skymavis.com>

consensus/consortium/v2/consortium.go

@@ -170,6 +170,16 @@ func (c *Consortium) IsSystemMessage(msg core.Message, header *types.Header) boo
 	return false
 }
 
+// In normal case, IsSystemTransaction in consortium/main.go is used instead of this function. This function
+// is only used in testing when we create standalone consortium v2 engine without the v1
+func (c *Consortium) IsSystemTransaction(tx *types.Transaction, header *types.Header) (bool, error) {
+	msg, err := tx.AsMessage(types.MakeSigner(c.chainConfig, header.Number), header.BaseFee)
+	if err != nil {
+		return false, err
+	}
+	return c.IsSystemMessage(msg, header), nil
+}
+
 // IsSystemContract implements consensus.PoSA, checking whether a contract is a system
 // contract or not
 // A system contract is a contract is defined in params.ConsortiumV2Contracts
@@ -191,11 +201,23 @@ func (c *Consortium) VerifyHeader(chain consensus.ChainHeaderReader, header *typ
 }
 
 // VerifyHeaders implements consensus.Engine, always returning an empty abort and results channels.
-// This method will be handled consortium/main.go instead
+// In normal case, VerifyHeaders in consortium/main.go is used instead of this function. This function
+// is only used in testing when we create standalone consortium v2 engine without the v1
 func (c *Consortium) VerifyHeaders(chain consensus.ChainHeaderReader, headers []*types.Header, seals []bool) (chan<- struct{}, <-chan error) {
 	abort := make(chan struct{})
 	results := make(chan error, len(headers))
 
+	go func() {
+		for i, header := range headers {
+			err := c.VerifyHeaderAndParents(chain, header, headers[:i])
+			select {
+			case <-abort:
+				return
+			case results <- err:
+			}
+		}
+	}()
+
 	return abort, results
 }
 
@@ -742,7 +764,7 @@ func (c *Consortium) processSystemTransactions(chain consensus.ChainHeaderReader
 
 	// If the parent's block includes the finality votes, distribute reward for the voters
 	if c.chainConfig.IsShillin(new(big.Int).Sub(header.Number, common.Big1)) {
-		parentHeader := chain.GetHeaderByHash(header.ParentHash)
+		parentHeader := chain.GetHeader(header.ParentHash, header.Number.Uint64()-1)
 		extraData, err := finality.DecodeExtra(parentHeader.Extra, true)
 		if err != nil {
 			return err

consensus/consortium/v2/consortium_test.go

@@ -2,6 +2,7 @@ package v2
 
 import (
 	"bytes"
+	"crypto/ecdsa"
 	"encoding/binary"
 	"errors"
 	"math/big"
@@ -17,6 +18,7 @@ import (
 	"github.com/ethereum/go-ethereum/core/rawdb"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/core/vm"
+	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/ethereum/go-ethereum/crypto/bls/blst"
 	blsCommon "github.com/ethereum/go-ethereum/crypto/bls/common"
 	"github.com/ethereum/go-ethereum/params"
@@ -972,3 +974,266 @@ func TestVerifyVote(t *testing.T) {
 		t.Errorf("Expect sucessful verification have %s", err)
 	}
 }
+
+func TestKnownBlockReorg(t *testing.T) {
+	db := rawdb.NewMemoryDatabase()
+
+	blsKeys := make([]blsCommon.SecretKey, 3)
+	ecdsaKeys := make([]*ecdsa.PrivateKey, 3)
+	validatorAddrs := make([]common.Address, 3)
+
+	for i := range blsKeys {
+		blsKey, err := blst.RandKey()
+		if err != nil {
+			t.Fatal(err)
+		}
+		blsKeys[i] = blsKey
+
+		secretKey, err := crypto.GenerateKey()
+		if err != nil {
+			t.Fatal(err)
+		}
+		ecdsaKeys[i] = secretKey
+		validatorAddrs[i] = crypto.PubkeyToAddress(secretKey.PublicKey)
+	}
+
+	for i := 0; i < len(blsKeys)-1; i++ {
+		for j := i; j < len(blsKeys); j++ {
+			if bytes.Compare(validatorAddrs[i][:], validatorAddrs[j][:]) > 0 {
+				validatorAddrs[i], validatorAddrs[j] = validatorAddrs[j], validatorAddrs[i]
+				blsKeys[i], blsKeys[j] = blsKeys[j], blsKeys[i]
+				ecdsaKeys[i], ecdsaKeys[j] = ecdsaKeys[j], ecdsaKeys[i]
+			}
+		}
+	}
+
+	chainConfig := params.ChainConfig{
+		ChainID:           big.NewInt(2021),
+		HomesteadBlock:    common.Big0,
+		EIP150Block:       common.Big0,
+		EIP155Block:       common.Big0,
+		EIP158Block:       common.Big0,
+		ConsortiumV2Block: common.Big0,
+		ShillinBlock:      big.NewInt(10),
+		Consortium: &params.ConsortiumConfig{
+			EpochV2: 10,
+		},
+	}
+
+	genesis := (&core.Genesis{
+		Config: &chainConfig,
+	}).MustCommit(db)
+
+	mock := &mockContract{
+		validators: make(map[common.Address]blsCommon.PublicKey),
+	}
+	mock.validators[validatorAddrs[0]] = blsKeys[0].PublicKey()
+	recents, _ := lru.NewARC(inmemorySnapshots)
+	signatures, _ := lru.NewARC(inmemorySignatures)
+
+	v2 := Consortium{
+		chainConfig: &chainConfig,
+		contract:    mock,
+		recents:     recents,
+		signatures:  signatures,
+		config:      chainConfig.Consortium,
+		db:          db,
+	}
+
+	chain, _ := core.NewBlockChain(db, nil, &chainConfig, &v2, vm.Config{}, nil, nil)
+	extraData := [consortiumCommon.ExtraVanity + consortiumCommon.ExtraSeal]byte{}
+
+	blocks, _ := core.GenerateConsortiumChain(
+		&chainConfig,
+		genesis,
+		&v2,
+		db,
+		9,
+		func(i int, bg *core.BlockGen) {
+			bg.SetCoinbase(validatorAddrs[0])
+			bg.SetExtra(extraData[:])
+			bg.SetDifficulty(big.NewInt(7))
+		},
+		true,
+		func(i int, bg *core.BlockGen) {
+			header := bg.Header()
+			hash := calculateSealHash(header, big.NewInt(2021))
+			sig, err := crypto.Sign(hash[:], ecdsaKeys[0])
+			if err != nil {
+				t.Fatalf("Failed to sign block, err %s", err)
+			}
+			copy(header.Extra[len(header.Extra)-consortiumCommon.ExtraSeal:], sig)
+			bg.SetExtra(header.Extra)
+		},
+	)
+
+	_, err := chain.InsertChain(blocks)
+	if err != nil {
+		t.Fatalf("Failed to insert block, err %s", err)
+	}
+
+	for i := range validatorAddrs {
+		mock.validators[validatorAddrs[i]] = blsKeys[i].PublicKey()
+	}
+
+	var checkpointValidators []finality.ValidatorWithBlsPub
+	for i := range validatorAddrs {
+		checkpointValidators = append(checkpointValidators, finality.ValidatorWithBlsPub{
+			Address:      validatorAddrs[i],
+			BlsPublicKey: blsKeys[i].PublicKey(),
+		})
+	}
+
+	// Prepare checkpoint block
+	blocks, _ = core.GenerateConsortiumChain(
+		&chainConfig,
+		blocks[len(blocks)-1],
+		&v2,
+		db,
+		1,
+		func(i int, bg *core.BlockGen) {
+			var extra finality.HeaderExtraData
+
+			bg.SetCoinbase(validatorAddrs[0])
+			bg.SetDifficulty(big.NewInt(7))
+			extra.CheckpointValidators = checkpointValidators
+			bg.SetExtra(extra.Encode(true))
+		},
+		true,
+		func(i int, bg *core.BlockGen) {
+			header := bg.Header()
+			hash := calculateSealHash(header, big.NewInt(2021))
+			sig, err := crypto.Sign(hash[:], ecdsaKeys[0])
+			if err != nil {
+				t.Fatalf("Failed to sign block, err %s", err)
+			}
+			copy(header.Extra[len(header.Extra)-consortiumCommon.ExtraSeal:], sig)
+			bg.SetExtra(header.Extra)
+		},
+	)
+
+	_, err = chain.InsertChain(blocks)
+	if err != nil {
+		t.Fatalf("Failed to insert block, err %s", err)
+	}
+
+	extraDataShillin := [consortiumCommon.ExtraVanity + 1 + consortiumCommon.ExtraSeal]byte{}
+	knownBlocks, _ := core.GenerateConsortiumChain(
+		&chainConfig,
+		blocks[len(blocks)-1],
+		&v2,
+		db,
+		1,
+		func(i int, bg *core.BlockGen) {
+			bg.SetCoinbase(validatorAddrs[2])
+			bg.SetExtra(extraDataShillin[:])
+			bg.SetDifficulty(big.NewInt(7))
+		},
+		true,
+		func(i int, bg *core.BlockGen) {
+			header := bg.Header()
+			hash := calculateSealHash(header, big.NewInt(2021))
+			sig, err := crypto.Sign(hash[:], ecdsaKeys[2])
+			if err != nil {
+				t.Fatalf("Failed to sign block, err %s", err)
+			}
+			copy(header.Extra[len(header.Extra)-consortiumCommon.ExtraSeal:], sig)
+			bg.SetExtra(header.Extra)
+		},
+	)
+
+	_, err = chain.InsertChain(knownBlocks)
+	if err != nil {
+		t.Fatalf("Failed to insert block, err %s", err)
+	}
+
+	header := chain.CurrentHeader()
+	if header.Number.Uint64() != 11 {
+		t.Fatalf("Expect head header to be %d, got %d", 11, header.Number.Uint64())
+	}
+	if header.Difficulty.Cmp(big.NewInt(7)) != 0 {
+		t.Fatalf("Expect header header to have difficulty %d, got %d", 7, header.Difficulty.Uint64())
+	}
+
+	justifiedBlocks, _ := core.GenerateConsortiumChain(
+		&chainConfig,
+		blocks[len(blocks)-1],
+		&v2,
+		db,
+		2,
+		func(i int, bg *core.BlockGen) {
+			if bg.Number().Uint64() == 11 {
+				bg.SetCoinbase(validatorAddrs[1])
+				bg.SetExtra(extraDataShillin[:])
+			} else {
+				bg.SetCoinbase(validatorAddrs[2])
+
+				var (
+					extra      finality.HeaderExtraData
+					voteBitset finality.FinalityVoteBitSet
+					signatures []blsCommon.Signature
+				)
+				voteBitset.SetBit(0)
+				voteBitset.SetBit(1)
+				voteBitset.SetBit(2)
+				extra.HasFinalityVote = 1
+				extra.FinalityVotedValidators = voteBitset
+
+				block := bg.PrevBlock(-1)
+				voteData := types.VoteData{
+					TargetNumber: block.NumberU64(),
+					TargetHash:   block.Hash(),
+				}
+				for i := range blsKeys {
+					signatures = append(signatures, blsKeys[i].Sign(voteData.Hash().Bytes()))
+				}
+
+				extra.AggregatedFinalityVotes = blst.AggregateSignatures(signatures)
+				bg.SetExtra(extra.Encode(true))
+			}
+
+			bg.SetDifficulty(big.NewInt(3))
+		},
+		true,
+		func(i int, bg *core.BlockGen) {
+			header := bg.Header()
+			hash := calculateSealHash(header, big.NewInt(2021))
+
+			var ecdsaKey *ecdsa.PrivateKey
+			if bg.Number().Uint64() == 11 {
+				ecdsaKey = ecdsaKeys[1]
+			} else {
+				ecdsaKey = ecdsaKeys[2]
+			}
+			sig, err := crypto.Sign(hash[:], ecdsaKey)
+			if err != nil {
+				t.Fatalf("Failed to sign block, err %s", err)
+			}
+			copy(header.Extra[len(header.Extra)-consortiumCommon.ExtraSeal:], sig)
+			bg.SetExtra(header.Extra)
+		},
+	)
+
+	_, err = chain.InsertChain(justifiedBlocks)
+	if err != nil {
+		t.Fatalf("Failed to insert block, err %s", err)
+	}
+
+	header = chain.CurrentHeader()
+	if header.Number.Uint64() != 12 {
+		t.Fatalf("Expect head header to be %d, got %d", 12, header.Number.Uint64())
+	}
+
+	_, err = chain.InsertChain(knownBlocks)
+	if err != nil {
+		t.Fatalf("Failed to insert block, err %s", err)
+	}
+	header = chain.CurrentHeader()
+	if header.Number.Uint64() != 12 {
+		t.Fatalf("Expect head header to be %d, got %d", 12, header.Number.Uint64())
+	}
+	header = chain.GetHeaderByNumber(11)
+	if header.Difficulty.Uint64() != 3 {
+		t.Fatalf("Expect head header to have difficulty %d, got %d", 3, header.Difficulty.Uint64())
+	}
+}

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