diff --git a/common/types.go b/common/types.go index 4c31429a53..d8db199883 100644 --- a/common/types.go +++ b/common/types.go @@ -279,19 +279,23 @@ func (a UnprefixedAddress) MarshalText() ([]byte, error) { // Extract validators from byte array. func RemoveItemFromArray(array []Address, items []Address) []Address { + // Create newArray to stop append change array value + newArray := make([]Address, len(array)) + copy(newArray, array) + if len(items) == 0 { - return array + return newArray } for _, item := range items { - for i := len(array) - 1; i >= 0; i-- { - if array[i] == item { - array = append(array[:i], array[i+1:]...) + for i := len(newArray) - 1; i >= 0; i-- { + if newArray[i] == item { + newArray = append(newArray[:i], newArray[i+1:]...) } } } - return array + return newArray } // Extract validators from byte array. diff --git a/common/types_test.go b/common/types_test.go index d7d310576d..1ec2bfc26c 100644 --- a/common/types_test.go +++ b/common/types_test.go @@ -158,8 +158,12 @@ func BenchmarkAddressHex(b *testing.B) { func TestRemoveItemInArray(t *testing.T) { array := []Address{HexToAddress("0x0000003"), HexToAddress("0x0000001"), HexToAddress("0x0000002"), HexToAddress("0x0000003")} remove := []Address{HexToAddress("0x0000002"), HexToAddress("0x0000004"), HexToAddress("0x0000003")} - array = RemoveItemFromArray(array, remove) - if len(array) != 1 { + newArray := RemoveItemFromArray(array, remove) + + if array[0] != HexToAddress("0x0000003") || array[2] != HexToAddress("0x0000002") { + t.Error("should keep the original item from array address") + } + if len(newArray) != 1 { t.Error("fail remove item from array address") } } diff --git a/consensus/XDPoS/engines/engine_v2/epochSwitch.go b/consensus/XDPoS/engines/engine_v2/epochSwitch.go index eb16de6bd5..1bb195a5e9 100644 --- a/consensus/XDPoS/engines/engine_v2/epochSwitch.go +++ b/consensus/XDPoS/engines/engine_v2/epochSwitch.go @@ -88,12 +88,16 @@ func (x *XDPoS_v2) isEpochSwitchAtRound(round utils.Round, parentHeader *types.H return true, epochNum, nil } - _, round, _, err := x.getExtraFields(parentHeader) + _, parentRound, _, err := x.getExtraFields(parentHeader) if err != nil { log.Error("[IsEpochSwitch] decode header error", "err", err, "header", parentHeader, "extra", common.Bytes2Hex(parentHeader.Extra)) return false, 0, err } - parentRound := round + if round <= parentRound { + // this round is no larger than parentRound, should return false + return false, epochNum, nil + } + epochStartRound := round - round%utils.Round(x.config.Epoch) return parentRound < epochStartRound, epochNum, nil } diff --git a/consensus/XDPoS/engines/engine_v2/testing_utils.go b/consensus/XDPoS/engines/engine_v2/testing_utils.go index cc60c41584..8a6f94c6b3 100644 --- a/consensus/XDPoS/engines/engine_v2/testing_utils.go +++ b/consensus/XDPoS/engines/engine_v2/testing_utils.go @@ -1,6 +1,7 @@ package engine_v2 import ( + "github.com/XinFinOrg/XDPoSChain/common" "github.com/XinFinOrg/XDPoSChain/consensus" "github.com/XinFinOrg/XDPoSChain/consensus/XDPoS/utils" ) @@ -74,6 +75,14 @@ func (x *XDPoS_v2) GetTimeoutPoolKeyListFaker() []string { return x.timeoutPool.PoolObjKeysList() } +// Fake the signer address, the signing function is incompatible +func (x *XDPoS_v2) AuthorizeFaker(signer common.Address) { + x.signLock.Lock() + defer x.signLock.Unlock() + + x.signer = signer +} + func (x *XDPoS_v2) GetForensicsFaker() *Forensics { return x.forensics } diff --git a/consensus/tests/engine_v2_tests/penalty_test.go b/consensus/tests/engine_v2_tests/penalty_test.go index a74c6c13b8..c4182f3e06 100644 --- a/consensus/tests/engine_v2_tests/penalty_test.go +++ b/consensus/tests/engine_v2_tests/penalty_test.go @@ -15,7 +15,7 @@ import ( func TestHookPenaltyV2Mining(t *testing.T) { config := params.TestXDPoSMockChainConfig - blockchain, _, _, signer, _, _ := PrepareXDCTestBlockChainForV2Engine(t, int(config.XDPoS.Epoch)*7, config, 0) + blockchain, _, _, _, _, _ := PrepareXDCTestBlockChainForV2Engine(t, int(config.XDPoS.Epoch)*7, config, 0) adaptor := blockchain.Engine().(*XDPoS.XDPoS) hooks.AttachConsensusV2Hooks(adaptor, blockchain, config) assert.NotNil(t, adaptor.EngineV2.HookPenalty) @@ -29,7 +29,8 @@ func TestHookPenaltyV2Mining(t *testing.T) { header6300 := blockchain.GetHeaderByNumber(config.XDPoS.Epoch * 7) penalty, err := adaptor.EngineV2.HookPenalty(blockchain, big.NewInt(int64(config.XDPoS.Epoch*7)), header6300.ParentHash, masternodes) assert.Nil(t, err) - // only 1 signer address not in the masternode list + // when we prepare the chain, we include all 5 signers as coinbase except one signer + // header6300 records 5 masternodes, so penalty contains 5-4=1 address assert.Equal(t, 1, len(penalty)) contains := false for _, mn := range common.RemoveItemFromArray(masternodes, penalty) { @@ -43,20 +44,23 @@ func TestHookPenaltyV2Mining(t *testing.T) { assert.Nil(t, err) err = adaptor.EngineV2.ProcessQCFaker(blockchain, extraField.QuorumCert) assert.Nil(t, err) + // coinbase is a faker signer headerMining := &types.Header{ ParentHash: header6300.ParentHash, Number: header6300.Number, GasLimit: params.TargetGasLimit, Time: header6300.Time, - Coinbase: signer, + Coinbase: acc1Addr, } // Force to make the node to be at its round to mine, otherwise won't pass the yourturn masternodes check - // We have 5 nodes in total and the node signer is always at the 4th(last) in the list. Hence int(config.XDPoS.Epoch)*7+4-900, the +4 means is to force to next 4 round and -900 is the relative round number to block number int(config.XDPoS.Epoch)*7 - adaptor.EngineV2.SetNewRoundFaker(blockchain, utils.Round(int(config.XDPoS.Epoch)*7+4-900), false) + // We have 19 nodes in total (20 candidates in snapshot - 1 penalty) and the fake signer is always at the 18th(last) in the list. Hence int(config.XDPoS.Epoch)*7+18-900, the +18 means is to force to next 18 round and -900 is the relative round number to block number int(config.XDPoS.Epoch)*7 + adaptor.EngineV2.SetNewRoundFaker(blockchain, utils.Round(int(config.XDPoS.Epoch)*7+18-900), false) + // The test default signer is not in the msaternodes, so we set the faker signer + adaptor.EngineV2.AuthorizeFaker(acc1Addr) err = adaptor.Prepare(blockchain, headerMining) assert.Nil(t, err) assert.Equal(t, 1, len(headerMining.Penalties)/common.AddressLength) - // 20 candidates (set by PrepareXDCTestBlockChainForV2Engine) - 3 penalty = 17 + // 20 candidates (set by PrepareXDCTestBlockChainForV2Engine) - 1 penalty = 19 assert.Equal(t, 19, len(headerMining.Validators)/common.AddressLength) } diff --git a/consensus/tests/engine_v2_tests/timeout_test.go b/consensus/tests/engine_v2_tests/timeout_test.go index 0b2345c09b..8663945662 100644 --- a/consensus/tests/engine_v2_tests/timeout_test.go +++ b/consensus/tests/engine_v2_tests/timeout_test.go @@ -16,14 +16,14 @@ import ( ) func TestCountdownTimeoutToSendTimeoutMessage(t *testing.T) { - blockchain, _, _, _, _, _ := PrepareXDCTestBlockChainForV2Engine(t, 2251, params.TestXDPoSMockChainConfig, 0) + blockchain, _, _, _, _, _ := PrepareXDCTestBlockChainForV2Engine(t, 901, params.TestXDPoSMockChainConfig, 0) engineV2 := blockchain.Engine().(*XDPoS.XDPoS).EngineV2 timeoutMsg := <-engineV2.BroadcastCh poolSize := engineV2.GetTimeoutPoolSizeFaker(timeoutMsg.(*utils.Timeout)) assert.Equal(t, poolSize, 1) assert.NotNil(t, timeoutMsg) - assert.Equal(t, uint64(1350), timeoutMsg.(*utils.Timeout).GapNumber) + assert.Equal(t, uint64(450), timeoutMsg.(*utils.Timeout).GapNumber) assert.Equal(t, utils.Round(1), timeoutMsg.(*utils.Timeout).Round) }