numofx · robertleifke · Sep 19, 2026 · Sep 18, 2026
diff --git a/services/execution/src/executor.submit.test.ts b/services/execution/src/executor.submit.test.ts
@@ -0,0 +1,146 @@
+import test from 'node:test';
+import assert from 'node:assert/strict';
+
+import { encodeAbiParameters, parseAbi } from 'viem';
+
+import type { AppConfig } from './config.js';
+import { MatchExecutor } from './executor.js';
+import { SignerNotOwnerError } from './signer-guard.js';
+import type { ExecuteMatchRequest, WithdrawRequest } from './types.js';
+import { WithdrawalRejectedError } from './withdrawal.js';
+
+const TRADE_MODULE = '0x12423B366F6F07130961900bE00d05Ea63Acd071';
+const WITHDRAWAL_MODULE = '0x0a10AE2f5D2482cE1e43bC309D430B8861C2b5aB';
+const WRAPPED_USDC = '0x364058aFF6f36E01505fB2Cc870f8B6BD4835e84';
+const MATCHING = '0x9E90A9cD13d859Bd6a08168082FB1F6F7405F191';
+const OWNER = '0x3448ac0A3283951A2AFD5B3A582329ECA43CB47B';
+const OTHER = '0x1661AA54fA390cd916722F971e4A9Fe4c01889fB';
+
+/**
+ * Port 1 is closed, so any RPC use fails immediately and unmistakably.
+ *
+ * That is the point: the guard runs before the queue and before `simulateContract`, so a refused
+ * submission must surface as SignerNotOwnerError and never as a connection error. An owner-signed
+ * one must do the opposite. These tests distinguish the two by which error arrives, which is what
+ * pins the guard to the submit boundary rather than to either handler.
+ */
+const config: AppConfig = {
+  port: 8081,
+  host: '127.0.0.1',
+  rpcUrl: 'http://127.0.0.1:1',
+  privateKey: '0x1111111111111111111111111111111111111111111111111111111111111111',
+  chainId: 8453,
+  executorAddress: '0x19E7E376E7C213B7E7e7e46cc70A5dD086DAff2A',
+  expectedActionOwner: undefined,
+  // Left unset exactly as infra/ leaves it: the settlement path's own signer check is inert here,
+  // which is why the boundary guard has to be the thing that fires.
+  expectedActionSigner: undefined,
+  dryRun: true,
+  waitForReceipt: false,
+  receiptTimeoutMs: 60_000,
+  withdrawalAssetAddresses: [],
+  withdrawalReceiptTimeoutMs: 30_000,
+};
+
+const deps = {
+  // Components are NAMED, matching @numo/abis. viem encodes a tuple by name when the ABI has
+  // them and positionally when it does not, so an unnamed ABI here makes every action encode as
+  // undefined -- which fails before the RPC and would make the "reaches the RPC" tests below pass
+  // for the wrong reason.
+  matchingAbi: parseAbi([
+    'function verifyAndMatch((uint256 subaccountId,uint256 nonce,address module,bytes data,uint256 expiry,address owner,address signer)[] actions,bytes[] signatures,bytes actionData)',
+  ]),
+  matchingAddress: MATCHING as `0x${string}`,
+  tradeModuleAddress: TRADE_MODULE as `0x${string}`,
+  withdrawal: { moduleAddress: WITHDRAWAL_MODULE as `0x${string}`, assetAddresses: [WRAPPED_USDC as `0x${string}`] },
+};
+
+const executor = () => MatchExecutor.create({ ...config }, deps);
+
+function matchRequest(signer: string = OWNER): ExecuteMatchRequest {
+  const action = (nonce: string) => ({
+    subaccount_id: '15',
+    nonce,
+    module: TRADE_MODULE,
+    data: '0x' as const,
+    expiry: '1789999999',
+    owner: OWNER,
+    signer,
+  });
+  return {
+    market: 'USDCcNGN-SPOT',
+    asset_address: WRAPPED_USDC,
+    module_address: TRADE_MODULE,
+    maker_order_id: 'maker-1',
+    taker_order_id: 'taker-1',
+    actions: [action('1'), action('2')],
+    signatures: [`0x${'ab'.repeat(65)}`, `0x${'cd'.repeat(65)}`],
+    order_data: {
+      taker_account: '19',
+      taker_fee: '0',
+      fill_details: [{ filled_account: '15', amount_filled: '1', price: '1', fee: '0' }],
+      manager_data: '0x',
+    },
+  } as ExecuteMatchRequest;
+}
+
+function withdrawRequest(signer: string = OWNER): WithdrawRequest {
+  return {
+    action: {
+      subaccount_id: '15',
+      nonce: '7328734720000000',
+      module: WITHDRAWAL_MODULE,
+      data: encodeAbiParameters([{ type: 'address' }, { type: 'uint256' }], [WRAPPED_USDC, 1_000_000n]),
+      expiry: String(Math.floor(Date.now() / 1000) + 600),
+      owner: OWNER,
+      signer,
+    },
+    signature: `0x${'ab'.repeat(65)}`,
+  } as WithdrawRequest;
+}
+
+async function errorFrom(run: () => Promise<unknown>): Promise<unknown> {
+  try {
+    await run();
+  } catch (error) {
+    return error;
+  }
+  assert.fail('expected a rejection, got none');
+}
+
+test('settlement: a non-owner signer is refused at the boundary, before any RPC', async () => {
+  const error = await errorFrom(() => executor().then((e) => e.execute(matchRequest(OTHER))));
+  assert.ok(error instanceof SignerNotOwnerError, `expected SignerNotOwnerError, got ${String(error)}`);
+  assert.equal(error.owner, OWNER);
+  assert.equal(error.signer, OTHER);
+  assert.equal(error.module, TRADE_MODULE);
+});
+
+test('settlement: an owner-signed match is unchanged — it reaches the RPC', async () => {
+  // Asserting the failure is the DEAD SOCKET, not merely "not SignerNotOwnerError". An earlier
+  // version of this test checked only the latter and passed while the request was dying in
+  // encoding, never reaching the RPC at all -- green for a reason that had nothing to do with the
+  // guard. Pinning the transport error is what makes this prove the guard let it through.
+  const error = await errorFrom(() => executor().then((e) => e.execute(matchRequest(OWNER))));
+  assert.ok(!(error instanceof SignerNotOwnerError), `guard rejected owner-signed traffic: ${String(error)}`);
+  assert.match(String((error as Error).message), /HTTP request failed/);
+});
+
+test('withdrawal: an owner-signed withdrawal is unchanged — it reaches the RPC', async () => {
+  const error = await errorFrom(() => executor().then((e) => e.withdraw(withdrawRequest(OWNER))));
+  assert.ok(!(error instanceof SignerNotOwnerError), `guard rejected owner-signed traffic: ${String(error)}`);
+  // describeSimulationRevert returns undefined for a transport failure, so the original error is
+  // rethrown rather than being reported as a withdrawal that would revert.
+  assert.ok(!(error instanceof WithdrawalRejectedError), `transport failure misreported as a revert: ${String(error)}`);
+  assert.match(String((error as Error).message), /HTTP request failed/);
+});
+
+test('withdrawal: a non-owner signer is refused, by the pre-existing policy check', async () => {
+  // Documents the overlap rather than hiding it. assertWithdrawalPolicy already rejects
+  // signer != owner and runs first, so on this path the boundary guard is a backstop that never
+  // fires today. It becomes load-bearing the moment that check is relaxed for a delegated signer,
+  // which is exactly when a forgotten path would otherwise open.
+  const error = await errorFrom(() => executor().then((e) => e.withdraw(withdrawRequest(OTHER))));
+  assert.ok(error instanceof WithdrawalRejectedError, `expected WithdrawalRejectedError, got ${String(error)}`);
+  assert.match(error.message, /signer must be action\.owner/);
+});
diff --git a/services/execution/src/executor.ts b/services/execution/src/executor.ts
@@ -14,6 +14,7 @@ import { privateKeyToAccount } from 'viem/accounts';
 import type { AppConfig } from './config.js';
 import { createKmsAccount } from '@numo/kms-signer';
 import { createSerialQueue } from './serial-queue.js';
