Xin 118 refactor yourturn in v1 and miner (#41)

* refactor wait log into adaptor

* refactor v1 yourturn

consensus/XDPoS/XDPoS.go

@@ -277,7 +277,7 @@ func (x *XDPoS) GetMasternodesByNumber(chain consensus.ChainReader, blockNumber
 	}
 }
 
-func (x *XDPoS) YourTurn(chain consensus.ChainReader, parent *types.Header, signer common.Address) (int, int, int, bool, error) {
+func (x *XDPoS) YourTurn(chain consensus.ChainReader, parent *types.Header, signer common.Address) (bool, error) {
 	switch x.config.BlockConsensusVersion(parent.Number) {
 	case params.ConsensusEngineVersion2:
 		return x.EngineV2.YourTurn(chain, parent, signer)

consensus/XDPoS/engines/engine_v1/engine.go

@@ -28,6 +28,13 @@ import (
 	lru "github.com/hashicorp/golang-lru"
 )
 
+const (
+	// timeout waiting for M1
+	waitPeriod = 10
+	// timeout for checkpoint.
+	waitPeriodCheckpoint = 20
+)
+
 // XDPoS is the delegated-proof-of-stake consensus engine proposed to support the
 // Ethereum testnet following the Ropsten attacks.
 type XDPoS_v1 struct {
@@ -383,8 +390,41 @@ func (x *XDPoS_v1) whoIsCreator(snap *SnapshotV1, header *types.Header) (common.
 	}
 	return m, nil
 }
+func (x *XDPoS_v1) YourTurn(chain consensus.ChainReader, parent *types.Header, signer common.Address) (bool, error) {
+	len, preIndex, curIndex, ok, err := x.yourTurn(chain, parent, signer)
 
-func (x *XDPoS_v1) YourTurn(chain consensus.ChainReader, parent *types.Header, signer common.Address) (int, int, int, bool, error) {
+	if err != nil {
+		log.Warn("Failed when trying to commit new work", "err", err)
+		return false, err
+	}
+	if !ok {
+		// in case some nodes are down
+		if preIndex == -1 {
+			// first block
+			return false, nil
+		}
+		if curIndex == -1 {
+			// you're not allowed to create this block
+			return false, nil
+		}
+		h := utils.Hop(len, preIndex, curIndex)
+		gap := waitPeriod * int64(h)
+		// Check nearest checkpoint block in hop range.
+		nearest := x.config.Epoch - (parent.Number.Uint64() % x.config.Epoch)
+		if uint64(h) >= nearest {
+			gap = waitPeriodCheckpoint * int64(h)
+		}
+		log.Info("Distance from the parent block", "seconds", gap, "hops", h)
+		waitedTime := time.Now().Unix() - parent.Time.Int64()
+		if gap > waitedTime {
+			return false, nil
+		}
+		log.Info("Wait enough. It's my turn", "waited seconds", waitedTime)
+	}
+	return true, nil
+}
+
+func (x *XDPoS_v1) yourTurn(chain consensus.ChainReader, parent *types.Header, signer common.Address) (int, int, int, bool, error) {
 	masternodes := x.GetMasternodes(chain, parent)
 
 	// if common.IsTestnet {
@@ -896,7 +936,7 @@ func (x *XDPoS_v1) calcDifficulty(chain consensus.ChainReader, parent *types.Hea
 	if x.config.SkipValidation {
 		return big.NewInt(1)
 	}
-	len, preIndex, curIndex, _, err := x.YourTurn(chain, parent, signer)
+	len, preIndex, curIndex, _, err := x.yourTurn(chain, parent, signer)
 	if err != nil {
 		return big.NewInt(int64(len + curIndex - preIndex))
 	}

consensus/XDPoS/engines/engine_v2/engine.go

@@ -321,7 +321,7 @@ func (x *XDPoS_v2) calcDifficulty(chain consensus.ChainReader, parent *types.Hea
 }
 
 // Check if it's my turm to mine a block. Note: The second return value `preIndex` is useless in V2 engine
-func (x *XDPoS_v2) YourTurn(chain consensus.ChainReader, parent *types.Header, signer common.Address) (int, int, int, bool, error) {
+func (x *XDPoS_v2) YourTurn(chain consensus.ChainReader, parent *types.Header, signer common.Address) (bool, error) {
 	x.lock.RLock()
 	defer x.lock.RUnlock()
 
@@ -329,7 +329,7 @@ func (x *XDPoS_v2) YourTurn(chain consensus.ChainReader, parent *types.Header, s
 	isEpochSwitch, _, err := x.IsEpochSwitchAtRound(round, parent)
 	if err != nil {
 		log.Error("[YourTurn]", "Error", err)
-		return 0, -1, -1, false, err
+		return false, err
 	}
 	var masterNodes []common.Address
 	if isEpochSwitch {
@@ -346,7 +346,7 @@ func (x *XDPoS_v2) YourTurn(chain consensus.ChainReader, parent *types.Header, s
 
 	if len(masterNodes) == 0 {
 		log.Error("[YourTurn] Fail to find any master nodes from current block round epoch", "Hash", parent.Hash(), "CurrentRound", round, "Number", parent.Number)
-		return 0, -1, -1, false, errors.New("Masternodes not found")
+		return false, errors.New("Masternodes not found")
 	}
 	leaderIndex := uint64(round) % x.config.Epoch % uint64(len(masterNodes))
 
@@ -359,10 +359,10 @@ func (x *XDPoS_v2) YourTurn(chain consensus.ChainReader, parent *types.Header, s
 	}
 
 	if masterNodes[leaderIndex] == signer {
-		return len(masterNodes), -1, curIndex, true, nil
+		return true, nil
 	}
 	log.Warn("[YourTurn] Not authorised signer", "signer", signer, "MN", masterNodes, "Hash", parent.Hash(), "masterNodes[leaderIndex]", masterNodes[leaderIndex], "signer", signer)
-	return len(masterNodes), -1, curIndex, false, nil
+	return false, nil
 }
 
 func (x *XDPoS_v2) IsAuthorisedAddress(chain consensus.ChainReader, header *types.Header, address common.Address) bool {

consensus/tests/authorised_masternode_test.go

@@ -118,22 +118,16 @@ func TestIsYourTurnConsensusV2(t *testing.T) {
 	}
 
 	// The first address is valid
-	numberOfMN, _, curIndex, isYourTurn, err := adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc0278C350152e15fa6FFC712a5A73D704Ce73E2E1"))
+	isYourTurn, err := adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc0278C350152e15fa6FFC712a5A73D704Ce73E2E1"))
 	assert.Nil(t, err)
-	assert.Equal(t, 3, numberOfMN)
-	assert.Equal(t, 0, curIndex)
 	assert.True(t, isYourTurn)
 
 	// The second and third address are not valid
-	numberOfMN, _, curIndex, isYourTurn, err = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc03d9e17Ae3fF2c6712E44e25B09Ac5ee91f6c9ff"))
+	isYourTurn, err = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc03d9e17Ae3fF2c6712E44e25B09Ac5ee91f6c9ff"))
 	assert.Nil(t, err)
-	assert.Equal(t, 3, numberOfMN)
-	assert.Equal(t, 1, curIndex)
 	assert.False(t, isYourTurn)
-	numberOfMN, _, curIndex, isYourTurn, err = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc065551F0dcAC6f00CAe11192D462db709bE3758c"))
+	isYourTurn, err = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc065551F0dcAC6f00CAe11192D462db709bE3758c"))
 	assert.Nil(t, err)
-	assert.Equal(t, 3, numberOfMN)
-	assert.Equal(t, 2, curIndex)
 	assert.False(t, isYourTurn)
 
 	// We continue to grow the chain which will increase the round number
@@ -145,16 +139,13 @@ func TestIsYourTurnConsensusV2(t *testing.T) {
 	}
 
 	adaptor.EngineV2.SetNewRoundFaker(1, false)
-	_, _, curIndex, isYourTurn, _ = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc0278C350152e15fa6FFC712a5A73D704Ce73E2E1"))
+	isYourTurn, _ = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc0278C350152e15fa6FFC712a5A73D704Ce73E2E1"))
-	assert.Equal(t, 0, curIndex)
 	assert.False(t, isYourTurn)
 
-	_, _, curIndex, isYourTurn, _ = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc03d9e17Ae3fF2c6712E44e25B09Ac5ee91f6c9ff"))
+	isYourTurn, _ = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc03d9e17Ae3fF2c6712E44e25B09Ac5ee91f6c9ff"))
-	assert.Equal(t, 1, curIndex)
 	assert.True(t, isYourTurn)
 
-	_, _, curIndex, isYourTurn, _ = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc065551F0dcAC6f00CAe11192D462db709bE3758c"))
+	isYourTurn, _ = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc065551F0dcAC6f00CAe11192D462db709bE3758c"))
-	assert.Equal(t, 2, curIndex)
 	assert.False(t, isYourTurn)
 
 }

miner/worker.go

@@ -34,7 +34,6 @@ import (
 	"github.com/XinFinOrg/XDPoSChain/common"
 	"github.com/XinFinOrg/XDPoSChain/consensus"
 	"github.com/XinFinOrg/XDPoSChain/consensus/XDPoS"
-	"github.com/XinFinOrg/XDPoSChain/consensus/XDPoS/utils"
 	"github.com/XinFinOrg/XDPoSChain/consensus/misc"
 	"github.com/XinFinOrg/XDPoSChain/contracts"
 	"github.com/XinFinOrg/XDPoSChain/core"
@@ -528,41 +527,17 @@ func (self *worker) commitNewWork() {
 	// Only try to commit new work if we are mining
 	if atomic.LoadInt32(&self.mining) == 1 {
 		// check if we are right after parent's coinbase in the list
-		// only go with XDPoS
 		if self.config.XDPoS != nil {
-			// get masternodes set from latest checkpoint
-			// TODO: refactor on yourturn with below condition for v1 v2
 			c := self.engine.(*XDPoS.XDPoS)
-			len, preIndex, curIndex, ok, err := c.YourTurn(self.chain, parent.Header(), self.coinbase)
+			ok, err := c.YourTurn(self.chain, parent.Header(), self.coinbase)
 			if err != nil {
 				log.Warn("Failed when trying to commit new work", "err", err)
 				return
 			}
 			if !ok {
 				log.Info("Not my turn to commit block. Waiting...")
-				// in case some nodes are down
-				if preIndex == -1 {
-					// first block
 				return
 			}
-				if curIndex == -1 {
-					// you're not allowed to create this block
-					return
-				}
-				h := utils.Hop(len, preIndex, curIndex)
-				gap := waitPeriod * int64(h)
-				// Check nearest checkpoint block in hop range.
-				nearest := self.config.XDPoS.Epoch - (parent.Header().Number.Uint64() % self.config.XDPoS.Epoch)
-				if uint64(h) >= nearest {
-					gap = waitPeriodCheckpoint * int64(h)
-				}
-				log.Info("Distance from the parent block", "seconds", gap, "hops", h)
-				waitedTime := time.Now().Unix() - parent.Header().Time.Int64()
-				if gap > waitedTime {
-					return
-				}
-				log.Info("Wait enough. It's my turn", "waited seconds", waitedTime)
-			}
 		}
 	}
 	tstamp := tstart.Unix()