fix(state-transition): group-check signature sets

src/state_transition/utils/signature_sets.zig

@@ -1,3 +1,4 @@
+const std = @import("std");
 const types = @import("consensus_types");
 pub const bls = @import("bls");
 const PublicKey = bls.PublicKey;
@@ -26,17 +27,19 @@ pub const AggregatedSignatureSet = struct {
 pub fn verifySingleSignatureSet(set: *const SingleSignatureSet) !bool {
     // All signatures are not trusted and must be group checked (p2.subgroup_check)
     const signature = try Signature.uncompress(&set.signature);
-    if (verify(&set.signing_root, &set.pubkey, &signature, .{})) {
-        return true;
-    } else |_| {
-        return false;
-    }
+    verify(&set.signing_root, &set.pubkey, &signature, .{ .sig_groupcheck = true }) catch return false;
+    return true;
 }
 
 pub fn verifyAggregatedSignatureSet(set: *const AggregatedSignatureSet) !bool {
     // All signatures are not trusted and must be group checked (p2.subgroup_check)
     const signature = try Signature.uncompress(&set.signature);
-    return fastAggregateVerify(&set.signing_root, set.pubkeys, &signature, .{});
+    return fastAggregateVerify(
+        &set.signing_root,
+        set.pubkeys,
+        &signature,
+        .{ .sig_groupcheck = true },
+    );
 }
 
 pub fn createSingleSignatureSetFromComponents(pubkey: *const PublicKey, signing_root: Root, signature: BLSSignature) SingleSignatureSet {
@@ -55,4 +58,55 @@ pub fn createAggregateSignatureSetFromComponents(pubkeys: []const PublicKey, sig
     };
 }
 
-// TODO: unit tests
+test "signature sets reject signatures outside G2 subgroup" {
+    const ikm = [_]u8{1} ** 32;
+    const signing_root = [_]u8{2} ** 32;
+    const secret_key = try bls.SecretKey.keyGen(&ikm, null);
+    const public_key = secret_key.toPublicKey();
+    const valid_signature = secret_key.sign(&signing_root, bls.DST, null);
+    const valid_signature_bytes = valid_signature.compress();
+
+    // Generated by adding [r]P to the valid signature, where P is the first decodable compressed
+    // curve point with an x-coordinate in the range 0...255.
+    var forged_signature_bytes: BLSSignature = undefined;
+    _ = try std.fmt.hexToBytes(
+        &forged_signature_bytes,
+        "99de7bff0cf5337f1d09fec80f5c9f5af01436b61656692b9520ca79f8211266" ++
+            "8bbedce24f69d56c171a39b0425e508017c04962b77d77d80233bd92f45ba9bb" ++
+            "93917964f576e8c5a1d2b9487c1e9b0d99021fb8ecfe5fec421d41c19674b609",
+    );
+    const forged_signature = try Signature.uncompress(&forged_signature_bytes);
+    try std.testing.expect(!forged_signature.subgroupCheck());
+
+    // The forged signature adds a torsion component, so decoding succeeds but subgroup validation
+    // must reject it.
+    const public_keys = [_]PublicKey{public_key};
+
+    const single_forged = SingleSignatureSet{
+        .pubkey = public_key,
+        .signing_root = signing_root,
+        .signature = forged_signature_bytes,
+    };
+    try std.testing.expect(!try verifySingleSignatureSet(&single_forged));
+
+    const aggregate_forged = AggregatedSignatureSet{
+        .pubkeys = &public_keys,
+        .signing_root = signing_root,
+        .signature = forged_signature_bytes,
+    };
+    try std.testing.expect(!try verifyAggregatedSignatureSet(&aggregate_forged));
+
+    const single_valid = SingleSignatureSet{
+        .pubkey = public_key,
+        .signing_root = signing_root,
+        .signature = valid_signature_bytes,
+    };
+    try std.testing.expect(try verifySingleSignatureSet(&single_valid));
+
+    const aggregate_valid = AggregatedSignatureSet{
+        .pubkeys = &public_keys,
+        .signing_root = signing_root,
+        .signature = valid_signature_bytes,
+    };
+    try std.testing.expect(try verifyAggregatedSignatureSet(&aggregate_valid));
+}