Fix issue when resync is not getting the right consensus config values (#221)

* Fix issue when resync is not getting the right consensus config values

* add test and fix log bug

* fix test

* delete temp file

Co-authored-by: Liam Lai <liam.icheng.lai@gmail.com>

consensus/XDPoS/engines/engine_v2/engine.go

@@ -121,16 +121,6 @@ func New(config *params.XDPoSConfig, db ethdb.Database, waitPeriodCh chan int) *
 	return engine
 }
 
-/* V2 Block
-SignerFn is a signer callback function to request a hash to be signed by a
-backing account.
-type SignerFn func(accounts.Account, []byte) ([]byte, error)
-
-sigHash returns the hash which is used as input for the delegated-proof-of-stake
-signing. It is the hash of the entire header apart from the 65 byte signature
-contained at the end of the extra data.
-*/
-
 func (x *XDPoS_v2) UpdateParams(header *types.Header) {
 	_, round, _, err := x.getExtraFields(header)
 	if err != nil {
@@ -148,6 +138,15 @@ func (x *XDPoS_v2) UpdateParams(header *types.Header) {
 	}()
 }
 
+/* V2 Block
+SignerFn is a signer callback function to request a hash to be signed by a
+backing account.
+type SignerFn func(accounts.Account, []byte) ([]byte, error)
+
+sigHash returns the hash which is used as input for the delegated-proof-of-stake
+signing. It is the hash of the entire header apart from the 65 byte signature
+contained at the end of the extra data.
+*/
 func (x *XDPoS_v2) SignHash(header *types.Header) (hash common.Hash) {
 	return sigHash(header)
 }
@@ -255,7 +254,7 @@ func (x *XDPoS_v2) YourTurn(chain consensus.ChainReader, parent *types.Header, s
 	_, parentRound, _, err := x.getExtraFields(parent)
 	minePeriod := x.config.V2.Config(uint64(parentRound) + 1).MinePeriod // plus 1 means current block
 	if waitedTime < int64(minePeriod) {
-		log.Trace("[YourTurn] wait after mine period", "minePeriod", x.config.V2.CurrentConfig.MinePeriod, "waitedTime", waitedTime)
+		log.Trace("[YourTurn] wait after mine period", "minePeriod", minePeriod, "waitedTime", waitedTime)
 		return false, nil
 	}
 

consensus/XDPoS/engines/engine_v2/utils.go

@@ -141,3 +141,17 @@ func (x *XDPoS_v2) getExtraFields(header *types.Header) (*types.QuorumCert, type
 	}
 	return decodedExtraField.QuorumCert, decodedExtraField.Round, masternodes, nil
 }
+
+func (x *XDPoS_v2) GetRoundNumber(header *types.Header) (types.Round, error) {
+	// If not v2 yet, return 0
+	if header.Number.Cmp(x.config.V2.SwitchBlock) <= 0 {
+		return types.Round(0), nil
+	} else {
+		var decodedExtraField types.ExtraFields_v2
+		err := utils.DecodeBytesExtraFields(header.Extra, &decodedExtraField)
+		if err != nil {
+			return types.Round(0), err
+		}
+		return decodedExtraField.Round, nil
+	}
+}

consensus/XDPoS/engines/engine_v2/verifyHeader.go

@@ -58,9 +58,6 @@ func (x *XDPoS_v2) verifyHeader(chain consensus.ChainReader, header *types.Heade
 	if parent == nil || parent.Number.Uint64() != number-1 || parent.Hash() != header.ParentHash {
 		return consensus.ErrUnknownAncestor
 	}
-	if parent.Number.Uint64() > x.config.V2.SwitchBlock.Uint64() && parent.Time.Uint64()+uint64(x.config.V2.CurrentConfig.MinePeriod) > header.Time.Uint64() {
-		return utils.ErrInvalidTimestamp
-	}
 
 	// Verify this is truely a v2 block first
 	quorumCert, round, _, err := x.getExtraFields(header)
@@ -68,6 +65,13 @@ func (x *XDPoS_v2) verifyHeader(chain consensus.ChainReader, header *types.Heade
 		log.Warn("[verifyHeader] decode extra field error", "err", err)
 		return utils.ErrInvalidV2Extra
 	}
+
+	minePeriod := uint64(x.config.V2.Config(uint64(round)).MinePeriod)
+	if parent.Number.Uint64() > x.config.V2.SwitchBlock.Uint64() && parent.Time.Uint64()+minePeriod > header.Time.Uint64() {
+		log.Warn("[verifyHeader] Fail to verify header due to invalid timestamp", "ParentTime", parent.Time.Uint64(), "MinePeriod", minePeriod, "HeaderTime", header.Time.Uint64(), "Hash", header.Hash().Hex())
+		return utils.ErrInvalidTimestamp
+	}
+
 	if round <= quorumCert.ProposedBlockInfo.Round {
 		return utils.ErrRoundInvalid
 	}

consensus/tests/engine_v2_tests/verify_header_test.go

@@ -259,6 +259,59 @@ func TestConfigSwitchOnDifferentCertThreshold(t *testing.T) {
 	assert.Equal(t, utils.ErrValidatorNotWithinMasternodes, err)
 }
 
+func TestConfigSwitchOnDifferentMindPeriod(t *testing.T) {
+	b, err := json.Marshal(params.TestXDPoSMockChainConfig)
+	assert.Nil(t, err)
+	configString := string(b)
+
+	var config params.ChainConfig
+	err = json.Unmarshal([]byte(configString), &config)
+	assert.Nil(t, err)
+	// Enable verify
+	config.XDPoS.V2.SkipV2Validation = false
+	// Block 901 is the first v2 block with round of 1
+	blockchain, _, _, _, _, _ := PrepareXDCTestBlockChainForV2Engine(t, 915, &config, nil)
+
+	adaptor := blockchain.Engine().(*XDPoS.XDPoS)
+
+	// Genrate 911 QC
+	proposedBlockInfo := &types.BlockInfo{
+		Hash:   blockchain.GetBlockByNumber(911).Hash(),
+		Round:  types.Round(11),
+		Number: blockchain.GetBlockByNumber(911).Number(),
+	}
+	voteForSign := &types.VoteForSign{
+		ProposedBlockInfo: proposedBlockInfo,
+		GapNumber:         450,
+	}
+
+	// Sign from acc 1, 2, 3
+	acc1SignedHash := SignHashByPK(acc1Key, types.VoteSigHash(voteForSign).Bytes())
+	acc2SignedHash := SignHashByPK(acc2Key, types.VoteSigHash(voteForSign).Bytes())
+	acc3SignedHash := SignHashByPK(acc3Key, types.VoteSigHash(voteForSign).Bytes())
+	var signaturesFirst []types.Signature
+	signaturesFirst = append(signaturesFirst, acc1SignedHash, acc2SignedHash, acc3SignedHash)
+	quorumCert := &types.QuorumCert{
+		ProposedBlockInfo: proposedBlockInfo,
+		Signatures:        signaturesFirst,
+		GapNumber:         450,
+	}
+
+	extra := types.ExtraFields_v2{
+		Round:      types.Round(12),
+		QuorumCert: quorumCert,
+	}
+	extraInBytes, _ := extra.EncodeToBytes()
+
+	// after 910 require 5 signs, but we only give 3 signs
+	block911 := blockchain.GetBlockByNumber(911).Header()
+	block911.Extra = extraInBytes
+	block911.Time = big.NewInt(blockchain.GetBlockByNumber(910).Time().Int64() + 2) //2 is previous config, should get the right config from round
+	err = adaptor.VerifyHeader(blockchain, block911, true)
+
+	assert.Equal(t, utils.ErrInvalidTimestamp, err)
+}
+
 func TestShouldFailIfNotEnoughQCSignatures(t *testing.T) {
 	b, err := json.Marshal(params.TestXDPoSMockChainConfig)
 	assert.Nil(t, err)

core/blockchain.go

@@ -18,6 +18,7 @@
 package core
 
 import (
+	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
@@ -2313,6 +2314,15 @@ func (bc *BlockChain) addBadBlock(block *types.Block) {
 func (bc *BlockChain) reportBlock(block *types.Block, receipts types.Receipts, err error) {
 	bc.addBadBlock(block)
 
+	// V2 specific logs
+	config, _ := json.Marshal(bc.chainConfig)
+
+	var roundNumber = types.Round(0)
+	engine, ok := bc.Engine().(*XDPoS.XDPoS)
+	if ok {
+		roundNumber, err = engine.EngineV2.GetRoundNumber(block.Header())
+	}
+
 	var receiptString string
 	for _, receipt := range receipts {
 		receiptString += fmt.Sprintf("\t%v\n", receipt)
@@ -2325,9 +2335,10 @@ Number: %v
 Hash: 0x%x
 %v
 
+Round: %v
 Error: %v
 ##############################
-`, bc.chainConfig, block.Number(), block.Hash(), receiptString, err))
+`, string(config), block.Number(), block.Hash(), receiptString, roundNumber, err))
 }
 
 // InsertHeaderChain attempts to insert the given header chain in to the local

params/config.go

@@ -401,7 +401,6 @@ func (v *V2) UpdateConfig(round uint64) {
 	for i := range v.configIndex {
 		if v.configIndex[i] <= round {
 			index = v.configIndex[i]
-		} else {
 			break
 		}
 	}
@@ -418,7 +417,6 @@ func (v *V2) Config(round uint64) *V2Config {
 	for i := range v.configIndex {
 		if v.configIndex[i] <= configRound {
 			index = v.configIndex[i]
-		} else {
 			break
 		}
 	}
@@ -433,9 +431,10 @@ func (v *V2) BuildConfigIndex() {
 	}
 
 	// sort, sort lib doesn't support type uint64, it's ok to have O(n^2)  because only few items in the list
+	// Make it descending order
 	for i := 0; i < len(list)-1; i++ {
 		for j := i + 1; j < len(list); j++ {
-			if list[i] > list[j] {
+			if list[i] < list[j] {
 				list[i], list[j] = list[j], list[i]
 			}
 		}

params/config_test.go

@@ -114,6 +114,6 @@ func TestV2Config(t *testing.T) {
 func TestBuildConfigIndex(t *testing.T) {
 	TestnetChainConfig.XDPoS.V2.BuildConfigIndex()
 	index := TestnetChainConfig.XDPoS.V2.ConfigIndex()
-	expected := []uint64{0, 10, 899}
+	expected := []uint64{899, 10, 0}
 	assert.Equal(t, expected, index)
 }