feat: high-availability validator signer #19031

spypsy · 2025-12-15T17:00:07Z

Introduces High-Availability validator signer. The signer uses a Postgres DB that will be used by multiple sequencer nodes, running with the same set of validator keys, in order to ensure that only one payload per duty is signed.

socket-security · 2025-12-15T18:29:56Z

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	npm/@types/node-pg-migrate@2.3.1
	npm/@types/pg@8.16.0
	npm/@middle-management/pglite-pg-adapter@0.0.3
	npm/pg@8.16.3
	npm/@electric-sql/pglite@0.2.17
	npm/node-pg-migrate@8.0.4
	npm/@aztec/validator-ha-signer@0.0.0-use.local

View full report

PhilWindle · 2026-01-04T18:19:20Z

yarn-project/validator-ha-signer/src/db/postgres.ts

+  /**
+   * Find an existing duty record
+   */
+  async findDuty(validatorAddress: EthAddress, slot: bigint, dutyType: DutyType): Promise<ValidatorDutyRecord | null> {


Why does return null?

PhilWindle · 2026-01-04T18:40:04Z

I feel like the database interactions are convoluted and have potential race conditions. IIUC we are manually performing quite a complicated check and set operation across multiple calls. Can this not be reduced to about 3 calls and have the DB provide the concurrency guarantees:

Insert record if not present, return record if present, We repeatedly call this if record is present and not yet signed.
Update signed record. Called upon success.
Delete failed record. Called upon failure.

alexghr · 2026-01-05T09:24:13Z

yarn-project/validator-ha-signer/src/db/postgres.test.ts

+    it('should initialize database schema successfully', async () => {
+      (mockClient.query as jest.Mock).mockResolvedValue({ rows: [], rowCount: 0 });
+
+      await db.initialize();
+
+      // Should start transaction
+      expect(mockClient.query).toHaveBeenCalledWith('BEGIN');
+
+      // Should run all schema setup statements (4 statements)
+      const calls = (mockClient.query as jest.Mock).mock.calls;
+      expect(calls.length).toBeGreaterThanOrEqual(6); // BEGIN + 4 schema statements + version + COMMIT
+
+      // Should commit transaction
+      expect(mockClient.query).toHaveBeenCalledWith('COMMIT');
+
+      // Should release client
+      expect(mockClient.release).toHaveBeenCalled();
+    });


I think this test is too brittle considering how tightly coupled it is to the migration being run.

alexghr · 2026-01-05T09:25:05Z

yarn-project/validator-ha-signer/migrations/1_initial-schema.ts

I see there's a src/db/schema.ts file too which defines tables and constraints. Are both files needed?

The SCHEMA_SETUP in schema.ts` could be removed I guess, and use what's in the migration code to initialize. Would that make sense?

alexghr · 2026-01-05T09:29:03Z

yarn-project/validator-ha-signer/src/db/postgres.test.ts

+    mockClient = mock<PoolClient>();
+
+    // Create mock pool
+    mockPool = mock<Pool>();
+    mockPool.connect.mockImplementation(() => Promise.resolve(mockClient));


Instead of using a mocked Posgres connection, we could instantiate pglite (Postgres compiled to wasm) in a temp folder. We could eliminate all mocked calls this way.

yep, was just looking into some form of postgres to use for proper testing bc this mock doesn't provide much confidence. Will check pglite 👍

alexghr · 2026-01-05T09:49:41Z

yarn-project/validator-ha-signer/src/db/schema.ts

+ */
+export const CLEANUP_OLD_DUTIES = `
+DELETE FROM validator_duties
+WHERE started_at < $1;
+`;


I don't think this query is using the idx_validator_duties_status index since it's defined on both status and started_at while the query does not specify status so it might require a full table scan.

Should this only clean 'completed' duties?

was just added to cleanup all duties if needed. Can add one for signed duties only.
Would the way to make this one more efficient be:

DELETE FROM validator_duties WHERE status IN ('signing', 'signed', 'failed') AND started_at < $1;

?

alexghr · 2026-01-05T09:58:25Z

yarn-project/validator-ha-signer/src/db/schema.ts

I like keeping the SQL queries separate but I'm slightly worried that if we change the query here and forget to update their uses in PostgresSlashingProtectionDatabase we'll only know of the issue at runtime since the unit tests use mocked clients that don't actually check the query syntax.

If we added pglite then we could run the queries in unit tests.

alexghr · 2026-01-05T11:34:03Z

yarn-project/validator-ha-signer/src/slashing_protection_service.ts

+  async recordSuccess(params: RecordSuccessParams): Promise<void> {
+    const { validatorAddress, slot, dutyType, signature, nodeId } = params;
+
+    await this.db.updateDutySigned(validatorAddress, slot, dutyType, signature.toString());
+
+    this.log.info(`Recorded successful signing for duty ${dutyType} at slot ${slot}`, {
+      validatorAddress: validatorAddress.toString(),
+      nodeId,
+    });
+  }


Is there any protection against rogue writes? E.g. some bug in the validator that causes NODE-FOO to call recordSuccess/recordFailure when the lock is actually held by NODE-BAR?

alexghr · 2026-01-05T11:38:09Z

yarn-project/validator-ha-signer/src/slashing_protection_service.test.ts

+      // Complete first node's signing after a short delay
+      await sleep(100);
+      await service.recordSuccess({


what would happen if the node never completes the signature? Does node 2 recover?

nope, as mentioned in another comment if a node fails during 'signing' we can't currently recover.

alexghr · 2026-01-05T11:41:56Z

yarn-project/validator-ha-signer/src/validator_ha_signer.ts

+   */
+  async signWithProtection(
+    validatorAddress: EthAddress,
+    signingRoot: Buffer32,


Nit: I'd rename signingRoot to just message or messageHash to match the rest of the codebase.

alexghr · 2026-01-05T11:43:36Z

yarn-project/validator-ha-signer/src/validator_ha_signer.ts

+    await this.slashingProtection.recordSuccess({
+      validatorAddress,
+      slot,
+      dutyType,
+      signature,
+      nodeId: this.config.nodeId,
+    });


This doesn't get retried so I think we'll lose signature in case of temporary network issues.

alexghr · 2026-01-05T11:44:17Z

yarn-project/validator-ha-signer/migrate.ts

This is the third(?) migration script

PhilWindle · 2026-01-05T14:04:54Z

yarn-project/validator-ha-signer/src/db/schema.ts

+/**
+ * Query to update a duty to 'signed' status
+ */
+export const UPDATE_DUTY_SIGNED = `


These UPDATE/DELETE queries should require the node to that which made the insert shouldn't they?

WHERE node_id = $5 or something.

updated to use lock_token

PhilWindle · 2026-01-06T16:21:14Z

yarn-project/validator-ha-signer/MIGRATIONS.md

+    spec:
+      initContainers:
+        - name: db-migrate
+          image: validator-image:latest


What's 'validator-image'?

sry didn't realize claude added this guide when I first created the migrations code.
Seems kinda useful, I've updated to correctly reflect how it currently works (which is with aztec migrate-ha-db command)

PhilWindle · 2026-01-06T16:22:15Z

yarn-project/validator-ha-signer/README.md

+    spec:
+      containers:
+        - name: migrate
+          image: your-validator-image


Should this be aztecprotocol/aztec?

And wouldn't it use the CLI command?

Updated REAME, it wasn't up-to-date

PhilWindle · 2026-01-06T16:35:06Z

yarn-project/validator-ha-signer/src/slashing_protection_service.ts

+   * @returns true if the update succeeded, false if token didn't match
+   */
+  async recordFailure(params: RecordFailureParams): Promise<boolean> {
+    const { validatorAddress, slot, dutyType, error, lockToken } = params;


This still feels redundant. Can't we simply delete the record?

hmm yeah I think you're right. other than logging/observability this is a better solution as it simplifies things even more.

PhilWindle · 2026-01-06T16:40:49Z

yarn-project/validator-ha-signer/src/slashing_protection_service.ts

+
+    while (true) {
+      // insert if not present, get existing if present
+      const { isNew, record } = await this.db.tryInsertOrGetExisting(params);


If node A acquires the lock and then dies, how long is node B going to keep looping? Is it infinite? At most it should be until the end of the slot, but I can't see where it would time out.

Further down it does:

} else if (record.status === DutyStatus.SIGNING) { // Another node is currently signing - check for timeout if (Date.now() - startTime > this.signingTimeoutMs) { this.log.warn(`Timeout waiting for signing to complete for duty ${dutyType} at slot ${slot}`, { validatorAddress: validatorAddress.toString(), timeoutMs: this.signingTimeoutMs, signingNodeId: record.nodeId, }); throw new DutyAlreadySignedError(slot, dutyType, 'unknown (timeout)'); } // .... keep looping }

Could switch from having a fixed timeout to having a deadline which would be the end of the slot.
Can implement that in the other PR

alexghr

Looks good but I'd like to see the HASigner check for invalid configs. The test changes would be nice to have though.

alexghr · 2026-01-07T11:12:00Z

yarn-project/validator-ha-signer/src/db/migrations/1_initial-schema.ts

+  pgm.sql(`
+    INSERT INTO schema_version (version)
+    VALUES (${SCHEMA_VERSION})
+    ON CONFLICT (version) DO NOTHING;
+  `);
+}


Doesn't pg-migrate already create a schema migration table?

alexghr · 2026-01-07T11:27:42Z

yarn-project/validator-ha-signer/src/db/postgres.test.ts

+    await db.close();
+  });
+
+  describe('schema setup', () => {


I think we can delete these tests as they're just testing PGlite functions correctly (which shouldn't be our concern)

alexghr · 2026-01-07T11:29:42Z

yarn-project/validator-ha-signer/src/db/postgres.test.ts

+    it('should handle large block numbers correctly', async () => {
+      const largeBlockNumber = 9007199254740991n;
+
+      const result = await db.query<InsertOrGetRow>(INSERT_OR_GET_DUTY, [


I think these queries should be tested through our PostgresSlashingProtectionDatabase implementation rather than going through the database. Doing it this way would also validate that the positional query parameters in the service are passed correctly

alexghr · 2026-01-07T11:32:35Z

yarn-project/validator-ha-signer/src/db/postgres.ts

+    const row = result.rows[0];
+    return {
+      isNew: row.is_new,
+      record: this.rowToRecord(row),


does this return the lock token?

It's worth adding a comment here specifying that on failing to get the lock the returned 'lock_token' from the db is going to be the empty string.

I did initially return the lock_token for non-new records. It shouldn't be an issue since this is a trusted setup but also wouldn't that defeat its purpose?

Oh I meant in the 'failed lock' case. It shouldn't return the lock token to Node B if Node A has the lock and the SQL handles this correctly.

alexghr · 2026-01-07T11:45:40Z

yarn-project/validator-ha-signer/src/db/schema.ts

+SELECT * FROM inserted
+UNION ALL
+SELECT


This looks quite complicated but I get how it's meant to work.

alexghr · 2026-01-07T12:11:50Z

yarn-project/validator-ha-signer/src/slashing_protection_service.test.ts

+      const result = await db.tryInsertOrGetExisting(params);
+      expect(result.record.status).toBe(DutyStatus.SIGNING);


This is an upsert so this doesn't actually assert that the insertion was done correctly. This function could have just inserted the row and this test wouldn't know.

NVM, I saw the tests further down in this test suite.

alexghr · 2026-01-07T12:25:20Z

yarn-project/validator-ha-signer/src/slashing_protection_service.test.ts

+      // First one should succeed, let it complete signing
+      await sleep(50);
+      const result = await db.tryInsertOrGetExisting(params1);


This makes node1 the winner. We can't assume the first promise will be the one to win. This should use a Promise.race

now I added a Promise.allSettled as per other comment I think this one becomes redundant

kept just one test where 3 nodes all fire at once with Promise.race and we check expected results

alexghr · 2026-01-07T12:28:43Z

yarn-project/validator-ha-signer/src/slashing_protection_service.ts

+          throw new SlashingProtectionError(slot, dutyType, record.messageHash, messageHash);
+        }
+        throw new DutyAlreadySignedError(slot, dutyType, record.nodeId);
+      } else if (record.status === DutyStatus.SIGNING) {


Are there any other statuses to check? "error" comes to mind

Ah error was removed! Can we add a

else { const _: never }

or throw an exception in case a new status gets added?

feels like it should error for unknown status, adding

alexghr · 2026-01-07T12:32:04Z

yarn-project/validator-ha-signer/src/validator_ha_signer.ts

+  private readonly slashingProtection: SlashingProtectionService | undefined;
+
+  constructor(
+    db: SlashingProtectionDatabase | undefined,


Shouldn't this be defined? Otherwise I'd expect the node to use a different Signer implementation.

alexghr · 2026-01-07T12:32:53Z

yarn-project/validator-ha-signer/src/validator_ha_signer.ts

+    } else {
+      this.slashingProtection = undefined;
+      this.log.info('Validator HA Signer initialized WITHOUT slashing protection');
+    }


this is dangerous: I want HA but I have no slashing protection. We should throw in this case IMHO

PhilWindle · 2026-01-07T12:38:11Z

yarn-project/validator-ha-signer/src/validator_ha_signer.ts

+ * );
+ * ```
+ */
+export class ValidatorHASigner {


I think this class shold be changed to only support fully protected signing, no fall through. Then we want a fatory to create the appropriate type at runtime.

spypsy

responses

Fixes [A-352](https://linear.app/aztec-labs/issue/A-352/add-postgresql-integration-for-recording-signed-duties) [A-353](https://linear.app/aztec-labs/issue/A-353/implement-core-slash-protection-logic) Introduces High-Availability validator signer. The signer uses a Postgres DB that will be used by multiple sequencer nodes, running with the same set of validator keys, in order to ensure that only one payload per duty is signed.

PhilWindle · 2026-01-08T09:42:05Z

yarn-project/validator-ha-signer/src/validator_ha_signer.ts

+ */
+export class ValidatorHASigner {
+  private readonly log: Logger;
+  private readonly slashingProtection: SlashingProtectionService | undefined;


This can't be undefined can it?

I've updated this in new PR FYI

PhilWindle · 2026-01-08T09:42:36Z

yarn-project/validator-ha-signer/src/validator_ha_signer.ts

+  ): Promise<Signature> {
+    // If slashing protection is disabled, just sign directly
+    if (!this.slashingProtection) {
+      this.log.info('Signing without slashing protection enabled', {


I don't think this branch is possible.

same here, removed

spypsy marked this pull request as ready for review December 16, 2025 12:48

spypsy added the ci-squash-and-merge label Dec 22, 2025

AztecBot force-pushed the spy/ha branch from 446720b to f3be0ea Compare December 22, 2025 20:21

AztecBot removed the ci-squash-and-merge label Dec 22, 2025

AztecBot enabled auto-merge December 22, 2025 20:21

AztecBot force-pushed the spy/ha branch from f3be0ea to 49b13c9 Compare December 22, 2025 20:24

PhilWindle reviewed Jan 4, 2026

View reviewed changes

alexghr reviewed Jan 5, 2026

View reviewed changes

PhilWindle reviewed Jan 5, 2026

View reviewed changes

PhilWindle reviewed Jan 6, 2026

View reviewed changes

spypsy requested a review from charlielye as a code owner January 7, 2026 09:34

alexghr reviewed Jan 7, 2026

View reviewed changes

PhilWindle requested changes Jan 7, 2026

View reviewed changes

spypsy commented Jan 7, 2026

View reviewed changes

alexghr approved these changes Jan 7, 2026

View reviewed changes

spypsy force-pushed the spy/ha branch from bbe0b60 to 0ad2b16 Compare January 7, 2026 16:51

spypsy added the ci-squash-and-merge label Jan 7, 2026

AztecBot force-pushed the spy/ha branch from 0ad2b16 to 689900d Compare January 7, 2026 17:09

AztecBot removed the ci-squash-and-merge label Jan 7, 2026

PhilWindle reviewed Jan 8, 2026

View reviewed changes

PhilWindle approved these changes Jan 8, 2026

View reviewed changes

AztecBot added this pull request to the merge queue Jan 8, 2026

Merged via the queue into next with commit 9b1bc1e Jan 8, 2026
17 checks passed

AztecBot deleted the spy/ha branch January 8, 2026 10:35

		const result = await db.tryInsertOrGetExisting(params);
		expect(result.record.status).toBe(DutyStatus.SIGNING);

feat: high-availability validator signer #19031

feat: high-availability validator signer #19031

Uh oh!

Conversation

spypsy commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

socket-security bot commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PhilWindle commented Jan 4, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexghr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

spypsy commented Dec 15, 2025 •

edited

Loading

socket-security bot commented Dec 15, 2025 •

edited

Loading