From f8a55beec6ab7d0e4ff88aa55b90332b61ab00df Mon Sep 17 00:00:00 2001 From: ilgyu Date: Fri, 24 Nov 2023 16:24:10 +0900 Subject: [PATCH] fix: Fix bug on gossip denial --- CHANGES.md | 8 +++++ .../GossipConsensusMessageCommunicator.cs | 35 ++++++++++++++++--- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 8ca1de2c626..6735aeb9c1c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,6 +6,14 @@ Version 3.8.1 To be released. + - (Libplanet.Net) Fixed a bug where `GossipConsensusMessageCommunicator` + does not clear `_peerCatchupRounds` on `OnStartHeight()`. [[#3519]] + - (Libplanet.Net) `GossipConsensusMessageCommunicator` now filters + `ConsensusVoteMsg` which height is different from latest `Context`. + [[#3519]] + +[#3519]: https://github.com/planetarium/libplanet/pull/3519 + Version 3.8.0 ------------- diff --git a/Libplanet.Net/Consensus/GossipConsensusMessageCommunicator.cs b/Libplanet.Net/Consensus/GossipConsensusMessageCommunicator.cs index 6acf5d0a5e9..07316ad88d1 100644 --- a/Libplanet.Net/Consensus/GossipConsensusMessageCommunicator.cs +++ b/Libplanet.Net/Consensus/GossipConsensusMessageCommunicator.cs @@ -66,6 +66,7 @@ public void PublishMessage(ConsensusMsg message) public void OnStartHeight(long height) { _height = height; + _peerCatchupRounds.Clear(); Gossip.ClearDenySet(); } @@ -86,6 +87,7 @@ private void ValidateMessageToReceive(Message message) { if (message.Content is ConsensusVoteMsg voteMsg) { + FilterDifferentHeightVote(voteMsg); FilterHigherRoundVoteSpam(voteMsg, message.Remote); } } @@ -98,10 +100,33 @@ private void ValidateMessageToReceive(Message message) /// to validate. private void ValidateMessageToSend(MessageContent content) { - if (content is ConsensusVoteMsg voteMsg && voteMsg.Round > _round) + if (content is ConsensusVoteMsg voteMsg) + { + if (voteMsg.Height != _height) + { + throw new InvalidConsensusMessageException( + $"Cannot send vote of height different from context's", voteMsg); + } + + if (voteMsg.Round > _round) + { + throw new InvalidConsensusMessageException( + $"Cannot send vote of round higher than context's", voteMsg); + } + } + } + + /// + /// Filter logic for different height s. + /// + /// to filter. + private void FilterDifferentHeightVote(ConsensusVoteMsg voteMsg) + { + if (voteMsg.Height != _height) { throw new InvalidConsensusMessageException( - $"Cannot send vote of round higher than context", voteMsg); + $"Filtered vote from different height: {voteMsg.Height}", + voteMsg); } } @@ -113,7 +138,8 @@ private void ValidateMessageToSend(MessageContent content) /// private void FilterHigherRoundVoteSpam(ConsensusVoteMsg voteMsg, BoundPeer peer) { - if (voteMsg.Round > _round) + if (voteMsg.Height == _height && + voteMsg.Round > _round) { if (!_peerCatchupRounds.ContainsKey(peer)) { @@ -130,7 +156,8 @@ private void FilterHigherRoundVoteSpam(ConsensusVoteMsg voteMsg, BoundPeer peer) { Gossip.DenyPeer(peer); throw new InvalidConsensusMessageException( - $"Repetitively found higher rounds, add {peer} to deny set", + $"Add {peer} to deny set, since repetitively found higher rounds: " + + $"{string.Join(", ", _peerCatchupRounds[peer])}", voteMsg); } }