test: add signature_sets unit tests and fix createSingleSignatureSetFromComponents by gorusys · Pull Request #313 · ChainSafe/lodestar-z

gorusys · 2026-04-13T18:05:57Z

No description provided.

…romComponents

gemini-code-assist · 2026-04-13T18:07:22Z

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a minor bug in the createSingleSignatureSetFromComponents function by ensuring the public key is correctly assigned. More significantly, it introduces a robust suite of unit tests for both single and aggregated signature set verification, enhancing the reliability and correctness of BLS signature handling within the state transition utilities.

Highlights

Bug Fix: Corrected the createSingleSignatureSetFromComponents function to properly dereference the public key pointer, ensuring correct assignment of the public key to the SingleSignatureSet struct.
Unit Tests for SingleSignatureSet: Added comprehensive unit tests for verifySingleSignatureSet, covering scenarios such as valid sets, mismatched signing roots, incorrect public key signers, and invalid signature byte encodings.
Unit Tests for AggregatedSignatureSet: Introduced unit tests for verifyAggregatedSignatureSet, including checks for valid single-key aggregates, mismatched signing roots, and cases where public keys do not correspond to the signers.
Function Validation Test: Included a test to verify that createSingleSignatureSetFromComponents produces a SingleSignatureSet equivalent to one created directly via a struct literal, confirming the function's correct behavior after the fix.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces a comprehensive suite of unit tests for signature set verification and updates the createSingleSignatureSetFromComponents function. Feedback focuses on adhering to the repository style guide, specifically regarding the use of assertions, out-pointers for large structs, and passing large arguments by constant pointer. Additionally, several lines in the new tests exceed the 100-column limit, and an improvement was suggested for a test case to ensure it validates individual struct fields rather than just the final verification result.

gemini-code-assist · 2026-04-13T18:08:58Z

 pub fn createSingleSignatureSetFromComponents(pubkey: *const PublicKey, signing_root: Root, signature: BLSSignature) SingleSignatureSet {
    return .{
-        .pubkey = pubkey,
+        .pubkey = pubkey.*,


The function createSingleSignatureSetFromComponents violates several rules from the Repository Style Guide:

Assertions: All function arguments must be asserted (Line 51). This function currently has no assertions. Even for simple constructors, the guide requires an average of two assertions per function to ensure data integrity.

Out-pointers: For structs larger than 16 bytes (this struct is ~224 bytes), the guide recommends initialization in-place using an out-pointer to avoid intermediate copy-move allocations and stack growth (Line 318).

Pass-by-pointer: Arguments larger than 16 bytes (like Root at 32 bytes and BLSSignature at 96 bytes) should be passed as *const to avoid unnecessary copies (Line 314).

While the function signature is not part of this diff, consider refactoring it to adhere to these safety and performance standards.

References

Assert all function arguments and return values, pre/postconditions and invariants. ^(link)

Construct larger structs in-place by passing an out pointer during initialization. ^(link)

If the argument type is more than 16 bytes, then pass the argument as *const. ^(link)

gemini-code-assist · 2026-04-13T18:08:58Z

+    const signature = sign(secret_key, &signing_root);
+    const public_key = secret_key.toPublicKey();
+    var pubkeys = [_]PublicKey{public_key};
+    const set = createAggregateSignatureSetFromComponents(pubkeys[0..], signing_root, signature.compress());


This line exceeds the 100-column limit (it is 108 columns), violating the Repository Style Guide (Line 400). Please wrap the function call.

const set = createAggregateSignatureSetFromComponents( pubkeys[0..], signing_root, signature.compress(), );

References

Hard limit all line lengths, without exception, to at most 100 columns. ^(link)

gemini-code-assist · 2026-04-13T18:08:58Z

+    const public_key = secret_key.toPublicKey();
+    signing_root[31] ^= 0xff;
+    var pubkeys = [_]PublicKey{public_key};
+    const set = createAggregateSignatureSetFromComponents(pubkeys[0..], signing_root, signature.compress());


This line exceeds the 100-column limit (108 columns), violating the Repository Style Guide (Line 400).

const set = createAggregateSignatureSetFromComponents( pubkeys[0..], signing_root, signature.compress(), );

References

Hard limit all line lengths, without exception, to at most 100 columns. ^(link)

gemini-code-assist · 2026-04-13T18:08:58Z

+    const signing_root = [_]u8{1} ** 32;
+    const signature = sign(secret_key_a, &signing_root);
+    var pubkeys = [_]PublicKey{secret_key_b.toPublicKey()};
+    const set = createAggregateSignatureSetFromComponents(pubkeys[0..], signing_root, signature.compress());


This line exceeds the 100-column limit (108 columns), violating the Repository Style Guide (Line 400).

const set = createAggregateSignatureSetFromComponents( pubkeys[0..], signing_root, signature.compress(), );

References

Hard limit all line lengths, without exception, to at most 100 columns. ^(link)

gemini-code-assist · 2026-04-13T18:08:58Z

+    try std.testing.expect(try verifySingleSignatureSet(&from_fn));
+    try std.testing.expect(try verifySingleSignatureSet(&literal));


The test 'createSingleSignatureSetFromComponents matches equivalent struct literal' should explicitly verify that the fields of the created struct match the expected values. Simply verifying the signature is a weak check for a factory function.

try std.testing.expect(from_fn.pubkey.isEqual(&literal.pubkey)); try std.testing.expectEqual(literal.signing_root, from_fn.signing_root); try std.testing.expectEqual(literal.signature, from_fn.signature); try std.testing.expect(try verifySingleSignatureSet(&from_fn));

GrapeBaBa · 2026-04-14T10:57:01Z

+    try std.testing.expect(try verifySingleSignatureSet(&set));
+}
+
+test "verifySingleSignatureSet returns false when signing root does not match signed message" {


can we make this group of similar tests using table driven approach to reduce verbose test code?

GrapeBaBa · 2026-04-14T10:58:03Z

+        .signing_root = signing_root,
+        .signature = signature.compress(),
+    };
+    try std.testing.expect(!try verifySingleSignatureSet(&set));


nit:use expectEqual false for negative check

gorusys added 2 commits April 13, 2026 19:37

test: add signature_sets unit tests and fix createSingleSignatureSetF…

1ad8e56

…romComponents

chore: rename the test ikm variables

80d3cee

gorusys requested a review from a team as a code owner April 13, 2026 18:05

gemini-code-assist Bot reviewed Apr 13, 2026

View reviewed changes

GrapeBaBa reviewed Apr 14, 2026

View reviewed changes

test: consolidate signature set verification cases

9671f39

gorusys requested a review from GrapeBaBa April 17, 2026 10:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: add signature_sets unit tests and fix createSingleSignatureSetFromComponents#313

test: add signature_sets unit tests and fix createSingleSignatureSetFromComponents#313
gorusys wants to merge 3 commits intoChainSafe:mainfrom
gorusys:test/signature_sets

gorusys commented Apr 13, 2026

Uh oh!

gemini-code-assist Bot commented Apr 13, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Uh oh!

GrapeBaBa Apr 14, 2026

Uh oh!

GrapeBaBa Apr 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		try std.testing.expect(try verifySingleSignatureSet(&from_fn));
		try std.testing.expect(try verifySingleSignatureSet(&literal));

Conversation

gorusys commented Apr 13, 2026

Uh oh!

gemini-code-assist Bot commented Apr 13, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

GrapeBaBa Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

GrapeBaBa Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants