Skip to content

test: add negative BLS verify and FAV cases #304

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gorusys wants to merge 7 commits into
base: main
Choose a base branch
from
Open

test: add negative BLS verify and FAV cases #304

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
74d8ceb
test: add negative BLS verify and FAV cases
gorusys Apr 2, 2026
44407ca
chore: wrap fastAggregateVerify msg comment & assert len >= 32
gorusys Apr 2, 2026
a8550ca
test: add positive BLS utils coverage
gorusys Apr 3, 2026
94e805d
test(bls): clarify variable names and use expectEqual
gorusys Apr 8, 2026
5e1eb80
chore(bls): drop redundant slice in bls sanity test
gorusys Apr 8, 2026
594cfa7
chore(bls): align function parameter names with doc comments
gorusys Apr 8, 2026
9cafa2d
refactor: deduplicate test key material in bls.zig tests
gorusys Apr 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 54 additions & 1 deletion src/state_transition/utils/bls.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ pub fn verify(msg: []const u8, pk: *const PublicKey, sig: *const Signature, in_p
try sig.verify(sig_groupcheck, msg, DST, null, pk, pk_validate);
}

/// `msg` must be at least 32 bytes; only the first 32 are passed to fast aggregate verification.
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The doc comment should start with a capital letter to comply with the style guide (Rule 304). Additionally, the line length is very close to the 100-column limit (Rule 400); wrapping it improves readability and ensures adherence to typographic constraints.

/// The `msg` must be at least 32 bytes; only the first 32 are passed to
/// fast aggregate verification.
References
  1. Comments are sentences, with a space after the slash, with a capital letter and a full stop. (link)
  2. Hard limit all line lengths, without exception, to at most 100 columns. (link)

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While the doc comment correctly identifies the requirement for msg length, the style guide (Rule 51) requires that all function arguments be asserted. Since the implementation performs a slice msg[0..32], an explicit std.debug.assert(msg.len >= 32); should be added to the function body to prevent out-of-bounds panics and provide better documentation of the invariant. This also helps meet the requirement of a minimum of two assertions per function.

References
  1. Assert all function arguments and return values, pre/postconditions and invariants. A function must not operate blindly on data it has not checked. The assertion density of the code must average a minimum of two assertions per function. (link)

pub fn fastAggregateVerify(msg: []const u8, pks: []const PublicKey, sig: *const Signature, in_pk_validate: ?bool, in_sigs_group_check: ?bool) !bool {
var pairing_buf: [bls.Pairing.sizeOf()]u8 align(bls.Pairing.buf_align) = undefined;

Expand All @@ -30,7 +31,6 @@ pub fn fastAggregateVerify(msg: []const u8, pks: []const PublicKey, sig: *const
return sig.fastAggregateVerify(sigs_groupcheck, &pairing_buf, msg[0..32], DST, pks, pks_validate) catch return false;
}

// TODO: unit tests
Copy link
Copy Markdown
Contributor

@GrapeBaBa GrapeBaBa Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps adding more positive tests for other functions if dropped this comment

test "bls - sanity" {
const ikm: [32]u8 = [_]u8{
0x93, 0xad, 0x7e, 0x65, 0xde, 0xad, 0x05, 0x2a, 0x08, 0x3a,
Expand All @@ -49,3 +49,56 @@ test "bls - sanity" {
const result = try fastAggregateVerify(&msg, pks_slice[0..], &sig, null, null);
try std.testing.expect(result);
}

test "bls - verify fails on wrong message" {
const ikm: [32]u8 = [_]u8{
0x93, 0xad, 0x7e, 0x65, 0xde, 0xad, 0x05, 0x2a, 0x08, 0x3a,
0x91, 0x0c, 0x8b, 0x72, 0x85, 0x91, 0x46, 0x4c, 0xca, 0x56,
0x60, 0x5b, 0xb0, 0x56, 0xed, 0xfe, 0x2b, 0x60, 0xa6, 0x3c,
0x48, 0x99,
};
const sk = try SecretKey.keyGen(ikm[0..], null);
var msg = [_]u8{1} ** 32;
const sig = sign(sk, &msg);
const pk = sk.toPublicKey();
msg[0] ^= 1;
try std.testing.expectError(bls.BlstError.VerifyFail, verify(&msg, &pk, &sig, null, null));
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The style guide prohibits the use of abbreviations for variable names (except for specific cases like sort indices). Please expand ikm, sk, msg, sig, and pk to their full descriptive names. Additionally, the call to expectError should be wrapped to stay within the 100-column limit.

    const initial_keying_material: [32]u8 = [_]u8{
        0x93, 0xad, 0x7e, 0x65, 0xde, 0xad, 0x05, 0x2a, 0x08, 0x3a,
        0x91, 0x0c, 0x8b, 0x72, 0x85, 0x91, 0x46, 0x4c, 0xca, 0x56,
        0x60, 0x5b, 0xb0, 0x56, 0xed, 0xfe, 0x2b, 0x60, 0xa6, 0x3c,
        0x48, 0x99,
    };
    const secret_key = try SecretKey.keyGen(initial_keying_material[0..], null);
    var message = [_]u8{1} ** 32;
    const signature = sign(secret_key, &message);
    const public_key = secret_key.toPublicKey();
    message[0] ^= 1;
    try std.testing.expectError(
        bls.BlstError.VerifyFail,
        verify(&message, &public_key, &signature, null, null),
    );
References
  1. Do not abbreviate variable names, unless the variable is a primitive integer type used as an argument to a sort function or matrix calculation. (link)

}

test "bls - verify fails on wrong public key" {
const ikm_a: [32]u8 = [_]u8{
0x93, 0xad, 0x7e, 0x65, 0xde, 0xad, 0x05, 0x2a, 0x08, 0x3a,
0x91, 0x0c, 0x8b, 0x72, 0x85, 0x91, 0x46, 0x4c, 0xca, 0x56,
0x60, 0x5b, 0xb0, 0x56, 0xed, 0xfe, 0x2b, 0x60, 0xa6, 0x3c,
0x48, 0x99,
};
const ikm_b: [32]u8 = [_]u8{
0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xaa,
0xbb, 0xcc, 0xdd, 0xee, 0xff, 0x00, 0x01, 0x02, 0x03, 0x04,
0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e,
0x0f, 0x10,
};
const sk_a = try SecretKey.keyGen(ikm_a[0..], null);
const sk_b = try SecretKey.keyGen(ikm_b[0..], null);
const msg = [_]u8{1} ** 32;
const sig = sign(sk_a, &msg);
const pk_b = sk_b.toPublicKey();
try std.testing.expectError(bls.BlstError.VerifyFail, verify(&msg, &pk_b, &sig, null, null));
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Expand abbreviations ikm_a, ikm_b, sk_a, sk_b, msg, sig, and pk_b to adhere to the style guide's rule against abbreviations. Qualifiers like 'a' and 'b' should be placed last. Also, ensure the expectError call is properly wrapped.

    const initial_keying_material_a: [32]u8 = [_]u8{
        0x93, 0xad, 0x7e, 0x65, 0xde, 0xad, 0x05, 0x2a, 0x08, 0x3a,
        0x91, 0x0c, 0x8b, 0x72, 0x85, 0x91, 0x46, 0x4c, 0xca, 0x56,
        0x60, 0x5b, 0xb0, 0x56, 0xed, 0xfe, 0x2b, 0x60, 0xa6, 0x3c,
        0x48, 0x99,
    };
    const initial_keying_material_b: [32]u8 = [_]u8{
        0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xaa,
        0xbb, 0xcc, 0xdd, 0xee, 0xff, 0x00, 0x01, 0x02, 0x03, 0x04,
        0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e,
        0x0f, 0x10,
    };
    const secret_key_a = try SecretKey.keyGen(initial_keying_material_a[0..], null);
    const secret_key_b = try SecretKey.keyGen(initial_keying_material_b[0..], null);
    const message = [_]u8{1} ** 32;
    const signature = sign(secret_key_a, &message);
    const public_key_b = secret_key_b.toPublicKey();
    try std.testing.expectError(
        bls.BlstError.VerifyFail,
        verify(&message, &public_key_b, &signature, null, null),
    );
References
  1. Do not abbreviate variable names. Qualifiers like 'a' and 'b' should be placed last. (link)

}

test "bls - fastAggregateVerify false on wrong message" {
const ikm: [32]u8 = [_]u8{
0x93, 0xad, 0x7e, 0x65, 0xde, 0xad, 0x05, 0x2a, 0x08, 0x3a,
0x91, 0x0c, 0x8b, 0x72, 0x85, 0x91, 0x46, 0x4c, 0xca, 0x56,
0x60, 0x5b, 0xb0, 0x56, 0xed, 0xfe, 0x2b, 0x60, 0xa6, 0x3c,
0x48, 0x99,
};
const sk = try SecretKey.keyGen(ikm[0..], null);
var msg = [_]u8{1} ** 32;
const sig = sign(sk, &msg);
const pk = sk.toPublicKey();
msg[31] ^= 0xff;
var pks = [_]PublicKey{pk};
const result = try fastAggregateVerify(&msg, pks[0..], &sig, null, null);
try std.testing.expect(!result);
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Expand abbreviations ikm, sk, msg, sig, pk, and pks. Additionally, avoid negations in assertions by using expectEqual(false, ...) to state the expected outcome positively.

    const initial_keying_material: [32]u8 = [_]u8{
        0x93, 0xad, 0x7e, 0x65, 0xde, 0xad, 0x05, 0x2a, 0x08, 0x3a,
        0x91, 0x0c, 0x8b, 0x72, 0x85, 0x91, 0x46, 0x4c, 0xca, 0x56,
        0x60, 0x5b, 0xb0, 0x56, 0xed, 0xfe, 0x2b, 0x60, 0xa6, 0x3c,
        0x48, 0x99,
    };
    const secret_key = try SecretKey.keyGen(initial_keying_material[0..], null);
    var message = [_]u8{1} ** 32;
    const signature = sign(secret_key, &message);
    const public_key = secret_key.toPublicKey();
    message[31] ^= 0xff;
    var public_keys = [_]PublicKey{public_key};
    const result = try fastAggregateVerify(&message, &public_keys, &signature, null, null);
    try std.testing.expectEqual(false, result);
References
  1. Do not abbreviate variable names. State invariants positively and avoid negations. (link)

Copy link
Copy Markdown
Contributor

@GrapeBaBa GrapeBaBa Apr 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gorusys nit but this is a good suggestion IMO

}
Loading