Merge pull request #81 from hash-laboratories-au/xin-176

xin-176 fix yourturn and new masternode bug

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-- {
+		for i := len(newArray) - 1; i >= 0; i-- {
-			if array[i] == item {
+			if newArray[i] == item {
-				array = append(array[:i], array[i+1:]...)
+				newArray = append(newArray[:i], newArray[i+1:]...)
 			}
 		}
 	}
 
-	return array
+	return newArray
 }
 
 // Extract validators from byte array.

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)
+	newArray := RemoveItemFromArray(array, remove)
-	if len(array) != 1 {
+
+	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")
 	}
 }

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
 }

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
 }

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
+	// 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+4-900), false)
+	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)
 }
 

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)
 }