move verify QC into verify header, fix broken tests etc (#61)

common/countdown/countdown.go

@@ -62,6 +62,8 @@ func (t *CountdownTimer) startTimer() {
 			if err != nil {
 				log.Error("OnTimeoutFn error", err)
 			}
+			log.Debug("Reset timer after timeout reached and OnTimeoutFn processed")
+			timer.Reset(t.timeoutDuration)
 		case <-t.resetc:
 			log.Debug("Reset countdown timer")
 			timer.Reset(t.timeoutDuration)

common/countdown/countdown_test.go

@@ -1,6 +1,7 @@
 package countdown
 
 import (
+	"fmt"
 	"testing"
 	"time"
 
@@ -59,7 +60,53 @@ firstReset:
 	// Now the countdown is paused after calling the callback function, let's reset it again
 	assert.True(t, countdown.isInitilised())
 	expectedTimeAfterReset := time.Now().Add(5000 * time.Millisecond)
+	<-called
+	// Always initilised
+	assert.True(t, countdown.isInitilised())
+	if time.Now().After(expectedTimeAfterReset) {
+		t.Log("Correctly reset the countdown second time")
+	} else {
+		t.Fatalf("Countdown did not reset correctly second time")
+	}
+}
+
+func TestCountdownShouldResetEvenIfErrored(t *testing.T) {
+	called := make(chan int)
+	OnTimeoutFn := func(time time.Time) error {
+		called <- 1
+		return fmt.Errorf("ERROR!")
+	}
+
+	countdown := NewCountDown(5000 * time.Millisecond)
+	countdown.OnTimeoutFn = OnTimeoutFn
+	// Check countdown did not start
+	assert.False(t, countdown.isInitilised())
 	countdown.Reset()
+	// Now the countdown should already started
+	assert.True(t, countdown.isInitilised())
+	expectedCalledTime := time.Now().Add(9000 * time.Millisecond)
+	resetTimer := time.NewTimer(4000 * time.Millisecond)
+
+firstReset:
+	for {
+		select {
+		case <-called:
+			if time.Now().After(expectedCalledTime) {
+				// Make sure the countdown runs forever
+				assert.True(t, countdown.isInitilised())
+				t.Log("Correctly reset the countdown once")
+			} else {
+				t.Fatalf("Countdown did not reset correctly first time")
+			}
+			break firstReset
+		case <-resetTimer.C:
+			countdown.Reset()
+		}
+	}
+
+	// Now the countdown is paused after calling the callback function, let's reset it again
+	assert.True(t, countdown.isInitilised())
+	expectedTimeAfterReset := time.Now().Add(5000 * time.Millisecond)
 	<-called
 	// Always initilised
 	assert.True(t, countdown.isInitilised())

consensus/XDPoS/XDPoS.go

@@ -445,6 +445,19 @@ func (x *XDPoS) GetAuthorisedSignersFromSnapshot(chain consensus.ChainReader, he
 	}
 }
 
+func (x *XDPoS) FindParentBlockToAssign(chain consensus.ChainReader, currentBlock *types.Block) *types.Block {
+	switch x.config.BlockConsensusVersion(currentBlock.Number()) {
+	case params.ConsensusEngineVersion2:
+		block := x.EngineV2.FindParentBlockToAssign(chain)
+		if block == nil {
+			return currentBlock
+		}
+		return block
+	default: // Default "v1"
+		return currentBlock
+	}
+}
+
 /**
 Caching
 */
@@ -502,7 +515,7 @@ func (x *XDPoS) initialV2FromLastV1(chain consensus.ChainReader, header *types.H
 	checkpointBlockNumber := header.Number.Uint64() - header.Number.Uint64()%x.config.Epoch
 	checkpointHeader := chain.GetHeaderByNumber(checkpointBlockNumber)
 	masternodes := x.EngineV1.GetMasternodesFromCheckpointHeader(checkpointHeader)
-	err := x.EngineV2.Initial(chain, header, masternodes)
+	err := x.EngineV2.Initial(chain, masternodes)
 	if err != nil {
 		return err
 	}

consensus/XDPoS/engines/engine_v2/engine.go

@@ -121,11 +121,11 @@ func (x *XDPoS_v2) SignHash(header *types.Header) (hash common.Hash) {
 	return sigHash(header)
 }
 
-func (x *XDPoS_v2) Initial(chain consensus.ChainReader, header *types.Header, masternodes []common.Address) error {
+func (x *XDPoS_v2) Initial(chain consensus.ChainReader, masternodes []common.Address) error {
 	log.Info("[Initial] initial v2 related parameters")
 
 	if x.highestQuorumCert.ProposedBlockInfo.Hash != (common.Hash{}) { // already initialized
-		log.Error("[Initial] Already initialized", "blockNum", header.Number, "Hash", header.Hash())
+		log.Error("[Initial] Already initialized", "x.highestQuorumCert.ProposedBlockInfo.Hash", x.highestQuorumCert.ProposedBlockInfo.Hash)
 		return nil
 	}
 
@@ -133,12 +133,14 @@ func (x *XDPoS_v2) Initial(chain consensus.ChainReader, header *types.Header, ma
 	defer x.lock.Unlock()
 	// Check header if it is the first consensus v2 block, if so, assign initial values to current round and highestQC
 
-	log.Info("[Initial] highest QC for consensus v2 first block", "BlockNum", header.Number.String(), "BlockHash", header.Hash())
+	log.Info("[Initial] highest QC for consensus v2 first block")
 	// Generate new parent blockInfo and put it into QC
+	// TODO: XIN-147 to initilise V2 engine in a more dynamic way
+	firstV2BlockHeader := chain.GetHeaderByNumber(x.config.V2.SwitchBlock.Uint64())
 	blockInfo := &utils.BlockInfo{
-		Hash:   header.Hash(),
+		Hash:   firstV2BlockHeader.Hash(),
 		Round:  utils.Round(0),
-		Number: header.Number,
+		Number: firstV2BlockHeader.Number,
 	}
 	quorumCert := &utils.QuorumCert{
 		ProposedBlockInfo: blockInfo,
@@ -148,7 +150,7 @@ func (x *XDPoS_v2) Initial(chain consensus.ChainReader, header *types.Header, ma
 	x.highestQuorumCert = quorumCert
 
 	// Initial snapshot
-	lastGapNum := header.Number.Uint64() - header.Number.Uint64()%x.config.Epoch - x.config.Gap
+	lastGapNum := firstV2BlockHeader.Number.Uint64() - firstV2BlockHeader.Number.Uint64()%x.config.Epoch - x.config.Gap
 	lastGapHeader := chain.GetHeaderByNumber(lastGapNum)
 
 	snap := newSnapshot(lastGapNum, lastGapHeader.Hash(), x.currentRound, x.highestQuorumCert, masternodes)
@@ -183,6 +185,7 @@ func (x *XDPoS_v2) Prepare(chain consensus.ChainReader, header *types.Header) er
 	x.lock.RUnlock()
 
 	if header.ParentHash != highestQC.ProposedBlockInfo.Hash {
+		log.Warn("[Prepare] parent hash and QC hash does not match", "blockNum", header.Number, "parentHash", header.ParentHash, "QCHash", highestQC.ProposedBlockInfo.Hash, "QCNumber", highestQC.ProposedBlockInfo.Number)
 		return consensus.ErrNotReadyToPropose
 	}
 
@@ -569,14 +572,11 @@ func (x *XDPoS_v2) verifyHeader(chain consensus.ChainReader, header *types.Heade
 		return utils.ErrInvalidV2Extra
 	}
 	quorumCert := decodedExtraField.QuorumCert
-	if quorumCert == nil || quorumCert.Signatures == nil || len(quorumCert.Signatures) == 0 {
+	err = x.verifyQC(chain, quorumCert)
-		return utils.ErrInvalidQC
+	if err != nil {
+		log.Warn("[verifyHeader] fail to verify QC", "QCNumber", quorumCert.ProposedBlockInfo.Number, "QCsigLength", len(quorumCert.Signatures))
+		return err
 	}
-
-	if quorumCert.ProposedBlockInfo.Hash == (common.Hash{}) {
-		return utils.ErrEmptyBlockInfoHash
-	}
-
 	// Nonces must be 0x00..0 or 0xff..f, zeroes enforced on checkpoints
 	if !bytes.Equal(header.Nonce[:], utils.NonceAuthVote) && !bytes.Equal(header.Nonce[:], utils.NonceDropVote) {
 		return utils.ErrInvalidVote
@@ -590,21 +590,25 @@ func (x *XDPoS_v2) verifyHeader(chain consensus.ChainReader, header *types.Heade
 		return utils.ErrInvalidUncleHash
 	}
 
-	// Verify v2 block that is on the epoch switch
+	isEpochSwitch, _, err := x.IsEpochSwitch(header) // Verify v2 block that is on the epoch switch
-	if header.Validators != nil {
+	if err != nil {
-		// Skip if it's the first v2 block as it wil inherit from last v1 epoch block
+		log.Error("[verifyHeader] error when checking if header is epoch switch header", "Hash", header.Hash(), "Number", header.Number, "Error", err)
-		if header.Number.Uint64() > x.config.V2.SwitchBlock.Uint64()+1 && header.Coinbase != (common.Address{}) {
+		return err
-			return utils.ErrInvalidCheckpointBeneficiary
+	}
-		}
+	if isEpochSwitch {
 		if !bytes.Equal(header.Nonce[:], utils.NonceDropVote) {
 			return utils.ErrInvalidCheckpointVote
 		}
-		if len(header.Validators) == 0 {
+		if header.Validators == nil || len(header.Validators) == 0 {
 			return utils.ErrEmptyEpochSwitchValidators
 		}
 		if len(header.Validators)%common.AddressLength != 0 {
 			return utils.ErrInvalidCheckpointSigners
 		}
+	} else {
+		if header.Validators != nil {
+			return utils.ErrInvalidFieldInNonEpochSwitch
+		}
 	}
 
 	// If all checks passed, validate any special fields for hard forks
@@ -1000,6 +1004,15 @@ func (x *XDPoS_v2) verifyQC(blockChainReader consensus.ChainReader, quorumCert *
 		return fmt.Errorf("Fail to verify QC due to failure in getting epoch switch info")
 	}
 
+	if quorumCert == nil {
+		log.Warn("[verifyQC] QC is Nil")
+		return utils.ErrInvalidQC
+	} else if (quorumCert.ProposedBlockInfo.Number.Uint64() > x.config.V2.SwitchBlock.Uint64()) && (quorumCert.Signatures == nil || (len(quorumCert.Signatures) < x.config.V2.CertThreshold)) {
+		//First V2 Block QC, QC Signatures is initial nil
+		log.Warn("[verifyHeader] Invalid QC Signature is nil or empty", "QC", quorumCert, "QCNumber", quorumCert.ProposedBlockInfo.Number, "Signatures len", len(quorumCert.Signatures))
+		return utils.ErrInvalidQC
+	}
+
 	var wg sync.WaitGroup
 	wg.Add(len(quorumCert.Signatures))
 	var haveError error
@@ -1049,6 +1062,10 @@ func (x *XDPoS_v2) processQC(blockChainReader consensus.ChainReader, quorumCert
 	}
 	// 2. Get QC from header and update lockQuorumCert(lockQuorumCert is the parent of highestQC)
 	proposedBlockHeader := blockChainReader.GetHeaderByHash(quorumCert.ProposedBlockInfo.Hash)
+	if proposedBlockHeader == nil {
+		log.Error("[processQC] Block not found using the QC", "quorumCert.ProposedBlockInfo.Hash", quorumCert.ProposedBlockInfo.Hash, "quorumCert.ProposedBlockInfo.Number", quorumCert.ProposedBlockInfo.Number)
+		return fmt.Errorf("Block not found, number: %v, hash: %v", quorumCert.ProposedBlockInfo.Number, quorumCert.ProposedBlockInfo.Hash)
+	}
 	if proposedBlockHeader.Number.Cmp(x.config.V2.SwitchBlock) > 0 {
 		// Extra field contain parent information
 		var decodedExtraField utils.ExtraFields_v2
@@ -1568,3 +1585,11 @@ func (x *XDPoS_v2) GetPreviousPenaltyByHash(chain consensus.ChainReader, hash co
 	header := chain.GetHeaderByHash(epochSwitchInfo.EpochSwitchBlockInfo.Hash)
 	return common.ExtractAddressFromBytes(header.Penalties)
 }
+
+func (x *XDPoS_v2) FindParentBlockToAssign(chain consensus.ChainReader) *types.Block {
+	parent := chain.GetBlock(x.highestQuorumCert.ProposedBlockInfo.Hash, x.highestQuorumCert.ProposedBlockInfo.Number.Uint64())
+	if parent == nil {
+		log.Error("[FindParentBlockToAssign] Can not find parent block from highestQC proposedBlockInfo", "x.highestQuorumCert.ProposedBlockInfo.Hash", x.highestQuorumCert.ProposedBlockInfo.Hash, "x.highestQuorumCert.ProposedBlockInfo.Number", x.highestQuorumCert.ProposedBlockInfo.Number.Uint64())
+	}
+	return parent
+}

consensus/XDPoS/utils/errors.go

@@ -80,9 +80,10 @@ var (
 
 	ErrEmptyEpochSwitchValidators = errors.New("empty validators list on epoch switch block")
 
-	ErrInvalidV2Extra     = errors.New("Invalid v2 extra in the block")
+	ErrInvalidV2Extra               = errors.New("Invalid v2 extra in the block")
-	ErrInvalidQC          = errors.New("Invalid QC content")
+	ErrInvalidQC                    = errors.New("Invalid QC content")
-	ErrEmptyBlockInfoHash = errors.New("BlockInfo hash is empty")
+	ErrEmptyBlockInfoHash           = errors.New("BlockInfo hash is empty")
+	ErrInvalidFieldInNonEpochSwitch = errors.New("Invalid field exist in a non-epoch swtich block")
 )
 
 type ErrIncomingMessageRoundNotEqualCurrentRound struct {

consensus/tests/adaptor_test.go

@@ -226,3 +226,31 @@ func TestGetCurrentEpochSwitchBlock(t *testing.T) {
 		assert.Equal(t, uint64(1), epochNum)
 	}
 }
+
+func TestGetParentBlock(t *testing.T) {
+	blockchain, _, block900, signer, signFn, _ := PrepareXDCTestBlockChainForV2Engine(t, 900, params.TestXDPoSMockChainConfig, 0)
+	adaptor := blockchain.Engine().(*XDPoS.XDPoS)
+
+	// V1
+	block := adaptor.FindParentBlockToAssign(blockchain, block900)
+	assert.Equal(t, block, block900)
+
+	// Initialise
+	err := adaptor.EngineV2.Initial(blockchain, []common.Address{})
+	assert.Nil(t, err)
+
+	// V2
+	blockNum := 901
+	blockCoinBase := "0x111000000000000000000000000000000123"
+	block901 := CreateBlock(blockchain, params.TestXDPoSMockChainConfig, block900, blockNum, 1, blockCoinBase, signer, signFn, nil)
+	blockchain.InsertBlock(block901)
+
+	// let's inject another one, but the highestedQC has not been updated, so it shall still point to 900
+	blockNum = 902
+	block902 := CreateBlock(blockchain, params.TestXDPoSMockChainConfig, block901, blockNum, 1, blockCoinBase, signer, signFn, nil)
+	blockchain.InsertBlock(block902)
+
+	block = adaptor.FindParentBlockToAssign(blockchain, block902)
+
+	assert.Equal(t, block900.Hash(), block.Hash())
+}

consensus/tests/authorised_masternode_test.go

@@ -103,18 +103,18 @@ func TestIsYourTurnConsensusV2(t *testing.T) {
 	blockchain.InsertBlock(currentBlock)
 
 	// Less then Mine Period
-	isYourTurn, err := adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc703c4b2bD70c169f5717101CaeE543299Fc946C7"))
+	isYourTurn, err := adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc0D3ab14BBaD3D99F4203bd7a11aCB94882050E7e"))
 	assert.Nil(t, err)
 	assert.False(t, isYourTurn)
 
 	time.Sleep(time.Duration(minePeriod) * time.Second)
-	// The first address is valid
+	// The second address is valid as the round starting from 1
-	isYourTurn, err = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc703c4b2bD70c169f5717101CaeE543299Fc946C7"))
+	isYourTurn, err = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc0D3ab14BBaD3D99F4203bd7a11aCB94882050E7e"))
 	assert.Nil(t, err)
 	assert.True(t, isYourTurn)
 
-	// The second and third address are not valid
+	// The first and third address are not valid
-	isYourTurn, err = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc0D3ab14BBaD3D99F4203bd7a11aCB94882050E7e"))
+	isYourTurn, err = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc703c4b2bD70c169f5717101CaeE543299Fc946C7"))
 	assert.Nil(t, err)
 	assert.False(t, isYourTurn)
 	isYourTurn, err = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc71562b71999873DB5b286dF957af199Ec94617F7"))
@@ -127,14 +127,14 @@ func TestIsYourTurnConsensusV2(t *testing.T) {
 	blockchain.InsertBlock(currentBlock)
 	time.Sleep(time.Duration(minePeriod) * time.Second)
 
-	adaptor.EngineV2.SetNewRoundFaker(1, false)
+	adaptor.EngineV2.SetNewRoundFaker(2, false)
-	isYourTurn, _ = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc703c4b2bD70c169f5717101CaeE543299Fc946C7"))
+	isYourTurn, _ = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc0D3ab14BBaD3D99F4203bd7a11aCB94882050E7e"))
 	assert.False(t, isYourTurn)
 
-	isYourTurn, _ = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc0D3ab14BBaD3D99F4203bd7a11aCB94882050E7e"))
+	isYourTurn, _ = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc71562b71999873DB5b286dF957af199Ec94617F7"))
 	assert.True(t, isYourTurn)
 
-	isYourTurn, _ = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc71562b71999873DB5b286dF957af199Ec94617F7"))
+	isYourTurn, _ = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc5F74529C0338546f82389402a01c31fB52c6f434"))
 	assert.False(t, isYourTurn)
 
 }

consensus/tests/test_helper.go

@@ -419,7 +419,7 @@ func PrepareXDCTestBlockChainForV2Engine(t *testing.T, numOfBlocks int, chainCon
 			checkpointBlockNumber := lastv1BlockNumber - lastv1BlockNumber%chainConfig.XDPoS.Epoch
 			checkpointHeader := blockchain.GetHeaderByNumber(checkpointBlockNumber)
 			masternodes := engine.EngineV1.GetMasternodesFromCheckpointHeader(checkpointHeader)
-			err := engine.EngineV2.Initial(blockchain, block.Header(), masternodes)
+			err := engine.EngineV2.Initial(blockchain, masternodes)
 			if err != nil {
 				panic(err)
 			}
@@ -513,8 +513,12 @@ func CreateBlock(blockchain *BlockChain, chainConfig *params.ChainConfig, starti
 		if err != nil {
 			panic(fmt.Errorf("Error generate QC by creating signedHash: %v", err))
 		}
+		// Sign from acc 1, 2, 3
+		acc1SignedHash := SignHashByPK(acc1Key, utils.VoteSigHash(proposedBlockInfo).Bytes())
+		acc2SignedHash := SignHashByPK(acc2Key, utils.VoteSigHash(proposedBlockInfo).Bytes())
+		acc3SignedHash := SignHashByPK(acc3Key, utils.VoteSigHash(proposedBlockInfo).Bytes())
 		var signatures []utils.Signature
-		signatures = append(signatures, signedHash)
+		signatures = append(signatures, signedHash, acc1SignedHash, acc2SignedHash, acc3SignedHash)
 		quorumCert := &utils.QuorumCert{
 			ProposedBlockInfo: proposedBlockInfo,
 			Signatures:        signatures,

miner/worker.go

@@ -527,7 +527,15 @@ func (self *worker) commitNewWork() {
 	defer self.currentMu.Unlock()
 
 	tstart := time.Now()
-	parent := self.chain.CurrentBlock()
+
+	c := self.engine.(*XDPoS.XDPoS)
+	var parent *types.Block
+	if c != nil {
+		parent = c.FindParentBlockToAssign(self.chain, self.chain.CurrentBlock())
+	} else {
+		parent = self.chain.CurrentBlock()
+	}
+
 	var signers map[common.Address]struct{}
 	if parent.Hash().Hex() == self.lastParentBlockCommit {
 		return
@@ -540,7 +548,6 @@ func (self *worker) commitNewWork() {
 	if atomic.LoadInt32(&self.mining) == 1 {
 		// check if we are right after parent's coinbase in the list
 		if self.config.XDPoS != nil {
-			c := self.engine.(*XDPoS.XDPoS)
 			ok, err := c.YourTurn(self.chain, parent.Header(), self.coinbase)
 			if err != nil {
 				log.Warn("Failed when trying to commit new work", "err", err)