misc fastUsdc vstorage cleanup (#10661)

refs: #10633

## Description
Miscellaneous small fixes, docs and refactorings. These are made in the course of #10633 but that has other changes that need more work.

Best reviewed by commit.


### Security Considerations
none


### Scaling Considerations
none

### Documentation Considerations
none

### Testing Considerations
CI

### Upgrade Considerations
not yet deployed

packages/boot/test/bootstrapTests/vaults-upgrade.test.ts

@@ -1,4 +1,3 @@
-/* eslint-disable @jessie.js/safe-await-separator */
 /**
  * @file Bootstrap test integration vaults with smart-wallet. The tests in this
  *   file are NOT independent; a single `test.before()` handler creates shared

packages/boot/test/bootstrapTests/vtransfer.test.ts

@@ -1,4 +1,3 @@
-/* eslint-disable @jessie.js/safe-await-separator -- confused by casting 'as' */
 import { test as anyTest } from '@agoric/zoe/tools/prepare-test-env-ava.js';
 
 import type { TestFn } from 'ava';

packages/boot/test/upgrading/upgrade-vats.test.ts

@@ -1,4 +1,3 @@
-/* eslint-disable @jessie.js/safe-await-separator -- test */
 import { test } from '@agoric/swingset-vat/tools/prepare-test-env-ava.js';
 
 import { BridgeId, deepCopyJsonable } from '@agoric/internal';

packages/fast-usdc/src/exos/status-manager.js

@@ -13,21 +13,29 @@ import { PendingTxStatus, TxStatus } from '../constants.js';
 /**
  * @import {MapStore, SetStore} from '@agoric/store';
  * @import {Zone} from '@agoric/zone';
- * @import {CctpTxEvidence, NobleAddress, SeenTxKey, PendingTxKey, PendingTx, EvmHash, LogFn} from '../types.js';
+ * @import {CctpTxEvidence, NobleAddress, PendingTx, EvmHash, LogFn} from '../types.js';
+ */
+
+/**
+ * @typedef {`pendingTx:${bigint}:${NobleAddress}`} PendingTxKey
+ * The string template is for developer visibility but not meant to ever be parsed.
+ *
+ * @typedef {`seenTx:${string}:${EvmHash}`} SeenTxKey
+ * The string template is for developer visibility but not meant to ever be parsed.
  */
 
 /**
  * Create the key for the pendingTxs MapStore.
  *
- * The key is a composite of `txHash` and `chainId` and not meant to be
- * parsable.
+ * The key is a composite but not meant to be parsable.
  *
  * @param {NobleAddress} addr
  * @param {bigint} amount
  * @returns {PendingTxKey}
  */
 const makePendingTxKey = (addr, amount) =>
-  `pendingTx:${JSON.stringify([addr, String(amount)])}`;
+  // amount can't contain colon
+  `pendingTx:${amount}:${addr}`;
 
 /**
  * Get the key for the pendingTxs MapStore.
@@ -43,15 +51,15 @@ const pendingTxKeyOf = evidence => {
 /**
  * Get the key for the seenTxs SetStore.
  *
- * The key is a composite of `NobleAddress` and transaction `amount` and not
- * meant to be parsable.
+ * The key is a composite but not meant to be parsable.
  *
  * @param {CctpTxEvidence} evidence
  * @returns {SeenTxKey}
  */
 const seenTxKeyOf = evidence => {
   const { txHash, chainId } = evidence;
-  return `seenTx:${JSON.stringify([txHash, chainId])}`;
+  // chainId can't contain colon
+  return `seenTx:${chainId}:${txHash}`;
 };
 
 /**
@@ -68,12 +76,12 @@ const seenTxKeyOf = evidence => {
  * XXX consider separate facets for `Advancing` and `Settling` capabilities.
  *
  * @param {Zone} zone
- * @param {() => Promise<StorageNode>} makeStatusNode
+ * @param {ERef<StorageNode>} transactionsNode
  * @param {StatusManagerPowers} caps
  */
 export const prepareStatusManager = (
   zone,
-  makeStatusNode,
+  transactionsNode,
   {
     log = makeTracer('Advancer', true),
   } = /** @type {StatusManagerPowers} */ ({}),
@@ -93,9 +101,8 @@ export const prepareStatusManager = (
    * @param {CctpTxEvidence['txHash']} hash
    * @param {TxStatus} status
    */
-  const recordStatus = (hash, status) => {
-    const statusNodeP = makeStatusNode();
-    const txnNodeP = E(statusNodeP).makeChildNode(hash);
+  const publishStatus = (hash, status) => {
+    const txnNodeP = E(transactionsNode).makeChildNode(hash);
     // Don't await, just writing to vstorage.
     void E(txnNodeP).setValue(status);
   };
@@ -109,7 +116,7 @@ export const prepareStatusManager = (
    * @param {CctpTxEvidence} evidence
    * @param {PendingTxStatus} status
    */
-  const recordPendingTx = (evidence, status) => {
+  const initPendingTx = (evidence, status) => {
     const seenKey = seenTxKeyOf(evidence);
     if (seenTxs.has(seenKey)) {
       throw makeError(`Transaction already seen: ${q(seenKey)}`);
@@ -121,9 +128,31 @@ export const prepareStatusManager = (
       pendingTxKeyOf(evidence),
       harden({ ...evidence, status }),
     );
-    recordStatus(evidence.txHash, status);
+    publishStatus(evidence.txHash, status);
   };
 
+  /**
+   * Update the pending transaction status.
+   *
+   * @param {{sender: NobleAddress, amount: bigint}} keyParts
+   * @param {PendingTxStatus} status
+   */
+  function setPendingTxStatus({ sender, amount }, status) {
+    const key = makePendingTxKey(sender, amount);
+    pendingTxs.has(key) || Fail`no advancing tx with ${{ sender, amount }}`;
+    const pending = pendingTxs.get(key);
+    const ix = pending.findIndex(tx => tx.status === PendingTxStatus.Advancing);
+    ix >= 0 || Fail`no advancing tx with ${{ sender, amount }}`;
+    const [prefix, tx, suffix] = [
+      pending.slice(0, ix),
+      pending[ix],
+      pending.slice(ix + 1),
+    ];
+    const txpost = { ...tx, status };
+    pendingTxs.set(key, harden([...prefix, txpost, ...suffix]));
+    publishStatus(tx.txHash, status);
+  }
+
   return zone.exo(
     'Fast USDC Status Manager',
     M.interface('StatusManagerI', {
@@ -156,10 +185,13 @@ export const prepareStatusManager = (
     {
       /**
        * Add a new transaction with ADVANCING status
+       *
+       * NB: this acts like observe() but skips recording the OBSERVED state
+       *
        * @param {CctpTxEvidence} evidence
        */
       advance(evidence) {
-        recordPendingTx(evidence, PendingTxStatus.Advancing);
+        initPendingTx(evidence, PendingTxStatus.Advancing);
       },
 
       /**
@@ -171,32 +203,18 @@ export const prepareStatusManager = (
        * @throws {Error} if nothing to advance
        */
       advanceOutcome(sender, amount, success) {
-        const key = makePendingTxKey(sender, amount);
-        pendingTxs.has(key) || Fail`no advancing tx with ${{ sender, amount }}`;
-        const pending = pendingTxs.get(key);
-        const ix = pending.findIndex(
-          tx => tx.status === PendingTxStatus.Advancing,
+        setPendingTxStatus(
+          { sender, amount },
+          success ? PendingTxStatus.Advanced : PendingTxStatus.AdvanceFailed,
         );
-        ix >= 0 || Fail`no advancing tx with ${{ sender, amount }}`;
-        const [prefix, tx, suffix] = [
-          pending.slice(0, ix),
-          pending[ix],
-          pending.slice(ix + 1),
-        ];
-        const status = success
-          ? PendingTxStatus.Advanced
-          : PendingTxStatus.AdvanceFailed;
-        const txpost = { ...tx, status };
-        pendingTxs.set(key, harden([...prefix, txpost, ...suffix]));
-        recordStatus(tx.txHash, status);
       },
 
       /**
        * Add a new transaction with OBSERVED status
        * @param {CctpTxEvidence} evidence
        */
       observe(evidence) {
-        recordPendingTx(evidence, PendingTxStatus.Observed);
+        initPendingTx(evidence, PendingTxStatus.Observed);
       },
 
       /**
@@ -247,7 +265,7 @@ export const prepareStatusManager = (
        * @param {EvmHash} txHash
        */
       disbursed(txHash) {
-        recordStatus(txHash, TxStatus.Disbursed);
+        publishStatus(txHash, TxStatus.Disbursed);
       },
 
       /**
@@ -259,7 +277,7 @@ export const prepareStatusManager = (
        */
       forwarded(txHash, address, amount) {
         if (txHash) {
-          recordStatus(txHash, TxStatus.Forwarded);
+          publishStatus(txHash, TxStatus.Forwarded);
         } else {
           // TODO store (early) `Minted` transactions to check against incoming evidence
           log(

packages/fast-usdc/src/fast-usdc.contract.js

@@ -95,8 +95,8 @@ export const contract = async (zcf, privateArgs, zone, tools) => {
     marshaller,
   );
 
-  const makeStatusNode = () => E(storageNode).makeChildNode(STATUS_NODE);
-  const statusManager = prepareStatusManager(zone, makeStatusNode);
+  const statusNode = E(storageNode).makeChildNode(STATUS_NODE);
+  const statusManager = prepareStatusManager(zone, statusNode);
 
   const { USDC } = terms.brands;
   const { withdrawToSeat } = tools.zoeTools;

packages/fast-usdc/src/types.ts

@@ -38,12 +38,6 @@ export interface PendingTx extends CctpTxEvidence {
   status: PendingTxStatus;
 }
 
-/** internal key for `StatusManager` exo */
-export type PendingTxKey = `pendingTx:${string}`;
-
-/** internal key for `StatusManager` exo */
-export type SeenTxKey = `seenTx:${string}`;
-
 export type FeeConfig = {
   flat: Amount<'nat'>;
   variableRate: Ratio;

packages/fast-usdc/src/util/agoric.js

@@ -5,7 +5,7 @@ export const queryFastUSDCLocalChainAccount = async (
   out = console,
 ) => {
   const agoricAddr = await vstorage.readLatest(
-    'published.fastUSDC.settlementAccount',
+    'published.fastUsdc.settlementAccount',
   );
   out.log(`Got Fast USDC Local Chain Account ${agoricAddr}`);
   return agoricAddr;

packages/fast-usdc/test/cli/transfer.test.ts

@@ -64,7 +64,7 @@ test('Transfer registers the noble forwarding account if it does not exist', asy
   const out = mockOut();
   const file = mockFile(path, JSON.stringify(config));
   const agoricSettlementAccount = 'agoric123456';
-  const settlementAccountVstoragePath = 'published.fastUSDC.settlementAccount';
+  const settlementAccountVstoragePath = 'published.fastUsdc.settlementAccount';
   const vstorageMock = makeVstorageMock({
     [settlementAccountVstoragePath]: agoricSettlementAccount,
   });
@@ -115,7 +115,7 @@ test('Transfer signs and broadcasts the depositForBurn message on Ethereum', asy
   const out = mockOut();
   const file = mockFile(path, JSON.stringify(config));
   const agoricSettlementAccount = 'agoric123456';
-  const settlementAccountVstoragePath = 'published.fastUSDC.settlementAccount';
+  const settlementAccountVstoragePath = 'published.fastUsdc.settlementAccount';
   const vstorageMock = makeVstorageMock({
     [settlementAccountVstoragePath]: agoricSettlementAccount,
   });

packages/fast-usdc/test/exos/advancer.test.ts

@@ -44,7 +44,7 @@ const createTestExtensions = (t, common: CommonSetup) => {
 
   const statusManager = prepareStatusManager(
     rootZone.subZone('status-manager'),
-    async () => storageNode.makeChildNode('status'),
+    storageNode.makeChildNode('status'),
   );
 
   const mockAccounts = prepareMockOrchAccounts(rootZone.subZone('accounts'), {
@@ -162,6 +162,7 @@ test('updates status to ADVANCING in happy path', async t => {
       mocks: { mockPoolAccount, resolveLocalTransferV },
     },
     brands: { usdc },
+    bootstrap: { storage },
   } = t.context;
 
   const mockEvidence = MockCctpTxEvidences.AGORIC_PLUS_OSMO();
@@ -174,14 +175,9 @@ test('updates status to ADVANCING in happy path', async t => {
   // wait for handleTransactionEvent to do work
   await eventLoopIteration();
 
-  const entries = statusManager.lookupPending(
-    mockEvidence.tx.forwardingAddress,
-    mockEvidence.tx.amount,
-  );
-
   t.deepEqual(
-    entries,
-    [{ ...mockEvidence, status: PendingTxStatus.Advancing }],
+    storage.data.get(`mockChainStorageRoot.status.${mockEvidence.txHash}`),
+    PendingTxStatus.Advancing,
     'ADVANCED status in happy path',
   );
 
@@ -210,6 +206,7 @@ test('updates status to ADVANCING in happy path', async t => {
 
 test('updates status to OBSERVED on insufficient pool funds', async t => {
   const {
+    bootstrap: { storage },
     extensions: {
       services: { makeAdvancer, statusManager },
       helpers: { inspectLogs },
@@ -229,14 +226,9 @@ test('updates status to OBSERVED on insufficient pool funds', async t => {
   void advancer.handleTransactionEvent(mockEvidence);
   await eventLoopIteration();
 
-  const entries = statusManager.lookupPending(
-    mockEvidence.tx.forwardingAddress,
-    mockEvidence.tx.amount,
-  );
-
   t.deepEqual(
-    entries,
-    [{ ...mockEvidence, status: PendingTxStatus.Observed }],
+    storage.data.get(`mockChainStorageRoot.status.${mockEvidence.txHash}`),
+    PendingTxStatus.Observed,
     'OBSERVED status on insufficient pool funds',
   );
 
@@ -248,6 +240,7 @@ test('updates status to OBSERVED on insufficient pool funds', async t => {
 
 test('updates status to OBSERVED if makeChainAddress fails', async t => {
   const {
+    bootstrap: { storage },
     extensions: {
       services: { advancer, statusManager },
       helpers: { inspectLogs },
@@ -257,14 +250,9 @@ test('updates status to OBSERVED if makeChainAddress fails', async t => {
   const mockEvidence = MockCctpTxEvidences.AGORIC_UNKNOWN_EUD();
   await advancer.handleTransactionEvent(mockEvidence);
 
-  const entries = statusManager.lookupPending(
-    mockEvidence.tx.forwardingAddress,
-    mockEvidence.tx.amount,
-  );
-
   t.deepEqual(
-    entries,
-    [{ ...mockEvidence, status: PendingTxStatus.Observed }],
+    storage.data.get(`mockChainStorageRoot.status.${mockEvidence.txHash}`),
+    PendingTxStatus.Observed,
     'OBSERVED status on makeChainAddress failure',
   );
 
@@ -276,6 +264,7 @@ test('updates status to OBSERVED if makeChainAddress fails', async t => {
 
 test('calls notifyAdvancingResult (AdvancedFailed) on failed transfer', async t => {
   const {
+    bootstrap: { storage },
     extensions: {
       services: { advancer, feeTools, statusManager },
       helpers: { inspectLogs, inspectNotifyCalls },
@@ -291,14 +280,9 @@ test('calls notifyAdvancingResult (AdvancedFailed) on failed transfer', async t
   resolveLocalTransferV();
   await eventLoopIteration();
 
-  const entries = statusManager.lookupPending(
-    mockEvidence.tx.forwardingAddress,
-    mockEvidence.tx.amount,
-  );
-
   t.deepEqual(
-    entries,
-    [{ ...mockEvidence, status: PendingTxStatus.Advancing }],
+    storage.data.get(`mockChainStorageRoot.status.${mockEvidence.txHash}`),
+    PendingTxStatus.Advancing,
     'tx is Advancing',
   );
 
@@ -333,6 +317,7 @@ test('calls notifyAdvancingResult (AdvancedFailed) on failed transfer', async t
 
 test('updates status to OBSERVED if pre-condition checks fail', async t => {
   const {
+    bootstrap: { storage },
     extensions: {
       services: { advancer, statusManager },
       helpers: { inspectLogs },
@@ -343,14 +328,9 @@ test('updates status to OBSERVED if pre-condition checks fail', async t => {
 
   await advancer.handleTransactionEvent(mockEvidence);
 
-  const entries = statusManager.lookupPending(
-    mockEvidence.tx.forwardingAddress,
-    mockEvidence.tx.amount,
-  );
-
   t.deepEqual(
-    entries,
-    [{ ...mockEvidence, status: PendingTxStatus.Observed }],
+    storage.data.get(`mockChainStorageRoot.status.${mockEvidence.txHash}`),
+    PendingTxStatus.Observed,
     'tx is recorded as OBSERVED',
   );