TXs signing (bypasses HA)

Author: spypsy

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

@@ -6,6 +6,7 @@ import { Fr } from '@aztec/foundation/curves/bn254';
 import type { EthAddress } from '@aztec/foundation/eth-address';
 import { Signature } from '@aztec/foundation/eth-signature';
 import { BufferReader, serializeToBuffer } from '@aztec/foundation/serialize';
+import { DutyType, type SigningContext } from '@aztec/validator-ha-signer/types';
 
 import type { L2Block } from '../block/l2_block.js';
 import type { L2BlockInfo } from '../block/l2_block_info.js';
@@ -125,7 +126,7 @@ export class BlockProposal extends Gossipable {
     archiveRoot: Fr,
     txHashes: TxHash[],
     txs: Tx[] | undefined,
-    payloadSigner: (payload: Buffer32) => Promise<Signature>,
+    payloadSigner: (payload: Buffer32, context: SigningContext) => Promise<Signature>,
   ): Promise<BlockProposal> {
     // Create a temporary proposal to get the payload to sign
     const tempProposal = new BlockProposal(
@@ -137,13 +138,23 @@ export class BlockProposal extends Gossipable {
       Signature.empty(),
     );
 
+    // Create the block signing context
+    const blockContext: SigningContext = {
+      slot: blockHeader.globalVariables.slotNumber,
+      blockNumber: blockHeader.globalVariables.blockNumber,
+      blockIndexWithinCheckpoint: indexWithinCheckpoint,
+      dutyType: DutyType.BLOCK_PROPOSAL,
+    };
+
     const hashed = getHashedSignaturePayload(tempProposal, SignatureDomainSeparator.blockProposal);
-    const sig = await payloadSigner(hashed);
+    const sig = await payloadSigner(hashed, blockContext);
 
     // If txs are provided, sign them as well
     let signedTxs: SignedTxs | undefined;
     if (txs) {
-      signedTxs = await SignedTxs.createFromSigner(txs, payloadSigner);
+      const txsSigningContext: SigningContext = { dutyType: DutyType.TXS };
+      const txsSigner = (payload: Buffer32) => payloadSigner(payload, txsSigningContext);
+      signedTxs = await SignedTxs.createFromSigner(txs, txsSigner);
     }
 
     return new BlockProposal(blockHeader, indexWithinCheckpoint, inHash, archiveRoot, txHashes, sig, signedTxs);

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

@@ -170,25 +170,14 @@ export class CheckpointProposal extends Gossipable {
       return new CheckpointProposal(checkpointHeader, archiveRoot, checkpointSignature);
     }
 
-    // Create a block-specific signer that uses BLOCK_PROPOSAL duty type
-    const blockSigner = (payload: Buffer32) => {
-      const blockContext: SigningContext = {
-        slot: lastBlockInfo.blockHeader.globalVariables.slotNumber,
-        blockNumber: lastBlockInfo.blockHeader.globalVariables.blockNumber,
-        blockIndexWithinCheckpoint: lastBlockInfo.indexWithinCheckpoint,
-        dutyType: DutyType.BLOCK_PROPOSAL,
-      };
-      return payloadSigner(payload, blockContext);
-    };
-
     const lastBlockProposal = await BlockProposal.createProposalFromSigner(
       lastBlockInfo.blockHeader,
       lastBlockInfo.indexWithinCheckpoint,
       checkpointHeader.inHash,
       archiveRoot,
       lastBlockInfo.txHashes,
       lastBlockInfo.txs,
-      blockSigner,
+      payloadSigner,
     );
 
     return new CheckpointProposal(checkpointHeader, archiveRoot, checkpointSignature, {

yarn-project/stdlib/src/tests/mocks.ts

@@ -565,7 +565,7 @@ export const makeBlockProposal = (options?: MakeBlockProposalOptions): Promise<B
     archiveRoot,
     txHashes,
     txs,
-    payload => Promise.resolve(signer.signMessage(payload)),
+    (_payload, _context) => Promise.resolve(signer.signMessage(_payload)),
   );
 };
 

yarn-project/validator-client/src/duties/validation_service.test.ts

@@ -4,6 +4,7 @@ import { Fr } from '@aztec/foundation/curves/bn254';
 import { EthAddress } from '@aztec/foundation/eth-address';
 import { makeCheckpointProposal, makeL2BlockHeader } from '@aztec/stdlib/testing';
 import { Tx } from '@aztec/stdlib/tx';
+import { DutyType } from '@aztec/validator-ha-signer/types';
 
 import { generatePrivateKey } from 'viem/accounts';
 
@@ -74,4 +75,76 @@ describe('ValidationService', () => {
     expect(attestations[0].getSender()).toEqual(addresses[0]);
     expect(attestations[1].getSender()).toEqual(addresses[1]);
   });
+
+  it('creates checkpoint proposal with different duty types for checkpoint and block', async () => {
+    // This test verifies the fix for HA double-signing issue where both checkpoint
+    // and block were incorrectly using the same CHECKPOINT_PROPOSAL duty type.
+    // Now they should use CHECKPOINT_PROPOSAL and BLOCK_PROPOSAL respectively.
+
+    const txs = await Promise.all([Tx.random(), Tx.random()]);
+    const l2BlockHeader = makeL2BlockHeader(1, 2, 3);
+    const blockHeader = l2BlockHeader.toBlockHeader();
+    const indexWithinCheckpoint = 0;
+    const archive = Fr.random();
+
+    // Create a spy keystore to capture signing contexts
+    const capturedContexts: Array<{ dutyType: DutyType; blockIndexWithinCheckpoint?: number }> = [];
+    const spyStore = {
+      ...store,
+      signMessageWithAddress: (address: EthAddress, message: Buffer32, context: any) => {
+        capturedContexts.push({
+          dutyType: context.dutyType,
+          blockIndexWithinCheckpoint: context.blockIndexWithinCheckpoint,
+        });
+        return store.signMessageWithAddress(address, message, context);
+      },
+      getAddress: (index: number) => store.getAddress(index),
+      getAddresses: () => store.getAddresses(),
+    };
+    const spyService = new ValidationService(spyStore as any);
+
+    // Create checkpoint header
+    const checkpointHeader = l2BlockHeader.toCheckpointHeader();
+
+    // Create checkpoint proposal with lastBlock
+    const proposal = await spyService.createCheckpointProposal(
+      checkpointHeader,
+      archive,
+      {
+        blockHeader,
+        indexWithinCheckpoint,
+        txs,
+      },
+      addresses[0],
+      { publishFullTxs: true },
+    );
+
+    // Verify proposal was created successfully
+    expect(proposal.getSender()).toEqual(addresses[0]);
+    expect(proposal.lastBlock).toBeDefined();
+
+    // Verify we captured signing operations:
+    // 1. CHECKPOINT_PROPOSAL for the checkpoint itself
+    // 2. BLOCK_PROPOSAL for the block itself
+    // 3. TXS for the SignedTxs
+    expect(capturedContexts.length).toBe(3);
+
+    // Find the checkpoint and block signatures
+    const checkpointSigs = capturedContexts.filter(c => c.dutyType === DutyType.CHECKPOINT_PROPOSAL);
+    const blockSigs = capturedContexts.filter(c => c.dutyType === DutyType.BLOCK_PROPOSAL);
+    const txsSigs = capturedContexts.filter(c => c.dutyType === DutyType.TXS);
+
+    // Should have exactly 1 checkpoint signature (no blockIndexWithinCheckpoint)
+    expect(checkpointSigs.length).toBe(1);
+    expect(checkpointSigs[0].blockIndexWithinCheckpoint).toBeUndefined();
+
+    // Should have exactly 2 block signatures (both with blockIndexWithinCheckpoint)
+    // One for the block proposal, one for the SignedTxs
+    expect(blockSigs.length).toBe(1);
+    expect(blockSigs[0].blockIndexWithinCheckpoint).toBe(indexWithinCheckpoint);
+
+    // Should have exactly 1 txs signature
+    expect(txsSigs.length).toBe(1);
+    expect(txsSigs[0].blockIndexWithinCheckpoint).toBeUndefined();
+  });
 });

