add tests

Signed-off-by: bytemare <[email protected]>

encoding.go

@@ -182,16 +182,6 @@ func (c *Configuration) Decode(data []byte) error {
 	return c.decode(header, data)
 }
 
-func (s *Signer) encodeNonceCommitments(out []byte) []byte {
-	for id, com := range s.NonceCommitments {
-		out = append(out, internal.Concatenate(internal.UInt64LE(id),
-			com.HidingNonce.Encode(),
-			com.BindingNonce.Encode(),
-			com.Commitment.Encode())...)
-	}
-	return out
-}
-
 // Encode serializes the client with its long term values, containing its secret share. This is useful for saving state
 // and backup.
 func (s *Signer) Encode() []byte {
@@ -216,6 +206,7 @@ func (s *Signer) Encode() []byte {
 	binary.LittleEndian.PutUint16(out[len(conf)+4:len(conf)+6], uint16(nLambdas))     // number of lambda entries
 
 	out = append(out, keyShare...)
+
 	for k, v := range s.LambdaRegistry {
 		b, err := hex.DecodeString(k)
 		if err != nil {
@@ -225,6 +216,7 @@ func (s *Signer) Encode() []byte {
 		out = append(out, b...)
 		out = append(out, v.Encode()...)
 	}
+
 	for id, com := range s.NonceCommitments {
 		out = append(out, internal.Concatenate(internal.UInt64LE(id),
 			com.HidingNonce.Encode(),
@@ -300,7 +292,7 @@ func (s *Signer) Decode(data []byte) error {
 	}
 
 	if err = conf.ValidateKeyShare(keyShare); err != nil {
-		return err
+		return fmt.Errorf("invalid key share: %w", err)
 	}
 
 	offset += ksLen

examples_test.go

@@ -22,7 +22,8 @@ func Example_signer() {
 	message := []byte("example message")
 	ciphersuite := frost.Ristretto255
 
-	// We assume you already have a pool of participants with distinct non-zero identifiers and their signing share.
+	// We assume you already have a pool of participants with distinct non-zero identifiers in [1:maxSingers]
+	// and their signing share.
 	// This example uses a centralised trusted dealer, but it is strongly recommended to use distributed key generation,
 	// e.g. from github.com/bytemare/dkg, which is compatible with FROST.
 	secretKeyShares, groupPublicKey, _ := debug.TrustedDealerKeygen(ciphersuite, nil, threshold, maxSigners)
@@ -38,6 +39,8 @@ func Example_signer() {
 	}
 
 	// This is how to set up the Configuration for FROST, the same for every signer and the coordinator.
+	// Note that every configuration setup for a Signer needs the public key shares of all other signers participating
+	// in a signing session (at least for the Sign() step).
 	configuration := &frost.Configuration{
 		Ciphersuite:           ciphersuite,
 		Threshold:             threshold,
@@ -50,28 +53,30 @@ func Example_signer() {
 		panic(err)
 	}
 
-	// Instantiate the participant.
+	// Instantiate the participant using its secret share.
+	// A participant (or Signer) can be backed up by serialization, and directly instantiated from that backup.
 	participant, err := configuration.Signer(participantSecretKeyShare)
 	if err != nil {
 		panic(err)
 	}
 
 	// Step 1: call Commit() on each participant. This will return the participant's single-use commitment for a
-	// a signature. Every commitment has an identifier that must be provided to Sign() to use that commitment.
-	// Send this to the coordinator or all other participants over an authenticated
+	// signature (which is independent of the future message to sign).
+	// Send this to the coordinator or all other participants (depending on your setup) over an authenticated
 	// channel (confidentiality is not required).
-	// A participant keeps an internal state during the protocol run across the two rounds.
+	// A participant (or Signer) keeps an internal state during the protocol run across the two rounds.
 	// A participant can pre-compute multiple commitments in advance: these commitments can be shared, but the
 	// participant keeps an internal state of corresponding values, so it must the same instance or a backup of it using
 	// the serialization functions.
 	com := participant.Commit()
 
-	// Step 2: collect the commitments from the other participants and coordinator-chosen the message to sign,
+	// Step 2: collect the commitments from the other participants and coordinator-chosen message to sign,
 	// and finalize by signing the message.
 	commitments := make(frost.CommitmentList, threshold)
 	commitments[0] = com
 
-	// This is not part of a participant's flow, but we need to collect the commitments of the other participants.
+	// This is not part of a participant's flow, but we need to collect the commitments of the other participants for
+	// the demo.
 	{
 		for i := uint64(1); i < threshold; i++ {
 			signer, err := configuration.Signer(secretKeyShares[i])
@@ -102,8 +107,8 @@ func Example_signer() {
 	// Output: Signing successful.
 }
 
-// Example_coordinator shows how to aggregate signature shares into the final signature, and verify a FROST signature
-// produced by multiple signers.
+// Example_coordinator shows how to aggregate signature shares produced by signers into the final signature
+// and verify a final FROST signature.
 func Example_coordinator() {
 	maxSigners := uint64(5)
 	threshold := uint64(3)
@@ -170,7 +175,7 @@ func Example_coordinator() {
 
 	// The coordinator assembles the shares. If the verify argument is set to true, AggregateSignatures will internally
 	// verify each signature share and return an error on the first that is invalid. It will also verify whether the
-	// signature is valid.
+	// output signature is valid.
 	signature, err := configuration.AggregateSignatures(message, signatureShares, commitments, true)
 	if err != nil {
 		panic(err)

tests/configuration_test.go

@@ -335,6 +335,27 @@ func TestConfiguration_VerifySignerPublicKeys_Duplicate_PublicKeys(t *testing.T)
 	}
 }
 
+func TestConfiguration_ValidatePublicKeyShare_InvalidConfiguration(t *testing.T) {
+	expectedErrorPrefix := "invalid group public key, the key is nil"
+	tt := &tableTest{
+		Ciphersuite: frost.Ristretto255,
+		threshold:   3,
+		maxSigners:  5,
+	}
+	configuration := &frost.Configuration{
+		Ciphersuite:           tt.Ciphersuite,
+		Threshold:             tt.threshold,
+		MaxSigners:            tt.maxSigners,
+		GroupPublicKey:        nil,
+		SignerPublicKeyShares: nil,
+	}
+
+	if err := configuration.ValidatePublicKeyShare(nil); err == nil ||
+		!strings.HasPrefix(err.Error(), expectedErrorPrefix) {
+		t.Fatalf("expected %q, got %q", expectedErrorPrefix, err)
+	}
+}
+
 func TestConfiguration_ValidatePublicKeyShare_Nil(t *testing.T) {
 	expectedErrorPrefix := "public key share is nil"
 	tt := &tableTest{
@@ -425,25 +446,22 @@ func TestConfiguration_ValidatePublicKeyShare_InvalidPublicKey(t *testing.T) {
 	}
 }
 
-func TestConfiguration_ValidateKeyShare_InvalidConf(t *testing.T) {
-	expectedErrorPrefix := internal.ErrInvalidCiphersuite
+func TestConfiguration_ValidateKeyShare_InvalidConfiguration(t *testing.T) {
+	expectedErrorPrefix := "invalid group public key, the key is nil"
 	tt := &tableTest{
 		Ciphersuite: frost.Ristretto255,
-		threshold:   2,
-		maxSigners:  3,
+		threshold:   3,
+		maxSigners:  5,
 	}
-	keyShares, groupPublicKey, _ := debug.TrustedDealerKeygen(tt.Ciphersuite, nil, tt.threshold, tt.maxSigners)
-	publicKeyShares := getPublicKeyShares(keyShares)
-
 	configuration := &frost.Configuration{
-		Ciphersuite:           2,
+		Ciphersuite:           tt.Ciphersuite,
 		Threshold:             tt.threshold,
 		MaxSigners:            tt.maxSigners,
-		GroupPublicKey:        groupPublicKey,
-		SignerPublicKeyShares: publicKeyShares,
+		GroupPublicKey:        nil,
+		SignerPublicKeyShares: nil,
 	}
 
-	if err := configuration.ValidateKeyShare(nil); err == nil || err.Error() != expectedErrorPrefix.Error() {
+	if err := configuration.ValidateKeyShare(nil); err == nil || err.Error() != expectedErrorPrefix {
 		t.Fatalf("expected %q, got %q", expectedErrorPrefix, err)
 	}
 }
@@ -894,6 +912,27 @@ func TestConfiguration_VerifySignatureShare_InvalidSignatureShare(t *testing.T)
 	}
 }
 
+func TestConfiguration_AggregateSignatures_InvalidConfiguration(t *testing.T) {
+	expectedErrorPrefix := "invalid group public key, the key is nil"
+	tt := &tableTest{
+		Ciphersuite: frost.Ristretto255,
+		threshold:   3,
+		maxSigners:  5,
+	}
+	configuration := &frost.Configuration{
+		Ciphersuite:           tt.Ciphersuite,
+		Threshold:             tt.threshold,
+		MaxSigners:            tt.maxSigners,
+		GroupPublicKey:        nil,
+		SignerPublicKeyShares: nil,
+	}
+
+	if _, err := configuration.AggregateSignatures(nil, nil, nil, false); err == nil ||
+		!strings.HasPrefix(err.Error(), expectedErrorPrefix) {
+		t.Fatalf("expected %q, got %q", expectedErrorPrefix, err)
+	}
+}
+
 func TestConfiguration_AggregateSignatures_InvalidCommitments(t *testing.T) {
 	expectedErrorPrefix := "invalid list of commitments: too few commitments: expected at least 3 but got 2"
 	tt := &tableTest{

tests/encoding_test.go

@@ -548,20 +548,37 @@ func TestEncoding_Signer_InvalidLambda(t *testing.T) {
 	})
 }
 
-func TestEncoding_Signer_InvalidKeyShare(t *testing.T) {
-	expectedErrorPrefix := "failed to decode key share:"
+func TestEncoding_Signer_BadKeyShare(t *testing.T) {
+	expectedErrorPrefix := "failed to decode key share: invalid group identifier"
 
 	testAll(t, func(t *testing.T, test *tableTest) {
 		s := makeSigners(t, test)[0]
 		confLen := len(s.Configuration.Encode())
 		offset := confLen + 6
 
 		// Set an invalid group in the key share encoding.
-		kse := s.KeyShare.Encode()
-		kse[0] = 2
+		encoded := s.Encode()
+		encoded = slices.Replace(encoded, offset, offset+1, []byte{2}...)
+
+		decoded := new(frost.Signer)
+		if err := decoded.Decode(encoded); err == nil || !strings.HasPrefix(err.Error(), expectedErrorPrefix) {
+			t.Fatalf("expected %q, got %q", expectedErrorPrefix, err)
+		}
+	})
+}
 
+func TestEncoding_Signer_InvalidKeyShare(t *testing.T) {
+	expectedErrorPrefix := "invalid key share: invalid identifier for public key share, the identifier is 0"
+
+	testAll(t, func(t *testing.T, test *tableTest) {
+		s := makeSigners(t, test)[0]
+		confLen := len(s.Configuration.Encode())
+		offset := confLen + 6 + 1
+
+		// Set an invalid identifier.
 		encoded := s.Encode()
-		encoded = slices.Replace(encoded, offset, offset+len(kse), kse...)
+		badID := [8]byte{}
+		encoded = slices.Replace(encoded, offset, offset+8, badID[:]...)
 
 		decoded := new(frost.Signer)
 		if err := decoded.Decode(encoded); err == nil || !strings.HasPrefix(err.Error(), expectedErrorPrefix) {
@@ -718,7 +735,7 @@ func TestEncoding_SignatureShare_InvalidLength2(t *testing.T) {
 	}
 }
 
-func TestEncoding_SignatureShare_ZeroID(t *testing.T) {
+func TestEncoding_SignatureShare_InvalidIdentifier(t *testing.T) {
 	// todo: check for zero id in all decodings
 	expectedError := errors.New("identifier cannot be 0")
 	encoded := make([]byte, 41)
@@ -909,6 +926,22 @@ func TestEncoding_Commitment_InvalidLength2(t *testing.T) {
 	})
 }
 
+func TestEncoding_Commitment_InvalidIdentifier(t *testing.T) {
+	expectedErrorPrefix := "identifier cannot be 0"
+
+	testAll(t, func(t *testing.T, test *tableTest) {
+		signer := makeSigners(t, test)[0]
+		com := signer.Commit()
+		com.SignerID = 0
+		encoded := com.Encode()
+
+		decoded := new(frost.Commitment)
+		if err := decoded.Decode(encoded); err == nil || !strings.HasPrefix(err.Error(), expectedErrorPrefix) {
+			t.Fatalf("expected %q, got %q", expectedErrorPrefix, err)
+		}
+	})
+}
+
 func TestEncoding_Commitment_InvalidHidingNonce(t *testing.T) {
 	expectedErrorPrefix := "invalid encoding of hiding nonce commitment: "
 
@@ -1092,6 +1125,15 @@ func TestEncoding_KeyShare_JSON(t *testing.T) {
 		if err := compareKeyShares(keyShare, decoded); err != nil {
 			t.Fatal(err)
 		}
+
+		// expect error
+		decoded = new(frost.KeyShare)
+		expectedError := errors.New("invalid group identifier")
+		encoded = replaceStringInBytes(encoded, fmt.Sprintf("\"group\":%d", test.ECGroup()), "\"group\":70")
+
+		if err := json.Unmarshal(encoded, decoded); err == nil || err.Error() != expectedError.Error() {
+			t.Fatalf("expected error %q, got %q", expectedError, err)
+		}
 	})
 }
 
@@ -1131,5 +1173,14 @@ func TestEncoding_PublicKeyShare_JSON(t *testing.T) {
 		if err := comparePublicKeyShare(keyShare, decoded); err != nil {
 			t.Fatal(err)
 		}
+
+		// expect error
+		decoded = new(frost.PublicKeyShare)
+		expectedError := errors.New("invalid group identifier")
+		encoded = replaceStringInBytes(encoded, fmt.Sprintf("\"group\":%d", test.ECGroup()), "\"group\":70")
+
+		if err := json.Unmarshal(encoded, decoded); err == nil || err.Error() != expectedError.Error() {
+			t.Fatalf("expected error %q, got %q", expectedError, err)
+		}
 	})
 }

tests/utils_test.go

@@ -117,6 +117,13 @@ func expectErrorPrefix(expectedErrorMessagePrefix string, f func() error) error
 	return nil
 }
 
+func replaceStringInBytes(data []byte, old, new string) []byte {
+	s := string(data)
+	s = strings.Replace(s, old, new, 1)
+
+	return []byte(s)
+}
+
 func TestConcatenate(t *testing.T) {
 	inputs := [][]byte{
 		{1, 2, 3},