fix(p2p): canonicalize yParity attestation signatures before L1 bundle

yarn-project/p2p/src/mem_pools/attestation_pool/attestation_pool.ts

@@ -434,7 +434,8 @@ export class AttestationPool {
         const ownKey = this.getSlotSignerKey(slotNumber, address);
         const payloadHash = attestation.getPayloadHash();
 
-        await this.attestationPerSlotAndSigner.set(ownKey, attestation.toBuffer());
+        // Store the signature in canonical (v ∈ {27, 28}) form; see tryAddCheckpointAttestation.
+        await this.attestationPerSlotAndSigner.set(ownKey, attestation.withNormalizedSignature().toBuffer());
         this.metrics.trackMempoolItemAdded(ownKey);
 
         // Track our own payload hash so that an equivocating attestation from another
@@ -614,7 +615,10 @@ export class AttestationPool {
       // equivocations are detected via the multimap but their bytes are not retained.
       const alreadyHasStored = await this.attestationPerSlotAndSigner.hasAsync(slotSignerKey);
       if (!alreadyHasStored) {
-        await this.attestationPerSlotAndSigner.set(slotSignerKey, attestation.toBuffer());
+        // Store the signature in canonical (v ∈ {27, 28}) form. `sender` recovered above, so the
+        // signature is non-empty and safe to normalize; this keeps a yParity-encoded (v = 0/1) variant
+        // from a malicious peer or an in-flight byte mutation out of the L1 bundle downstream.
+        await this.attestationPerSlotAndSigner.set(slotSignerKey, attestation.withNormalizedSignature().toBuffer());
         this.metrics.trackMempoolItemAdded(slotSignerKey);
       }
 

yarn-project/p2p/src/mem_pools/attestation_pool/attestation_pool_test_suite.ts

@@ -1,7 +1,9 @@
 import { IndexWithinCheckpoint, SlotNumber } from '@aztec/foundation/branded-types';
 import { Secp256k1Signer } from '@aztec/foundation/crypto/secp256k1-signer';
 import { Fr } from '@aztec/foundation/curves/bn254';
-import type { BlockProposal, CheckpointAttestation, CheckpointProposalCore } from '@aztec/stdlib/p2p';
+import { Signature } from '@aztec/foundation/eth-signature';
+import type { BlockProposal, CheckpointProposalCore } from '@aztec/stdlib/p2p';
+import { CheckpointAttestation } from '@aztec/stdlib/p2p';
 import { CheckpointHeader } from '@aztec/stdlib/rollup';
 import {
   makeBlockHeader,
@@ -62,7 +64,44 @@ export function describeAttestationPool(getAttestationPool: () => AttestationPoo
     expect(a1Buffer).toEqual(expect.arrayContaining(a2Buffer));
   };
 
+  /**
+   * Rewrites the attester signature into yParity form (27 -> 0, 28 -> 1), keeping (r, s). The recovery
+   * bit is unchanged so it still recovers to the same signer, mimicking a peer that gossips (or mutates
+   * in flight) a signature with a non-canonical recovery byte.
+   */
+  const toYParityForm = (attestation: CheckpointAttestation): CheckpointAttestation => {
+    const sig = attestation.signature;
+    const yParityV = sig.v === 27 ? 0 : sig.v === 28 ? 1 : sig.v;
+    return new CheckpointAttestation(
+      attestation.payload,
+      new Signature(sig.r, sig.s, yParityV),
+      attestation.proposerSignature,
+    );
+  };
+
   describe('CheckpointAttestation', () => {
+    it('normalizes yParity (v=0/v=1) attester signatures on ingress from both gossip and own keys', async () => {
+      const slotNumber = 421;
+      const canonical = createCheckpointAttestationsForSlot(slotNumber);
+      const payloadHash = canonical[0].getPayloadHash();
+      const expectedSenders = new Set([canonical[0].getSender()!.toString(), canonical[1].getSender()!.toString()]);
+
+      const gossiped = toYParityForm(canonical[0]);
+      expect([0, 1]).toContain(gossiped.signature.v);
+      await ap.tryAddCheckpointAttestation(gossiped);
+
+      const own = toYParityForm(canonical[1]);
+      expect([0, 1]).toContain(own.signature.v);
+      await ap.addOwnCheckpointAttestations([own]);
+
+      const retrieved = await ap.getCheckpointAttestationsForSlotAndProposal(SlotNumber(slotNumber), payloadHash);
+      expect(retrieved.length).toBe(2);
+      for (const attestation of retrieved) {
+        expect([27, 28]).toContain(attestation.signature.v);
+        expect(expectedSenders.has(attestation.getSender()!.toString())).toBe(true);
+      }
+    });
+
     it('should add attestations to pool', async () => {
       const slotNumber = 420;
       const archive = Fr.random();

yarn-project/stdlib/src/block/proposal/attestations_and_signers.test.ts

@@ -0,0 +1,61 @@
+import { Buffer32 } from '@aztec/foundation/buffer';
+import { EthAddress } from '@aztec/foundation/eth-address';
+import { Signature } from '@aztec/foundation/eth-signature';
+
+import { TEST_COORDINATION_SIGNATURE_CONTEXT } from '../../tests/mocks.js';
+import { CommitteeAttestationsAndSigners } from './attestations_and_signers.js';
+import { CommitteeAttestation } from './committee_attestation.js';
+
+function popcount(hex: `0x${string}`): number {
+  let count = 0;
+  for (const byte of Buffer.from(hex.slice(2), 'hex')) {
+    let b = byte;
+    while (b) {
+      count += b & 1;
+      b >>= 1;
+    }
+  }
+  return count;
+}
+
+/** A signing attestation with the given recovery byte and non-zero (r, s). */
+function signing(v: number): CommitteeAttestation {
+  return new CommitteeAttestation(EthAddress.random(), new Signature(Buffer32.random(), Buffer32.random(), v));
+}
+
+describe('CommitteeAttestationsAndSigners.packAttestations', () => {
+  it('keeps the bitmap popcount consistent with getSigners() for yParity (v=0/v=1) signatures', () => {
+    const attestations = [
+      signing(27),
+      signing(0), // yParity form of a v=27 signature
+      CommitteeAttestation.fromAddress(EthAddress.random()), // non-signing member (empty signature)
+      signing(1), // yParity form of a v=28 signature
+      signing(28),
+    ];
+    const bundle = new CommitteeAttestationsAndSigners(attestations, TEST_COORDINATION_SIGNATURE_CONTEXT);
+
+    const packed = bundle.getPackedAttestations();
+    // Four signing members, one empty slot.
+    expect(bundle.getSigners().length).toEqual(4);
+    expect(popcount(packed.signatureIndices)).toEqual(bundle.getSigners().length);
+  });
+
+  it('packs every signature slot with a canonical v so L1 ECDSA.recover accepts it at proving time', () => {
+    const attestations = [signing(0), signing(1), signing(27), signing(28)];
+    const bundle = new CommitteeAttestationsAndSigners(attestations, TEST_COORDINATION_SIGNATURE_CONTEXT);
+
+    const unpacked = CommitteeAttestation.fromPacked(bundle.getPackedAttestations(), attestations.length);
+    for (const attestation of unpacked) {
+      expect([27, 28]).toContain(attestation.signature.v);
+    }
+  });
+
+  it('still packs a genuinely empty signature as an address-only slot', () => {
+    const address = EthAddress.random();
+    const attestations = [signing(27), CommitteeAttestation.fromAddress(address)];
+    const bundle = new CommitteeAttestationsAndSigners(attestations, TEST_COORDINATION_SIGNATURE_CONTEXT);
+
+    expect(bundle.getSigners().map(s => s.toString())).not.toContain(address.toString());
+    expect(popcount(bundle.getPackedAttestations().signatureIndices)).toEqual(1);
+  });
+});

yarn-project/stdlib/src/block/proposal/attestations_and_signers.ts

@@ -76,8 +76,10 @@ export class CommitteeAttestationsAndSigners implements Signable {
     let totalDataSize = 0;
     for (const attestation of viemAttestations) {
       const signature = attestation.signature;
-      // Check if signature is empty (v = 0)
-      const isEmpty = signature.v === 0;
+      // A slot is empty (a non-signing member, packed as its address) only when r, s and v are all zero
+      // — matching Signature.isEmpty() and getSigners(), so the bitmap popcount and the signers list can
+      // never disagree (which would revert propose() with SignersSizeMismatch).
+      const isEmpty = signature.v === 0 && BigInt(signature.r) === 0n && BigInt(signature.s) === 0n;
 
       if (!isEmpty) {
         totalDataSize += 65; // v (1) + r (32) + s (32)
@@ -93,17 +95,19 @@ export class CommitteeAttestationsAndSigners implements Signable {
     for (const [i, attestation] of viemAttestations.entries()) {
       const signature = attestation.signature;
 
-      // Check if signature is empty
-      const isEmpty = signature.v === 0;
+      // Empty iff r, s and v are all zero (see the size-tally loop above).
+      const isEmpty = signature.v === 0 && BigInt(signature.r) === 0n && BigInt(signature.s) === 0n;
 
       if (!isEmpty) {
         // Set bit in bitmap (bit 7-0 in each byte, left to right)
         const byteIndex = Math.floor(i / 8);
         const bitIndex = 7 - (i % 8);
         signatureIndices[byteIndex] = (signatureIndices[byteIndex] ?? 0) | (1 << bitIndex);
 
-        // Pack signature: v + r + s
-        signaturesOrAddresses[dataIndex] = signature.v;
+        // Pack signature: v + r + s. Canonicalize a yParity recovery byte (v = 0/1) to 27/28 — it
+        // recovers to the same signer, but L1 ECDSA.recover only accepts 27/28. Any other value is left
+        // as-is so a genuinely malformed signature still fails on L1 rather than being silently rewritten.
+        signaturesOrAddresses[dataIndex] = signature.v === 0 || signature.v === 1 ? signature.v + 27 : signature.v;
         dataIndex++;
 
         // Pack r (32 bytes)

yarn-project/stdlib/src/p2p/attestation_utils.test.ts

@@ -1,12 +1,14 @@
 import { SlotNumber } from '@aztec/foundation/branded-types';
 import { Secp256k1Signer } from '@aztec/foundation/crypto/secp256k1-signer';
 import { Fr } from '@aztec/foundation/curves/bn254';
+import { Signature } from '@aztec/foundation/eth-signature';
 
 import { jest } from '@jest/globals';
 
+import { CommitteeAttestationsAndSigners } from '../block/proposal/attestations_and_signers.js';
 import { CheckpointHeader } from '../rollup/index.js';
 import { TEST_COORDINATION_SIGNATURE_CONTEXT } from '../tests/mocks.js';
-import { trimAttestations } from './attestation_utils.js';
+import { orderAttestations, trimAttestations } from './attestation_utils.js';
 import { CheckpointAttestation } from './checkpoint_attestation.js';
 import { CheckpointProposal } from './checkpoint_proposal.js';
 import { ConsensusPayload } from './consensus_payload.js';
@@ -32,6 +34,86 @@ function makeSignerAndAttestation() {
   return { signer, attestation: makeAttestation(signer), address: signer.address };
 }
 
+/**
+ * Rewrites the attester signature's recovery byte into yParity form (27 -> 0, 28 -> 1) while keeping
+ * (r, s). Since the recovery bit is unchanged, the signature still recovers to the same signer, but the
+ * non-canonical `v` is what a malicious committee member (or a peer mutating the byte in flight) can
+ * emit to make `packAttestations` and `getSigners` disagree.
+ */
+function toYParityForm(attestation: CheckpointAttestation): CheckpointAttestation {
+  const sig = attestation.signature;
+  const yParityV = sig.v === 27 ? 0 : sig.v === 28 ? 1 : sig.v;
+  return new CheckpointAttestation(
+    attestation.payload,
+    new Signature(sig.r, sig.s, yParityV),
+    attestation.proposerSignature,
+  );
+}
+
+function popcount(hex: `0x${string}`): number {
+  let count = 0;
+  for (const byte of Buffer.from(hex.slice(2), 'hex')) {
+    let b = byte;
+    while (b) {
+      count += b & 1;
+      b >>= 1;
+    }
+  }
+  return count;
+}
+
+describe('orderAttestations', () => {
+  it('normalizes yParity (v=0/v=1) attester signatures to canonical v', () => {
+    // Generate enough signers that both recovery bits (v=27 -> 0 and v=28 -> 1) are exercised.
+    const items = Array.from({ length: 8 }, () => makeSignerAndAttestation());
+    const committee = items.map(i => i.address);
+    const yParityAttestations = items.map(i => toYParityForm(i.attestation));
+
+    // Sanity check that the fixture actually produced non-canonical signatures for both recovery bits.
+    const emittedVs = new Set(yParityAttestations.map(a => a.signature.v));
+    expect([...emittedVs].every(v => v === 0 || v === 1)).toBe(true);
+
+    const ordered = orderAttestations(yParityAttestations, committee);
+
+    for (const [i, attestation] of ordered.entries()) {
+      expect([27, 28]).toContain(attestation.signature.v);
+      expect(attestation.address.toString()).toEqual(committee[i].toString());
+    }
+  });
+
+  it('produces an L1-consistent bundle (popcount === signers.length) for yParity signatures', () => {
+    const items = Array.from({ length: 8 }, () => makeSignerAndAttestation());
+    const committee = items.map(i => i.address);
+    const yParityAttestations = items.map(i => toYParityForm(i.attestation));
+
+    const ordered = orderAttestations(yParityAttestations, committee);
+    const bundle = new CommitteeAttestationsAndSigners(ordered, TEST_COORDINATION_SIGNATURE_CONTEXT);
+
+    const packed = bundle.getPackedAttestations();
+    expect(popcount(packed.signatureIndices)).toEqual(bundle.getSigners().length);
+    // Every signing slot must carry a canonical v so L1 ECDSA.recover (used at proving time) accepts it.
+    for (const attestation of ordered) {
+      if (!attestation.signature.isEmpty()) {
+        expect([27, 28]).toContain(attestation.signature.v);
+      }
+    }
+  });
+
+  it('leaves non-signing committee members as empty address-only slots', () => {
+    const items = Array.from({ length: 3 }, () => makeSignerAndAttestation());
+    const missing = Secp256k1Signer.random().address;
+    const committee = [...items.map(i => i.address), missing];
+
+    const ordered = orderAttestations(
+      items.map(i => toYParityForm(i.attestation)),
+      committee,
+    );
+
+    expect(ordered[3].address.toString()).toEqual(missing.toString());
+    expect(ordered[3].signature.isEmpty()).toBe(true);
+  });
+});
+
 describe('trimAttestations', () => {
   it('returns attestations unchanged when count <= required', () => {
     const items = Array.from({ length: 3 }, () => makeSignerAndAttestation());

yarn-project/stdlib/src/p2p/attestation_utils.ts

@@ -1,3 +1,4 @@
+import { normalizeSignature } from '@aztec/foundation/crypto/secp256k1-signer';
 import type { EthAddress } from '@aztec/foundation/eth-address';
 
 import { CommitteeAttestation } from '../block/index.js';
@@ -18,9 +19,15 @@ export function orderAttestations(
   for (const attestation of attestations) {
     const sender = attestation.getSender();
     if (sender) {
+      // Canonicalize the recovery byte to v ∈ {27, 28} before it reaches the L1 bundle. A committee
+      // member (or a peer mutating the byte in flight) can emit an equivalent signature in yParity form
+      // (v = 0/1); left as-is it makes `packAttestations` (empty iff v === 0) and `getSigners` (empty iff
+      // r,s,v all zero) disagree, reverting `propose()` with SignersSizeMismatch, and a v = 1 byte would
+      // later revert epoch proving in ECDSA.recover. `sender` recovered, so the signature is non-empty
+      // and low-s, and normalizeSignature preserves the recovered address.
       attestationMap.set(
         sender.toString(),
-        CommitteeAttestation.fromAddressAndSignature(sender, attestation.signature),
+        CommitteeAttestation.fromAddressAndSignature(sender, normalizeSignature(attestation.signature)),
       );
     }
   }

yarn-project/stdlib/src/p2p/checkpoint_attestation.ts

@@ -1,5 +1,6 @@
 import { CheckpointProposalHash, type SlotNumber } from '@aztec/foundation/branded-types';
 import { type BaseBuffer32, Buffer32 } from '@aztec/foundation/buffer';
+import { normalizeSignature } from '@aztec/foundation/crypto/secp256k1-signer';
 import type { Fr } from '@aztec/foundation/curves/bn254';
 import type { EthAddress } from '@aztec/foundation/eth-address';
 import { Signature } from '@aztec/foundation/eth-signature';
@@ -97,6 +98,17 @@ export class CheckpointAttestation extends Gossipable {
     return this.cachedProposer ?? undefined;
   }
 
+  /**
+   * Returns a copy with the attester signature canonicalized to v ∈ {27, 28}. Callers store attestations
+   * received from gossip verbatim; normalizing on ingress keeps a signature emitted in yParity form
+   * (v = 0/1) — whether by a malicious committee member or a peer mutating the byte in flight — from
+   * reaching the L1 bundle in a non-canonical form. Must only be called once the signature has recovered
+   * a sender (so it is non-empty and low-s); `normalizeSignature` throws on an all-zero signature.
+   */
+  withNormalizedSignature(): CheckpointAttestation {
+    return new CheckpointAttestation(this.payload, normalizeSignature(this.signature), this.proposerSignature);
+  }
+
   getPayload(): Buffer {
     return this.payload.getPayloadToSign();
   }