feat: Implement latest signature spec test by GrapeBaBa · Pull Request #458 · blockblaz/zeam

GrapeBaBa · 2025-12-27T14:08:03Z

This PR is blocked by leanEthereum/leanSpec#243

Copilot

Pull request overview

This PR implements signature verification spec tests for the latest signature scheme, migrating from individual attestations to aggregated attestations with corresponding signature groups. The changes introduce SSZ-based signature parsing from JSON fixtures and add comprehensive debugging support for signature verification.

Key Changes:

Migrated from individual Attestation to AggregatedAttestation with aggregation bits
Restructured BlockSignatures to separate proposer signatures from attestation signature groups
Added Rust-based JSON-to-SSZ signature conversion utilities

Reviewed changes

Copilot reviewed 25 out of 26 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
`rust/hashsig-glue/src/lib.rs`	Added extensive debug logging, SSZ roundtrip validation, and JSON-to-SSZ signature conversion function
`rust/hashsig-glue/Cargo.toml`	Updated leansig dependency and added dev dependencies for Poseidon2 consistency testing
`pkgs/xmss/src/hashsig.zig`	Added FFI binding for JSON-to-SSZ signature conversion
`pkgs/types/src/block.zig`	Restructured attestations to use aggregation, refactored `BlockSignatures` structure, added aggregation logic
`pkgs/types/src/attestation.zig`	Added utility functions for aggregation bits manipulation and validator index extraction
`pkgs/types/src/state.zig`	Updated attestation processing to handle aggregated attestations and check for duplicates
`pkgs/state-transition/src/transition.zig`	Modified signature verification to validate per-validator signatures from aggregated groups
`pkgs/spectest/src/runner/verify_signatures_runner.zig`	New test runner for signature verification spec tests
Multiple test files	Updated to use `AggregatedAttestations` instead of `Attestations`

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-27T14:10:32Z

@@ -7,6 +7,7 @@ use std::ffi::CStr;
 use std::os::raw::c_char;
 use std::ptr;
 use std::slice;


The import serde_json::Value is only used in newly added helper functions (json_u32, json_get, etc.) but is not used in the existing code or tests. While this is acceptable since these functions are part of the new hashsig_signature_ssz_from_json feature, consider grouping JSON-related imports together or adding a comment to clarify their purpose.

Suggested change

use std::slice;

use std::slice;

// Used by JSON helper functions (e.g., json_u32, json_get) for hashsig_signature_ssz_from_json.

Copilot · 2025-12-27T14:10:32Z

+        // Debug: print first 36 bytes of signature
+        eprintln!("[hashsig_verify_ssz] pubkey_len={}, sig_len={}, epoch={}", pubkey_len, signature_len, epoch);
+        eprintln!("[hashsig_verify_ssz] sig first 36 bytes: {:02x?}", &sig_data[..36.min(sig_data.len())]);
+        eprintln!("[hashsig_verify_ssz] message: {:02x?}", message_array);


Extensive debug logging with eprintln! should be removed or placed behind a feature flag before merging to production. These debug statements (lines 395-518) will impact performance and clutter stderr in production environments.

Copilot · 2025-12-27T14:10:32Z

