From 92857e50e5b393626c95223d8ec353c1f90f7dd0 Mon Sep 17 00:00:00 2001 From: Liam Date: Sat, 2 Apr 2022 23:59:39 -0500 Subject: [PATCH] xin-177 check penalty only on epoch switch block and Add Hook on initial (#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 | 27 +++++++++++------ consensus/XDPoS/engines/engine_v2/mining.go | 17 +++-------- .../XDPoS/engines/engine_v2/verifyHeader.go | 29 +++++++++++-------- .../engine_v2_tests/verify_header_test.go | 15 ++++++---- eth/backend.go | 1 + 5 files changed, 50 insertions(+), 39 deletions(-) diff --git a/consensus/XDPoS/engines/engine_v2/engine.go b/consensus/XDPoS/engines/engine_v2/engine.go index adf6d02a5049..c1eb7ebbf5d0 100644 --- a/consensus/XDPoS/engines/engine_v2/engine.go +++ b/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 diff --git a/consensus/XDPoS/engines/engine_v2/mining.go b/consensus/XDPoS/engines/engine_v2/mining.go index b672780ce2dc..5d3805e32dbd 100644 --- a/consensus/XDPoS/engines/engine_v2/mining.go +++ b/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 diff --git a/consensus/XDPoS/engines/engine_v2/verifyHeader.go b/consensus/XDPoS/engines/engine_v2/verifyHeader.go index 819712c7acb5..2b78022fe608 100644 --- a/consensus/XDPoS/engines/engine_v2/verifyHeader.go +++ b/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 diff --git a/consensus/tests/engine_v2_tests/verify_header_test.go b/consensus/tests/engine_v2_tests/verify_header_test.go index a380b1c69377..5a698379ae7d 100644 --- a/consensus/tests/engine_v2_tests/verify_header_test.go +++ b/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) diff --git a/eth/backend.go b/eth/backend.go index a2451606cb56..519f528a5671 100644 --- a/eth/backend.go +++ b/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()