+import { assertSignerIsOwner, type SubmittedAction } from './signer-guard.js';
 import type { ExecuteMatchRequest, ExecuteMatchResponse, WithdrawRequest } from './types.js';
 import {
   WithdrawalRejectedError,
@@ -77,23 +78,36 @@ export class MatchExecutor {
     this.walletClient = createWalletClient({ account: this.account, chain: this.chain, transport: http(config.rpcUrl) });
   }
 
-  async execute(request: ExecuteMatchRequest): Promise<ExecuteMatchResponse> {
-    assertPayloadConsistency(request, {
-      tradeModuleAddress: this.deps.tradeModuleAddress,
-      expectedActionOwner: this.config.expectedActionOwner,
-      expectedActionSigner: this.config.expectedActionSigner,
-    });
-
-    const args = buildVerifyAndMatchArgs(request);
+  /**
+   * The one place an action reaches `Matching.verifyAndMatch`.
+   *
+   * Both callers -- settlement and withdrawal -- route through here so the signer guard is applied
+   * once at the boundary rather than once per handler. A seventh module, or a third caller, inherits
+   * it by construction instead of needing to remember it.
+   *
+   * The guard runs BEFORE the queue: a refusal costs no queue slot and no simulation. Everything
+   * that differs between the two callers -- the ABI, and what a failed simulation means -- is passed
+   * in, so this method decides nothing about them.
+   */
+  private async submitVerifyAndMatch(
+    args: readonly [readonly SubmittedAction[], readonly `0x${string}`[], `0x${string}`],
+    options: { abi: Abi; onSimulationError?: (error: unknown) => never },
+  ): Promise<`0x${string}` | 'dry-run'> {
+    assertSignerIsOwner(args[0]);
 
-    const txHash = await this.enqueueSend(async () => {
-      await this.publicClient.simulateContract({
-        account: this.account,
-        address: this.deps.matchingAddress,
-        abi: this.deps.matchingAbi,
-        functionName: 'verifyAndMatch',
-        args,
-      });
+    return this.enqueueSend(async () => {
+      try {
+        await this.publicClient.simulateContract({
+          account: this.account,
+          address: this.deps.matchingAddress,
+          abi: options.abi,
+          functionName: 'verifyAndMatch',
+          args,
+        });
+      } catch (error) {
+        options.onSimulationError?.(error);
+        throw error;
+      }
 
       if (this.config.dryRun) {
         return 'dry-run' as const;
@@ -102,12 +116,24 @@ export class MatchExecutor {
       return this.walletClient.writeContract({
         account: this.account,
         address: this.deps.matchingAddress,
-        abi: this.deps.matchingAbi,
+        abi: options.abi,
         functionName: 'verifyAndMatch',
         args,
         chain: this.chain,
       });
     });
+  }
+
+  async execute(request: ExecuteMatchRequest): Promise<ExecuteMatchResponse> {
+    assertPayloadConsistency(request, {
+      tradeModuleAddress: this.deps.tradeModuleAddress,
+      expectedActionOwner: this.config.expectedActionOwner,
+      expectedActionSigner: this.config.expectedActionSigner,
+    });
+
+    const args = buildVerifyAndMatchArgs(request);
+
+    const txHash = await this.submitVerifyAndMatch(args, { abi: this.deps.matchingAbi });
 
     if (txHash === 'dry-run') {
       return { accepted: true, tx_hash: 'dry-run' };
@@ -158,35 +184,15 @@ export class MatchExecutor {
     const args = buildWithdrawArgs(request);
     const abi = [...this.deps.matchingAbi, ...withdrawalRevertErrorsAbi] as Abi;
 
-    const txHash = await this.enqueueSend(async () => {
-      try {
-        await this.publicClient.simulateContract({
-          account: this.account,
-          address: this.deps.matchingAddress,
-          abi,
-          functionName: 'verifyAndMatch',
-          args,
-        });
-      } catch (error) {
+    const txHash = await this.submitVerifyAndMatch(args, {
+      abi,
+      onSimulationError: (error) => {
         const revert = describeSimulationRevert(error);
         if (revert !== undefined) {
           throw new WithdrawalRejectedError(`withdrawal would revert: ${revert}`, revert);
         }
         throw error;
-      }
-
-      if (this.config.dryRun) {
-        return 'dry-run' as const;
-      }
-
-      return this.walletClient.writeContract({
-        account: this.account,
-        address: this.deps.matchingAddress,
-        abi,
-        functionName: 'verifyAndMatch',
-        args,
-        chain: this.chain,
-      });
+      },
     });
 
     if (txHash === 'dry-run') {

diff --git a/services/execution/src/signer-guard.test.ts b/services/execution/src/signer-guard.test.ts
@@ -0,0 +1,90 @@
+import test from 'node:test';
+import assert from 'node:assert/strict';
+
+import { SignerNotOwnerError, assertSignerIsOwner, type GuardLog, type SubmittedAction } from './signer-guard.js';
+
+// The live module set on Base, so "module-agnostic" is asserted against the real addresses rather
+// than against placeholders.
+const DEPOSIT = '0x6540f8d9Eb599b045C05E45cb6a5B1730a806658' as `0x${string}`;
+const TRANSFER = '0xEd8f114982FDBb03B70D4AC427bec7A355Cb78e4' as `0x${string}`;
+const WITHDRAWAL = '0x0a10AE2f5D2482cE1e43bC309D430B8861C2b5aB' as `0x${string}`;
+const LIQUIDATE = '0x25CF912A21e25226F1Bd99E2ADA959cC80dC4338' as `0x${string}`;
+const RFQ = '0x8399328AC53a279A3564E49c2cbC82Ce95ee62D3' as `0x${string}`;
+const TRADE = '0x12423B366F6F07130961900bE00d05Ea63Acd071' as `0x${string}`;
+
+const OWNER = '0x3448ac0A3283951A2AFD5B3A582329ECA43CB47B' as `0x${string}`;
+const OTHER = '0x1661AA54fA390cd916722F971e4A9Fe4c01889fB' as `0x${string}`;
+
+function action(over: Partial<SubmittedAction> = {}): SubmittedAction {
+  return { subaccountId: 15n, module: TRADE, owner: OWNER, signer: OWNER, ...over };
+}
+
+/** node:assert's throws() returns undefined, so the error has to be caught to be inspected. */
+function thrownBy(run: () => void): SignerNotOwnerError {
+  try {
+    run();
+  } catch (error) {
+    assert.ok(error instanceof SignerNotOwnerError, `expected SignerNotOwnerError, got ${String(error)}`);
+    return error;
+  }
+  assert.fail('expected a rejection, got none');
+}
+
+function capture(): { log: GuardLog; lines: { level: string; message: string; fields: Record<string, unknown> }[] } {
+  const lines: { level: string; message: string; fields: Record<string, unknown> }[] = [];
+  return { log: (level, message, fields) => lines.push({ level, message, fields }), lines };
+}
+
+test('owner-signed actions pass on every live module', () => {
+  for (const module of [DEPOSIT, TRANSFER, WITHDRAWAL, LIQUIDATE, RFQ, TRADE]) {
+    assertSignerIsOwner([action({ module })], capture().log);
+  }
+});
+
+test('a non-owner signer is rejected on every live module', () => {
+  // The rule is module-agnostic on purpose: a session key registered for this owner would be
+  // chain-valid through all six, so a guard that only covered trade or only covered withdrawal
+  // would leave the others open.
+  for (const module of [DEPOSIT, TRANSFER, WITHDRAWAL, LIQUIDATE, RFQ, TRADE]) {
+    assert.throws(() => assertSignerIsOwner([action({ module, signer: OTHER })], capture().log), SignerNotOwnerError);
+  }
+});
+
+test('an empty action list passes', () => {
+  assertSignerIsOwner([], capture().log);
+});
+
+test('the first offending action is the one reported', () => {
+  const { log, lines } = capture();
+  const actions = [action(), action({ module: RFQ, signer: OTHER }), action({ module: TRANSFER, signer: OTHER })];
+  const error = thrownBy(() => assertSignerIsOwner(actions, log));
+  assert.equal(error.index, 1);
+  assert.equal(error.module, RFQ);
+  assert.equal(lines.length, 1, 'one refusal, not one per offending action');
+});
+
+test('the refusal logs signer, owner and module', () => {
+  const { log, lines } = capture();
+  assert.throws(() => assertSignerIsOwner([action({ signer: OTHER })], log));
+  assert.equal(lines[0]?.level, 'error');
+  assert.equal(lines[0]?.message, 'action_signer_not_owner');
+  assert.deepEqual(lines[0]?.fields, {
+    index: 0,
+    owner: OWNER,
+    signer: OTHER,
+    module: TRADE,
+    subaccount_id: '15',
+  });
+});
+
+test('the error names both addresses, so a log line identifies the key', () => {
+  const error = thrownBy(() => assertSignerIsOwner([action({ signer: OTHER })], capture().log));
+  assert.equal(error.name, 'SignerNotOwnerError');
+  assert.match(error.message, new RegExp(OTHER));
+  assert.match(error.message, new RegExp(OWNER));
+});
+
+test('addresses are compared checksum-insensitively', () => {
+  // Both builders run getAddress, but a differing case must never read as a different signer.
+  assertSignerIsOwner([action({ owner: OWNER.toLowerCase() as `0x${string}` })], capture().log);
+});