From c5bb3c5849746157db2bcdc3b30c275e5059dfa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Thu, 23 Jul 2026 19:59:17 +0000 Subject: [PATCH] test(entry): cover signature-share validation and message type pkg/beacon/entry had only marshaling round-trip tests (~14% coverage). Add unit tests for the logic that can be exercised without a live broadcast channel or relay chain: - extractAndValidateShare, the gatekeeper that accepts or rejects a signature share received from another group member - all three rejection paths (unparseable share, unknown sender, share that does not verify) plus the accept path, built on real BLS sign/verify. - The SignatureShareMessage value object (constructor, SenderID) and its wire Type identifier, which must stay stable for cross-client network compatibility. - Unmarshal rejecting a member index that overflows the uint8 range. Raises package coverage from ~14% to ~27%. The remaining uncovered code (SignAndSubmit orchestration, relay entry submission) drives a broadcast channel and the beacon chain and is exercised by integration tests. --- pkg/beacon/entry/entry_test.go | 74 ++++++++++++++++++++++++++++++++ pkg/beacon/entry/message_test.go | 67 +++++++++++++++++++++++++++++ 2 files changed, 141 insertions(+) create mode 100644 pkg/beacon/entry/entry_test.go create mode 100644 pkg/beacon/entry/message_test.go diff --git a/pkg/beacon/entry/entry_test.go b/pkg/beacon/entry/entry_test.go new file mode 100644 index 0000000000..4be516bbb5 --- /dev/null +++ b/pkg/beacon/entry/entry_test.go @@ -0,0 +1,74 @@ +package entry + +import ( + "math/big" + "testing" + + bn256 "github.com/ethereum/go-ethereum/crypto/bn256/cloudflare" + + "github.com/keep-network/keep-core/pkg/bls" + "github.com/keep-network/keep-core/pkg/protocol/group" +) + +// extractAndValidateShare is the gatekeeper that decides whether a signature +// share received from another group member is trustworthy. A member that +// accepted a bad share would contribute to a corrupt relay entry, so each +// rejection path matters as much as the accept path. +func TestExtractAndValidateShare(t *testing.T) { + senderID := group.MemberIndex(2) + secretKey := big.NewInt(1234567) + + // previousEntry is the G1 point being signed; the share is the point + // signed with the sender's secret key and the public key share is the + // matching G2 public key. + previousEntry := new(bn256.G1).ScalarBaseMult(big.NewInt(7)) + validShare := bls.SignG1(secretKey, previousEntry) + publicKeyShare := new(bn256.G2).ScalarBaseMult(secretKey) + + t.Run("valid share", func(t *testing.T) { + message := NewSignatureShareMessage(senderID, validShare.Marshal(), "s") + shares := map[group.MemberIndex]*bn256.G2{senderID: publicKeyShare} + + share, err := extractAndValidateShare(message, shares, previousEntry) + if err != nil { + t.Fatalf("unexpected error: [%v]", err) + } + if share == nil { + t.Fatal("expected a non-nil share") + } + // The returned share must be the one carried by the message. + if !bls.VerifyG1(publicKeyShare, previousEntry, share) { + t.Error("returned share does not verify against the public key share") + } + }) + + t.Run("unparseable share bytes", func(t *testing.T) { + message := NewSignatureShareMessage(senderID, []byte{0x01, 0x02}, "s") + shares := map[group.MemberIndex]*bn256.G2{senderID: publicKeyShare} + + if _, err := extractAndValidateShare(message, shares, previousEntry); err == nil { + t.Error("expected an error for share bytes that are not a valid G1 point") + } + }) + + t.Run("no public key share for sender", func(t *testing.T) { + message := NewSignatureShareMessage(senderID, validShare.Marshal(), "s") + shares := map[group.MemberIndex]*bn256.G2{} // sender absent + + if _, err := extractAndValidateShare(message, shares, previousEntry); err == nil { + t.Error("expected an error when the sender has no known public key share") + } + }) + + t.Run("share does not verify", func(t *testing.T) { + // A share signed with a different secret key than the public key share + // advertises must be rejected. + wrongShare := bls.SignG1(big.NewInt(7654321), previousEntry) + message := NewSignatureShareMessage(senderID, wrongShare.Marshal(), "s") + shares := map[group.MemberIndex]*bn256.G2{senderID: publicKeyShare} + + if _, err := extractAndValidateShare(message, shares, previousEntry); err == nil { + t.Error("expected an error for a share that does not verify") + } + }) +} diff --git a/pkg/beacon/entry/message_test.go b/pkg/beacon/entry/message_test.go new file mode 100644 index 0000000000..0e905e0a2b --- /dev/null +++ b/pkg/beacon/entry/message_test.go @@ -0,0 +1,67 @@ +package entry + +import ( + "testing" + + "google.golang.org/protobuf/proto" + + "github.com/keep-network/keep-core/internal/testutils" + "github.com/keep-network/keep-core/pkg/beacon/entry/gen/pb" + "github.com/keep-network/keep-core/pkg/protocol/group" +) + +func TestNewSignatureShareMessage(t *testing.T) { + senderID := group.MemberIndex(7) + shareBytes := []byte{0x01, 0x02, 0x03} + sessionID := "session-1" + + message := NewSignatureShareMessage(senderID, shareBytes, sessionID) + + if message.SenderID() != senderID { + t.Errorf( + "unexpected sender ID\nexpected: [%v]\nactual: [%v]", + senderID, + message.SenderID(), + ) + } + testutils.AssertBytesEqual(t, shareBytes, message.shareBytes) + if message.sessionID != sessionID { + t.Errorf( + "unexpected session ID\nexpected: [%v]\nactual: [%v]", + sessionID, + message.sessionID, + ) + } +} + +// The Type string is the wire identifier used to route this message on the +// broadcast channel. Changing it silently would break network compatibility +// with other clients, so it must stay stable. +func TestSignatureShareMessageType(t *testing.T) { + message := &SignatureShareMessage{} + + if got := message.Type(); got != "relay/signature/share" { + t.Errorf( + "unexpected message type\nexpected: [relay/signature/share]\nactual: [%v]", + got, + ) + } +} + +// MemberIndex is a uint8 in the protocol but uint32 on the wire; Unmarshal must +// reject values that would overflow the uint8 rather than silently truncating +// them into a valid-looking but wrong member index. +func TestUnmarshalRejectsOverflowingMemberIndex(t *testing.T) { + overflowing, err := proto.Marshal(&pb.SignatureShare{ + SenderID: 256, // maxMemberIndex is 255 + Share: []byte{0x01}, + SessionID: "session-1", + }) + if err != nil { + t.Fatalf("failed to marshal test fixture: [%v]", err) + } + + if err := (&SignatureShareMessage{}).Unmarshal(overflowing); err == nil { + t.Fatal("expected an error for a member index exceeding the uint8 range") + } +}