XIN-159, 160 and 161 (ethereum#69)

* XIN-159, 160 and 161

* update the bft handler to make sure we don't process dis-qualified messages

* add verify header missing checks and its tests

consensus/XDPoS/engines/engine_v2/engine.go

@@ -590,10 +590,13 @@ func (x *XDPoS_v2) verifyHeader(chain consensus.ChainReader, header *types.Heade
 
 	// Verify this is truely a v2 block first
 
-	quorumCert, _, _, err := x.getExtraFields(header)
+	quorumCert, round, _, err := x.getExtraFields(header)
 	if err != nil {
 		return utils.ErrInvalidV2Extra
 	}
+	if round <= quorumCert.ProposedBlockInfo.Round {
+		return utils.ErrRoundInvalid
+	}
 
 	err = x.verifyQC(chain, quorumCert)
 	if err != nil {
@@ -613,6 +616,10 @@ func (x *XDPoS_v2) verifyHeader(chain consensus.ChainReader, header *types.Heade
 		return utils.ErrInvalidUncleHash
 	}
 
+	if header.Difficulty.Cmp(big.NewInt(1)) != 0 {
+		return utils.ErrInvalidDifficulty
+	}
+
 	isEpochSwitch, _, err := x.IsEpochSwitch(header) // Verify v2 block that is on the epoch switch
 	if err != nil {
 		log.Error("[verifyHeader] error when checking if header is epoch switch header", "Hash", header.Hash(), "Number", header.Number, "Error", err)
@@ -628,6 +635,7 @@ func (x *XDPoS_v2) verifyHeader(chain consensus.ChainReader, header *types.Heade
 		if len(header.Validators)%common.AddressLength != 0 {
 			return utils.ErrInvalidCheckpointSigners
 		}
+		// TODO: Add checkMasternodesOnEpochSwitch
 	} else {
 		if len(header.Validators) != 0 {
 			log.Warn("[verifyHeader] Validators shall not have values in non-epochSwitch block", "Hash", header.Hash(), "Number", header.Number, "Validators", header.Validators)
@@ -652,47 +660,54 @@ 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.Time.Uint64()+uint64(x.config.V2.MinePeriod) > header.Time.Uint64() {
+	if parent.Number.Uint64() > x.config.V2.SwitchBlock.Uint64() && parent.Time.Uint64()+uint64(x.config.V2.MinePeriod) > header.Time.Uint64() {
 		return utils.ErrInvalidTimestamp
 	}
-	// TODO: verifySeal XIN-135
+	// TODO: item 9. check validator
 
-	x.verifiedHeaders.Add(header.Hash(), true)
-	return nil
-}
+	_, penalties, err := x.calcMasternodes(chain, header.Number, header.ParentHash)
+	if err != nil {
+		log.Error("[verifyHeader] Fail to calculate master nodes list with penalty", "Number", header.Number, "Hash", header.Hash())
+		return err
+	}
 
-// Utils for test to get current Pool size
-func (x *XDPoS_v2) GetVotePoolSize(vote *utils.Vote) int {
-	return x.votePool.Size(vote)
-}
+	if !utils.CompareSignersLists(common.ExtractAddressFromBytes(header.Penalties), penalties) {
+		return utils.ErrPenaltyListDoesNotMatch
+	}
 
-// Utils for test to get Timeout Pool Size
-func (x *XDPoS_v2) GetTimeoutPoolSize(timeout *utils.Timeout) int {
-	return x.timeoutPool.Size(timeout)
+	x.verifiedHeaders.Add(header.Hash(), true)
+	return nil
 }
 
 /*
 	SyncInfo workflow
 */
 // Verify syncInfo and trigger process QC or TC if successful
-func (x *XDPoS_v2) VerifySyncInfoMessage(chain consensus.ChainReader, syncInfo *utils.SyncInfo) error {
+func (x *XDPoS_v2) VerifySyncInfoMessage(chain consensus.ChainReader, syncInfo *utils.SyncInfo) (bool, error) {
 	/*
-		1. Verify items including:
+		1. Check QC and TC against highest QC TC. Skip if none of them need to be updated
+		2. Verify items including:
 				- verifyQC
 				- verifyTC
-		2. Broadcast(Not part of consensus)
+		3. Broadcast(Not part of consensus)
 	*/
+
+	if (x.highestQuorumCert.ProposedBlockInfo.Round >= syncInfo.HighestQuorumCert.ProposedBlockInfo.Round) && (x.highestTimeoutCert.Round >= syncInfo.HighestTimeoutCert.Round) {
+		log.Warn("[VerifySyncInfoMessage] Round from incoming syncInfo message is no longer qualified", "Highest QC Round", x.highestQuorumCert.ProposedBlockInfo.Round, "Incoming SyncInfo QC Round", syncInfo.HighestQuorumCert.ProposedBlockInfo.Round, "highestTimeoutCert Round", x.highestTimeoutCert.Round, "Incoming syncInfo TC Round", syncInfo.HighestTimeoutCert.Round)
+		return false, nil
+	}
+
 	err := x.verifyQC(chain, syncInfo.HighestQuorumCert)
 	if err != nil {
 		log.Warn("SyncInfo message verification failed due to QC", err)
-		return err
+		return false, err
 	}
 	err = x.verifyTC(chain, syncInfo.HighestTimeoutCert)
 	if err != nil {
 		log.Warn("SyncInfo message verification failed due to TC", err)
-		return err
+		return false, err
 	}
-	return nil
+	return true, nil
 }
 
 func (x *XDPoS_v2) SyncInfoHandler(chain consensus.ChainReader, syncInfo *utils.SyncInfo) error {
@@ -714,17 +729,19 @@ func (x *XDPoS_v2) SyncInfoHandler(chain consensus.ChainReader, syncInfo *utils.
 */
 func (x *XDPoS_v2) VerifyVoteMessage(chain consensus.ChainReader, vote *utils.Vote) (bool, error) {
 	/*
-		  1. Get masterNode list from snapshot
-		  2. Check signature:
+		  1. Check vote round with current round for fast fail(disqualifed)
+		  2. Get masterNode list from snapshot
+		  3. Check signature:
 					- Use ecRecover to get the public key
 					- Use the above public key to find out the xdc address
 					- Use the above xdc address to check against the master node list from step 1(For the running epoch)
 			4. Broadcast(Not part of consensus)
 	*/
-	err := x.VerifyBlockInfo(chain, vote.ProposedBlockInfo)
-	if err != nil {
-		return false, err
+	if vote.ProposedBlockInfo.Round < x.currentRound {
+		log.Warn("[VerifyVoteMessage] Disqualified vote message as the proposed round does not match currentRound", "vote.ProposedBlockInfo.Round", vote.ProposedBlockInfo.Round, "currentRound", x.currentRound)
+		return false, nil
 	}
+
 	snapshot, err := x.getSnapshot(chain, vote.ProposedBlockInfo.Number.Uint64(), false)
 	if err != nil {
 		log.Error("[VerifyVoteMessage] fail to get snapshot for a vote message", "BlockNum", vote.ProposedBlockInfo.Number, "Hash", vote.ProposedBlockInfo.Hash, "Error", err.Error())
@@ -770,7 +787,13 @@ func (x *XDPoS_v2) voteHandler(chain consensus.ChainReader, voteMsg *utils.Vote)
 			return nil
 		}
 
-		err := x.onVotePoolThresholdReached(chain, pooledVotes, voteMsg, proposedBlockHeader)
+		err := x.VerifyBlockInfo(chain, voteMsg.ProposedBlockInfo)
+		if err != nil {
+			x.votePool.ClearPoolKeyByObj(voteMsg)
+			return err
+		}
+
+		err = x.onVotePoolThresholdReached(chain, pooledVotes, voteMsg, proposedBlockHeader)
 		if err != nil {
 			return err
 		}
@@ -1487,41 +1510,6 @@ func (x *XDPoS_v2) isExtendingFromAncestor(blockChainReader consensus.ChainReade
 	return false, nil
 }
 
-/*
-	Testing tools
-*/
-
-func (x *XDPoS_v2) SetNewRoundFaker(blockChainReader consensus.ChainReader, newRound utils.Round, resetTimer bool) {
-	x.lock.Lock()
-	defer x.lock.Unlock()
-	// Reset a bunch of things
-	if resetTimer {
-		x.timeoutWorker.Reset(blockChainReader)
-	}
-	x.currentRound = newRound
-}
-
-// for test only
-func (x *XDPoS_v2) ProcessQC(chain consensus.ChainReader, qc *utils.QuorumCert) error {
-	x.lock.Lock()
-	defer x.lock.Unlock()
-	return x.processQC(chain, qc)
-}
-
-// Utils for test to check currentRound value
-func (x *XDPoS_v2) GetCurrentRound() utils.Round {
-	x.lock.RLock()
-	defer x.lock.RUnlock()
-	return x.currentRound
-}
-
-// Utils for test to check currentRound value
-func (x *XDPoS_v2) GetProperties() (utils.Round, *utils.QuorumCert, *utils.QuorumCert, utils.Round, *utils.BlockInfo) {
-	x.lock.RLock()
-	defer x.lock.RUnlock()
-	return x.currentRound, x.lockQuorumCert, x.highestQuorumCert, x.highestVotedRound, x.highestCommitBlock
-}
-
 // Get master nodes over extra data of epoch switch block.
 func (x *XDPoS_v2) GetMasternodesFromEpochSwitchHeader(epochSwitchHeader *types.Header) []common.Address {
 	if epochSwitchHeader == nil {
@@ -1539,7 +1527,7 @@ func (x *XDPoS_v2) GetMasternodesFromEpochSwitchHeader(epochSwitchHeader *types.
 func (x *XDPoS_v2) IsEpochSwitch(header *types.Header) (bool, uint64, error) {
 	// Return true directly if we are examing the last v1 block. This could happen if the calling function is examing parent block
 	if header.Number.Cmp(x.config.V2.SwitchBlock) == 0 {
-		log.Info("[IsEpochSwitch] examing last v1 block 👯‍♂️")
+		log.Info("[IsEpochSwitch] examing last v1 block")
 		return true, header.Number.Uint64() / x.config.Epoch, nil
 	}
 

consensus/XDPoS/engines/engine_v2/testing_utils.go

@@ -0,0 +1,59 @@
+package engine_v2
+
+import (
+	"github.com/XinFinOrg/XDPoSChain/consensus"
+	"github.com/XinFinOrg/XDPoSChain/consensus/XDPoS/utils"
+)
+
+/*
+	Testing tools
+*/
+
+func (x *XDPoS_v2) SetNewRoundFaker(blockChainReader consensus.ChainReader, newRound utils.Round, resetTimer bool) {
+	x.lock.Lock()
+	defer x.lock.Unlock()
+	// Reset a bunch of things
+	if resetTimer {
+		x.timeoutWorker.Reset(blockChainReader)
+	}
+	x.currentRound = newRound
+}
+
+// for test only
+func (x *XDPoS_v2) ProcessQCFaker(chain consensus.ChainReader, qc *utils.QuorumCert) error {
+	x.lock.Lock()
+	defer x.lock.Unlock()
+	return x.processQC(chain, qc)
+}
+
+// Utils for test to check currentRound value
+func (x *XDPoS_v2) GetCurrentRoundFaker() utils.Round {
+	x.lock.RLock()
+	defer x.lock.RUnlock()
+	return x.currentRound
+}
+
+// Utils for test to get current Pool size
+func (x *XDPoS_v2) GetVotePoolSizeFaker(vote *utils.Vote) int {
+	return x.votePool.Size(vote)
+}
+
+// Utils for test to get Timeout Pool Size
+func (x *XDPoS_v2) GetTimeoutPoolSizeFaker(timeout *utils.Timeout) int {
+	return x.timeoutPool.Size(timeout)
+}
+
+// WARN: This function is designed for testing purpose only!
+// Utils for test to check currentRound values
+func (x *XDPoS_v2) GetPropertiesFaker() (utils.Round, *utils.QuorumCert, *utils.QuorumCert, *utils.TimeoutCert, utils.Round, *utils.BlockInfo) {
+	x.lock.RLock()
+	defer x.lock.RUnlock()
+	return x.currentRound, x.lockQuorumCert, x.highestQuorumCert, x.highestTimeoutCert, x.highestVotedRound, x.highestCommitBlock
+}
+
+// WARN: This function is designed for testing purpose only!
+// Utils for tests to set engine specific values
+func (x *XDPoS_v2) SetPropertiesFaker(highestQC *utils.QuorumCert, highestTC *utils.TimeoutCert) {
+	x.highestQuorumCert = highestQC
+	x.highestTimeoutCert = highestTC
+}

consensus/XDPoS/utils/errors.go

@@ -85,6 +85,9 @@ var (
 	ErrInvalidTC                    = errors.New("Invalid TC content")
 	ErrEmptyBlockInfoHash           = errors.New("BlockInfo hash is empty")
 	ErrInvalidFieldInNonEpochSwitch = errors.New("Invalid field exist in a non-epoch swtich block")
+
+	ErrPenaltyListDoesNotMatch = errors.New("Incoming block penalty list does not match")
+	ErrRoundInvalid            = errors.New("Invalid Round, it shall be bigger than QC round")
 )
 
 type ErrIncomingMessageRoundNotEqualCurrentRound struct {

consensus/tests/initial_test.go

@@ -24,7 +24,7 @@ func TestInitialFirstV2Blcok(t *testing.T) {
 	err := adaptor.EngineV2.Initial(blockchain, header)
 	assert.Nil(t, err)
 
-	round, _, highQC, _, _ := adaptor.EngineV2.GetProperties()
+	round, _, highQC, _, _, _ := adaptor.EngineV2.GetPropertiesFaker()
 	blockInfo := &utils.BlockInfo{
 		Hash:   header.Hash(),
 		Round:  utils.Round(0),
@@ -98,7 +98,7 @@ func TestInitialOtherV2Block(t *testing.T) {
 	err = adaptor.EngineV2.Initial(blockchain, block.Header())
 	assert.Nil(t, err)
 
-	round, _, highQC, _, _ := adaptor.EngineV2.GetProperties()
+	round, _, highQC, _, _, _ := adaptor.EngineV2.GetPropertiesFaker()
 	expectedQuorumCert := &utils.QuorumCert{
 		ProposedBlockInfo: blockInfo,
 		Signatures:        []utils.Signature{},

consensus/tests/mine_test.go

@@ -47,7 +47,7 @@ func TestYourTurnInitialV2(t *testing.T) {
 	assert.Nil(t, err)
 	// round=1, so masternode[1] has YourTurn = True
 	assert.True(t, b)
-	assert.Equal(t, adaptor.EngineV2.GetCurrentRound(), utils.Round(1))
+	assert.Equal(t, adaptor.EngineV2.GetCurrentRoundFaker(), utils.Round(1))
 
 	snap, err := adaptor.EngineV2.GetSnapshot(blockchain, block900.Header())
 	assert.Nil(t, err)

consensus/tests/penalty_test.go

@@ -36,7 +36,7 @@ func TestHookPenaltyV2Mining(t *testing.T) {
 	// set adaptor round/qc to that of 6299
 	err = utils.DecodeBytesExtraFields(header6300.Extra, &extraField)
 	assert.Nil(t, err)
-	err = adaptor.EngineV2.ProcessQC(blockchain, extraField.QuorumCert)
+	err = adaptor.EngineV2.ProcessQCFaker(blockchain, extraField.QuorumCert)
 	assert.Nil(t, err)
 	headerMining := &types.Header{
 		ParentHash: header6300.ParentHash,