+        let pk_roundtrip = pk.as_ssz_bytes();
+        if pk_roundtrip != pk_data {


Duplicate SSZ roundtrip check for pubkey. The same roundtrip validation is performed at lines 427-443 and again at lines 465-489. Consider removing the duplicate check or consolidating the logic.

Copilot · 2025-12-27T14:10:32Z

+        let sig_roundtrip = sig.as_ssz_bytes();
+        if sig_roundtrip != sig_data {


Duplicate SSZ roundtrip check for signature. The same roundtrip validation is performed at lines 445-461 and again at lines 491-515. Consider removing the duplicate check or consolidating the logic.

Copilot · 2025-12-27T14:10:33Z


-        // Serialize attestations list
        var attestations_array = json.Array.init(allocator);
+        errdefer attestations_array.deinit();


Good defensive programming: the errdefer ensures cleanup on error paths. However, verify that all error paths after line 64 properly trigger this cleanup, especially in the loop at line 66-67 which appends to the array.

Copilot · 2025-12-27T14:10:33Z

+            const stored_target_root = try self.historical_block_hashes.get(target_slot);
+            const has_correct_source_root = std.mem.eql(u8, &attestation_data.source.root, &stored_source_root);
+            const has_correct_target_root = std.mem.eql(u8, &attestation_data.target.root, &stored_target_root);
+            const has_known_root = has_correct_source_root or has_correct_target_root;


The logic change from requiring both has_correct_source_root AND has_correct_target_root to requiring only has_known_root (OR condition) represents a significant behavioral change. The original logic at line 388 required !has_correct_source_root or !has_correct_target_root to skip, which meant both had to be correct to proceed. The new logic allows proceeding if either is correct. Ensure this relaxed validation aligns with the protocol specification.

Suggested change

const has_known_root = has_correct_source_root or has_correct_target_root;

const has_known_root = has_correct_source_root and has_correct_target_root;

Copilot · 2025-12-27T14:10:33Z

+    // Debug: print signature info
+    const sig = &signed_block.signature.proposer_signature;
+    std.debug.print("fixture {s}: signature first 32 bytes: {x}\n", .{ ctx.fixture_label, sig[0..32].* });
+    std.debug.print("fixture {s}: signature last 32 bytes: {x}\n", .{ ctx.fixture_label, sig[sig.len - 32 ..].* });
+
+    // Debug: print proposer attestation data and computed message hash
+    const proposer_att = signed_block.message.proposer_attestation;
+    std.debug.print("fixture {s}: proposer_attestation.validator_id: {d}\n", .{ ctx.fixture_label, proposer_att.validator_id });


Extensive debug printing (lines 206-236) should be removed or placed behind a debug flag. These print statements will clutter test output and impact test execution performance.

Copilot · 2025-12-27T14:10:33Z

+        if (att_sigs_obj.get("data")) |data_val| {
+            const arr = try expect.expectArrayValue(FixtureError, data_val, ctx, "signature.attestationSignatures.data");
+            for (arr.items) |_| {
+                // TODO: Parse actual attestation signatures if needed


The TODO comment indicates incomplete implementation. Non-empty attestation signatures are not supported, which limits test coverage. Consider tracking this limitation in an issue and linking it here.

Suggested change

// TODO: Parse actual attestation signatures if needed

// Limitation: non-empty attestation signatures are currently unsupported in spectest verification.

// Tracked in issue: https://github.com/<org>/<repo>/issues/<id>

Copilot · 2025-12-27T14:10:33Z

+                for (arr.items) |_| {
+                    // TODO: Parse actual attestations if needed
+                    std.debug.print("fixture {s} case {s}: non-empty attestations not yet supported\n", .{ ctx.fixture_label, ctx.case_name });


Another TODO indicating incomplete implementation. Block body attestations are not parsed, limiting test coverage for blocks with attestations. Consider tracking this limitation.

Suggested change

for (arr.items) |_| {

// TODO: Parse actual attestations if needed

std.debug.print("fixture {s} case {s}: non-empty attestations not yet supported\n", .{ ctx.fixture_label, ctx.case_name });

if (arr.items.len != 0) {

// Block body attestations are currently not parsed by this runner.

// Treat fixtures with non-empty attestations as unsupported to make this

// limitation explicit and avoid silently ignoring attestation data.

Copilot

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-15T04:07:12Z

+            _ => verify_with_scheme::<HashSigSchemeTest>(pk_data, sig_data, epoch, message_array)
+                .or_else(|_| verify_with_scheme::<HashSigSchemeProd>(pk_data, sig_data, epoch, message_array)),


The fallback logic attempts test scheme first, then production scheme for unknown signature lengths. This means production signatures with unknown lengths will be verified twice (failing test verification, then succeeding on production). Consider ordering the fallback to try production first since it's the more common case, or add additional length constants to avoid the double verification.

Suggested change

_ => verify_with_scheme::<HashSigSchemeTest>(pk_data, sig_data, epoch, message_array)

.or_else(|_| verify_with_scheme::<HashSigSchemeProd>(pk_data, sig_data, epoch, message_array)),

_ => verify_with_scheme::<HashSigSchemeProd>(pk_data, sig_data, epoch, message_array)

.or_else(|_| verify_with_scheme::<HashSigSchemeTest>(pk_data, sig_data, epoch, message_array)),

Copilot

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-15T08:44:10Z

+) FixtureError!std.json.Array {
+    const value = getField(obj, field_names) orelse {
+        std.debug.print(
+            "fixture {s} case {s}{}: missing field {s}\n",


The format string contains '{}' which appears to be a placeholder formatting artifact. This should likely be removed or replaced with a proper format specifier.

Copilot · 2026-01-15T08:44:11Z

+    };
+
+    // Re-serialize just the signature object and let Rust parse/SSZ-encode it.
+    var json_buf = std.ArrayList(u8).init(ctx.allocator);


The function accesses ctx.allocator but the Context type is defined elsewhere. Based on the usage pattern, this should be allocator (the function parameter) instead of ctx.allocator.

Copilot · 2026-01-15T08:44:11Z

+            _ => verify_with_scheme::<HashSigSchemeTest>(pk_data, sig_data, epoch, message_array)
+                .or_else(|_| {
+                    verify_with_scheme::<HashSigSchemeProd>(pk_data, sig_data, epoch, message_array)
+                }),


When the signature length doesn't match either constant, verification is attempted twice - first with Test scheme, then with Prod scheme. This could be optimized by checking which scheme's signature length is closer, or documenting why trying Test first is preferred.

Suggested change

_ => verify_with_scheme::<HashSigSchemeTest>(pk_data, sig_data, epoch, message_array)

.or_else(|_| {

verify_with_scheme::<HashSigSchemeProd>(pk_data, sig_data, epoch, message_array)

}),

_ => {

// When the length does not exactly match either known size, prefer the scheme

// whose expected signature length is closer to the provided length, but still

// fall back to the other scheme if the first attempt fails.

let test_diff = signature_len.abs_diff(TEST_SIGNATURE_SSZ_LEN);

let prod_diff = signature_len.abs_diff(PROD_SIGNATURE_SSZ_LEN);

if test_diff <= prod_diff {

verify_with_scheme::<HashSigSchemeTest>(

pk_data,

sig_data,

epoch,

message_array,

)

.or_else(|_| {

verify_with_scheme::<HashSigSchemeProd>(

pk_data,

sig_data,

epoch,

message_array,

)

})

} else {

verify_with_scheme::<HashSigSchemeProd>(

pk_data,

sig_data,

epoch,

message_array,

)

.or_else(|_| {

verify_with_scheme::<HashSigSchemeTest>(

pk_data,

sig_data,

epoch,

message_array,

)

})

}

}

Signed-off-by: Chen Kai <281165273grape@gmail.com>

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Signed-off-by: grapebaba <grapebaba@grapebabadeMacBook-Pro.local>

Copilot

Pull request overview

Copilot reviewed 14 out of 15 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-22T15:09:30Z

+            _ => verify_with_scheme::<HashSigSchemeTest>(pk_data, sig_data, epoch, message_array)
+                .or_else(|_| {
+                    verify_with_scheme::<HashSigSchemeProd>(pk_data, sig_data, epoch, message_array)
+                }),


For unknown signature lengths, attempting verification with test scheme first then prod scheme may be inefficient. Consider logging a warning or returning an error immediately for unrecognized signature lengths instead of double verification attempts.

Copilot · 2026-01-22T15:09:30Z

+    let sibling_size: usize = 8 * 4;
+    let hash_size: usize = 8 * 4;


Magic numbers 8 and 4 are used to calculate sizes. Consider defining these as named constants (e.g., FIELD_ELEMENT_COUNT and U32_BYTES) to improve clarity and maintainability.

Copilot · 2026-01-22T15:09:30Z

+            if (proof.proof_data.len == 0) {
+                return types.StateTransitionError.InvalidBlockSignatures;
+            }
+        } else {


The else branch contains only a comment about future work. Consider adding validation logic or at minimum a check that proof_data is non-empty in production mode to ensure consistency.

Suggested change

} else {

} else {

if (proof.proof_data.len == 0) {

return types.StateTransitionError.InvalidBlockSignatures;

}

Copilot · 2026-01-22T15:09:31Z

+fn parsePublicKeyHex(input: []const u8) !types.Bytes52 {
+    const hex_str = if (std.mem.startsWith(u8, input, "0x")) input[2..] else input;
+    if (hex_str.len != 104) {


Magic number 104 represents the expected hex string length for a 52-byte public key (52 * 2). Consider defining this as a constant or calculating it from types.Bytes52 size to maintain consistency.

Suggested change

fn parsePublicKeyHex(input: []const u8) !types.Bytes52 {

const hex_str = if (std.mem.startsWith(u8, input, "0x")) input[2..] else input;

if (hex_str.len != 104) {

const PublicKeyHexLen: usize = 2 * @sizeOf(types.Bytes52);

fn parsePublicKeyHex(input: []const u8) !types.Bytes52 {

const hex_str = if (std.mem.startsWith(u8, input, "0x")) input[2..] else input;

if (hex_str.len != PublicKeyHexLen) {

Copilot AI review requested due to automatic review settings December 27, 2025 14:08

Copilot AI reviewed Dec 27, 2025

View reviewed changes

Copilot AI review requested due to automatic review settings January 15, 2026 04:05

GrapeBaBa force-pushed the sig_spec_test branch from 40df3ca to b642b70 Compare January 15, 2026 04:05

Copilot AI reviewed Jan 15, 2026

View reviewed changes

GrapeBaBa force-pushed the sig_spec_test branch from b642b70 to 03fa466 Compare January 15, 2026 05:35

Copilot AI review requested due to automatic review settings January 15, 2026 08:42

Copilot AI reviewed Jan 15, 2026

View reviewed changes

GrapeBaBa and others added 10 commits January 21, 2026 22:15

feat: impl sig spec test

4924e83

Signed-off-by: Chen Kai <281165273grape@gmail.com>

feat: add signature spec test

0233f7d

Signed-off-by: Chen Kai <281165273grape@gmail.com>

fix: fix signature test

a27a789

Signed-off-by: Chen Kai <281165273grape@gmail.com>

fix: bootstrap spectest:run when missing generated index

5e56317

fix(spectest): support verify_signatures attestations + proofs

ff26975

chore: fix zig fmt step and rust lint

71c50b4

Apply suggestion from @Copilot

164aca4

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

fix(spectest): stop using ctx allocator in signature parsing

ffe61b6

Merge branch 'main' of github.com:blockblaz/zeam into sig_spec_test

c45ec58

refactor: align latest spec test

ebb40d1

Signed-off-by: grapebaba <grapebaba@grapebabadeMacBook-Pro.local>

Copilot AI review requested due to automatic review settings January 22, 2026 15:07

GrapeBaBa force-pushed the sig_spec_test branch from 22a8ad4 to ebb40d1 Compare January 22, 2026 15:07

Merge branch 'main' into sig_spec_test

f6abf9c

Copilot AI reviewed Jan 22, 2026

View reviewed changes

GrapeBaBa marked this pull request as ready for review January 22, 2026 15:10

GrapeBaBa closed this Jan 22, 2026

	use std::slice;
	use std::slice;
	// Used by JSON helper functions (e.g., json_u32, json_get) for hashsig_signature_ssz_from_json.

		let pk_roundtrip = pk.as_ssz_bytes();
		if pk_roundtrip != pk_data {

		let sig_roundtrip = sig.as_ssz_bytes();
		if sig_roundtrip != sig_data {

	const has_known_root = has_correct_source_root or has_correct_target_root;
	const has_known_root = has_correct_source_root and has_correct_target_root;

	// TODO: Parse actual attestation signatures if needed
	// Limitation: non-empty attestation signatures are currently unsupported in spectest verification.
	// Tracked in issue: https://github.com/<org>/<repo>/issues/<id>

-                for (arr.items) |_| {
-                    // TODO: Parse actual attestations if needed
-                    std.debug.print("fixture {s} case {s}: non-empty attestations not yet supported\n", .{ ctx.fixture_label, ctx.case_name });
+                if (arr.items.len != 0) {
+                    // Block body attestations are currently not parsed by this runner.
+                    // Treat fixtures with non-empty attestations as unsupported to make this
+                    // limitation explicit and avoid silently ignoring attestation data.

		_ => verify_with_scheme::<HashSigSchemeTest>(pk_data, sig_data, epoch, message_array)
		.or_else(\|_\| verify_with_scheme::<HashSigSchemeProd>(pk_data, sig_data, epoch, message_array)),

		let sibling_size: usize = 8 * 4;
		let hash_size: usize = 8 * 4;

Conversation

GrapeBaBa commented Dec 27, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants