Skip to content

Commit

Permalink
Fail fast in equivocation tests and fix typos (#681)
Browse files Browse the repository at this point in the history 
We want to fail fast in equivocation tests as there is not much point
proceeding with the tests if a test case fails.

While at it, fix typos in tests.
  • Loading branch information
@masih
masih authored Oct 2, 2024
1 parent 895250c commit ed5b294
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 22 deletions.
1 change: 0 additions & 1 deletion equivocation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,5 +139,4 @@ func (ef *equivocationFilter) ProcessReceive(peerID peer.ID, m *gpbft.GMessage)
// add the message
ef.seenMessages[key] = equivMessage{signature: m.Signature, origin: peerID}
}

}
42 changes: 21 additions & 21 deletions equivocation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ import (

"github.com/filecoin-project/go-f3/gpbft"
"github.com/libp2p/go-libp2p/core/peer"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

var localGoodPID = peer.ID("1local")
var remotePID0 = peer.ID("0remote")
var remotePID5 = peer.ID("5remote")

func TestEquivactionFilter_ProcessBroadcast(t *testing.T) {
func TestEquivocationFilter_ProcessBroadcast(t *testing.T) {
localPID := localGoodPID
ef := newEquivocationFilter(localPID)

Expand All @@ -22,42 +22,42 @@ func TestEquivactionFilter_ProcessBroadcast(t *testing.T) {
Vote: gpbft.Payload{Instance: 1, Round: 1, Phase: gpbft.Phase(1)},
Signature: []byte("signature1"),
}
assert.True(t, ef.ProcessBroadcast(msg1), "First message should be processed")
require.True(t, ef.ProcessBroadcast(msg1), "First message should be processed")

// Test case 2: Duplicate message with same signature should be processed
msg2 := &gpbft.GMessage{
Sender: gpbft.ActorID(1),
Vote: gpbft.Payload{Instance: 1, Round: 1, Phase: gpbft.Phase(1)},
Signature: []byte("signature1"),
}
assert.True(t, ef.ProcessBroadcast(msg2), "Duplicate message with same signature should be processed")
require.True(t, ef.ProcessBroadcast(msg2), "Duplicate message with same signature should be processed")

// Test case 3 Message with same key but different signature should not be processed
msg3 := &gpbft.GMessage{
Sender: gpbft.ActorID(1),
Vote: gpbft.Payload{Instance: 1, Round: 1, Phase: gpbft.Phase(1)},
Signature: []byte("signature2"),
}
assert.False(t, ef.ProcessBroadcast(msg3), "Message with same key but different signature should not be processed")
require.False(t, ef.ProcessBroadcast(msg3), "Message with same key but different signature should not be processed")

// Test case 4: Message with new instance should be processed
msg4 := &gpbft.GMessage{
Sender: gpbft.ActorID(1),
Vote: gpbft.Payload{Instance: 2, Round: 1, Phase: gpbft.Phase(1)},
Signature: []byte("signature3"),
}
assert.True(t, ef.ProcessBroadcast(msg4), "Message with new instance should be processed")
require.True(t, ef.ProcessBroadcast(msg4), "Message with new instance should be processed")

// Test case 5: Message with past instance should not be processed
msg5 := &gpbft.GMessage{
Sender: gpbft.ActorID(1),
Vote: gpbft.Payload{Instance: 1, Round: 1, Phase: gpbft.Phase(1)},
Signature: []byte("signature4"),
}
assert.False(t, ef.ProcessBroadcast(msg5), "Message with past instance should not be processed")
require.False(t, ef.ProcessBroadcast(msg5), "Message with past instance should not be processed")
}

func TestEquivactionFilter_formKey(t *testing.T) {
func TestEquivocationFilter_formKey(t *testing.T) {
ef := newEquivocationFilter(localGoodPID)

msg := &gpbft.GMessage{
Expand All @@ -71,10 +71,10 @@ func TestEquivactionFilter_formKey(t *testing.T) {
Phase: gpbft.Phase(1),
}

assert.Equal(t, expectedKey, ef.formKey(msg), "Keys should match")
require.Equal(t, expectedKey, ef.formKey(msg), "Keys should match")
}

func TestEquivactionFilter_remoteEquivocationResolution(t *testing.T) {
func TestEquivocationFilter_remoteEquivocationResolution(t *testing.T) {
localPID := localGoodPID
ef := newEquivocationFilter(localPID)

Expand All @@ -83,7 +83,7 @@ func TestEquivactionFilter_remoteEquivocationResolution(t *testing.T) {
Vote: gpbft.Payload{Instance: 1, Round: 1, Phase: gpbft.Phase(1)},
Signature: []byte("signature1"),
}
assert.True(t, ef.ProcessBroadcast(msg1), "First message should be processed")
require.True(t, ef.ProcessBroadcast(msg1), "First message should be processed")
ef.ProcessReceive(localPID, msg1)

msg2 := &gpbft.GMessage{
Expand All @@ -92,15 +92,15 @@ func TestEquivactionFilter_remoteEquivocationResolution(t *testing.T) {
Signature: []byte("signature2"),
}
ef.ProcessReceive(remotePID5, msg2)
assert.Contains(t, ef.activeSenders[msg2.Sender].origins, remotePID5)
require.Contains(t, ef.activeSenders[msg2.Sender].origins, remotePID5)

msg3 := &gpbft.GMessage{
Sender: gpbft.ActorID(1),
Vote: gpbft.Payload{Instance: 1, Round: 2, Phase: gpbft.Phase(1)},
Signature: []byte("signature3"),
}

assert.True(t, ef.ProcessBroadcast(msg3), "local sender should still be able to broadcast")
require.True(t, ef.ProcessBroadcast(msg3), "local sender should still be able to broadcast")
ef.ProcessReceive(localPID, msg1)

// lower PeerID sender comes along
Expand All @@ -110,17 +110,17 @@ func TestEquivactionFilter_remoteEquivocationResolution(t *testing.T) {
Signature: []byte("signature4"),
}
ef.ProcessReceive(remotePID0, msg4)
assert.Contains(t, ef.activeSenders[msg2.Sender].origins, remotePID0)
require.Contains(t, ef.activeSenders[msg2.Sender].origins, remotePID0)

msg5 := &gpbft.GMessage{
Sender: gpbft.ActorID(1),
Vote: gpbft.Payload{Instance: 1, Round: 3, Phase: gpbft.Phase(1)},
Signature: []byte("signature5"),
}

assert.False(t, ef.ProcessBroadcast(msg5), "we should have backed off")
require.False(t, ef.ProcessBroadcast(msg5), "we should have backed off")

assert.False(t, ef.ProcessBroadcast(msg3), "trying to re-broadcast is now not allowed")
require.False(t, ef.ProcessBroadcast(msg3), "trying to re-broadcast is now not allowed")
}

func TestEquivocationFilter_PeerIDBasedEquivocationHandling(t *testing.T) {
Expand All @@ -133,7 +133,7 @@ func TestEquivocationFilter_PeerIDBasedEquivocationHandling(t *testing.T) {
Vote: gpbft.Payload{Instance: 1, Round: 1, Phase: gpbft.Phase(1)},
Signature: []byte("signature1"),
}
assert.True(t, ef.ProcessBroadcast(msg1), "Local message should be processed")
require.True(t, ef.ProcessBroadcast(msg1), "Local message should be processed")

// Remote equivocation with higher PeerID
msg2 := &gpbft.GMessage{
Expand All @@ -142,10 +142,10 @@ func TestEquivocationFilter_PeerIDBasedEquivocationHandling(t *testing.T) {
Signature: []byte("signature2"),
}
ef.ProcessReceive(remotePID5, msg2)
assert.Contains(t, ef.activeSenders[msg2.Sender].origins, remotePID5, "Higher PeerID should be recorded")
require.Contains(t, ef.activeSenders[msg2.Sender].origins, remotePID5, "Higher PeerID should be recorded")

// Local broadcast after higher PeerID equivocation
assert.True(t, ef.ProcessBroadcast(msg1), "Local message should still be processed after higher PeerID equivocation")
require.True(t, ef.ProcessBroadcast(msg1), "Local message should still be processed after higher PeerID equivocation")

// Remote equivocation with lower PeerID
msg3 := &gpbft.GMessage{
Expand All @@ -154,8 +154,8 @@ func TestEquivocationFilter_PeerIDBasedEquivocationHandling(t *testing.T) {
Signature: []byte("signature3"),
}
ef.ProcessReceive(remotePID0, msg3)
assert.Contains(t, ef.activeSenders[msg3.Sender].origins, remotePID0, "Lower PeerID should be recorded")
require.Contains(t, ef.activeSenders[msg3.Sender].origins, remotePID0, "Lower PeerID should be recorded")

// Local broadcast after lower PeerID equivocation
assert.False(t, ef.ProcessBroadcast(msg1), "Local message should not be processed after lower PeerID equivocation")
require.False(t, ef.ProcessBroadcast(msg1), "Local message should not be processed after lower PeerID equivocation")
}

0 comments on commit ed5b294

Please sign in to comment.