Fix #1834 (#1838)

* Fix 1834 * Fix UT * Remove useless condition Co-authored-by: Erik Zhang <erik@neo.org>
neo-project · Aug 13, 2020 · e6d68dc · e6d68dc
1 parent fcb7ad1
commit e6d68dc
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 19 deletions.
diff --git a/src/neo/Consensus/ConsensusContext.cs b/src/neo/Consensus/ConsensusContext.cs
@@ -35,7 +35,7 @@ internal class ConsensusContext : IDisposable, ISerializable
         public ConsensusPayload[] LastChangeViewPayloads;
         // LastSeenMessage array stores the height of the last seen message, for each validator.
         // if this node never heard from validator i, LastSeenMessage[i] will be -1.
-        public Dictionary<ECPoint, int> LastSeenMessage { get; private set; }
+        public Dictionary<ECPoint, uint> LastSeenMessage { get; private set; }
 
         /// <summary>
         /// Store all verified unsorted transactions' senders' fee currently in the consensus context.
@@ -55,7 +55,14 @@ internal class ConsensusContext : IDisposable, ISerializable
         public bool WatchOnly => MyIndex < 0;
         public Header PrevHeader => Snapshot.GetHeader(Block.PrevHash);
         public int CountCommitted => CommitPayloads.Count(p => p != null);
-        public int CountFailed => LastSeenMessage?.Count(p => p.Value < (((int)Block.Index) - 1)) ?? 0;
+        public int CountFailed
+        {
+            get
+            {
+                if (LastSeenMessage == null) return 0;
+                return Validators.Count(p => !LastSeenMessage.TryGetValue(p, out var value) || value < (Block.Index - 1));
+            }
+        }
         public bool ValidatorsChanged
         {
             get
@@ -397,13 +404,13 @@ public void Reset(byte viewNumber)
                 if (ValidatorsChanged || LastSeenMessage is null)
                 {
                     var previous_last_seen_message = LastSeenMessage;
-                    LastSeenMessage = new Dictionary<ECPoint, int>();
+                    LastSeenMessage = new Dictionary<ECPoint, uint>();
                     foreach (var validator in Validators)
                     {
-                        if (previous_last_seen_message != null && previous_last_seen_message.TryGetValue(validator, out int value))
+                        if (previous_last_seen_message != null && previous_last_seen_message.TryGetValue(validator, out var value))
                             LastSeenMessage[validator] = value;
                         else
-                            LastSeenMessage[validator] = (int)Snapshot.Height;
+                            LastSeenMessage[validator] = Snapshot.Height;
                     }
                 }
                 keyPair = null;
@@ -434,7 +441,7 @@ public void Reset(byte viewNumber)
             Block.Transactions = null;
             TransactionHashes = null;
             PreparationPayloads = new ConsensusPayload[Validators.Length];
-            if (MyIndex >= 0) LastSeenMessage[Validators[MyIndex]] = (int)Block.Index;
+            if (MyIndex >= 0) LastSeenMessage[Validators[MyIndex]] = Block.Index;
         }
 
         public void Save()

diff --git a/src/neo/Consensus/ConsensusService.cs b/src/neo/Consensus/ConsensusService.cs
@@ -291,7 +291,7 @@ private void OnConsensusPayload(ConsensusPayload payload)
             {
                 return;
             }
-            context.LastSeenMessage[context.Validators[payload.ValidatorIndex]] = (int)payload.BlockIndex;
+            context.LastSeenMessage[context.Validators[payload.ValidatorIndex]] = payload.BlockIndex;
             foreach (IP2PPlugin plugin in Plugin.P2PPlugins)
                 if (!plugin.OnConsensusMessage(payload))
                     return;

diff --git a/tests/neo.UnitTests/Consensus/UT_Consensus.cs b/tests/neo.UnitTests/Consensus/UT_Consensus.cs
@@ -170,10 +170,6 @@ public void ConsensusService_SingleNodeActors_OnStart_PrepReq_PrepResponses_Comm
             Console.WriteLine("Forcing Failed nodes for recovery request... ");
             mockContext.Object.CountFailed.Should().Be(0);
             mockContext.Object.LastSeenMessage.Clear();
-            foreach (var validator in mockContext.Object.Validators)
-            {
-                mockContext.Object.LastSeenMessage[validator] = -1;
-            }
             mockContext.Object.CountFailed.Should().Be(7);
             Console.WriteLine("\nWaiting for recovery due to failed nodes... ");
             var backupOnRecoveryDueToFailedNodes = subscriber.ExpectMsg<LocalNode.SendDirectly>();
@@ -275,10 +271,6 @@ public void ConsensusService_SingleNodeActors_OnStart_PrepReq_PrepResponses_Comm
             Console.WriteLine($"Generated keypairs PKey:");
             //refresh LastSeenMessage
             mockContext.Object.LastSeenMessage.Clear();
-            foreach (var item in mockContext.Object.Validators)
-            {
-                mockContext.Object.LastSeenMessage[item] = -1;
-            }
             for (int i = 0; i < mockContext.Object.Validators.Length; i++)
                 Console.WriteLine($"{mockContext.Object.Validators[i]}/{Contract.CreateSignatureContract(mockContext.Object.Validators[i]).ScriptHash}");
             var updatedContract = Contract.CreateMultiSigContract(mockContext.Object.M, mockContext.Object.Validators);
@@ -398,10 +390,6 @@ public void ConsensusService_SingleNodeActors_OnStart_PrepReq_PrepResponses_Comm
             Console.WriteLine($"\nModifying CountFailed and asserting 7...");
             // This will ensure a non-deterministic behavior after last recovery
             mockContext.Object.LastSeenMessage.Clear();
-            foreach (var validator in mockContext.Object.Validators)
-            {
-                mockContext.Object.LastSeenMessage[validator] = -1;
-            }
             mockContext.Object.CountFailed.Should().Be(7);
 
             TellConsensusPayload(actorConsensus, rmPayload);