xin-177 check penalty only on epoch switch block and Add Hook on init…

…ial (ethereum#78)

* check penalty only on epoch switch block

* skip calculate penalty on first v2 block

* clean code, its doing same thing

consensus/XDPoS/engines/engine_v2/engine.go

@@ -1000,16 +1000,25 @@ func (x *XDPoS_v2) calcMasternodes(chain consensus.ChainReader, blockNum *big.In
 		return nil, nil, err
 	}
 	candidates := snap.NextEpochMasterNodes
-	if x.HookPenalty != nil {
-		penalties, err := x.HookPenalty(chain, blockNum, parentHash, candidates)
-		if err != nil {
-			log.Error("[calcMasternodes] Adaptor v2 HookPenalty has error", "err", err)
-			return nil, nil, err
-		}
-		masternodes := common.RemoveItemFromArray(candidates, penalties)
-		return masternodes, penalties, nil
+
+	if blockNum.Uint64() == x.config.V2.SwitchBlock.Uint64()+1 {
+		log.Info("[calcMasternodes] examing first v2 block")
+		return candidates, []common.Address{}, nil
+	}
+
+	if x.HookPenalty == nil {
+		log.Info("[calcMasternodes] no hook penalty defined")
+		return candidates, []common.Address{}, nil
 	}
-	return candidates, []common.Address{}, nil
+
+	penalties, err := x.HookPenalty(chain, blockNum, parentHash, candidates)
+	if err != nil {
+		log.Error("[calcMasternodes] Adaptor v2 HookPenalty has error", "err", err)
+		return nil, nil, err
+	}
+	masternodes := common.RemoveItemFromArray(candidates, penalties)
+	return masternodes, penalties, nil
+
 }
 
 // Given hash, get master node from the epoch switch block of the epoch

consensus/XDPoS/engines/engine_v2/mining.go

@@ -25,19 +25,10 @@ func (x *XDPoS_v2) yourturn(chain consensus.ChainReader, round utils.Round, pare
 	}
 	var masterNodes []common.Address
 	if isEpochSwitch {
-		if x.config.V2.SwitchBlock.Cmp(parent.Number) == 0 {
-			// the initial master nodes of v1->v2 switch contains penalties node
-			_, _, masterNodes, err = x.getExtraFields(parent)
-			if err != nil {
-				log.Error("[yourturn] Cannot find snapshot at gap num of last V1", "err", err, "number", x.config.V2.SwitchBlock.Uint64())
-				return false, err
-			}
-		} else {
-			masterNodes, _, err = x.calcMasternodes(chain, big.NewInt(0).Add(parent.Number, big.NewInt(1)), parent.Hash())
-			if err != nil {
-				log.Error("[yourturn] Cannot calcMasternodes at gap num ", "err", err, "parent number", parent.Number)
-				return false, err
-			}
+		masterNodes, _, err = x.calcMasternodes(chain, big.NewInt(0).Add(parent.Number, big.NewInt(1)), parent.Hash())
+		if err != nil {
+			log.Error("[yourturn] Cannot calcMasternodes at gap num ", "err", err, "parent number", parent.Number)
+			return false, err
 		}
 	} else {
 		// this block and parent belong to the same epoch

consensus/XDPoS/engines/engine_v2/verifyHeader.go

@@ -109,9 +109,24 @@ func (x *XDPoS_v2) verifyHeader(chain consensus.ChainReader, header *types.Heade
 		if !isLegit {
 			return utils.ErrValidatorsNotLegit
 		}
+
+		_, 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
+		}
+
+		if !utils.CompareSignersLists(common.ExtractAddressFromBytes(header.Penalties), penalties) {
+			return utils.ErrPenaltyListDoesNotMatch
+		}
+
 	} else {
 		if len(header.Validators) != 0 {
-			log.Warn("[verifyHeader] Validators shall not have values in non-epochSwitch block", "Hash", header.Hash(), "Number", header.Number, "ValidatorsLength", len(header.Validators))
+			log.Warn("[verifyHeader] Validators shall not have values in non-epochSwitch block", "Hash", header.Hash(), "Number", header.Number, "header.Validators", header.Validators)
+			return utils.ErrInvalidFieldInNonEpochSwitch
+		}
+		if len(header.Penalties) != 0 {
+			log.Warn("[verifyHeader] Penalties shall not have values in non-epochSwitch block", "Hash", header.Hash(), "Number", header.Number, "header.Penalties", header.Penalties)
 			return utils.ErrInvalidFieldInNonEpochSwitch
 		}
 	}
@@ -121,16 +136,6 @@ func (x *XDPoS_v2) verifyHeader(chain consensus.ChainReader, header *types.Heade
 		return err
 	}
 
-	_, 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
-	}
-
-	if !utils.CompareSignersLists(common.ExtractAddressFromBytes(header.Penalties), penalties) {
-		return utils.ErrPenaltyListDoesNotMatch
-	}
-
 	// Check its validator
 	masterNodes := x.GetMasternodes(chain, header)
 	verified, validatorAddress, err := x.verifyMsgSignature(sigHash(header), header.Validator, masterNodes)
@@ -165,7 +170,7 @@ func (x *XDPoS_v2) verifyHeader(chain consensus.ChainReader, header *types.Heade
 func (x *XDPoS_v2) isValidatorsLegit(chain consensus.ChainReader, header *types.Header) (bool, error) {
 	snap, err := x.getSnapshot(chain, header.Number.Uint64(), false)
 	if err != nil {
-		log.Error("[checkMasternodesOnEpochSwitch] Error while trying to get snapshot", "BlockNumber", header.Number.Int64(), "Hash", header.Hash().Hex(), "error", err)
+		log.Error("[isValidatorsLegit] Error while trying to get snapshot", "BlockNumber", header.Number.Int64(), "Hash", header.Hash().Hex(), "error", err)
 		return false, err
 	}
 	// snap.NextEpochMasterNodes

consensus/tests/engine_v2_tests/verify_header_test.go

@@ -80,12 +80,22 @@ func TestShouldVerifyBlock(t *testing.T) {
 	err = adaptor.VerifyHeader(blockchain, invalidValidatorsSignerBlock, true)
 	assert.Equal(t, utils.ErrInvalidCheckpointSigners, err)
 
+	invalidPenaltiesExistBlock := blockchain.GetBlockByNumber(901).Header()
+	invalidPenaltiesExistBlock.Penalties = common.Hex2BytesFixed("123131231", 20)
+	err = adaptor.VerifyHeader(blockchain, invalidPenaltiesExistBlock, true)
+	assert.Equal(t, utils.ErrPenaltyListDoesNotMatch, err)
+
 	// non-epoch switch
 	invalidValidatorsExistBlock := blockchain.GetBlockByNumber(902).Header()
 	invalidValidatorsExistBlock.Validators = []byte{123}
 	err = adaptor.VerifyHeader(blockchain, invalidValidatorsExistBlock, true)
 	assert.Equal(t, utils.ErrInvalidFieldInNonEpochSwitch, err)
 
+	invalidPenaltiesExistBlock = blockchain.GetBlockByNumber(902).Header()
+	invalidPenaltiesExistBlock.Penalties = common.Hex2BytesFixed("123131231", 20)
+	err = adaptor.VerifyHeader(blockchain, invalidPenaltiesExistBlock, true)
+	assert.Equal(t, utils.ErrInvalidFieldInNonEpochSwitch, err)
+
 	merkleRoot := "35999dded35e8db12de7e6c1471eb9670c162eec616ecebbaf4fddd4676fb123"
 	parentNotExistBlock := blockchain.GetBlockByNumber(901).Header()
 	parentNotExistBlock.ParentHash = common.HexToHash(merkleRoot)
@@ -143,11 +153,6 @@ func TestShouldVerifyBlock(t *testing.T) {
 	err = adaptor.VerifyHeader(blockchain, invalidRoundBlock, true)
 	assert.Equal(t, utils.ErrRoundInvalid, err)
 
-	invalidPenaltiesExistBlock := blockchain.GetBlockByNumber(902).Header()
-	invalidPenaltiesExistBlock.Penalties = common.Hex2BytesFixed("123131231", 20)
-	err = adaptor.VerifyHeader(blockchain, invalidPenaltiesExistBlock, true)
-	assert.Equal(t, utils.ErrPenaltyListDoesNotMatch, err)
-
 	// Not valid validator
 	coinbaseValidatorMismatchBlock := blockchain.GetBlockByNumber(902).Header()
 	notQualifiedSigner, notQualifiedSignFn, err := getSignerAndSignFn(voterKey)

eth/backend.go

@@ -285,6 +285,7 @@ func New(ctx *node.ServiceContext, config *Config, XDCXServ *XDCx.XDCX, lendingS
 			XDPoS1.0 Specific hooks
 		*/
 		hooks.AttachConsensusV1Hooks(c, eth.blockchain, chainConfig)
+		hooks.AttachConsensusV2Hooks(c, eth.blockchain, chainConfig)
 
 		eth.txPool.IsSigner = func(address common.Address) bool {
 			currentHeader := eth.blockchain.CurrentHeader()