From aa16b7f7359896d149ef340324b4c06c508105d4 Mon Sep 17 00:00:00 2001
From: jsolman <jsolinsky@gmail.com>
Date: Mon, 18 Mar 2019 14:47:35 -0700
Subject: [PATCH 01/26] Prevent commit after ChangeView.

---
 neo/Consensus/ConsensusService.cs | 35 ++++++++++++++++---------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/neo/Consensus/ConsensusService.cs b/neo/Consensus/ConsensusService.cs
index f89f537229..4ce9bfb768 100644
--- a/neo/Consensus/ConsensusService.cs
+++ b/neo/Consensus/ConsensusService.cs
@@ -293,19 +293,24 @@ private void OnRecoveryMessageReceived(ConsensusPayload payload, RecoveryMessage
                         ReverifyAndProcessPayload(changeViewPayload);
                 }
                 if (message.ViewNumber != context.ViewNumber) return;
-                if (!context.CommitSent())
+                if (!context.ViewChanging())
                 {
-                    if (!context.RequestSentOrReceived())
+                    if (!context.CommitSent())
                     {
-                        ConsensusPayload prepareRequestPayload = message.GetPrepareRequestPayload(context, payload);
-                        if (prepareRequestPayload != null)
-                            ReverifyAndProcessPayload(prepareRequestPayload);
-                        else if (context.IsPrimary())
-                            SendPrepareRequest();
+                        if (!context.RequestSentOrReceived())
+                        {
+                            ConsensusPayload prepareRequestPayload = message.GetPrepareRequestPayload(context, payload);
+                            if (prepareRequestPayload != null)
+                                ReverifyAndProcessPayload(prepareRequestPayload);
+                            else if (context.IsPrimary())
+                                SendPrepareRequest();
+                        }
+
+                        ConsensusPayload[] prepareResponsePayloads =
+                            message.GetPrepareResponsePayloads(context, payload);
+                        foreach (ConsensusPayload prepareResponsePayload in prepareResponsePayloads)
+                            ReverifyAndProcessPayload(prepareResponsePayload);
                     }
-                    ConsensusPayload[] prepareResponsePayloads = message.GetPrepareResponsePayloads(context, payload);
-                    foreach (ConsensusPayload prepareResponsePayload in prepareResponsePayloads)
-                        ReverifyAndProcessPayload(prepareResponsePayload);
                 }
                 ConsensusPayload[] commitPayloads = message.GetCommitPayloadsFromRecoveryMessage(context, payload);
                 foreach (ConsensusPayload commitPayload in commitPayloads)
