diff --git a/consensus/XDPoS/engines/engine_v2/engine.go b/consensus/XDPoS/engines/engine_v2/engine.go index a1c30fe6db..cfd12eed45 100644 --- a/consensus/XDPoS/engines/engine_v2/engine.go +++ b/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,7 +426,7 @@ 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 @@ -434,10 +434,10 @@ func (x *XDPoS_v2) IsAuthorisedAddress(chain consensus.ChainReader, header *type 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) @@ -445,7 +445,7 @@ func (x *XDPoS_v2) IsAuthorisedAddress(chain consensus.ChainReader, header *type } } - 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 +} diff --git a/consensus/XDPoS/engines/engine_v2/utils.go b/consensus/XDPoS/engines/engine_v2/utils.go index be99cad26a..a6105b72e5 100644 --- a/consensus/XDPoS/engines/engine_v2/utils.go +++ b/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 +} diff --git a/consensus/tests/proposed_block_test.go b/consensus/tests/proposed_block_test.go index d03bc28a43..67d49f01ee 100644 --- a/consensus/tests/proposed_block_test.go +++ b/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()) +} diff --git a/consensus/tests/timeout_test.go b/consensus/tests/timeout_test.go index aa1b2920f7..baba42f861 100644 --- a/consensus/tests/timeout_test.go +++ b/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 TestSyncInfoAfterReachTimeoutSnycThreadhold(t *testing.T) { - blockchain, _, _, _, _, _ := PrepareXDCTestBlockChainForV2Engine(t, 2251, params.TestXDPoSMockChainConfig, 0) +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, 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 diff --git a/consensus/tests/verify_block_test.go b/consensus/tests/verify_block_test.go index 1f5f4f7915..d916aa518f 100644 --- a/consensus/tests/verify_block_test.go +++ b/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) + +} diff --git a/eth/bft/bft_handler.go b/eth/bft/bft_handler.go index 14e01157f2..ce1ce899df 100644 --- a/eth/bft/bft_handler.go +++ b/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 }