feat: error handling for bad event packets

Agoric · Jan 29, 2025 · 450b953 · 450b953
1 parent 947cca2
commit 450b953
Show file tree

Hide file tree

Showing 2 changed files with 146 additions and 31 deletions.
diff --git a/packages/fast-usdc/src/exos/settler.js b/packages/fast-usdc/src/exos/settler.js
@@ -19,14 +19,61 @@ import {
  * @import {Amount, Brand, NatValue, Payment} from '@agoric/ertp';
  * @import {Denom, OrchestrationAccount, ChainHub, ChainAddress} from '@agoric/orchestration';
  * @import {WithdrawToSeat} from '@agoric/orchestration/src/utils/zoe-tools'
- * @import {IBCChannelID, VTransferIBCEvent} from '@agoric/vats';
+ * @import {IBCChannelID, IBCPacket, VTransferIBCEvent} from '@agoric/vats';
  * @import {Zone} from '@agoric/zone';
  * @import {HostOf, HostInterface} from '@agoric/async-flow';
  * @import {TargetRegistration} from '@agoric/vats/src/bridge-target.js';
  * @import {NobleAddress, LiquidityPoolKit, FeeConfig, EvmHash, LogFn, CctpTxEvidence} from '../types.js';
  * @import {StatusManager} from './status-manager.js';
  */
 
+/**
+ * @param {IBCPacket} data
+ * @param {string} remoteDenom
+ * @returns {{ nfa: NobleAddress, amount: bigint, EUD: string } | {error: object[]}}
+ */
+const decodeEventPacket = ({ data }, remoteDenom) => {
+  // NB: may not be a FungibleTokenPacketData or even JSON
+  /** @type {FungibleTokenPacketData} */
+  let tx;
+  try {
+    tx = JSON.parse(atob(data));
+  } catch (e) {
+    return { error: ['could not parse packet data', data] };
+  }
+
+  // given the sourceChannel check, we can be certain of this cast
+  const nfa = /** @type {NobleAddress} */ (tx.sender);
+
+  if (tx.denom !== remoteDenom) {
+    const { denom: actual } = tx;
+    return { error: ['unexpected denom', { actual, expected: remoteDenom }] };
+  }
+
+  let EUD;
+  try {
+    ({ EUD } = decodeAddressHook(tx.receiver).query);
+    if (!EUD) {
+      return { error: ['no EUD parameter', tx.receiver] };
+    }
+    if (typeof EUD !== 'string') {
+      return { error: ['EUD is not a string', EUD] };
+    }
+  } catch (e) {
+    return { error: ['no query params', tx.receiver] };
+  }
+
+  let amount;
+  try {
+    amount = BigInt(tx.amount);
+  } catch (e) {
+    return { error: ['invalid amount', tx.amount] };
+  }
+
+  return { nfa, amount, EUD };
+};
+harden(decodeEventPacket);
+
 /**
  * NOTE: not meant to be parsable.
  *
@@ -158,38 +205,13 @@ export const prepareSettler = (
             return;
           }
 
-          // TODO: why is it safe to cast this without a runtime check?
-          const tx = /** @type {FungibleTokenPacketData} */ (
-            JSON.parse(atob(packet.data))
-          );
-
-          // given the sourceChannel check, we can be certain of this cast
-          const nfa = /** @type {NobleAddress} */ (tx.sender);
-
-          if (tx.denom !== remoteDenom) {
-            const { denom: actual } = tx;
-            log('unexpected denom', { actual, expected: remoteDenom });
-            return;
-          }
-
-          let EUD;
-          try {
-            ({ EUD } = decodeAddressHook(tx.receiver).query);
-            if (!EUD) {
-              log('no EUD parameter', tx.receiver);
-              return;
-            }
-            if (typeof EUD !== 'string') {
-              log('EUD is not a string', EUD);
-              return;
-            }
-          } catch (e) {
-            log('no query params', tx.receiver);
+          const decoded = decodeEventPacket(event.packet, remoteDenom);
+          if ('error' in decoded) {
+            log('invalid event packet', decoded.error);
             return;
           }
 
-          const amount = BigInt(tx.amount); // TODO: what if this throws?
-
+          const { nfa, amount, EUD } = decoded;
           const { self } = this.facets;
           const found = statusManager.dequeueStatus(nfa, amount);
           log('dequeued', found, 'for', nfa, amount);

diff --git a/packages/fast-usdc/test/exos/settler.test.ts b/packages/fast-usdc/test/exos/settler.test.ts
@@ -1,10 +1,14 @@
 import { test as anyTest } from '@agoric/zoe/tools/prepare-test-env-ava.js';
 import type { TestFn } from 'ava';
 
-import { decodeAddressHook } from '@agoric/cosmic-proto/address-hooks.js';
+import {
+  decodeAddressHook,
+  encodeAddressHook,
+} from '@agoric/cosmic-proto/address-hooks.js';
 import { defaultMarshaller } from '@agoric/internal/src/storage-test-utils.js';
 import { eventLoopIteration } from '@agoric/internal/src/testing-utils.js';
 import fetchedChainInfo from '@agoric/orchestration/src/fetched-chain-info.js';
+import { buildVTransferEvent } from '@agoric/orchestration/tools/ibc-mocks.js';
 import type { Zone } from '@agoric/zone';
 import { PendingTxStatus, TxStatus } from '../../src/constants.js';
 import {
@@ -737,3 +741,92 @@ test('slow path, and forward fails (terminal state)', async t => {
 test.todo('creator facet methods');
 
 test.todo('ignored packets');
+
+test('bad packet data', async t => {
+  const { makeSettler, defaultSettlerParams, repayer, accounts, inspectLogs } =
+    t.context;
+  const settler = makeSettler({
+    repayer,
+    settlementAccount: accounts.settlement.account,
+    ...defaultSettlerParams,
+  });
+
+  await settler.tap.receiveUpcall(
+    buildVTransferEvent({ sourceChannel: 'channel-21' }),
+  );
+  t.deepEqual(inspectLogs().at(-1), [
+    'invalid event packet',
+    ['unexpected denom', { actual: 'uatom', expected: 'uusdc' }],
+  ]);
+
+  await settler.tap.receiveUpcall(
+    buildVTransferEvent({ sourceChannel: 'channel-21', denom: 'uusdc' }),
+  );
+  t.deepEqual(inspectLogs().at(-1), [
+    'invalid event packet',
+    ['no query params', 'agoric1fakeLCAAddress'],
+  ]);
+
+  await settler.tap.receiveUpcall(
+    buildVTransferEvent({
+      sourceChannel: 'channel-21',
+      denom: 'uusdc',
+      receiver: encodeAddressHook(
+        // valid but meaningless address
+        'agoric1c9gyu460lu70rtcdp95vummd6032psmpdx7wdy',
+        {},
+      ),
+    }),
+  );
+  t.deepEqual(inspectLogs().at(-1), [
+    'invalid event packet',
+    [
+      'no EUD parameter',
+      'agoric10rchps2sfet5lleu7xhs6ztgeehkm5lz5rpkz0cqzs95zdge',
+    ],
+  ]);
+
+  await settler.tap.receiveUpcall(
+    buildVTransferEvent({
+      sourceChannel: 'channel-21',
+      denom: 'uusdc',
+      receiver: encodeAddressHook(
+        // valid but meaningless address
+        'agoric1c9gyu460lu70rtcdp95vummd6032psmpdx7wdy',
+        { EUD: 'cosmos1foo' },
+      ),
+      // @ts-expect-error intentionally bad
+      amount: 'bad',
+    }),
+  );
+  t.deepEqual(inspectLogs().at(-1), [
+    'invalid event packet',
+    ['invalid amount', 'bad'],
+  ]);
+
+  const goodEvent = buildVTransferEvent({
+    sourceChannel: 'channel-21',
+    denom: 'uusdc',
+    receiver: encodeAddressHook(
+      // valid but meaningless address
+      'agoric1c9gyu460lu70rtcdp95vummd6032psmpdx7wdy',
+      { EUD: 'cosmos1foo' },
+    ),
+  });
+  await settler.tap.receiveUpcall(goodEvent);
+  t.deepEqual(inspectLogs().at(-1), [
+    '⚠️ tap: minted before observed',
+    'cosmos1AccAddress',
+    10n,
+  ]);
+
+  const badJson = {
+    ...goodEvent,
+    packet: { ...goodEvent.packet, data: 'not json' },
+  };
+  await settler.tap.receiveUpcall(badJson);
+  t.deepEqual(inspectLogs().at(-1), [
+    'invalid event packet',
+    ['could not parse packet data', 'not json'],
+  ]);
+});