fix: Fix bug on gossip denial

planetarium · Nov 24, 2023 · f8a55be · f8a55be
1 parent 8c75a48
commit f8a55be
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 4 deletions.
diff --git 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
@@ -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)
         /// <param name="content"><see cref="MessageContent"/> to validate.</param>
         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);
+                }
+            }
+        }
+
+        /// <summary>
+        /// Filter logic for different height <see cref="ConsensusVoteMsg"/>s.
+        /// </summary>
+        /// <param name="voteMsg"><see cref="ConsensusVoteMsg"/> to filter.</param>
+        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)
         /// </param>
         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);
                 }
             }