diff --git a/packages/fast-usdc/src/constants.js b/packages/fast-usdc/src/constants.js index e75e06172917..d4c939fa285a 100644 --- a/packages/fast-usdc/src/constants.js +++ b/packages/fast-usdc/src/constants.js @@ -2,8 +2,8 @@ * Status values for the StatusManager. Listed in order: * * - 'OBSERVED': TX is observed via EventFeed - * - 'ADVANCED': advance funds are available and IBC transfer is initiated (but - * not necessarily settled) + * - 'SKIPPED': advance was skipped due to failed checks + * - 'ADVANCED': IBC transfer is initiated (but not necessarily settled) * - 'SETTLED': settlement for matching advance received and funds dispersed * * @enum {(typeof TxStatus)[keyof typeof TxStatus]} @@ -11,7 +11,9 @@ export const TxStatus = /** @type {const} */ ({ /** when TX is observed via EventFeed */ Observed: 'OBSERVED', - /** advance funds are available and IBC transfer is initiated */ + /** advance was skipped due to failed checks */ + Skipped: 'SKIPPED', + /** IBC transfer is initiated */ Advanced: 'ADVANCED', /** settlement for matching advance received and funds dispersed */ Settled: 'SETTLED', diff --git a/packages/fast-usdc/src/exos/README.md b/packages/fast-usdc/src/exos/README.md index eb77220baf4d..54a99a8f1329 100644 --- a/packages/fast-usdc/src/exos/README.md +++ b/packages/fast-usdc/src/exos/README.md @@ -1,54 +1,31 @@ -## **StatusManager** sequence diagram, showing `TxStatus` state transitions +## **StatusManager** state diagram, showing `TxStatus` transitions ### Current Implementation ```mermaid -sequenceDiagram - participant I as Initial - participant O as OBSERVED - participant A as ADVANCED - participant S as SETTLED - - Note over O,S: Currently Implemented State Transitions - - I->>O: statusManager.observe()
When TX observed via EventFeed - O->>A: statusManager.advance()
When IBC transfer initiated - A->>S: statusManager.settle()
When settlement received + dispersed - - Note over O,A: Status updated when transfer starts - Note over A,S: Requires matching settlement transfer +stateDiagram-v2 + [*] --> Observed: EventFeed .observe() + Observed --> Advanced: Advancer .advance() + Advanced --> Settled: Settler .settle() after fees + Observed --> Skipped: Advancer .skip() + Skipped --> Settled: Settler .settle() sans fees + Settled --> [*] ``` -### Additional States to Consider (Not Implemented) +### Additional Scenarios to Consider ```mermaid -sequenceDiagram - participant I as Initial - participant O as OBSERVED - participant AG as ADVANCING - participant A as ADVANCED - participant S as SETTLED - participant AS as ADVANCE_SKIPPED - participant AF as ADVANCE_FAILED - participant OS as ORPHANED_SETTLE - - Note over O,S: Normal Flow - I->>O: statusManager.observe() - O->>AG: initiate IBC transfer
(if validation passes) - AG->>A: IBC transfer settled - A->>S: settlement received - - Note over O,AS: Early Failures - O-->>AS: validation fails
(missing chain config,
invalid path,
no pool funds) - - Note over AG,AF: IBC Failures - AG-->>AF: timeout,
unexpected error,
insufficient funds - - Note over O,OS: Edge Cases - I-->>OS: Settlement received without OBSERVED - O-->>S: OBSERVED->SETTLED without ADVANCING/ADVANCED
is this a valid state transition?
behavior is slowUSDC? - AG-->>S: Settlement received while ADVANCING
wait for ADVANCED to SETTLE/disperse funds? - - Note over AS,AF: Future: Queue for retry? - Note over OS: Future: Wait for OBSERVE? +stateDiagram-v2 + [*] --> Observed: EventFeed .observe() + Observed --> Advanced: Advancer .advance() + Advanced --> Settled: Settler .settle() after fees + note right of Advanced + When IBC transfer starts; + needs to account for failure + end note + Observed --> Skipped: Advancer .skip() + Skipped --> Settled: Settler .settle() sans fees + Settled --> [*] + + [*] --> [*]: Unobserved settlement ``` diff --git a/packages/fast-usdc/src/exos/advancer.js b/packages/fast-usdc/src/exos/advancer.js index cde929b66886..3fa36f09a8df 100644 --- a/packages/fast-usdc/src/exos/advancer.js +++ b/packages/fast-usdc/src/exos/advancer.js @@ -94,8 +94,13 @@ export const prepareAdvancer = ( params: { EUD }, } = addressTools.getQueryParams(event.aux.recipientAddress); if (!EUD) { + statusManager.skip( + event.tx.forwardingAddress, + event.tx.amount, + entryIndex, + ); throw makeError( - `'recipientAddress' does not contain EUD param: ${q(event.aux.recipientAddress)}`, + `recipientAddress does not contain EUD param: ${q(event.aux.recipientAddress)}`, ); } @@ -104,6 +109,7 @@ export const prepareAdvancer = ( // 2. ensure we can find chainID // 3. ~~ensure valid PFM path~~ best observable via .transfer() vow rejection + // XXX this can throw, and should make a status update in the catch const { chainId } = chainHub.getChainInfoByAddress(EUD); /** @type {ChainAddress} */ diff --git a/packages/fast-usdc/src/exos/settler.js b/packages/fast-usdc/src/exos/settler.js index e13dcff36add..8757b0974ee6 100644 --- a/packages/fast-usdc/src/exos/settler.js +++ b/packages/fast-usdc/src/exos/settler.js @@ -6,11 +6,11 @@ import { M } from '@endo/patterns'; import { addressTools } from '../utils/address.js'; /** - * @import { FungibleTokenPacketData } from '@agoric/cosmic-proto/ibc/applications/transfer/v2/packet.js'; + * @import {FungibleTokenPacketData} from '@agoric/cosmic-proto/ibc/applications/transfer/v2/packet.js'; * @import {Denom} from '@agoric/orchestration'; * @import {IBCChannelID, VTransferIBCEvent} from '@agoric/vats'; * @import {Zone} from '@agoric/zone'; - * @import { NobleAddress } from '../types.js'; + * @import {NobleAddress} from '../types.js'; * @import {StatusManager} from './status-manager.js'; */ diff --git a/packages/fast-usdc/src/exos/status-manager.js b/packages/fast-usdc/src/exos/status-manager.js index d456dd3438e7..a8cdb1cede94 100644 --- a/packages/fast-usdc/src/exos/status-manager.js +++ b/packages/fast-usdc/src/exos/status-manager.js @@ -29,6 +29,15 @@ export const prepareStatusManager = zone => { valueShape: M.arrayOf(StatusManagerValueShape), }); + /** @param {StatusManagerValue[]} entries */ + const getPendingIdx = entries => + entries.findIndex( + ({ status }) => + // TODO since we remove SETTLED, maybe this is not needed + // or could become `status !== TxStatus.Observed` + status === TxStatus.Advanced || status === TxStatus.Skipped, + ); + return zone.exo( 'Fast USDC Status Manager', M.interface('StatusManagerI', { @@ -36,6 +45,7 @@ export const prepareStatusManager = zone => { advance: M.call(M.string(), M.bigint(), M.number()).returns( M.undefined(), ), + skip: M.call(M.string(), M.bigint(), M.number()).returns(M.undefined()), hasPendingSettlement: M.call(M.string(), M.bigint()).returns(M.boolean()), settle: M.call(M.string(), M.bigint()).returns(M.undefined()), view: M.call(M.string(), M.bigint()) @@ -82,6 +92,9 @@ export const prepareStatusManager = zone => { advance(address, amount, index) { const key = /** @type {StatusManagerKey} */ (`${address}-${amount}`); const mutableEntries = [...txs.get(key)]; + if (!mutableEntries[index]) { + throw makeError(`Cannot find ${q(key)} entry at index ${q(index)}`); + } if (mutableEntries[index].status !== TxStatus.Observed) { throw makeError( `Cannot advance ${q(key)} entry ${q(index)} with status: ${q(mutableEntries[index].status)}`, @@ -94,7 +107,32 @@ export const prepareStatusManager = zone => { txs.set(key, harden(mutableEntries)); }, /** - * Find an `ADVANCED` tx waiting to be `SETTLED` + * Mark an `OBSERVED` transaction as `SKIPPED`. + * + * @param {string} address + * @param {bigint} amount + * @param {number} index + */ + skip(address, amount, index) { + const key = /** @type {StatusManagerKey} */ (`${address}-${amount}`); + const mutableEntries = [...txs.get(key)]; + if (!mutableEntries[index]) { + throw makeError(`Cannot find ${q(key)} entry at index ${q(index)}`); + } + // TODO, consider ADVANCED -> SKIPPED for failed IBC Transfer + if (mutableEntries[index].status !== TxStatus.Observed) { + throw makeError( + `Cannot skip ${q(key)} entry ${q(index)} with status: ${q(mutableEntries[index].status)}`, + ); + } + mutableEntries[index] = { + ...mutableEntries[index], + status: TxStatus.Skipped, + }; + txs.set(key, harden(mutableEntries)); + }, + /** + * Find an `ADVANCED` or `SKIPPED` tx waiting to be `SETTLED` * * @param {string} address * @param {bigint} amount @@ -103,11 +141,7 @@ export const prepareStatusManager = zone => { hasPendingSettlement(address, amount) { const key = /** @type {StatusManagerKey} */ (`${address}-${amount}`); const entries = txs.get(key); - const unsettledIdx = entries.findIndex( - ({ status }) => status === TxStatus.Advanced, - ); - // TODO: if OBSERVED -> SETTLED is a valid transition, and - // (unsettledIdx > -1) === false, look for `OBSERVED` entry + const unsettledIdx = getPendingIdx(entries); return unsettledIdx > -1; }, /** @@ -121,16 +155,13 @@ export const prepareStatusManager = zone => { settle(address, amount) { const key = /** @type {StatusManagerKey} */ (`${address}-${amount}`); const mutableEntries = [...txs.get(key)]; - const unsettledIdx = mutableEntries.findIndex( - ({ status }) => status === TxStatus.Advanced, - ); + const unsettledIdx = getPendingIdx(mutableEntries); if (unsettledIdx === -1) { - throw makeError(`No pending advance found for ${q(key)}`); + throw makeError(`No unsettled entry for ${q(key)}`); } - mutableEntries[unsettledIdx] = { - ...mutableEntries[unsettledIdx], - status: TxStatus.Settled, - }; + // TODO, vstorage update for `TxStatus.Settled` + // delete entry once SETTLED + mutableEntries.splice(unsettledIdx, 1); txs.set(key, harden(mutableEntries)); }, /** @@ -144,7 +175,7 @@ export const prepareStatusManager = zone => { * @param {NobleAddress} address * @param {bigint} amount * @param {number} index - * @returns {StatusManagerValue} + * @returns {StatusManagerValue | undefined} */ /** * View a StatusManagerValue or a list of them diff --git a/packages/fast-usdc/test/exos/advancer.test.ts b/packages/fast-usdc/test/exos/advancer.test.ts index 5c13b62504a4..285b0cab9b13 100644 --- a/packages/fast-usdc/test/exos/advancer.test.ts +++ b/packages/fast-usdc/test/exos/advancer.test.ts @@ -148,3 +148,33 @@ test('advancer does not update status on failed transfer', async t => { '"[Error: simulated error]"', ]); }); + +test('advancer updated status to SKIPPED if pre-condition checks fail', async t => { + const { localDenom, makeAdvancer, statusManager, rootZone, vowTools } = + t.context; + + const { poolAccount, poolAccountTransferVResolver } = prepareMockOrchAccounts( + rootZone.subZone('poolAcct2'), + { vowTools, log: t.log }, + ); + + const advancer = makeAdvancer({ poolAccount, localDenom }); + t.truthy(advancer, 'advancer instantiates'); + + // simulate input from EventFeed + const mockCttpTxEvidence = MockCctpTxEvidences.AGORIC_NO_PARAMS(); + t.throws(() => advancer.handleEvent(mockCttpTxEvidence), { + message: + 'recipientAddress does not contain EUD param: "agoric16kv2g7snfc4q24vg3pjdlnnqgngtjpwtetd2h689nz09lcklvh5s8u37ek"', + }); + + const entries = statusManager.view( + mockCttpTxEvidence.tx.forwardingAddress, + mockCttpTxEvidence.tx.amount, + ); + t.deepEqual( + entries, + [{ ...mockCttpTxEvidence, status: TxStatus.Skipped }], + 'tx status is still OBSERVED', + ); +}); diff --git a/packages/fast-usdc/test/exos/settler.test.ts b/packages/fast-usdc/test/exos/settler.test.ts index 89d3b45abf4a..7a5686e46bd1 100644 --- a/packages/fast-usdc/test/exos/settler.test.ts +++ b/packages/fast-usdc/test/exos/settler.test.ts @@ -88,11 +88,8 @@ test('StatusManger gets `SETTLED` update in happy path', async t => { cctpTxEvidence.tx.amount, index, ); - t.is( - entry?.status, - TxStatus.Settled, - 'StatusManger entry updated with SETTLED status', - ); + // TODO, confirm vstorage write for TxStatus.SETTLED + t.is(entry, undefined, 'SETTLED entry removed from StatusManger'); }); test.todo("StatusManager does not receive update when we can't settle"); diff --git a/packages/fast-usdc/test/exos/status-manager.test.ts b/packages/fast-usdc/test/exos/status-manager.test.ts index c762622e59b7..e18915459e24 100644 --- a/packages/fast-usdc/test/exos/status-manager.test.ts +++ b/packages/fast-usdc/test/exos/status-manager.test.ts @@ -18,7 +18,7 @@ test('observe creates new entry with OBSERVED status', t => { index, ); - t.deepEqual(entry?.status, TxStatus.Observed); + t.is(entry?.status, TxStatus.Observed); }); test('advance updates status from OBSERVED to ADVANCED', t => { @@ -40,30 +40,87 @@ test('advance updates status from OBSERVED to ADVANCED', t => { index, ); - t.deepEqual(entry?.status, TxStatus.Advanced); + t.is(entry?.status, TxStatus.Advanced); }); -test('settle updates status from ADVANCED to SETTLED', t => { +test('skip updates status from OBSERVED to SKIPPED', t => { const zone = provideDurableZone('status-test'); const statusManager = prepareStatusManager(zone.subZone('status-manager')); const evidence = MockCctpTxEvidences.AGORIC_PLUS_OSMO(); const index = statusManager.observe(evidence); - statusManager.advance( + statusManager.skip(evidence.tx.forwardingAddress, evidence.tx.amount, index); + + const entry = statusManager.view( evidence.tx.forwardingAddress, evidence.tx.amount, index, ); - statusManager.settle(evidence.tx.forwardingAddress, evidence.tx.amount); - const entry = statusManager.view( + t.is(entry?.status, TxStatus.Skipped); +}); + +test('settle updates status from ADVANCED to SETTLED', t => { + const zone = provideDurableZone('status-test'); + const statusManager = prepareStatusManager(zone.subZone('status-manager')); + + const evidence = MockCctpTxEvidences.AGORIC_PLUS_OSMO(); + const index = statusManager.observe(evidence); + + statusManager.advance( evidence.tx.forwardingAddress, evidence.tx.amount, index, ); + t.is( + statusManager.view(evidence.tx.forwardingAddress, evidence.tx.amount, index) + ?.status, + TxStatus.Advanced, + 'sanity check that advance worked', + ); + + statusManager.settle(evidence.tx.forwardingAddress, evidence.tx.amount); + + // TODO, check vstorage for TxStatus.Settled + t.is( + statusManager.view( + evidence.tx.forwardingAddress, + evidence.tx.amount, + index, + ), + undefined, + 'Settled entry should be deleted', + ); +}); + +test('settle updates status from SKIPPED to SETTLED', t => { + const zone = provideDurableZone('status-test'); + const statusManager = prepareStatusManager(zone.subZone('status-manager')); + + const evidence = MockCctpTxEvidences.AGORIC_PLUS_OSMO(); + const index = statusManager.observe(evidence); + + statusManager.skip(evidence.tx.forwardingAddress, evidence.tx.amount, index); + t.is( + statusManager.view(evidence.tx.forwardingAddress, evidence.tx.amount, index) + ?.status, + TxStatus.Skipped, + 'sanity check that skip worked', + ); + + statusManager.settle(evidence.tx.forwardingAddress, evidence.tx.amount); - t.deepEqual(entry?.status, TxStatus.Settled); + // TODO, check vstorage for TxStatus.Settled + t.is( + statusManager.view( + evidence.tx.forwardingAddress, + evidence.tx.amount, + index, + ), + undefined, + 'Settled entry should be deleted', + ); }); test('cannot ADVANCE without OBSERVE first', t => { @@ -91,19 +148,19 @@ test('cannot SETTLE without ADVANCE first', t => { const statusManager = prepareStatusManager(zone.subZone('status-manager')); const evidence = MockCctpTxEvidences.AGORIC_PLUS_OSMO(); - const index = statusManager.observe(evidence); + statusManager.observe(evidence); t.throws( () => statusManager.settle(evidence.tx.forwardingAddress, evidence.tx.amount), { message: - 'No pending advance found for "noble1x0ydg69dh6fqvr27xjvp6maqmrldam6yfelqkd-150000000"', + 'No unsettled entry for "noble1x0ydg69dh6fqvr27xjvp6maqmrldam6yfelqkd-150000000"', }, ); }); -test('cannot ADVANCE an entry that is already SETTLED', t => { +test('cannot ADVANCE an entry that is already SETTLED or ADVANCED', t => { const zone = provideDurableZone('status-test'); const statusManager = prepareStatusManager(zone.subZone('status-manager')); @@ -115,6 +172,19 @@ test('cannot ADVANCE an entry that is already SETTLED', t => { evidence.tx.amount, index, ); + t.throws( + () => + statusManager.advance( + evidence.tx.forwardingAddress, + evidence.tx.amount, + index, + ), + { + message: + 'Cannot advance "noble1x0ydg69dh6fqvr27xjvp6maqmrldam6yfelqkd-150000000" entry 0 with status: "ADVANCED"', + }, + ); + statusManager.settle(evidence.tx.forwardingAddress, evidence.tx.amount); t.throws( @@ -126,7 +196,48 @@ test('cannot ADVANCE an entry that is already SETTLED', t => { ), { message: - 'Cannot advance "noble1x0ydg69dh6fqvr27xjvp6maqmrldam6yfelqkd-150000000" entry 0 with status: "SETTLED"', + 'Cannot find "noble1x0ydg69dh6fqvr27xjvp6maqmrldam6yfelqkd-150000000" entry at index 0', + }, + ); +}); + +test('cannot SKIP an entry that is already SETTLED or ADVANCED', t => { + const zone = provideDurableZone('status-test'); + const statusManager = prepareStatusManager(zone.subZone('status-manager')); + + // setup transaction in SETTLED state + const evidence = MockCctpTxEvidences.AGORIC_PLUS_OSMO(); + const index = statusManager.observe(evidence); + statusManager.advance( + evidence.tx.forwardingAddress, + evidence.tx.amount, + index, + ); + t.throws( + () => + statusManager.skip( + evidence.tx.forwardingAddress, + evidence.tx.amount, + index, + ), + { + message: + 'Cannot skip "noble1x0ydg69dh6fqvr27xjvp6maqmrldam6yfelqkd-150000000" entry 0 with status: "ADVANCED"', + }, + ); + + statusManager.settle(evidence.tx.forwardingAddress, evidence.tx.amount); + + t.throws( + () => + statusManager.skip( + evidence.tx.forwardingAddress, + evidence.tx.amount, + index, + ), + { + message: + 'Cannot find "noble1x0ydg69dh6fqvr27xjvp6maqmrldam6yfelqkd-150000000" entry at index 0', }, ); }); @@ -159,9 +270,9 @@ test('settle SETTLES first matched entry', t => { evidence.tx.forwardingAddress, evidence.tx.amount, ); - t.is(entries0.length, 2); - t.is(entries0?.[0].status, TxStatus.Settled); - t.is(entries0?.[1].status, TxStatus.Advanced); + t.is(entries0.length, 1); + // TODO, check vstorage for TxStatus.Settled + t.is(entries0?.[0].status, TxStatus.Advanced, 'first settled entry deleted'); // settle again wih same args settled 2nd advance statusManager.settle(evidence.tx.forwardingAddress, evidence.tx.amount); @@ -169,8 +280,8 @@ test('settle SETTLES first matched entry', t => { evidence.tx.forwardingAddress, evidence.tx.amount, ); - t.is(entries1?.[0].status, TxStatus.Settled); - t.is(entries1?.[1].status, TxStatus.Settled); + // TODO, check vstorage for TxStatus.Settled + t.is(entries1?.length, 0, 'settled entries are deleted'); }); test('StatusManagerKey logic handles addresses with hyphens', async t => { @@ -197,16 +308,15 @@ test('StatusManagerKey logic handles addresses with hyphens', async t => { statusManager.advance(evidence.tx.forwardingAddress, evidence.tx.amount, idx); t.is( statusManager.view(evidence.tx.forwardingAddress, evidence.tx.amount, idx) - .status, + ?.status, TxStatus.Advanced, 'able to advance StatusManagerKey with multiple hyphens', ); statusManager.settle(evidence.tx.forwardingAddress, evidence.tx.amount); t.is( - statusManager.view(evidence.tx.forwardingAddress, evidence.tx.amount, idx) - .status, - TxStatus.Settled, + statusManager.view(evidence.tx.forwardingAddress, evidence.tx.amount, idx), + undefined, 'able to settle StatusManagerKey with multiple hyphens', ); }); diff --git a/packages/fast-usdc/test/fixtures.ts b/packages/fast-usdc/test/fixtures.ts index 138e137930f3..69363c988c75 100644 --- a/packages/fast-usdc/test/fixtures.ts +++ b/packages/fast-usdc/test/fixtures.ts @@ -3,7 +3,11 @@ import { buildVTransferEvent } from '@agoric/orchestration/tools/ibc-mocks.js'; import fetchedChainInfo from '@agoric/orchestration/src/fetched-chain-info.js'; import type { CctpTxEvidence } from '../src/types.js'; -const mockScenarios = ['AGORIC_PLUS_OSMO', 'AGORIC_PLUS_DYDX'] as const; +const mockScenarios = [ + 'AGORIC_PLUS_OSMO', + 'AGORIC_PLUS_DYDX', + 'AGORIC_NO_PARAMS', +] as const; type MockScenario = (typeof mockScenarios)[number]; @@ -49,6 +53,25 @@ export const MockCctpTxEvidences: Record< }, chainId: 1, }), + AGORIC_NO_PARAMS: (receiverAddress?: string) => ({ + blockHash: + '0x70d7343e04f8160892e94f02d6a9b9f255663ed0ac34caca98544c8143fee699', + blockNumber: 21037669n, + blockTimestamp: 1730762099n, + txHash: + '0xa81bc6105b60a234c7c50ac17816ebcd5561d366df8bf3be59ff387552761799', + tx: { + amount: 200000000n, + forwardingAddress: 'noble1x0ydg69dh6fqvr27xjvp6maqmrldam6yfelyyy', + }, + aux: { + forwardingChannel: 'channel-21', + recipientAddress: + receiverAddress || + 'agoric16kv2g7snfc4q24vg3pjdlnnqgngtjpwtetd2h689nz09lcklvh5s8u37ek', + }, + chainId: 1, + }), }; const nobleDefaultVTransferParams = { @@ -83,4 +106,13 @@ export const MockVTransferEvents: Record< recieverAddress || MockCctpTxEvidences.AGORIC_PLUS_DYDX().aux.recipientAddress, }), + AGORIC_NO_PARAMS: (recieverAddress?: string) => + buildVTransferEvent({ + ...nobleDefaultVTransferParams, + amount: MockCctpTxEvidences.AGORIC_NO_PARAMS().tx.amount, + sender: MockCctpTxEvidences.AGORIC_NO_PARAMS().tx.forwardingAddress, + receiver: + recieverAddress || + MockCctpTxEvidences.AGORIC_NO_PARAMS().aux.recipientAddress, + }), };