Fix chain comparison bug and add tests (#734)

Return false if chains of same length are not equal and assert assert it
in unit tests.

Author: masih

gpbft/chain.go

@@ -83,7 +83,7 @@ func (ts *TipSet) Equal(b *TipSet) bool {
 	}
 	return ts.Epoch == b.Epoch &&
 		bytes.Equal(ts.Key, b.Key) &&
-		ts.PowerTable == b.PowerTable &&
+		ts.PowerTable.Equals(b.PowerTable) &&
 		ts.Commitments == b.Commitments
 }
 
@@ -216,7 +216,9 @@ func (c ECChain) Eq(other ECChain) bool {
 		return false
 	}
 	for i := range c {
-		c[i].Equal(&other[i])
+		if !c[i].Equal(&other[i]) {
+			return false
+		}
 	}
 	return true
 }

gpbft/chain_test.go

@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/filecoin-project/go-f3/gpbft"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -107,3 +108,79 @@ func TestECChain(t *testing.T) {
 		require.Equal(t, second[1], gpbft.TipSet{Epoch: 1, Key: []byte{2}})
 	})
 }
+
+func TestECChain_Eq(t *testing.T) {
+	t.Parallel()
+	var (
+		commitThis              = [32]byte{0x01}
+		commitThat              = [32]byte{0x02}
+		ptThis                  = gpbft.MakeCid([]byte("fish"))
+		ptThat                  = gpbft.MakeCid([]byte("lobster"))
+		ts1                     = gpbft.TipSet{Epoch: 1, Key: []byte("barreleye1"), PowerTable: ptThat, Commitments: commitThis}
+		ts2                     = gpbft.TipSet{Epoch: 2, Key: []byte("barreleye2"), PowerTable: ptThat, Commitments: commitThis}
+		ts3                     = gpbft.TipSet{Epoch: 3, Key: []byte("barreleye3"), PowerTable: ptThat, Commitments: commitThis}
+		ts1DifferentCommitments = gpbft.TipSet{1, []byte("barreleye1"), ptThat, commitThat}
+		ts1DifferentPowerTable  = gpbft.TipSet{1, []byte("barreleye1"), ptThis, commitThis}
+	)
+	for _, tt := range []struct {
+		name   string
+		one    *gpbft.ECChain
+		other  *gpbft.ECChain
+		expect bool
+	}{
+		{
+			name:   "Equal chains",
+			one:    &gpbft.ECChain{ts1, ts2},
+			other:  &gpbft.ECChain{ts1, ts2},
+			expect: true,
+		},
+		{
+			name:   "Different chains",
+			one:    &gpbft.ECChain{ts1, ts2},
+			other:  &gpbft.ECChain{ts1, ts3},
+			expect: false,
+		},
+		{
+			name:   "Same chain compared with itself",
+			one:    &gpbft.ECChain{ts1, ts2},
+			other:  &gpbft.ECChain{ts1, ts2},
+			expect: true,
+		},
+		{
+			name:   "Different lengths",
+			one:    &gpbft.ECChain{ts1},
+			other:  &gpbft.ECChain{ts1, ts2},
+			expect: false,
+		},
+		{
+			name:   "Zero chains (empty chains)",
+			one:    &gpbft.ECChain{},
+			other:  &gpbft.ECChain{},
+			expect: true,
+		},
+		{
+			name:   "One zero chain",
+			one:    &gpbft.ECChain{ts1},
+			other:  &gpbft.ECChain{},
+			expect: false,
+		},
+		{
+			name:   "Different commitments",
+			one:    &gpbft.ECChain{ts1},
+			other:  &gpbft.ECChain{ts1DifferentCommitments},
+			expect: false,
+		},
+		{
+			name:   "Different power table",
+			one:    &gpbft.ECChain{ts1},
+			other:  &gpbft.ECChain{ts1DifferentPowerTable},
+			expect: false,
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			assert.Equal(t, tt.expect, tt.one.Eq(*tt.other), "Unexpected equality result for one compared to other: %s", tt.name)
+			assert.Equal(t, tt.expect, tt.other.Eq(*tt.one), "Unexpected equality result for other compared to one: %s", tt.name)
+		})
+	}
+}