check against master node list before sending out anything (ethereum#67)

* check against master node list before sending out anything

* remove duplicated signatures from QC

* add break when checking allowed to send

consensus/XDPoS/engines/engine_v2/engine.go

@@ -415,7 +415,7 @@ func (x *XDPoS_v2) YourTurn(chain consensus.ChainReader, parent *types.Header, s
 	if masterNodes[leaderIndex] == signer {
 		return true, nil
 	}
-	log.Warn("[YourTurn] Not authorised signer", "signer", signer, "MN", masterNodes, "Hash", parent.Hash(), "masterNodes[leaderIndex]", masterNodes[leaderIndex], "signer", signer)
+	log.Warn("[YourTurn] Not authorised signer", "signer", signer, "MN", masterNodes, "Hash", parent.Hash().Hex(), "masterNodes[leaderIndex]", masterNodes[leaderIndex], "signer", signer)
 	return false, nil
 }
 
@@ -426,26 +426,26 @@ func (x *XDPoS_v2) IsAuthorisedAddress(chain consensus.ChainReader, header *type
 	var extraField utils.ExtraFields_v2
 	err := utils.DecodeBytesExtraFields(header.Extra, &extraField)
 	if err != nil {
-		log.Error("[IsAuthorisedAddress] Fail to decode v2 extra data", "Hash", header.Hash(), "Extra", header.Extra, "Error", err)
+		log.Error("[IsAuthorisedAddress] Fail to decode v2 extra data", "Hash", header.Hash().Hex(), "Extra", header.Extra, "Error", err)
 		return false
 	}
 	blockRound := extraField.Round
 
 	masterNodes := x.GetMasternodes(chain, header)
 
 	if len(masterNodes) == 0 {
-		log.Error("[IsAuthorisedAddress] Fail to find any master nodes from current block round epoch", "Hash", header.Hash(), "Round", blockRound, "Number", header.Number)
+		log.Error("[IsAuthorisedAddress] Fail to find any master nodes from current block round epoch", "Hash", header.Hash().Hex(), "Round", blockRound, "Number", header.Number)
 		return false
 	}
-	// leaderIndex := uint64(blockRound) % x.config.Epoch % uint64(len(masterNodes))
+
 	for index, masterNodeAddress := range masterNodes {
 		if masterNodeAddress == address {
 			log.Debug("[IsAuthorisedAddress] Found matching master node address", "index", index, "Address", address, "MasterNodes", masterNodes)
 			return true
 		}
 	}
 
-	log.Warn("Not authorised address", "Address", address.Hex(), "Hash", header.Hash())
+	log.Warn("Not authorised address", "Address", address.Hex(), "Hash", header.Hash().Hex())
 	for index, mn := range masterNodes {
 		log.Warn("Master node list item", "mn", mn.Hex(), "index", index)
 	}
@@ -714,7 +714,10 @@ func (x *XDPoS_v2) VerifyVoteMessage(chain consensus.ChainReader, vote *utils.Vo
 	}
 	verified, err := x.verifyMsgSignature(utils.VoteSigHash(vote.ProposedBlockInfo), vote.Signature, snapshot.NextEpochMasterNodes)
 	if err != nil {
-		log.Error("[VerifyVoteMessage] Error while verifying vote message", "Error", err.Error())
+		for i, mn := range snapshot.NextEpochMasterNodes {
+			log.Warn("[VerifyVoteMessage] Master node list item", "index", i, "Master node", mn.Hex())
+		}
+		log.Warn("[VerifyVoteMessage] Error while verifying vote message", "votedBlockNum", vote.ProposedBlockInfo.Number.Uint64(), "votedBlockHash", vote.ProposedBlockInfo.Hash.Hex(), "Error", err.Error())
 	}
 	return verified, err
 }
@@ -913,7 +916,7 @@ func (x *XDPoS_v2) onTimeoutPoolThresholdReached(blockChainReader consensus.Chai
 /*
 	Proposed Block workflow
 */
-func (x *XDPoS_v2) ProposedBlockHandler(blockChainReader consensus.ChainReader, blockHeader *types.Header) error {
+func (x *XDPoS_v2) ProposedBlockHandler(chain consensus.ChainReader, blockHeader *types.Header) error {
 	x.lock.Lock()
 	defer x.lock.Unlock()
 
@@ -933,7 +936,7 @@ func (x *XDPoS_v2) ProposedBlockHandler(blockChainReader consensus.ChainReader,
 	quorumCert := decodedExtraField.QuorumCert
 	round := decodedExtraField.Round
 
-	err = x.verifyQC(blockChainReader, quorumCert)
+	err = x.verifyQC(chain, quorumCert)
 	if err != nil {
 		log.Error("[ProposedBlockHandler] Fail to verify QC", "Extra round", round, "QC proposed BlockInfo Hash", quorumCert.ProposedBlockInfo.Hash)
 		return err
@@ -945,17 +948,23 @@ func (x *XDPoS_v2) ProposedBlockHandler(blockChainReader consensus.ChainReader,
 		Round:  round,
 		Number: blockHeader.Number,
 	}
-	err = x.processQC(blockChainReader, quorumCert)
+	err = x.processQC(chain, quorumCert)
 	if err != nil {
 		log.Error("[ProposedBlockHandler] Fail to processQC", "QC proposed blockInfo round number", quorumCert.ProposedBlockInfo.Round, "QC proposed blockInfo hash", quorumCert.ProposedBlockInfo.Hash)
 		return err
 	}
-	verified, err := x.verifyVotingRule(blockChainReader, blockInfo, quorumCert)
+
+	err = x.allowedToSend(chain, blockHeader, "vote")
+	if err != nil {
+		return err
+	}
+
+	verified, err := x.verifyVotingRule(chain, blockInfo, quorumCert)
 	if err != nil {
 		return err
 	}
 	if verified {
-		return x.sendVote(blockChainReader, blockInfo)
+		return x.sendVote(chain, blockInfo)
 	} else {
 		log.Info("Failed to pass the voting rule verification", "ProposeBlockHash", blockInfo.Hash)
 	}
@@ -1022,20 +1031,26 @@ func (x *XDPoS_v2) verifyQC(blockChainReader consensus.ChainReader, quorumCert *
 		return fmt.Errorf("Fail to verify QC due to failure in getting epoch switch info")
 	}
 
+	signatures, duplicates := UniqueSignatures(quorumCert.Signatures)
+	if len(duplicates) != 0 {
+		for _, d := range duplicates {
+			log.Warn("[verifyQC] duplicated signature in QC", "duplicate", common.Bytes2Hex(d))
+		}
+	}
 	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)) {
+	} else if (quorumCert.ProposedBlockInfo.Number.Uint64() > x.config.V2.SwitchBlock.Uint64()) && (signatures == nil || (len(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))
+		log.Warn("[verifyHeader] Invalid QC Signature is nil or empty", "QC", quorumCert, "QCNumber", quorumCert.ProposedBlockInfo.Number, "Signatures len", len(signatures))
 		return utils.ErrInvalidQC
 	}
 
 	var wg sync.WaitGroup
-	wg.Add(len(quorumCert.Signatures))
+	wg.Add(len(signatures))
 	var haveError error
 
-	for _, signature := range quorumCert.Signatures {
+	for _, signature := range signatures {
 		go func(sig utils.Signature) {
 			defer wg.Done()
 			verified, err := x.verifyMsgSignature(utils.VoteSigHash(quorumCert.ProposedBlockInfo), sig, epochInfo.Masternodes)
@@ -1272,6 +1287,7 @@ func (x *XDPoS_v2) sendTimeout(chain consensus.ChainReader) error {
 		log.Error("[sendTimeout] Error while checking if the currentBlock is epoch switch", "currentRound", x.currentRound, "currentBlockNum", currentBlockHeader.Number, "currentBlockHash", currentBlockHeader.Hash(), "epochNum", epochNum)
 		return err
 	}
+
 	if isEpochSwitch {
 		// Notice this +1 is because we expect a block whos is the child of currentHeader
 		currentNumber := currentBlockHeader.Number.Uint64() + 1
@@ -1339,7 +1355,7 @@ func (x *XDPoS_v2) verifyMsgSignature(signedHashToBeVerified common.Hash, signat
 		}
 	}
 
-	return false, fmt.Errorf("Masternodes does not contain signer address. Master node list %v, Signer address: %v", masternodes, signerAddress)
+	return false, fmt.Errorf("Masternodes list does not contain signer address, Signer address: %v", signerAddress.Hex())
 }
 
 /*
@@ -1350,7 +1366,13 @@ func (x *XDPoS_v2) OnCountdownTimeout(time time.Time, chain interface{}) error {
 	x.lock.Lock()
 	defer x.lock.Unlock()
 
-	err := x.sendTimeout(chain.(consensus.ChainReader))
+	// Check if we are within the master node list
+	err := x.allowedToSend(chain.(consensus.ChainReader), chain.(consensus.ChainReader).CurrentHeader(), "timeout")
+	if err != nil {
+		return err
+	}
+
+	err = x.sendTimeout(chain.(consensus.ChainReader))
 	if err != nil {
 		log.Error("Error while sending out timeout message at time: ", time)
 		return err
@@ -1689,3 +1711,28 @@ func (x *XDPoS_v2) FindParentBlockToAssign(chain consensus.ChainReader) *types.B
 	}
 	return parent
 }
+
+func (x *XDPoS_v2) allowedToSend(chain consensus.ChainReader, blockHeader *types.Header, sendType string) error {
+	allowedToSend := false
+	// Don't hold the signFn for the whole signing operation
+	x.signLock.RLock()
+	signer := x.signer
+	x.signLock.RUnlock()
+	// Check if the node can send this sendType
+	masterNodes := x.GetMasternodes(chain, blockHeader)
+	for i, mn := range masterNodes {
+		if signer == mn {
+			log.Debug("[allowedToSend] Yes, I'm allowed to send", "sendType", sendType, "MyAddress", signer.Hex(), "Index in master node list", i)
+			allowedToSend = true
+			break
+		}
+	}
+	if !allowedToSend {
+		for _, mn := range masterNodes {
+			log.Debug("[allowedToSend] Master node list", "masterNodeAddress", mn.Hash())
+		}
+		log.Warn("[allowedToSend] Not in the Masternode list, not suppose to send", "sendType", sendType, "MyAddress", signer.Hex())
+		return fmt.Errorf("Not in the master node list, not suppose to %v", sendType)
+	}
+	return nil
+}

consensus/XDPoS/engines/engine_v2/utils.go

@@ -68,3 +68,19 @@ func decodeMasternodesFromHeaderExtra(checkpointHeader *types.Header) []common.A
 	}
 	return masternodes
 }
+
+func UniqueSignatures(signatureSlice []utils.Signature) ([]utils.Signature, []utils.Signature) {
+	keys := make(map[string]bool)
+	list := []utils.Signature{}
+	duplicates := []utils.Signature{}
+	for _, signature := range signatureSlice {
+		hexOfSig := common.Bytes2Hex(signature)
+		if _, value := keys[hexOfSig]; !value {
+			keys[hexOfSig] = true
+			list = append(list, signature)
+		} else {
+			duplicates = append(duplicates, signature)
+		}
+	}
+	return list, duplicates
+}

consensus/tests/proposed_block_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/XinFinOrg/XDPoSChain/accounts/abi/bind/backends"
 	"github.com/XinFinOrg/XDPoSChain/consensus/XDPoS"
 	"github.com/XinFinOrg/XDPoSChain/consensus/XDPoS/utils"
 	"github.com/XinFinOrg/XDPoSChain/params"
@@ -350,3 +351,24 @@ func TestShouldSendVoteMsg(t *testing.T) {
 		assert.Equal(t, round, vote.(*utils.Vote).ProposedBlockInfo.Round)
 	}
 }
+
+func TestProposedBlockMessageHandlerNotGenerateVoteIfSignerNotInMNlist(t *testing.T) {
+	blockchain, _, currentBlock, _, _, _ := PrepareXDCTestBlockChainForV2Engine(t, 906, params.TestXDPoSMockChainConfig, 0)
+	engineV2 := blockchain.Engine().(*XDPoS.XDPoS).EngineV2
+	differentSigner, differentSignFn, err := backends.SimulateWalletAddressAndSignFn()
+	assert.Nil(t, err)
+	// Let's change the address
+	engineV2.Authorize(differentSigner, differentSignFn)
+
+	// Set current round to 5
+	engineV2.SetNewRoundFaker(blockchain, utils.Round(5), false)
+
+	var extraField utils.ExtraFields_v2
+	err = utils.DecodeBytesExtraFields(currentBlock.Extra(), &extraField)
+	if err != nil {
+		t.Fatal("Fail to decode extra data", err)
+	}
+
+	err = engineV2.ProposedBlockHandler(blockchain, currentBlock.Header())
+	assert.Equal(t, "Not in the master node list, not suppose to vote", err.Error())
+}

consensus/tests/timeout_test.go

@@ -3,8 +3,10 @@ package tests
 import (
 	"fmt"
 	"testing"
+	"time"
 
 	"github.com/XinFinOrg/XDPoSChain/accounts"
+	"github.com/XinFinOrg/XDPoSChain/accounts/abi/bind/backends"
 	"github.com/XinFinOrg/XDPoSChain/consensus/XDPoS"
 	"github.com/XinFinOrg/XDPoSChain/consensus/XDPoS/utils"
 	"github.com/XinFinOrg/XDPoSChain/log"
@@ -25,9 +27,28 @@ func TestCountdownTimeoutToSendTimeoutMessage(t *testing.T) {
 	assert.Equal(t, utils.Round(1), timeoutMsg.(*utils.Timeout).Round)
 }
 
+func TestCountdownTimeoutNotToSendTimeoutMessageIfNotInMasternodeList(t *testing.T) {
+	blockchain, _, _, _, _, _ := PrepareXDCTestBlockChainForV2Engine(t, 901, params.TestXDPoSMockChainConfig, 0)
+
+	engineV2 := blockchain.Engine().(*XDPoS.XDPoS).EngineV2
+	differentSigner, differentSignFn, err := backends.SimulateWalletAddressAndSignFn()
+	assert.Nil(t, err)
+	// Let's change the address
+	engineV2.Authorize(differentSigner, differentSignFn)
+
+	engineV2.SetNewRoundFaker(blockchain, 1, true)
+
+	select {
+	case <-engineV2.BroadcastCh:
+		t.Fatalf("Not suppose to receive timeout msg")
+	case <-time.After(15 * time.Second): //Countdown is only 1s wait, let's wait for 3s here
+	}
+}
+
 func TestSyncInfoAfterReachTimeoutSnycThreadhold(t *testing.T) {
-	blockchain, _, _, _, _, _ := PrepareXDCTestBlockChainForV2Engine(t, 2251, params.TestXDPoSMockChainConfig, 0)
+	blockchain, _, _, _, _, _ := PrepareXDCTestBlockChainForV2Engine(t, 901, params.TestXDPoSMockChainConfig, 0)
 	engineV2 := blockchain.Engine().(*XDPoS.XDPoS).EngineV2
+	engineV2.SetNewRoundFaker(blockchain, 1, true)
 
 	// Because messages are sending async and on random order, so use this way to test
 	var timeoutCounter, syncInfoCounter int

consensus/tests/verify_block_test.go

@@ -2,8 +2,10 @@ package tests
 
 import (
 	"encoding/json"
+	"fmt"
 	"testing"
 
+	"github.com/XinFinOrg/XDPoSChain/accounts"
 	"github.com/XinFinOrg/XDPoSChain/consensus/XDPoS"
 	"github.com/XinFinOrg/XDPoSChain/consensus/XDPoS/utils"
 	"github.com/XinFinOrg/XDPoSChain/params"
@@ -39,3 +41,51 @@ func TestShouldVerifyBlock(t *testing.T) {
 	err = adaptor.VerifyHeader(blockchain, nonEpochSwitchWithValidators, true)
 	assert.Equal(t, utils.ErrInvalidFieldInNonEpochSwitch, err)
 }
+
+func TestShouldFailIfNotEnoughQCSignatures(t *testing.T) {
+	b, err := json.Marshal(params.TestXDPoSMockChainConfig)
+	assert.Nil(t, err)
+	configString := string(b)
+
+	var config params.ChainConfig
+	err = json.Unmarshal([]byte(configString), &config)
+	assert.Nil(t, err)
+	// Enable verify
+	config.XDPoS.V2.SkipV2Validation = false
+	// Skip the mining time validation by set mine time to 0
+	config.XDPoS.V2.MinePeriod = 0
+	// Block 901 is the first v2 block with round of 1
+	blockchain, _, currentBlock, signer, signFn, _ := PrepareXDCTestBlockChainForV2Engine(t, 902, &config, 0)
+	adaptor := blockchain.Engine().(*XDPoS.XDPoS)
+
+	parentBlock := blockchain.GetBlockByNumber(901)
+	proposedBlockInfo := &utils.BlockInfo{
+		Hash:   parentBlock.Hash(),
+		Round:  utils.Round(1),
+		Number: parentBlock.Number(),
+	}
+	signedHash, err := signFn(accounts.Account{Address: signer}, utils.VoteSigHash(proposedBlockInfo).Bytes())
+	assert.Nil(t, err)
+	var signatures []utils.Signature
+	// Duplicate the signatures
+	signatures = append(signatures, signedHash, signedHash, signedHash, signedHash, signedHash, signedHash)
+	quorumCert := &utils.QuorumCert{
+		ProposedBlockInfo: proposedBlockInfo,
+		Signatures:        signatures,
+	}
+
+	extra := utils.ExtraFields_v2{
+		Round:      utils.Round(2),
+		QuorumCert: quorumCert,
+	}
+	extraInBytes, err := extra.EncodeToBytes()
+	if err != nil {
+		panic(fmt.Errorf("Error encode extra into bytes: %v", err))
+	}
+	headerWithDuplicatedSignatures := currentBlock.Header()
+	headerWithDuplicatedSignatures.Extra = extraInBytes
+	// Happy path
+	err = adaptor.VerifyHeader(blockchain, headerWithDuplicatedSignatures, true)
+	assert.Equal(t, utils.ErrInvalidQC, err)
+
+}

eth/bft/bft_handler.go

@@ -79,11 +79,10 @@ func (b *Bfter) SetConsensusFuns(engine consensus.Engine) {
 	}
 }
 
-// TODO: rename
 func (b *Bfter) Vote(vote *utils.Vote) error {
-	log.Trace("Receive Vote", "hash", vote.Hash(), "voted block hash", vote.ProposedBlockInfo.Hash.Hex(), "number", vote.ProposedBlockInfo.Number, "round", vote.ProposedBlockInfo.Round, "signature", vote.Signature)
+	log.Trace("Receive Vote", "hash", vote.Hash().Hex(), "voted block hash", vote.ProposedBlockInfo.Hash.Hex(), "number", vote.ProposedBlockInfo.Number, "round", vote.ProposedBlockInfo.Round)
 	if exist, _ := b.knownVotes.ContainsOrAdd(vote.Hash(), true); exist {
-		log.Info("Discarded vote, known vote", "vote hash", vote.Hash(), "voted block hash", vote.ProposedBlockInfo.Hash.Hex(), "number", vote.ProposedBlockInfo.Number, "round", vote.ProposedBlockInfo.Round)
+		log.Debug("Discarded vote, known vote", "vote hash", vote.Hash(), "voted block hash", vote.ProposedBlockInfo.Hash.Hex(), "number", vote.ProposedBlockInfo.Number, "round", vote.ProposedBlockInfo.Round)
 		return nil
 	}