@@ -319,7 +324,7 @@ private void OnRecoveryMessageReceived(ConsensusPayload payload, RecoveryMessage
 
         private void OnPrepareRequestReceived(ConsensusPayload payload, PrepareRequest message)
         {
-            if (context.RequestSentOrReceived()) return;
+            if (context.RequestSentOrReceived() || context.ViewChanging()) return;
             if (payload.ValidatorIndex != context.PrimaryIndex) return;
             Log($"{nameof(OnPrepareRequestReceived)}: height={payload.BlockIndex} view={message.ViewNumber} index={payload.ValidatorIndex} tx={message.TransactionHashes.Length}");
             if (message.Timestamp <= context.PrevHeader().Timestamp || message.Timestamp > TimeProvider.Current.UtcNow.AddMinutes(10).ToTimestamp())
@@ -379,7 +384,7 @@ private void OnPrepareRequestReceived(ConsensusPayload payload, PrepareRequest m
 
         private void OnPrepareResponseReceived(ConsensusPayload payload, PrepareResponse message)
         {
-            if (context.PreparationPayloads[payload.ValidatorIndex] != null) return;
+            if (context.PreparationPayloads[payload.ValidatorIndex] != null || context.ViewChanging()) return;
             if (context.PreparationPayloads[context.PrimaryIndex] != null && !message.PreparationHash.Equals(context.PreparationPayloads[context.PrimaryIndex].Hash))
                 return;
             Log($"{nameof(OnPrepareResponseReceived)}: height={payload.BlockIndex} view={message.ViewNumber} index={payload.ValidatorIndex}");
@@ -473,12 +478,8 @@ private void OnTimer(Timer timer)
         private void OnTransaction(Transaction transaction)
         {
             if (transaction.Type == TransactionType.MinerTransaction) return;
-            if (!context.IsBackup() || !context.RequestSentOrReceived() || context.ResponseSent() || context.BlockSent())
+            if (!context.IsBackup() || context.ViewChanging() || !context.RequestSentOrReceived() || context.ResponseSent() || context.BlockSent())
                 return;
-            // If we are changing view but we already have enough preparation payloads to commit in the current view,
-            // we must keep on accepting transactions in the current view to be able to create the block.
-            if (context.ViewChanging() &&
-                context.PreparationPayloads.Count(p => p != null) < context.M()) return;
             if (context.Transactions.ContainsKey(transaction.Hash)) return;
             if (!context.TransactionHashes.Contains(transaction.Hash)) return;
             AddTransaction(transaction, true);

From d686236ed946d5538515c2c66b64c2cbc18072a1 Mon Sep 17 00:00:00 2001
From: jsolman <jsolinsky@gmail.com>
Date: Mon, 18 Mar 2019 15:29:01 -0700
Subject: [PATCH 02/26] Minor simplification.

---
 neo/Consensus/ConsensusService.cs | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/neo/Consensus/ConsensusService.cs b/neo/Consensus/ConsensusService.cs
index 4ce9bfb768..7a372cc42e 100644
--- a/neo/Consensus/ConsensusService.cs
+++ b/neo/Consensus/ConsensusService.cs
@@ -293,24 +293,21 @@ private void OnRecoveryMessageReceived(ConsensusPayload payload, RecoveryMessage
                         ReverifyAndProcessPayload(changeViewPayload);
                 }
                 if (message.ViewNumber != context.ViewNumber) return;
-                if (!context.ViewChanging())
+                if (!context.ViewChanging() && !context.CommitSent())
                 {
-                    if (!context.CommitSent())
+                    if (!context.RequestSentOrReceived())
                     {
-                        if (!context.RequestSentOrReceived())
-                        {
-                            ConsensusPayload prepareRequestPayload = message.GetPrepareRequestPayload(context, payload);
-                            if (prepareRequestPayload != null)
-                                ReverifyAndProcessPayload(prepareRequestPayload);
-                            else if (context.IsPrimary())
-                                SendPrepareRequest();
-                        }
-
-                        ConsensusPayload[] prepareResponsePayloads =
-                            message.GetPrepareResponsePayloads(context, payload);
-                        foreach (ConsensusPayload prepareResponsePayload in prepareResponsePayloads)
-                            ReverifyAndProcessPayload(prepareResponsePayload);
+                        ConsensusPayload prepareRequestPayload = message.GetPrepareRequestPayload(context, payload);
+                        if (prepareRequestPayload != null)
+                            ReverifyAndProcessPayload(prepareRequestPayload);
+                        else if (context.IsPrimary())
+                            SendPrepareRequest();
                     }
+
+                    ConsensusPayload[] prepareResponsePayloads =
+                        message.GetPrepareResponsePayloads(context, payload);
+                    foreach (ConsensusPayload prepareResponsePayload in prepareResponsePayloads)
+                        ReverifyAndProcessPayload(prepareResponsePayload);
                 }
                 ConsensusPayload[] commitPayloads = message.GetCommitPayloadsFromRecoveryMessage(context, payload);
                 foreach (ConsensusPayload commitPayload in commitPayloads)

From 0d98768ed996385e9876b1c6f4e8f418870add17 Mon Sep 17 00:00:00 2001
From: jsolman <jsolinsky@gmail.com>
Date: Mon, 18 Mar 2019 15:30:31 -0700
Subject: [PATCH 03/26] Minimize differences.

---
 neo/Consensus/ConsensusService.cs | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/neo/Consensus/ConsensusService.cs b/neo/Consensus/ConsensusService.cs
index 7a372cc42e..f11f3d9fb3 100644
--- a/neo/Consensus/ConsensusService.cs
+++ b/neo/Consensus/ConsensusService.cs
@@ -303,9 +303,7 @@ private void OnRecoveryMessageReceived(ConsensusPayload payload, RecoveryMessage
                         else if (context.IsPrimary())
                             SendPrepareRequest();
                     }
-
-                    ConsensusPayload[] prepareResponsePayloads =
-                        message.GetPrepareResponsePayloads(context, payload);
+                    ConsensusPayload[] prepareResponsePayloads = message.GetPrepareResponsePayloads(context, payload);
                     foreach (ConsensusPayload prepareResponsePayload in prepareResponsePayloads)
                         ReverifyAndProcessPayload(prepareResponsePayload);
                 }

From d9ea44e1986f17635eb49329c880d865e0c226b2 Mon Sep 17 00:00:00 2001
From: jsolman <jsolinsky@gmail.com>
Date: Mon, 18 Mar 2019 17:31:44 -0700
Subject: [PATCH 04/26] Don't consider view changing once F nodes committed.

---
 neo/Consensus/Helper.cs | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/neo/Consensus/Helper.cs b/neo/Consensus/Helper.cs
index cb74c2850b..14e1dbea02 100644
--- a/neo/Consensus/Helper.cs
+++ b/neo/Consensus/Helper.cs
@@ -1,4 +1,5 @@
-using Neo.Network.P2P.Payloads;
+using System.Linq;
+using Neo.Network.P2P.Payloads;
 using Neo.Persistence;
 using System.Runtime.CompilerServices;
 
@@ -26,8 +27,11 @@ internal static class Helper
         public static bool CommitSent(this IConsensusContext context) => context.CommitPayloads[context.MyIndex] != null;
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static bool BlockSent(this IConsensusContext context) => context.Block != null;
+
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        public static bool ViewChanging(this IConsensusContext context) => context.ChangeViewPayloads[context.MyIndex]?.GetDeserializedMessage<ChangeView>().NewViewNumber > context.ViewNumber;
+        public static bool ViewChanging(this IConsensusContext context) => context.ChangeViewPayloads[context.MyIndex]?.GetDeserializedMessage<ChangeView>().NewViewNumber > context.ViewNumber && !context.FNodesValidCommitted();
+
+        public static bool FNodesValidCommitted(this IConsensusContext context) => context.CommitPayloads.Count(p => p != null) >= context.F();
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static uint GetPrimaryIndex(this IConsensusContext context, byte viewNumber)

From 57065f876954b30bb6a23638f9c13f63f9333e11 Mon Sep 17 00:00:00 2001
From: jsolman <jsolinsky@gmail.com>
Date: Mon, 18 Mar 2019 17:56:11 -0700
Subject: [PATCH 05/26] Add a comment describing current potential issues.

---
 neo/Consensus/Helper.cs | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/neo/Consensus/Helper.cs b/neo/Consensus/Helper.cs
index 14e1dbea02..2589539972 100644
--- a/neo/Consensus/Helper.cs
+++ b/neo/Consensus/Helper.cs
@@ -31,6 +31,11 @@ internal static class Helper
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static bool ViewChanging(this IConsensusContext context) => context.ChangeViewPayloads[context.MyIndex]?.GetDeserializedMessage<ChangeView>().NewViewNumber > context.ViewNumber && !context.FNodesValidCommitted();
 
+        // A possible attack can happen if the last node to commit is malicious and either sends change view after his
+        // commit to stall nodes in a higher view, or if he refuses to send recovery messages. In addition, if a node
+        // asking change views crashes and comes back when nodes are committed in more than one view, it is possible for
+        // the restart node to accept recovery from any committed node, thus potentially splitting nodes among views
+        // and stalling the network.
         public static bool FNodesValidCommitted(this IConsensusContext context) => context.CommitPayloads.Count(p => p != null) >= context.F();
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]

From 442bfe81f3f91e8dd5f4cf87163e61af8cb27071 Mon Sep 17 00:00:00 2001
From: jsolman <jsolinsky@gmail.com>
Date: Mon, 18 Mar 2019 18:43:17 -0700
Subject: [PATCH 06/26] Save consensus context upon changing view to prevent
 split commit.

---
 neo/Consensus/ConsensusService.cs | 38 ++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/neo/Consensus/ConsensusService.cs b/neo/Consensus/ConsensusService.cs
index f11f3d9fb3..3cd6af2f8d 100644
--- a/neo/Consensus/ConsensusService.cs
+++ b/neo/Consensus/ConsensusService.cs
@@ -116,6 +116,7 @@ private void CheckExpectedView(byte viewNumber)
                 if (message is null || message.NewViewNumber < viewNumber)
                     localNode.Tell(new LocalNode.SendDirectly { Inventory = context.MakeChangeView(viewNumber) });
                 InitializeConsensus(viewNumber);
+                context.Save();
             }
         }
 
@@ -182,25 +183,27 @@ private void OnChangeViewReceived(ConsensusPayload payload, ChangeView message)
             // ChangeView messages include a Timestamp when the change view is sent, thus if a node restarts
             // and issues a change view for the same view, it will have a different hash and will correctly respond
             // again; however replay attacks of the ChangeView message from arbitrary nodes will not trigger an
-            // additonal recovery message response.
+            // additional recovery message response.
             if (!knownHashes.Add(payload.Hash)) return;
             if (message.NewViewNumber <= context.ViewNumber)
             {
-                bool shouldSendRecovery = false;
-                // Limit recovery to sending from `f` nodes when the request is from a lower view number.
-                int allowedRecoveryNodeCount = context.F();
-                for (int i = 0; i < allowedRecoveryNodeCount; i++)
+                if (!context.CommitSent())
                 {
-                    var eligibleResponders = context.Validators.Length - 1;
-                    var chosenIndex = (payload.ValidatorIndex + i + message.NewViewNumber) % eligibleResponders;
-                    if (chosenIndex >= payload.ValidatorIndex) chosenIndex++;
-                    if (chosenIndex != context.MyIndex) continue;
-                    shouldSendRecovery = true;
-                    break;
-                }
-
-                if (!shouldSendRecovery) return;
+                    bool shouldSendRecovery = false;
+                    // Limit recovery to sending from `f` nodes when the request is from a lower view number.
+                    int allowedRecoveryNodeCount = context.F();
+                    for (int i = 0; i < allowedRecoveryNodeCount; i++)
+                    {
+                        var eligibleResponders = context.Validators.Length - 1;
+                        var chosenIndex = (payload.ValidatorIndex + i + message.NewViewNumber) % eligibleResponders;
+                        if (chosenIndex >= payload.ValidatorIndex) chosenIndex++;
+                        if (chosenIndex != context.MyIndex) continue;
+                        shouldSendRecovery = true;
+                        break;
+                    }
 
+                    if (!shouldSendRecovery) return;
+                }
                 Log($"send recovery from view: {message.ViewNumber} to view: {context.ViewNumber}");
                 localNode.Tell(new LocalNode.SendDirectly { Inventory = context.MakeRecoveryMessage() });
             }
@@ -438,11 +441,14 @@ private void OnStart(Start options)
                     CheckPreparations();
                     return;
                 }
+                InitializeConsensus(context.ViewNumber);
             }
-            InitializeConsensus(0);
+            else
+                InitializeConsensus(0);
+
             // Issue a ChangeView with NewViewNumber of 0 to request recovery messages on start-up.
             if (context.BlockIndex == Blockchain.Singleton.HeaderHeight + 1)
-                localNode.Tell(new LocalNode.SendDirectly { Inventory = context.MakeChangeView(0) });
+                localNode.Tell(new LocalNode.SendDirectly {Inventory = context.MakeChangeView(context.ViewNumber)});
         }
 
         private void OnTimer(Timer timer)

From f471b32b5799fd86768ff412a4692c42231af1ed Mon Sep 17 00:00:00 2001
From: jsolman <jsolinsky@gmail.com>
Date: Mon, 18 Mar 2019 18:54:20 -0700
Subject: [PATCH 07/26] Adjust comment concerning potential issues.

---
 neo/Consensus/Helper.cs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/neo/Consensus/Helper.cs b/neo/Consensus/Helper.cs
index 2589539972..3ea1bab7b4 100644
--- a/neo/Consensus/Helper.cs
+++ b/neo/Consensus/Helper.cs
@@ -33,9 +33,9 @@ internal static class Helper
 
         // A possible attack can happen if the last node to commit is malicious and either sends change view after his
         // commit to stall nodes in a higher view, or if he refuses to send recovery messages. In addition, if a node
-        // asking change views crashes and comes back when nodes are committed in more than one view, it is possible for
-        // the restart node to accept recovery from any committed node, thus potentially splitting nodes among views
-        // and stalling the network.
+        // asking change views loses network or crashes and comes back when nodes are committed in more than one view,
+        // it is possible for that node to accept recovery from any committed node, thus potentially splitting nodes
+        // among views and stalling the network.
         public static bool FNodesValidCommitted(this IConsensusContext context) => context.CommitPayloads.Count(p => p != null) >= context.F();
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]

From 7d5b2013fa4d17943eb7f42facb443a39fce5d28 Mon Sep 17 00:00:00 2001
From: jsolman <jsolinsky@gmail.com>
Date: Mon, 18 Mar 2019 19:42:01 -0700
Subject: [PATCH 08/26] Further improve comment.

---
 neo/Consensus/Helper.cs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/neo/Consensus/Helper.cs b/neo/Consensus/Helper.cs
index 3ea1bab7b4..f20bc5f665 100644
--- a/neo/Consensus/Helper.cs
+++ b/neo/Consensus/Helper.cs
@@ -33,9 +33,9 @@ internal static class Helper
 
         // A possible attack can happen if the last node to commit is malicious and either sends change view after his
         // commit to stall nodes in a higher view, or if he refuses to send recovery messages. In addition, if a node
-        // asking change views loses network or crashes and comes back when nodes are committed in more than one view,
-        // it is possible for that node to accept recovery from any committed node, thus potentially splitting nodes
-        // among views and stalling the network.
+        // asking change views loses network or crashes and comes back when nodes are committed in more than one higher
+        // numbered view, it is possible for the node accepting recovery and commit in any of the higher views, thus
+        // potentially splitting nodes among views and stalling the network.
         public static bool FNodesValidCommitted(this IConsensusContext context) => context.CommitPayloads.Count(p => p != null) >= context.F();
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]

From 470b35c63705c2770fe40346c90b6e2f1d2d148d Mon Sep 17 00:00:00 2001
From: jsolman <jsolinsky@gmail.com>
Date: Mon, 18 Mar 2019 20:20:50 -0700
Subject: [PATCH 09/26] Reduce to minimal changes to fix the main issue that
 can cause the network to stall creating blocks.

---
 neo/Consensus/ConsensusService.cs | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/neo/Consensus/ConsensusService.cs b/neo/Consensus/ConsensusService.cs
index 3cd6af2f8d..97c570ca88 100644
--- a/neo/Consensus/ConsensusService.cs
+++ b/neo/Consensus/ConsensusService.cs
@@ -116,7 +116,6 @@ private void CheckExpectedView(byte viewNumber)
                 if (message is null || message.NewViewNumber < viewNumber)
                     localNode.Tell(new LocalNode.SendDirectly { Inventory = context.MakeChangeView(viewNumber) });
                 InitializeConsensus(viewNumber);
-                context.Save();
             }
         }
 
@@ -441,14 +440,11 @@ private void OnStart(Start options)
                     CheckPreparations();
                     return;
                 }
-                InitializeConsensus(context.ViewNumber);
             }
-            else
-                InitializeConsensus(0);
-
+            InitializeConsensus(0);
             // Issue a ChangeView with NewViewNumber of 0 to request recovery messages on start-up.
             if (context.BlockIndex == Blockchain.Singleton.HeaderHeight + 1)
-                localNode.Tell(new LocalNode.SendDirectly {Inventory = context.MakeChangeView(context.ViewNumber)});
+                localNode.Tell(new LocalNode.SendDirectly { Inventory = context.MakeChangeView(0) });
         }
 
         private void OnTimer(Timer timer)

From bfb729e569ea69ff4c79238206155137726ef2d5 Mon Sep 17 00:00:00 2001
From: jsolman <jsolinsky@gmail.com>
Date: Mon, 18 Mar 2019 21:51:47 -0700
Subject: [PATCH 10/26] Rename helper method that checks if F nodes are already
 committed.

---
 neo/Consensus/Helper.cs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/neo/Consensus/Helper.cs b/neo/Consensus/Helper.cs
index f20bc5f665..e49418fd41 100644
--- a/neo/Consensus/Helper.cs
+++ b/neo/Consensus/Helper.cs
@@ -29,14 +29,14 @@ internal static class Helper
         public static bool BlockSent(this IConsensusContext context) => context.Block != null;
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        public static bool ViewChanging(this IConsensusContext context) => context.ChangeViewPayloads[context.MyIndex]?.GetDeserializedMessage<ChangeView>().NewViewNumber > context.ViewNumber && !context.FNodesValidCommitted();
+        public static bool ViewChanging(this IConsensusContext context) => context.ChangeViewPayloads[context.MyIndex]?.GetDeserializedMessage<ChangeView>().NewViewNumber > context.ViewNumber && !context.FNodesCommitted();
 
         // A possible attack can happen if the last node to commit is malicious and either sends change view after his
         // commit to stall nodes in a higher view, or if he refuses to send recovery messages. In addition, if a node
         // asking change views loses network or crashes and comes back when nodes are committed in more than one higher
         // numbered view, it is possible for the node accepting recovery and commit in any of the higher views, thus
         // potentially splitting nodes among views and stalling the network.
-        public static bool FNodesValidCommitted(this IConsensusContext context) => context.CommitPayloads.Count(p => p != null) >= context.F();
+        public static bool FNodesCommitted(this IConsensusContext context) => context.CommitPayloads.Count(p => p != null) >= context.F();
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static uint GetPrimaryIndex(this IConsensusContext context, byte viewNumber)

From 76bd5763f215748a3a8e6eabde7a07f849cd71f9 Mon Sep 17 00:00:00 2001
From: jsolman <jsolinsky@gmail.com>
Date: Mon, 18 Mar 2019 22:02:45 -0700
Subject: [PATCH 11/26] Greater than F nodes must have committed for it to not
 be possible for changing view and coming to consensus.

---
 neo/Consensus/Helper.cs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/neo/Consensus/Helper.cs b/neo/Consensus/Helper.cs
index e49418fd41..1a841534f9 100644
--- a/neo/Consensus/Helper.cs
+++ b/neo/Consensus/Helper.cs
@@ -36,7 +36,7 @@ internal static class Helper
         // asking change views loses network or crashes and comes back when nodes are committed in more than one higher
         // numbered view, it is possible for the node accepting recovery and commit in any of the higher views, thus
         // potentially splitting nodes among views and stalling the network.
-        public static bool FNodesCommitted(this IConsensusContext context) => context.CommitPayloads.Count(p => p != null) >= context.F();
+        public static bool FNodesCommitted(this IConsensusContext context) => context.CommitPayloads.Count(p => p != null) > context.F();
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static uint GetPrimaryIndex(this IConsensusContext context, byte viewNumber)

From ee72d589ea9fad2de3355fd4f96e5df796989dbd Mon Sep 17 00:00:00 2001
From: jsolman <jsolinsky@gmail.com>
Date: Mon, 18 Mar 2019 22:19:52 -0700
Subject: [PATCH 12/26] Rename flag to MoreThanFNodesCommitted.

---
 neo/Consensus/Helper.cs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/neo/Consensus/Helper.cs b/neo/Consensus/Helper.cs
index 1a841534f9..bc2478ac48 100644
--- a/neo/Consensus/Helper.cs
+++ b/neo/Consensus/Helper.cs
@@ -29,14 +29,14 @@ internal static class Helper
         public static bool BlockSent(this IConsensusContext context) => context.Block != null;
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        public static bool ViewChanging(this IConsensusContext context) => context.ChangeViewPayloads[context.MyIndex]?.GetDeserializedMessage<ChangeView>().NewViewNumber > context.ViewNumber && !context.FNodesCommitted();
+        public static bool ViewChanging(this IConsensusContext context) => context.ChangeViewPayloads[context.MyIndex]?.GetDeserializedMessage<ChangeView>().NewViewNumber > context.ViewNumber && !context.MoreThanFNodesCommitted();
 
         // A possible attack can happen if the last node to commit is malicious and either sends change view after his
         // commit to stall nodes in a higher view, or if he refuses to send recovery messages. In addition, if a node
         // asking change views loses network or crashes and comes back when nodes are committed in more than one higher
         // numbered view, it is possible for the node accepting recovery and commit in any of the higher views, thus
         // potentially splitting nodes among views and stalling the network.
-        public static bool FNodesCommitted(this IConsensusContext context) => context.CommitPayloads.Count(p => p != null) > context.F();
+        public static bool MoreThanFNodesCommitted(this IConsensusContext context) => context.CommitPayloads.Count(p => p != null) > context.F();
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static uint GetPrimaryIndex(this IConsensusContext context, byte viewNumber)

From aa365fd7a7228162dbb8e61bd5786c0455ebeb38 Mon Sep 17 00:00:00 2001
From: jsolman <jsolinsky@gmail.com>
Date: Tue, 19 Mar 2019 20:25:29 -0700
Subject: [PATCH 13/26] Make a new flag for clarity. Add TODO for fixing
 counting committed payloads.

---
 neo/Consensus/ConsensusService.cs | 8 ++++----
 neo/Consensus/Helper.cs           | 5 ++++-
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/neo/Consensus/ConsensusService.cs b/neo/Consensus/ConsensusService.cs
index bb445e9710..ea6630d1e9 100644
--- a/neo/Consensus/ConsensusService.cs
+++ b/neo/Consensus/ConsensusService.cs
@@ -299,7 +299,7 @@ private void OnRecoveryMessageReceived(ConsensusPayload payload, RecoveryMessage
                         if (ReverifyAndProcessPayload(changeViewPayload)) validChangeViews++;
                 }
                 if (message.ViewNumber != context.ViewNumber) return;
-                if (!context.ViewChanging() && !context.CommitSent())
+                if (!context.ViewChangingIfAllowed() && !context.CommitSent())
                 {
                     if (!context.RequestSentOrReceived())
                     {
@@ -335,7 +335,7 @@ private void OnRecoveryMessageReceived(ConsensusPayload payload, RecoveryMessage
 
         private void OnPrepareRequestReceived(ConsensusPayload payload, PrepareRequest message)
         {
-            if (context.RequestSentOrReceived() || context.ViewChanging()) return;
+            if (context.RequestSentOrReceived() || context.ViewChangingIfAllowed()) return;
             if (payload.ValidatorIndex != context.PrimaryIndex) return;
             Log($"{nameof(OnPrepareRequestReceived)}: height={payload.BlockIndex} view={message.ViewNumber} index={payload.ValidatorIndex} tx={message.TransactionHashes.Length}");
             if (message.Timestamp <= context.PrevHeader().Timestamp || message.Timestamp > TimeProvider.Current.UtcNow.AddMinutes(10).ToTimestamp())
@@ -395,7 +395,7 @@ private void OnPrepareRequestReceived(ConsensusPayload payload, PrepareRequest m
 
         private void OnPrepareResponseReceived(ConsensusPayload payload, PrepareResponse message)
         {
-            if (context.PreparationPayloads[payload.ValidatorIndex] != null || context.ViewChanging()) return;
+            if (context.PreparationPayloads[payload.ValidatorIndex] != null || context.ViewChangingIfAllowed()) return;
             if (context.PreparationPayloads[context.PrimaryIndex] != null && !message.PreparationHash.Equals(context.PreparationPayloads[context.PrimaryIndex].Hash))
                 return;
             Log($"{nameof(OnPrepareResponseReceived)}: height={payload.BlockIndex} view={message.ViewNumber} index={payload.ValidatorIndex}");
@@ -489,7 +489,7 @@ private void OnTimer(Timer timer)
         private void OnTransaction(Transaction transaction)
         {
             if (transaction.Type == TransactionType.MinerTransaction) return;
-            if (!context.IsBackup() || context.ViewChanging() || !context.RequestSentOrReceived() || context.ResponseSent() || context.BlockSent())
+            if (!context.IsBackup() || context.ViewChangingIfAllowed() || !context.RequestSentOrReceived() || context.ResponseSent() || context.BlockSent())
                 return;
             if (context.Transactions.ContainsKey(transaction.Hash)) return;
             if (!context.TransactionHashes.Contains(transaction.Hash)) return;
diff --git a/neo/Consensus/Helper.cs b/neo/Consensus/Helper.cs
index bc2478ac48..d633a1c3b2 100644
--- a/neo/Consensus/Helper.cs
+++ b/neo/Consensus/Helper.cs
@@ -29,13 +29,16 @@ internal static class Helper
         public static bool BlockSent(this IConsensusContext context) => context.Block != null;
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        public static bool ViewChanging(this IConsensusContext context) => context.ChangeViewPayloads[context.MyIndex]?.GetDeserializedMessage<ChangeView>().NewViewNumber > context.ViewNumber && !context.MoreThanFNodesCommitted();
+        public static bool ViewChanging(this IConsensusContext context) => context.ChangeViewPayloads[context.MyIndex]?.GetDeserializedMessage<ChangeView>().NewViewNumber > context.ViewNumber;
+
+        public static bool ViewChangingIfAllowed(this IConsensusContext context) => context.ViewChanging() && !context.MoreThanFNodesCommitted();
 
         // A possible attack can happen if the last node to commit is malicious and either sends change view after his
         // commit to stall nodes in a higher view, or if he refuses to send recovery messages. In addition, if a node
         // asking change views loses network or crashes and comes back when nodes are committed in more than one higher
         // numbered view, it is possible for the node accepting recovery and commit in any of the higher views, thus
         // potentially splitting nodes among views and stalling the network.
+        // TODO: Fix this to properly count all committed payloads including at lower view numbers.
         public static bool MoreThanFNodesCommitted(this IConsensusContext context) => context.CommitPayloads.Count(p => p != null) > context.F();
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]

From 495917682b29a583a0f12b5700b635f9e18ca405 Mon Sep 17 00:00:00 2001
From: jsolman <jsolinsky@gmail.com>
Date: Wed, 20 Mar 2019 12:41:32 -0700
Subject: [PATCH 14/26] Keep track of commit payloads in all views to support
 counting commits.

---
 neo/Consensus/ConsensusContext.cs             | 14 ++---
 neo/Consensus/ConsensusService.cs             | 56 ++++++++++++++-----
 neo/Consensus/Helper.cs                       |  1 -
 .../RecoveryMessage.CommitPayloadCompact.cs   |  5 ++
 neo/Consensus/RecoveryMessage.cs              |  2 +-
 5 files changed, 53 insertions(+), 25 deletions(-)

diff --git a/neo/Consensus/ConsensusContext.cs b/neo/Consensus/ConsensusContext.cs
index b7562898dc..8de1b68cda 100644
--- a/neo/Consensus/ConsensusContext.cs
+++ b/neo/Consensus/ConsensusContext.cs
@@ -61,7 +61,7 @@ public Block CreateBlock()
                 ContractParametersContext sc = new ContractParametersContext(Block);
                 for (int i = 0, j = 0; i < Validators.Length && j < this.M(); i++)
                 {
-                    if (CommitPayloads[i] == null) continue;
+                    if (CommitPayloads[i] == null || CommitPayloads[i].ConsensusMessage.ViewNumber != ViewNumber) continue;
                     sc.AddSignature(contract, Validators[i], CommitPayloads[i].GetDeserializedMessage<Commit>().Signature);
                     j++;
                 }
@@ -146,12 +146,10 @@ public ConsensusPayload MakeChangeView(byte newViewNumber)
 
         public ConsensusPayload MakeCommit()
         {
-            if (CommitPayloads[MyIndex] == null)
-                CommitPayloads[MyIndex] = MakeSignedPayload(new Commit
-                {
-                    Signature = MakeHeader()?.Sign(keyPair)
-                });
-            return CommitPayloads[MyIndex];
+            return CommitPayloads[MyIndex] ?? (CommitPayloads[MyIndex] = MakeSignedPayload(new Commit
+            {
+                Signature = MakeHeader()?.Sign(keyPair)
+            }));
         }
 
         private Block _header = null;
@@ -267,6 +265,7 @@ public void Reset(byte viewNumber)
                 MyIndex = -1;
                 ChangeViewPayloads = new ConsensusPayload[Validators.Length];
                 LastChangeViewPayloads = new ConsensusPayload[Validators.Length];
+                CommitPayloads = new ConsensusPayload[Validators.Length];
                 keyPair = null;
                 for (int i = 0; i < Validators.Length; i++)
                 {
@@ -290,7 +289,6 @@ public void Reset(byte viewNumber)
             Timestamp = 0;
             TransactionHashes = null;
             PreparationPayloads = new ConsensusPayload[Validators.Length];
-            CommitPayloads = new ConsensusPayload[Validators.Length];
             _header = null;
         }
 
diff --git a/neo/Consensus/ConsensusService.cs b/neo/Consensus/ConsensusService.cs
index ea6630d1e9..2c1e50bd36 100644
--- a/neo/Consensus/ConsensusService.cs
+++ b/neo/Consensus/ConsensusService.cs
@@ -99,7 +99,7 @@ private void ChangeTimer(TimeSpan delay)
 
         private void CheckCommits()
         {
-            if (context.CommitPayloads.Count(p => p != null) >= context.M() && context.TransactionHashes.All(p => context.Transactions.ContainsKey(p)))
+            if (context.CommitPayloads.Count(p => p != null && p.ConsensusMessage.ViewNumber == context.ViewNumber) >= context.M() && context.TransactionHashes.All(p => context.Transactions.ContainsKey(p)))
             {
                 Block block = context.CreateBlock();
                 Log($"relay block: {block.Hash}");
@@ -220,17 +220,33 @@ private void OnChangeViewReceived(ConsensusPayload payload, ChangeView message)
 
         private void OnCommitReceived(ConsensusPayload payload, Commit commit)
         {
-            if (context.CommitPayloads[payload.ValidatorIndex] != null) return;
-            Log($"{nameof(OnCommitReceived)}: height={payload.BlockIndex} view={commit.ViewNumber} index={payload.ValidatorIndex}");
-            byte[] hashData = context.MakeHeader()?.GetHashData();
-            if (hashData == null)
+            ref ConsensusPayload existingCommitPayload = ref context.CommitPayloads[payload.ValidatorIndex];
+            if (commit.ViewNumber == context.ViewNumber)
             {
-                context.CommitPayloads[payload.ValidatorIndex] = payload;
+                Log($"{nameof(OnCommitReceived)}: height={payload.BlockIndex} view={commit.ViewNumber} index={payload.ValidatorIndex}");
+
+                byte[] hashData = context.MakeHeader()?.GetHashData();
+                if (hashData == null)
+                {
+                    existingCommitPayload = payload;
+                }
+                else if (Crypto.Default.VerifySignature(hashData, commit.Signature,
+                    context.Validators[payload.ValidatorIndex].EncodePoint(false)))
+                {
+                    existingCommitPayload = payload;
+                    CheckCommits();
+                }
+                return;
             }
-            else if (Crypto.Default.VerifySignature(hashData, commit.Signature, context.Validators[payload.ValidatorIndex].EncodePoint(false)))
+            // Receiving commit from another view
+            Log($"{nameof(OnCommitReceived)}: record commit for different view={commit.ViewNumber} index={payload.ValidatorIndex} height={payload.BlockIndex}");
+            if (existingCommitPayload == null)
+                existingCommitPayload = payload;
+            else
             {
-                context.CommitPayloads[payload.ValidatorIndex] = payload;
-                CheckCommits();
+                var existingViewNumber = existingCommitPayload.ConsensusMessage.ViewNumber;
+                if (existingViewNumber == context.ViewNumber || commit.ViewNumber <= existingViewNumber) return;
+                existingCommitPayload = payload;
             }
         }
 
@@ -248,8 +264,8 @@ private void OnConsensusPayload(ConsensusPayload payload)
             }
             if (payload.ValidatorIndex >= context.Validators.Length) return;
             ConsensusMessage message = payload.ConsensusMessage;
-            if (message.ViewNumber != context.ViewNumber && message.Type != ConsensusMessageType.ChangeView &&
-                                                            message.Type != ConsensusMessageType.RecoveryMessage)
+            if (message.ViewNumber != context.ViewNumber && (message.Type == ConsensusMessageType.PrepareRequest ||
+                                                             message.Type == ConsensusMessageType.PrepareResponse))
                 return;
             switch (message)
             {
@@ -281,15 +297,24 @@ private void OnPersistCompleted(Block block)
 
         private void OnRecoveryMessageReceived(ConsensusPayload payload, RecoveryMessage message)
         {
-            if (message.ViewNumber < context.ViewNumber) return;
-            Log($"{nameof(OnRecoveryMessageReceived)}: height={payload.BlockIndex} view={message.ViewNumber} index={payload.ValidatorIndex}");
             // isRecovering is always set to false again after OnRecoveryMessageReceived
             isRecovering = true;
             int validChangeViews = 0, totalChangeViews = 0, validPrepReq=0, totalPrepReq = 0;
             int validPrepResponses = 0, totalPrepResponses = 0, validCommits = 0, totalCommits = 0;
 
+            Log($"{nameof(OnRecoveryMessageReceived)}: height={payload.BlockIndex} view={message.ViewNumber} index={payload.ValidatorIndex}");
             try
             {
+                ConsensusPayload[] commitPayloads;
+                if (message.ViewNumber < context.ViewNumber)
+                {
+                    // Ensure we know about all commits form lower view numbers.
+                    commitPayloads = message.GetCommitPayloadsFromRecoveryMessage(context, payload);
+                    totalCommits = commitPayloads.Length;
+                    foreach (ConsensusPayload commitPayload in commitPayloads)
+                        if (ReverifyAndProcessPayload(commitPayload)) validCommits++;
+                    return;
+                }
                 if (message.ViewNumber > context.ViewNumber)
                 {
                     if (context.CommitSent()) return;
@@ -317,7 +342,7 @@ private void OnRecoveryMessageReceived(ConsensusPayload payload, RecoveryMessage
                     foreach (ConsensusPayload prepareResponsePayload in prepareResponsePayloads)
                         if (ReverifyAndProcessPayload(prepareResponsePayload)) validPrepResponses++;
                 }
-                ConsensusPayload[] commitPayloads = message.GetCommitPayloadsFromRecoveryMessage(context, payload);
+                commitPayloads = message.GetCommitPayloadsFromRecoveryMessage(context, payload);
                 totalCommits = commitPayloads.Length;
                 foreach (ConsensusPayload commitPayload in commitPayloads)
                     if (ReverifyAndProcessPayload(commitPayload)) validCommits++;
@@ -361,7 +386,8 @@ private void OnPrepareRequestReceived(ConsensusPayload payload, PrepareRequest m
             byte[] hashData = context.MakeHeader().GetHashData();
             for (int i = 0; i < context.CommitPayloads.Length; i++)
                 if (context.CommitPayloads[i] != null)
-                    if (!Crypto.Default.VerifySignature(hashData, context.CommitPayloads[i].GetDeserializedMessage<Commit>().Signature, context.Validators[i].EncodePoint(false)))
+                    if (context.CommitPayloads[i].ConsensusMessage.ViewNumber == context.ViewNumber &&
+                        !Crypto.Default.VerifySignature(hashData, context.CommitPayloads[i].GetDeserializedMessage<Commit>().Signature, context.Validators[i].EncodePoint(false)))
                         context.CommitPayloads[i] = null;
             Dictionary<UInt256, Transaction> mempoolVerified = Blockchain.Singleton.MemPool.GetVerifiedTransactions().ToDictionary(p => p.Hash);
 
diff --git a/neo/Consensus/Helper.cs b/neo/Consensus/Helper.cs
index d633a1c3b2..665d1433b9 100644
--- a/neo/Consensus/Helper.cs
+++ b/neo/Consensus/Helper.cs
@@ -38,7 +38,6 @@ internal static class Helper
         // asking change views loses network or crashes and comes back when nodes are committed in more than one higher
         // numbered view, it is possible for the node accepting recovery and commit in any of the higher views, thus
         // potentially splitting nodes among views and stalling the network.
-        // TODO: Fix this to properly count all committed payloads including at lower view numbers.
         public static bool MoreThanFNodesCommitted(this IConsensusContext context) => context.CommitPayloads.Count(p => p != null) > context.F();
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
diff --git a/neo/Consensus/RecoveryMessage.CommitPayloadCompact.cs b/neo/Consensus/RecoveryMessage.CommitPayloadCompact.cs
index 58dd0e34e5..d9347b71bc 100644
--- a/neo/Consensus/RecoveryMessage.CommitPayloadCompact.cs
+++ b/neo/Consensus/RecoveryMessage.CommitPayloadCompact.cs
@@ -8,17 +8,20 @@ partial class RecoveryMessage
     {
         public class CommitPayloadCompact : ISerializable
         {
+            public byte ViewNumber;
             public ushort ValidatorIndex;
             public byte[] Signature;
             public byte[] InvocationScript;
 
             int ISerializable.Size =>
+                sizeof(byte) +                  //ViewNumber
                 sizeof(ushort) +                //ValidatorIndex
                 Signature.Length +              //Signature
                 InvocationScript.GetVarSize();  //InvocationScript
 
             void ISerializable.Deserialize(BinaryReader reader)
             {
+                ViewNumber = reader.ReadByte();
                 ValidatorIndex = reader.ReadUInt16();
                 Signature = reader.ReadBytes(64);
                 InvocationScript = reader.ReadVarBytes(1024);
@@ -29,6 +32,7 @@ public static CommitPayloadCompact FromPayload(ConsensusPayload payload)
                 Commit message = payload.GetDeserializedMessage<Commit>();
                 return new CommitPayloadCompact
                 {
+                    ViewNumber = message.ViewNumber,
                     ValidatorIndex = payload.ValidatorIndex,
                     Signature = message.Signature,
                     InvocationScript = payload.Witness.InvocationScript
@@ -37,6 +41,7 @@ public static CommitPayloadCompact FromPayload(ConsensusPayload payload)
 
             void ISerializable.Serialize(BinaryWriter writer)
             {
+                writer.Write(ViewNumber);
                 writer.Write(ValidatorIndex);
                 writer.Write(Signature);
                 writer.WriteVarBytes(InvocationScript);
diff --git a/neo/Consensus/RecoveryMessage.cs b/neo/Consensus/RecoveryMessage.cs
index 731d8258e5..817ecb5cd5 100644
--- a/neo/Consensus/RecoveryMessage.cs
+++ b/neo/Consensus/RecoveryMessage.cs
@@ -71,7 +71,7 @@ internal ConsensusPayload[] GetCommitPayloadsFromRecoveryMessage(IConsensusConte
                 ValidatorIndex = p.ValidatorIndex,
                 ConsensusMessage = new Commit
                 {
-                    ViewNumber = ViewNumber,
+                    ViewNumber = p.ViewNumber,
                     Signature = p.Signature
                 },
                 Witness = new Witness

From 68f523579cfc2556b00a2b9be6afe8ffba87ba3c Mon Sep 17 00:00:00 2001
From: jsolman <jsolinsky@gmail.com>
Date: Wed, 20 Mar 2019 12:47:50 -0700
Subject: [PATCH 15/26] Rename flag for clarity.

---
 neo/Consensus/ConsensusService.cs | 8 ++++----
 neo/Consensus/Helper.cs           | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/neo/Consensus/ConsensusService.cs b/neo/Consensus/ConsensusService.cs
index 2c1e50bd36..6bd6bbf1a6 100644
--- a/neo/Consensus/ConsensusService.cs
+++ b/neo/Consensus/ConsensusService.cs
@@ -324,7 +324,7 @@ private void OnRecoveryMessageReceived(ConsensusPayload payload, RecoveryMessage
                         if (ReverifyAndProcessPayload(changeViewPayload)) validChangeViews++;
                 }
                 if (message.ViewNumber != context.ViewNumber) return;
-                if (!context.ViewChangingIfAllowed() && !context.CommitSent())
+                if (!context.NotAcceptingPayloadsDueToViewChanging() && !context.CommitSent())
                 {
                     if (!context.RequestSentOrReceived())
                     {
@@ -360,7 +360,7 @@ private void OnRecoveryMessageReceived(ConsensusPayload payload, RecoveryMessage
 
         private void OnPrepareRequestReceived(ConsensusPayload payload, PrepareRequest message)
         {
-            if (context.RequestSentOrReceived() || context.ViewChangingIfAllowed()) return;
+            if (context.RequestSentOrReceived() || context.NotAcceptingPayloadsDueToViewChanging()) return;
             if (payload.ValidatorIndex != context.PrimaryIndex) return;
             Log($"{nameof(OnPrepareRequestReceived)}: height={payload.BlockIndex} view={message.ViewNumber} index={payload.ValidatorIndex} tx={message.TransactionHashes.Length}");
             if (message.Timestamp <= context.PrevHeader().Timestamp || message.Timestamp > TimeProvider.Current.UtcNow.AddMinutes(10).ToTimestamp())
@@ -421,7 +421,7 @@ private void OnPrepareRequestReceived(ConsensusPayload payload, PrepareRequest m
 
         private void OnPrepareResponseReceived(ConsensusPayload payload, PrepareResponse message)
         {
-            if (context.PreparationPayloads[payload.ValidatorIndex] != null || context.ViewChangingIfAllowed()) return;
+            if (context.PreparationPayloads[payload.ValidatorIndex] != null || context.NotAcceptingPayloadsDueToViewChanging()) return;
             if (context.PreparationPayloads[context.PrimaryIndex] != null && !message.PreparationHash.Equals(context.PreparationPayloads[context.PrimaryIndex].Hash))
                 return;
             Log($"{nameof(OnPrepareResponseReceived)}: height={payload.BlockIndex} view={message.ViewNumber} index={payload.ValidatorIndex}");
@@ -515,7 +515,7 @@ private void OnTimer(Timer timer)
         private void OnTransaction(Transaction transaction)
         {
             if (transaction.Type == TransactionType.MinerTransaction) return;
-            if (!context.IsBackup() || context.ViewChangingIfAllowed() || !context.RequestSentOrReceived() || context.ResponseSent() || context.BlockSent())
+            if (!context.IsBackup() || context.NotAcceptingPayloadsDueToViewChanging() || !context.RequestSentOrReceived() || context.ResponseSent() || context.BlockSent())
                 return;
             if (context.Transactions.ContainsKey(transaction.Hash)) return;
             if (!context.TransactionHashes.Contains(transaction.Hash)) return;
diff --git a/neo/Consensus/Helper.cs b/neo/Consensus/Helper.cs
index 665d1433b9..74897cda3c 100644
--- a/neo/Consensus/Helper.cs
+++ b/neo/Consensus/Helper.cs
@@ -31,7 +31,7 @@ internal static class Helper
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static bool ViewChanging(this IConsensusContext context) => context.ChangeViewPayloads[context.MyIndex]?.GetDeserializedMessage<ChangeView>().NewViewNumber > context.ViewNumber;
 
-        public static bool ViewChangingIfAllowed(this IConsensusContext context) => context.ViewChanging() && !context.MoreThanFNodesCommitted();
+        public static bool NotAcceptingPayloadsDueToViewChanging(this IConsensusContext context) => context.ViewChanging() && !context.MoreThanFNodesCommitted();
 
         // A possible attack can happen if the last node to commit is malicious and either sends change view after his
         // commit to stall nodes in a higher view, or if he refuses to send recovery messages. In addition, if a node

From eee35ccf4ae99d39e3f5ece68a9594681e4b361b Mon Sep 17 00:00:00 2001
From: jsolman <jsolinsky@gmail.com>
Date: Wed, 20 Mar 2019 13:12:24 -0700
Subject: [PATCH 16/26] Remove unnecessary change of recording commit from
 another view.

---
 neo/Consensus/ConsensusService.cs | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/neo/Consensus/ConsensusService.cs b/neo/Consensus/ConsensusService.cs
index 6bd6bbf1a6..0a18a84ff3 100644
--- a/neo/Consensus/ConsensusService.cs
+++ b/neo/Consensus/ConsensusService.cs
@@ -239,15 +239,9 @@ private void OnCommitReceived(ConsensusPayload payload, Commit commit)
                 return;
             }
             // Receiving commit from another view
+            if (existingCommitPayload != null) return;
             Log($"{nameof(OnCommitReceived)}: record commit for different view={commit.ViewNumber} index={payload.ValidatorIndex} height={payload.BlockIndex}");
-            if (existingCommitPayload == null)
-                existingCommitPayload = payload;
-            else
-            {
-                var existingViewNumber = existingCommitPayload.ConsensusMessage.ViewNumber;
-                if (existingViewNumber == context.ViewNumber || commit.ViewNumber <= existingViewNumber) return;
-                existingCommitPayload = payload;
-            }
+            existingCommitPayload = payload;
         }
 
         private void OnConsensusPayload(ConsensusPayload payload)

From 2e7d8adbcfbeaf406c50635d5d1f9a7bb1b29076 Mon Sep 17 00:00:00 2001
From: jsolman <jsolinsky@gmail.com>
Date: Wed, 20 Mar 2019 14:55:50 -0700
Subject: [PATCH 17/26] Fix typo in comment.

---
 neo/Consensus/ConsensusService.cs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/neo/Consensus/ConsensusService.cs b/neo/Consensus/ConsensusService.cs
index 0a18a84ff3..7600eb5cb4 100644
--- a/neo/Consensus/ConsensusService.cs
+++ b/neo/Consensus/ConsensusService.cs
@@ -302,7 +302,7 @@ private void OnRecoveryMessageReceived(ConsensusPayload payload, RecoveryMessage
                 ConsensusPayload[] commitPayloads;
                 if (message.ViewNumber < context.ViewNumber)
                 {
-                    // Ensure we know about all commits form lower view numbers.
+                    // Ensure we know about all commits from lower view numbers.
                     commitPayloads = message.GetCommitPayloadsFromRecoveryMessage(context, payload);
                     totalCommits = commitPayloads.Length;
                     foreach (ConsensusPayload commitPayload in commitPayloads)

From 28b45ccc3e0a082b9b6b1d6da6ef72db2daa3288 Mon Sep 17 00:00:00 2001
From: jsolman <jsolinsky@gmail.com>
Date: Wed, 20 Mar 2019 15:05:11 -0700
Subject: [PATCH 18/26] If we already have a commit for the validator at the
 current view, we can return early.

---
 neo/Consensus/ConsensusService.cs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/neo/Consensus/ConsensusService.cs b/neo/Consensus/ConsensusService.cs
index 7600eb5cb4..a009d690a2 100644
--- a/neo/Consensus/ConsensusService.cs
+++ b/neo/Consensus/ConsensusService.cs
@@ -223,6 +223,7 @@ private void OnCommitReceived(ConsensusPayload payload, Commit commit)
             ref ConsensusPayload existingCommitPayload = ref context.CommitPayloads[payload.ValidatorIndex];
             if (commit.ViewNumber == context.ViewNumber)
             {
+                if (existingCommitPayload != null && context.ViewNumber == existingCommitPayload.ConsensusMessage.ViewNumber) return;
                 Log($"{nameof(OnCommitReceived)}: height={payload.BlockIndex} view={commit.ViewNumber} index={payload.ValidatorIndex}");
 
                 byte[] hashData = context.MakeHeader()?.GetHashData();

From b6bf4da1fa119496abc00c049be1de91535ed2d2 Mon Sep 17 00:00:00 2001
From: jsolman <jsolinsky@gmail.com>
Date: Wed, 20 Mar 2019 15:50:35 -0700
Subject: [PATCH 19/26] Add warning log for receiving a different commit from a
 validator. A validator should never change their commit at a given height.

---
 neo/Consensus/ConsensusService.cs | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/neo/Consensus/ConsensusService.cs b/neo/Consensus/ConsensusService.cs
index a009d690a2..79896fdb8b 100644
--- a/neo/Consensus/ConsensusService.cs
+++ b/neo/Consensus/ConsensusService.cs
@@ -221,9 +221,15 @@ private void OnChangeViewReceived(ConsensusPayload payload, ChangeView message)
         private void OnCommitReceived(ConsensusPayload payload, Commit commit)
         {
             ref ConsensusPayload existingCommitPayload = ref context.CommitPayloads[payload.ValidatorIndex];
+            if (existingCommitPayload != null)
+            {
+                if (existingCommitPayload.Hash != payload.Hash)
+                    Log($"{nameof(OnCommitReceived)}: Warning, different commit from validator! height={payload.BlockIndex} index={payload.ValidatorIndex} view={commit.ViewNumber} existingView={existingCommitPayload.ConsensusMessage.ViewNumber}");
+                return;
+            }
+
             if (commit.ViewNumber == context.ViewNumber)
             {
-                if (existingCommitPayload != null && context.ViewNumber == existingCommitPayload.ConsensusMessage.ViewNumber) return;
                 Log($"{nameof(OnCommitReceived)}: height={payload.BlockIndex} view={commit.ViewNumber} index={payload.ValidatorIndex}");
 
                 byte[] hashData = context.MakeHeader()?.GetHashData();
@@ -240,7 +246,6 @@ private void OnCommitReceived(ConsensusPayload payload, Commit commit)
                 return;
             }
             // Receiving commit from another view
-            if (existingCommitPayload != null) return;
             Log($"{nameof(OnCommitReceived)}: record commit for different view={commit.ViewNumber} index={payload.ValidatorIndex} height={payload.BlockIndex}");
             existingCommitPayload = payload;
         }

From ce7685b08d76a7f971b71dcad59b9daf083b5d03 Mon Sep 17 00:00:00 2001
From: erikzhang <erik@neo.org>
Date: Thu, 21 Mar 2019 11:50:17 +0800
Subject: [PATCH 20/26] minor changes

---
 neo/Consensus/ConsensusContext.cs | 2 +-
 neo/Consensus/ConsensusService.cs | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/neo/Consensus/ConsensusContext.cs b/neo/Consensus/ConsensusContext.cs
index 8de1b68cda..622b71b621 100644
--- a/neo/Consensus/ConsensusContext.cs
+++ b/neo/Consensus/ConsensusContext.cs
@@ -61,7 +61,7 @@ public Block CreateBlock()
                 ContractParametersContext sc = new ContractParametersContext(Block);
                 for (int i = 0, j = 0; i < Validators.Length && j < this.M(); i++)
                 {
-                    if (CommitPayloads[i] == null || CommitPayloads[i].ConsensusMessage.ViewNumber != ViewNumber) continue;
+                    if (CommitPayloads[i]?.ConsensusMessage.ViewNumber != ViewNumber) continue;
                     sc.AddSignature(contract, Validators[i], CommitPayloads[i].GetDeserializedMessage<Commit>().Signature);
                     j++;
                 }
diff --git a/neo/Consensus/ConsensusService.cs b/neo/Consensus/ConsensusService.cs
index 79896fdb8b..fee22be3ff 100644
--- a/neo/Consensus/ConsensusService.cs
+++ b/neo/Consensus/ConsensusService.cs
@@ -99,7 +99,7 @@ private void ChangeTimer(TimeSpan delay)
 
         private void CheckCommits()
         {
-            if (context.CommitPayloads.Count(p => p != null && p.ConsensusMessage.ViewNumber == context.ViewNumber) >= context.M() && context.TransactionHashes.All(p => context.Transactions.ContainsKey(p)))
+            if (context.CommitPayloads.Count(p => p?.ConsensusMessage.ViewNumber == context.ViewNumber) >= context.M() && context.TransactionHashes.All(p => context.Transactions.ContainsKey(p)))
             {
                 Block block = context.CreateBlock();
                 Log($"relay block: {block.Hash}");
@@ -299,7 +299,7 @@ private void OnRecoveryMessageReceived(ConsensusPayload payload, RecoveryMessage
         {
             // isRecovering is always set to false again after OnRecoveryMessageReceived
             isRecovering = true;
-            int validChangeViews = 0, totalChangeViews = 0, validPrepReq=0, totalPrepReq = 0;
+            int validChangeViews = 0, totalChangeViews = 0, validPrepReq = 0, totalPrepReq = 0;
             int validPrepResponses = 0, totalPrepResponses = 0, validCommits = 0, totalCommits = 0;
 
             Log($"{nameof(OnRecoveryMessageReceived)}: height={payload.BlockIndex} view={message.ViewNumber} index={payload.ValidatorIndex}");

From b8beafc31ef826a7fa1fff368d39a9855bedeb46 Mon Sep 17 00:00:00 2001
From: erikzhang <erik@neo.org>
Date: Thu, 21 Mar 2019 11:59:41 +0800
Subject: [PATCH 21/26] use `LogLevel.Warning`

---
 neo/Consensus/ConsensusService.cs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/neo/Consensus/ConsensusService.cs b/neo/Consensus/ConsensusService.cs
index fee22be3ff..6665f0e9bc 100644
--- a/neo/Consensus/ConsensusService.cs
+++ b/neo/Consensus/ConsensusService.cs
@@ -224,7 +224,7 @@ private void OnCommitReceived(ConsensusPayload payload, Commit commit)
             if (existingCommitPayload != null)
             {
                 if (existingCommitPayload.Hash != payload.Hash)
-                    Log($"{nameof(OnCommitReceived)}: Warning, different commit from validator! height={payload.BlockIndex} index={payload.ValidatorIndex} view={commit.ViewNumber} existingView={existingCommitPayload.ConsensusMessage.ViewNumber}");
+                    Log($"{nameof(OnCommitReceived)}: different commit from validator! height={payload.BlockIndex} index={payload.ValidatorIndex} view={commit.ViewNumber} existingView={existingCommitPayload.ConsensusMessage.ViewNumber}", LogLevel.Warning);
                 return;
             }
 

From 7ba991b87ca3110b08da43c8edec58f744f566b8 Mon Sep 17 00:00:00 2001
From: erikzhang <erik@neo.org>
Date: Thu, 21 Mar 2019 12:39:21 +0800
Subject: [PATCH 22/26] Check `ViewNumber` in `OnPrepareRequestReceived()` and
 `OnPrepareResponseReceived()`

---
 neo/Consensus/ConsensusService.cs | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/neo/Consensus/ConsensusService.cs b/neo/Consensus/ConsensusService.cs
index bd3c283f20..23bc21e748 100644
--- a/neo/Consensus/ConsensusService.cs
+++ b/neo/Consensus/ConsensusService.cs
@@ -263,14 +263,10 @@ private void OnConsensusPayload(ConsensusPayload payload)
                 return;
             }
             if (payload.ValidatorIndex >= context.Validators.Length) return;
-            ConsensusMessage message = payload.ConsensusMessage;
-            if (message.ViewNumber != context.ViewNumber && (message.Type == ConsensusMessageType.PrepareRequest ||
-                                                             message.Type == ConsensusMessageType.PrepareResponse))
-                return;
             foreach (IP2PPlugin plugin in Plugin.P2PPlugins)
                 if (!plugin.OnConsensusMessage(payload))
                     return;
-            switch (message)
+            switch (payload.ConsensusMessage)
             {
                 case ChangeView view:
                     OnChangeViewReceived(payload, view);
@@ -364,7 +360,7 @@ private void OnRecoveryMessageReceived(ConsensusPayload payload, RecoveryMessage
         private void OnPrepareRequestReceived(ConsensusPayload payload, PrepareRequest message)
         {
             if (context.RequestSentOrReceived() || context.NotAcceptingPayloadsDueToViewChanging()) return;
-            if (payload.ValidatorIndex != context.PrimaryIndex) return;
+            if (payload.ValidatorIndex != context.PrimaryIndex || message.ViewNumber != context.ViewNumber) return;
             Log($"{nameof(OnPrepareRequestReceived)}: height={payload.BlockIndex} view={message.ViewNumber} index={payload.ValidatorIndex} tx={message.TransactionHashes.Length}");
             if (message.Timestamp <= context.PrevHeader().Timestamp || message.Timestamp > TimeProvider.Current.UtcNow.AddMinutes(10).ToTimestamp())
             {
@@ -424,6 +420,7 @@ private void OnPrepareRequestReceived(ConsensusPayload payload, PrepareRequest m
 
         private void OnPrepareResponseReceived(ConsensusPayload payload, PrepareResponse message)
         {
+            if (message.ViewNumber != context.ViewNumber) return;
             if (context.PreparationPayloads[payload.ValidatorIndex] != null || context.NotAcceptingPayloadsDueToViewChanging()) return;
             if (context.PreparationPayloads[context.PrimaryIndex] != null && !message.PreparationHash.Equals(context.PreparationPayloads[context.PrimaryIndex].Hash))
                 return;

From 6fc48644c6639a8ace019e7dc485b3e61dc00a41 Mon Sep 17 00:00:00 2001
From: erikzhang <erik@neo.org>
Date: Thu, 21 Mar 2019 12:49:41 +0800
Subject: [PATCH 23/26] Simplify `OnRecoveryMessageReceived()`

---
 neo/Consensus/ConsensusService.cs | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/neo/Consensus/ConsensusService.cs b/neo/Consensus/ConsensusService.cs
index 23bc21e748..e75ab93499 100644
--- a/neo/Consensus/ConsensusService.cs
+++ b/neo/Consensus/ConsensusService.cs
@@ -304,16 +304,6 @@ private void OnRecoveryMessageReceived(ConsensusPayload payload, RecoveryMessage
             Log($"{nameof(OnRecoveryMessageReceived)}: height={payload.BlockIndex} view={message.ViewNumber} index={payload.ValidatorIndex}");
             try
             {
-                ConsensusPayload[] commitPayloads;
-                if (message.ViewNumber < context.ViewNumber)
-                {
-                    // Ensure we know about all commits from lower view numbers.
-                    commitPayloads = message.GetCommitPayloadsFromRecoveryMessage(context, payload);
-                    totalCommits = commitPayloads.Length;
-                    foreach (ConsensusPayload commitPayload in commitPayloads)
-                        if (ReverifyAndProcessPayload(commitPayload)) validCommits++;
-                    return;
-                }
                 if (message.ViewNumber > context.ViewNumber)
                 {
                     if (context.CommitSent()) return;
@@ -322,8 +312,7 @@ private void OnRecoveryMessageReceived(ConsensusPayload payload, RecoveryMessage
                     foreach (ConsensusPayload changeViewPayload in changeViewPayloads)
                         if (ReverifyAndProcessPayload(changeViewPayload)) validChangeViews++;
                 }
-                if (message.ViewNumber != context.ViewNumber) return;
-                if (!context.NotAcceptingPayloadsDueToViewChanging() && !context.CommitSent())
+                if (message.ViewNumber == context.ViewNumber && !context.NotAcceptingPayloadsDueToViewChanging() && !context.CommitSent())
                 {
                     if (!context.RequestSentOrReceived())
                     {
@@ -341,10 +330,14 @@ private void OnRecoveryMessageReceived(ConsensusPayload payload, RecoveryMessage
                     foreach (ConsensusPayload prepareResponsePayload in prepareResponsePayloads)
                         if (ReverifyAndProcessPayload(prepareResponsePayload)) validPrepResponses++;
                 }
-                commitPayloads = message.GetCommitPayloadsFromRecoveryMessage(context, payload);
-                totalCommits = commitPayloads.Length;
-                foreach (ConsensusPayload commitPayload in commitPayloads)
-                    if (ReverifyAndProcessPayload(commitPayload)) validCommits++;
+                if (message.ViewNumber <= context.ViewNumber)
+                {
+                    // Ensure we know about all commits from lower view numbers.
+                    ConsensusPayload[] commitPayloads = message.GetCommitPayloadsFromRecoveryMessage(context, payload);
+                    totalCommits = commitPayloads.Length;
+                    foreach (ConsensusPayload commitPayload in commitPayloads)
+                        if (ReverifyAndProcessPayload(commitPayload)) validCommits++;
+                }
             }
             finally
             {

From b7e79810506ea1ba5eac7a837895361910d65fad Mon Sep 17 00:00:00 2001
From: erikzhang <erik@neo.org>
Date: Thu, 21 Mar 2019 13:02:39 +0800
Subject: [PATCH 24/26] minor change

---
 neo/Consensus/ConsensusService.cs | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/neo/Consensus/ConsensusService.cs b/neo/Consensus/ConsensusService.cs
index e75ab93499..73b1614dd4 100644
--- a/neo/Consensus/ConsensusService.cs
+++ b/neo/Consensus/ConsensusService.cs
@@ -377,9 +377,8 @@ private void OnPrepareRequestReceived(ConsensusPayload payload, PrepareRequest m
             context.PreparationPayloads[payload.ValidatorIndex] = payload;
             byte[] hashData = context.MakeHeader().GetHashData();
             for (int i = 0; i < context.CommitPayloads.Length; i++)
-                if (context.CommitPayloads[i] != null)
-                    if (context.CommitPayloads[i].ConsensusMessage.ViewNumber == context.ViewNumber &&
-                        !Crypto.Default.VerifySignature(hashData, context.CommitPayloads[i].GetDeserializedMessage<Commit>().Signature, context.Validators[i].EncodePoint(false)))
+                if (context.CommitPayloads[i]?.ConsensusMessage.ViewNumber == context.ViewNumber)
+                    if (!Crypto.Default.VerifySignature(hashData, context.CommitPayloads[i].GetDeserializedMessage<Commit>().Signature, context.Validators[i].EncodePoint(false)))
                         context.CommitPayloads[i] = null;
             Dictionary<UInt256, Transaction> mempoolVerified = Blockchain.Singleton.MemPool.GetVerifiedTransactions().ToDictionary(p => p.Hash);
 

From 9838afc3ad47a238eec9ceae807288aecff330b6 Mon Sep 17 00:00:00 2001
From: jsolman <jsolinsky@gmail.com>
Date: Thu, 21 Mar 2019 00:15:04 -0700
Subject: [PATCH 25/26] Fix a merge issue. OnConsensusPayload method should not
 be accepting PrepareRequest and PrepareResponse from different views.

---
 neo/Consensus/ConsensusService.cs | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/neo/Consensus/ConsensusService.cs b/neo/Consensus/ConsensusService.cs
index 73b1614dd4..b38a673b49 100644
--- a/neo/Consensus/ConsensusService.cs
+++ b/neo/Consensus/ConsensusService.cs
@@ -266,7 +266,11 @@ private void OnConsensusPayload(ConsensusPayload payload)
             foreach (IP2PPlugin plugin in Plugin.P2PPlugins)
                 if (!plugin.OnConsensusMessage(payload))
                     return;
-            switch (payload.ConsensusMessage)
+            ConsensusMessage message = payload.ConsensusMessage;
+            if (message.ViewNumber != context.ViewNumber && (message.Type == ConsensusMessageType.PrepareRequest ||
+                                                             message.Type == ConsensusMessageType.PrepareResponse))
+                return;
+            switch (message)
             {
                 case ChangeView view:
                     OnChangeViewReceived(payload, view);

From 663a8ae0099fa23289bdf29547808ea6496293c5 Mon Sep 17 00:00:00 2001
From: jsolman <jsolinsky@gmail.com>
Date: Thu, 21 Mar 2019 00:21:52 -0700
Subject: [PATCH 26/26] Revert "Fix a merge issue. OnConsensusPayload method
 should not be accepting PrepareRequest and PrepareResponse from different
 views."

This reverts commit 9838afc3ad47a238eec9ceae807288aecff330b6.
---
 neo/Consensus/ConsensusService.cs | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/neo/Consensus/ConsensusService.cs b/neo/Consensus/ConsensusService.cs
index b38a673b49..73b1614dd4 100644
--- a/neo/Consensus/ConsensusService.cs
+++ b/neo/Consensus/ConsensusService.cs
@@ -266,11 +266,7 @@ private void OnConsensusPayload(ConsensusPayload payload)
             foreach (IP2PPlugin plugin in Plugin.P2PPlugins)
                 if (!plugin.OnConsensusMessage(payload))
                     return;
-            ConsensusMessage message = payload.ConsensusMessage;
-            if (message.ViewNumber != context.ViewNumber && (message.Type == ConsensusMessageType.PrepareRequest ||
-                                                             message.Type == ConsensusMessageType.PrepareResponse))
-                return;
-            switch (message)
+            switch (payload.ConsensusMessage)
             {
                 case ChangeView view:
                     OnChangeViewReceived(payload, view);