Fix a bug in pubsub message ID hash function (#791)

The prior implementation I believe intended to include additional fields
into account when calculating the message ID from data. But was only
returning hash of data for both manifest and GPBFT messages.

Fix this by writing the data to the hasher, _then_ computing the hash
with no suffix.

internal/psutil/psutil.go

@@ -28,8 +28,10 @@ func pubsubMsgIdHashData(m *pubsub_pb.Message) string {
 	if _, err := hasher.Write(topic); err != nil {
 		panic(err)
 	}
-
-	hash := blake2b.Sum256(m.Data)
+	if _, err := hasher.Write(m.Data); err != nil {
+		panic(err)
+	}
+	hash := hasher.Sum(nil)
 	return string(hash[:])
 }
 
@@ -53,8 +55,10 @@ func pubsubMsgIdHashDataAndSender(m *pubsub_pb.Message) string {
 	if _, err := hasher.Write(m.From); err != nil {
 		panic(err)
 	}
-
-	hash := blake2b.Sum256(m.Data)
+	if _, err := hasher.Write(m.Data); err != nil {
+		panic(err)
+	}
+	hash := hasher.Sum(nil)
 	return string(hash[:])
 }
 

internal/psutil/psutil_test.go

@@ -0,0 +1,121 @@
+package psutil_test
+
+import (
+	"testing"
+
+	"github.com/filecoin-project/go-f3/internal/psutil"
+	pubsub_pb "github.com/libp2p/go-libp2p-pubsub/pb"
+	"github.com/stretchr/testify/require"
+)
+
+func TestManifestMessageIdFn(t *testing.T) {
+	for _, test := range []struct {
+		name          string
+		one           *pubsub_pb.Message
+		other         *pubsub_pb.Message
+		expectEqualID bool
+	}{
+		{
+			name: "same topic different data",
+			one: &pubsub_pb.Message{
+				Topic: topic("fish"),
+				From:  []byte("barreleye"),
+				Data:  []byte("undadasea"),
+			},
+			other: &pubsub_pb.Message{
+				Topic: topic("fish"),
+				From:  []byte("barreleye"),
+				Data:  []byte("lobstermuncher"),
+			},
+			expectEqualID: false,
+		},
+		{
+			name: "same data different topic",
+			one: &pubsub_pb.Message{
+				Topic: topic("fish"),
+				From:  []byte("barreleye"),
+				Data:  []byte("undadasea"),
+			},
+			other: &pubsub_pb.Message{
+				Topic: topic("lobster"),
+				From:  []byte("barreleye"),
+				Data:  []byte("undadasea"),
+			},
+			expectEqualID: false,
+		},
+		{
+			name: "same data and topic different sender",
+			one: &pubsub_pb.Message{
+				Topic: topic("fish"),
+				From:  []byte("barreleye"),
+				Data:  []byte("undadasea"),
+			},
+			other: &pubsub_pb.Message{
+				Topic: topic("fish"),
+				From:  []byte("fisherman"),
+				Data:  []byte("undadasea"),
+			},
+			expectEqualID: false,
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			this, that := psutil.ManifestMessageIdFn(test.one), psutil.ManifestMessageIdFn(test.other)
+			require.Equal(t, test.expectEqualID, this == that)
+		})
+	}
+}
+
+func TestGPBFTMessageIdFn(t *testing.T) {
+	for _, test := range []struct {
+		name          string
+		one           *pubsub_pb.Message
+		other         *pubsub_pb.Message
+		expectEqualID bool
+	}{
+		{
+			name: "same topic different data",
+			one: &pubsub_pb.Message{
+				Topic: topic("fish"),
+				Data:  []byte("undadasea"),
+			},
+			other: &pubsub_pb.Message{
+				Topic: topic("fish"),
+				Data:  []byte("lobstermuncher"),
+			},
+			expectEqualID: false,
+		},
+		{
+			name: "same data different topic",
+			one: &pubsub_pb.Message{
+				Topic: topic("fish"),
+				Data:  []byte("undadasea"),
+			},
+			other: &pubsub_pb.Message{
+				Topic: topic("lobster"),
+				Data:  []byte("undadasea"),
+			},
+			expectEqualID: false,
+		},
+		{
+			name: "same data and topic different sender",
+			one: &pubsub_pb.Message{
+				Topic: topic("fish"),
+				From:  []byte("barreleye"),
+				Data:  []byte("undadasea"),
+			},
+			other: &pubsub_pb.Message{
+				Topic: topic("fish"),
+				From:  []byte("fisherman"),
+				Data:  []byte("undadasea"),
+			},
+			expectEqualID: true,
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			this, that := psutil.GPBFTMessageIdFn(test.one), psutil.GPBFTMessageIdFn(test.other)
+			require.Equal(t, test.expectEqualID, this == that)
+		})
+	}
+}
+
+func topic(s string) *string { return &s }