Go-ibft ignores some future messages (#67)

* Go-ibft ignores some future messages

* Additional test case (same height, lower message round)

* Test for greater height and lower round added

---------

Co-authored-by: Stefan Negovanović <stefan@ethernal.tech>

core/ibft.go

@@ -1074,12 +1074,16 @@ func (i *IBFT) AddMessage(message *proto.Message) {
 	if i.isAcceptableMessage(message) {
 		i.messages.AddMessage(message)
 
-		msgs := i.messages.GetValidMessages(
-			message.View,
-			message.Type,
-			func(_ *proto.Message) bool { return true })
-		if i.backend.HasQuorum(message.View.Height, msgs, message.Type) {
-			i.messages.SignalEvent(message)
+		// Signal event if the quorum is reached. Since the subscriptions refer to the state height,
+		// no need to call this if the message height is not equal to the state height
+		if message.View.Height == i.state.getHeight() {
+			msgs := i.messages.GetValidMessages(
+				message.View,
+				message.Type,
+				func(_ *proto.Message) bool { return true })
+			if i.backend.HasQuorum(message.View.Height, msgs, message.Type) {
+				i.messages.SignalEvent(message)
+			}
 		}
 	}
 }
@@ -1102,8 +1106,12 @@ func (i *IBFT) isAcceptableMessage(message *proto.Message) bool {
 		return false
 	}
 
-	// Make sure the message round is >= the current state round
-	return message.View.Round >= i.state.getRound()
+	// Make sure if the heights are the same, the message round is >= the current state round
+	if i.state.getHeight() == message.View.Height {
+		return message.View.Round >= i.state.getRound()
+	}
+
+	return true
 }
 
 // ExtendRoundTimeout extends each round's timer by the specified amount.

core/ibft_test.go

@@ -1117,54 +1117,80 @@ func TestIBFT_IsAcceptableMessage(t *testing.T) {
 
 	testTable := []struct {
 		name          string
-		view          *proto.View
-		currentView   *proto.View
+		msgView       *proto.View
+		stateView     *proto.View
 		invalidSender bool
 		acceptable    bool
 	}{
 		{
-			"invalid sender",
-			nil,
-			baseView,
-			true,
-			false,
+			name:          "invalid sender",
+			msgView:       nil,
+			stateView:     baseView,
+			invalidSender: true,
+			acceptable:    false,
 		},
 		{
-			"malformed message",
-			nil,
-			baseView,
-			false,
-			false,
+			name:          "malformed message",
+			msgView:       nil,
+			stateView:     baseView,
+			invalidSender: false,
+			acceptable:    false,
 		},
 		{
-			"higher height number",
-			&proto.View{
+			name: "higher height, same round number",
+			msgView: &proto.View{
 				Height: baseView.Height + 100,
 				Round:  baseView.Round,
 			},
-			baseView,
-			false,
-			true,
+			stateView:     baseView,
+			invalidSender: false,
+			acceptable:    true,
 		},
 		{
-			"higher round number",
-			&proto.View{
+			name: "higher height, lower round number",
+			msgView: &proto.View{
+				Height: baseView.Height + 100,
+				Round:  baseView.Round,
+			},
+			stateView: &proto.View{
 				Height: baseView.Height,
 				Round:  baseView.Round + 1,
 			},
-			baseView,
-			false,
-			true,
+			invalidSender: false,
+			acceptable:    true,
 		},
 		{
-			"lower height number",
-			baseView,
-			&proto.View{
+			name: "same heights, higher round number",
+			msgView: &proto.View{
+				Height: baseView.Height,
+				Round:  baseView.Round + 1,
+			},
+			stateView:     baseView,
+			invalidSender: false,
+			acceptable:    true,
+		},
+		{
+			name: "same heights, lower round number",
+			msgView: &proto.View{
+				Height: baseView.Height,
+				Round:  baseView.Round + 1,
+			},
+			stateView: &proto.View{
+				Height: baseView.Height,
+				Round:  baseView.Round + 2,
+			},
+			invalidSender: false,
+			acceptable:    false,
+		},
+		{
+			name:    "lower height number",
+			msgView: baseView,
+			stateView: &proto.View{
 				Height: baseView.Height + 1,
 				Round:  baseView.Round,
 			},
-			false,
-			false,
+			invalidSender: false,
+			acceptable:    false,
 		},
 	}
 
@@ -1184,10 +1210,10 @@ func TestIBFT_IsAcceptableMessage(t *testing.T) {
 				}
 			)
 			i := NewIBFT(log, backend, transport)
-			i.state.view = testCase.currentView
+			i.state.view = testCase.stateView
 
 			message := &proto.Message{
-				View: testCase.view,
+				View: testCase.msgView,
 			}
 
 			assert.Equal(t, testCase.acceptable, i.isAcceptableMessage(message))