yarn-project/validator-client/src/duties/validation_service.ts

@@ -54,22 +54,16 @@ export class ValidationService {
     proposerAttesterAddress: EthAddress | undefined,
     options: BlockProposalOptions,
   ): Promise<BlockProposal> {
-    const context: SigningContext = {
-      slot: blockHeader.globalVariables.slotNumber,
-      blockNumber: blockHeader.globalVariables.blockNumber,
-      blockIndexWithinCheckpoint,
-      dutyType: DutyType.BLOCK_PROPOSAL,
-    };
-
     // For testing: change the new archive to trigger state_mismatch validation failure
     if (options.broadcastInvalidBlockProposal) {
       archive = Fr.random();
       this.log.warn(`Creating INVALID block proposal for slot ${blockHeader.globalVariables.slotNumber}`);
     }
 
-    // Create a signer that uses the appropriate address and captures the context
+    // Create a signer that uses the appropriate address
     const address = proposerAttesterAddress ?? this.keyStore.getAddress(0);
-    const payloadSigner = (payload: Buffer32) => this.keyStore.signMessageWithAddress(address, payload, context);
+    const payloadSigner = (payload: Buffer32, context: SigningContext) =>
+      this.keyStore.signMessageWithAddress(address, payload, context);
 
     return BlockProposal.createProposalFromSigner(
       blockHeader,

yarn-project/validator-client/src/key_store/ha_key_store.test.ts

@@ -5,7 +5,7 @@ import type { Signature } from '@aztec/foundation/eth-signature';
 import type { EthRemoteSignerConfig } from '@aztec/node-keystore';
 import type { AztecAddress } from '@aztec/stdlib/aztec-address';
 import { DutyAlreadySignedError, SlashingProtectionError } from '@aztec/validator-ha-signer/errors';
-import { DutyType, type SigningContext } from '@aztec/validator-ha-signer/types';
+import { DutyType, type SigningContext, isHAProtectedContext } from '@aztec/validator-ha-signer/types';
 import type { ValidatorHASigner } from '@aztec/validator-ha-signer/validator-ha-signer';
 
 import { beforeEach, describe, expect, it, jest } from '@jest/globals';
@@ -89,9 +89,8 @@ describe('HAKeyStore', () => {
     mockHASigner.signWithProtection.mockResolvedValue(mockSignature);
   });
 
-  describe('ValidatorKeyStore interface delegation (AUTH_REQUEST bypasses HA)', () => {
+  describe('ValidatorKeyStore interface delegation (non-HA duties bypass HA)', () => {
     let haKeyStore: HAKeyStore;
-    const authRequestContext: SigningContext = { dutyType: DutyType.AUTH_REQUEST };
 
     beforeEach(() => {
       haKeyStore = new HAKeyStore(mockBaseKeyStore, mockHASigner);
@@ -109,40 +108,84 @@ describe('HAKeyStore', () => {
       expect(mockBaseKeyStore.getAddresses).toHaveBeenCalled();
     });
 
-    it('should delegate signTypedData to base key store for AUTH_REQUEST (bypasses HA)', async () => {
-      const result = await haKeyStore.signTypedData(mockTypedData, authRequestContext);
-      expect(result).toEqual([mockSignature]);
-      expect(mockBaseKeyStore.signTypedData).toHaveBeenCalledWith(mockTypedData, authRequestContext);
-      expect(mockHASigner.signWithProtection).not.toHaveBeenCalled();
-    });
+    describe('AUTH_REQUEST duties bypass HA', () => {
+      const authRequestContext: SigningContext = { dutyType: DutyType.AUTH_REQUEST };
 
-    it('should delegate signMessage to base key store for AUTH_REQUEST (bypasses HA)', async () => {
-      const result = await haKeyStore.signMessage(SIGNING_ROOT, authRequestContext);
-      expect(result).toEqual([mockSignature]);
-      expect(mockBaseKeyStore.signMessage).toHaveBeenCalledWith(SIGNING_ROOT, authRequestContext);
-      expect(mockHASigner.signWithProtection).not.toHaveBeenCalled();
-    });
+      it('should delegate signTypedData to base key store for AUTH_REQUEST', async () => {
+        const result = await haKeyStore.signTypedData(mockTypedData, authRequestContext);
+        expect(result).toEqual([mockSignature]);
+        expect(mockBaseKeyStore.signTypedData).toHaveBeenCalledWith(mockTypedData, authRequestContext);
+        expect(mockHASigner.signWithProtection).not.toHaveBeenCalled();
+      });
 
-    it('should delegate signTypedDataWithAddress without HA for AUTH_REQUEST', async () => {
-      const result = await haKeyStore.signTypedDataWithAddress(VALIDATOR_ADDRESS, mockTypedData, authRequestContext);
-      expect(result).toBe(mockSignature);
-      expect(mockBaseKeyStore.signTypedDataWithAddress).toHaveBeenCalledWith(
-        VALIDATOR_ADDRESS,
-        mockTypedData,
-        authRequestContext,
-      );
-      expect(mockHASigner.signWithProtection).not.toHaveBeenCalled();
+      it('should delegate signMessage to base key store for AUTH_REQUEST', async () => {
+        const result = await haKeyStore.signMessage(SIGNING_ROOT, authRequestContext);
+        expect(result).toEqual([mockSignature]);
+        expect(mockBaseKeyStore.signMessage).toHaveBeenCalledWith(SIGNING_ROOT, authRequestContext);
+        expect(mockHASigner.signWithProtection).not.toHaveBeenCalled();
+      });
+
+      it('should delegate signTypedDataWithAddress without HA for AUTH_REQUEST', async () => {
+        const result = await haKeyStore.signTypedDataWithAddress(VALIDATOR_ADDRESS, mockTypedData, authRequestContext);
+        expect(result).toBe(mockSignature);
+        expect(mockBaseKeyStore.signTypedDataWithAddress).toHaveBeenCalledWith(
+          VALIDATOR_ADDRESS,
+          mockTypedData,
+          authRequestContext,
+        );
+        expect(mockHASigner.signWithProtection).not.toHaveBeenCalled();
+      });
+
+      it('should delegate signMessageWithAddress without HA for AUTH_REQUEST', async () => {
+        const result = await haKeyStore.signMessageWithAddress(VALIDATOR_ADDRESS, SIGNING_ROOT, authRequestContext);
+        expect(result).toBe(mockSignature);
+        expect(mockBaseKeyStore.signMessageWithAddress).toHaveBeenCalledWith(
+          VALIDATOR_ADDRESS,
+          SIGNING_ROOT,
+          authRequestContext,
+        );
+        expect(mockHASigner.signWithProtection).not.toHaveBeenCalled();
+      });
     });
 
-    it('should delegate signMessageWithAddress without HA for AUTH_REQUEST', async () => {
-      const result = await haKeyStore.signMessageWithAddress(VALIDATOR_ADDRESS, SIGNING_ROOT, authRequestContext);
-      expect(result).toBe(mockSignature);
-      expect(mockBaseKeyStore.signMessageWithAddress).toHaveBeenCalledWith(
-        VALIDATOR_ADDRESS,
-        SIGNING_ROOT,
-        authRequestContext,
-      );
-      expect(mockHASigner.signWithProtection).not.toHaveBeenCalled();
+    describe('TXS duties bypass HA', () => {
+      const txsContext: SigningContext = { dutyType: DutyType.TXS };
+
+      it('should delegate signTypedData to base key store for TXS', async () => {
+        const result = await haKeyStore.signTypedData(mockTypedData, txsContext);
+        expect(result).toEqual([mockSignature]);
+        expect(mockBaseKeyStore.signTypedData).toHaveBeenCalledWith(mockTypedData, txsContext);
+        expect(mockHASigner.signWithProtection).not.toHaveBeenCalled();
+      });
+
+      it('should delegate signMessage to base key store for TXS', async () => {
+        const result = await haKeyStore.signMessage(SIGNING_ROOT, txsContext);
+        expect(result).toEqual([mockSignature]);
+        expect(mockBaseKeyStore.signMessage).toHaveBeenCalledWith(SIGNING_ROOT, txsContext);
+        expect(mockHASigner.signWithProtection).not.toHaveBeenCalled();
+      });
+
+      it('should delegate signTypedDataWithAddress without HA for TXS', async () => {
+        const result = await haKeyStore.signTypedDataWithAddress(VALIDATOR_ADDRESS, mockTypedData, txsContext);
+        expect(result).toBe(mockSignature);
+        expect(mockBaseKeyStore.signTypedDataWithAddress).toHaveBeenCalledWith(
+          VALIDATOR_ADDRESS,
+          mockTypedData,
+          txsContext,
+        );
+        expect(mockHASigner.signWithProtection).not.toHaveBeenCalled();
+      });
+
+      it('should delegate signMessageWithAddress without HA for TXS', async () => {
+        const result = await haKeyStore.signMessageWithAddress(VALIDATOR_ADDRESS, SIGNING_ROOT, txsContext);
+        expect(result).toBe(mockSignature);
+        expect(mockBaseKeyStore.signMessageWithAddress).toHaveBeenCalledWith(
+          VALIDATOR_ADDRESS,
+          SIGNING_ROOT,
+          txsContext,
+        );
+        expect(mockHASigner.signWithProtection).not.toHaveBeenCalled();
+      });
     });
   });
 
@@ -331,4 +374,69 @@ describe('HAKeyStore', () => {
       expect(mockHASigner.stop).toHaveBeenCalledTimes(1);
     });
   });
+
+  describe('isHAProtectedContext type guard', () => {
+    it('should return false for AUTH_REQUEST', () => {
+      const context: SigningContext = { dutyType: DutyType.AUTH_REQUEST };
+      expect(isHAProtectedContext(context)).toBe(false);
+    });
+
+    it('should return false for TXS', () => {
+      const context: SigningContext = { dutyType: DutyType.TXS };
+      expect(isHAProtectedContext(context)).toBe(false);
+    });
+
+    it('should return true for BLOCK_PROPOSAL', () => {
+      const context: SigningContext = {
+        slot: SlotNumber(100),
+        blockNumber: BlockNumber(50),
+        dutyType: DutyType.BLOCK_PROPOSAL,
+        blockIndexWithinCheckpoint: 0,
+      };
+      expect(isHAProtectedContext(context)).toBe(true);
+    });
+
+    it('should return true for ATTESTATION', () => {
+      const context: SigningContext = {
+        slot: SlotNumber(100),
+        blockNumber: BlockNumber(50),
+        dutyType: DutyType.ATTESTATION,
+      };
+      expect(isHAProtectedContext(context)).toBe(true);
+    });
+
+    it('should return true for CHECKPOINT_PROPOSAL', () => {
+      const context: SigningContext = {
+        slot: SlotNumber(100),
+        blockNumber: BlockNumber(50),
+        dutyType: DutyType.CHECKPOINT_PROPOSAL,
+      };
+      expect(isHAProtectedContext(context)).toBe(true);
+    });
+
+    it('should return true for ATTESTATIONS_AND_SIGNERS', () => {
+      const context: SigningContext = {
+        slot: SlotNumber(100),
+        blockNumber: BlockNumber(50),
+        dutyType: DutyType.ATTESTATIONS_AND_SIGNERS,
+      };
+      expect(isHAProtectedContext(context)).toBe(true);
+    });
+
+    it('should return true for GOVERNANCE_VOTE', () => {
+      const context: SigningContext = {
+        slot: SlotNumber(100),
+        dutyType: DutyType.GOVERNANCE_VOTE,
+      };
+      expect(isHAProtectedContext(context)).toBe(true);
+    });
+
+    it('should return true for SLASHING_VOTE', () => {
+      const context: SigningContext = {
+        slot: SlotNumber(100),
+        dutyType: DutyType.SLASHING_VOTE,
+      };
+      expect(isHAProtectedContext(context)).toBe(true);
+    });
+  });
 });