From 8b18171ff3f20ad8f839e51e3d1ba504a60535b5 Mon Sep 17 00:00:00 2001 From: 0xPatrick Date: Thu, 19 Dec 2024 18:09:50 -0500 Subject: [PATCH] chore: tx statuses in vstorage - ensure "minted before observed" `Observe` statuses are recorded in vstorage - ensure "minted while Advancing" `Advanced` and `AdvanceFailed` statuses are recorded in vstorage --- packages/fast-usdc/src/exos/advancer.js | 40 ++++----------- packages/fast-usdc/src/exos/settler.js | 38 +++++++------- packages/fast-usdc/src/exos/status-manager.js | 49 ++++++++++++++++--- packages/fast-usdc/test/exos/advancer.test.ts | 19 ++++--- packages/fast-usdc/test/exos/settler.test.ts | 13 ++--- .../fast-usdc/test/fast-usdc.contract.test.ts | 4 +- 6 files changed, 87 insertions(+), 76 deletions(-) diff --git a/packages/fast-usdc/src/exos/advancer.js b/packages/fast-usdc/src/exos/advancer.js index 168ce7c0632..31529de56ba 100644 --- a/packages/fast-usdc/src/exos/advancer.js +++ b/packages/fast-usdc/src/exos/advancer.js @@ -164,25 +164,13 @@ export const prepareAdvancerKit = ( const destination = chainHub.makeChainAddress(EUD); const fullAmount = toAmount(evidence.tx.amount); - const { - tx: { forwardingAddress }, - txHash, - } = evidence; - const { borrowerFacet, notifyFacet, poolAccount } = this.state; - if ( - notifyFacet.forwardIfMinted( - destination, - forwardingAddress, - fullAmount, - txHash, - ) - ) { - // settlement already received; tx will Forward. - // do not add to `pendingSettleTxs` by calling `.observe()` - log('⚠️ minted before Observed'); - return; - } + // do not advance if we've already received a mint/settlement + const mintedEarly = notifyFacet.checkMintedEarly( + evidence, + destination, + ); + if (mintedEarly) return; // throws if requested does not exceed fees const advanceAmount = feeTools.calculateAdvance(fullAmount); @@ -206,7 +194,7 @@ export const prepareAdvancerKit = ( forwardingAddress: evidence.tx.forwardingAddress, fullAmount, tmpSeat, - txHash, + txHash: evidence.txHash, }); } catch (error) { log('Advancer error:', error); @@ -225,13 +213,7 @@ export const prepareAdvancerKit = ( */ onFulfilled(result, ctx) { const { poolAccount, intermediateRecipient } = this.state; - const { - destination, - advanceAmount, - // eslint-disable-next-line no-unused-vars - tmpSeat, - ...detail - } = ctx; + const { destination, advanceAmount, tmpSeat: _, ...detail } = ctx; const transferV = E(poolAccount).transfer( destination, { denom: usdc.denom, value: advanceAmount.value }, @@ -296,11 +278,7 @@ export const prepareAdvancerKit = ( onRejected(error, ctx) { const { notifyFacet } = this.state; log('Advance transfer rejected', error); - const { - // eslint-disable-next-line no-unused-vars - advanceAmount, - ...restCtx - } = ctx; + const { advanceAmount: _, ...restCtx } = ctx; notifyFacet.notifyAdvancingResult(restCtx, false); }, }, diff --git a/packages/fast-usdc/src/exos/settler.js b/packages/fast-usdc/src/exos/settler.js index 5ff573dfa13..47344a17fe2 100644 --- a/packages/fast-usdc/src/exos/settler.js +++ b/packages/fast-usdc/src/exos/settler.js @@ -8,7 +8,11 @@ import { M } from '@endo/patterns'; import { decodeAddressHook } from '@agoric/cosmic-proto/address-hooks.js'; import { PendingTxStatus } from '../constants.js'; import { makeFeeTools } from '../utils/fees.js'; -import { EvmHashShape, makeNatAmountShape } from '../type-guards.js'; +import { + CctpTxEvidenceShape, + EvmHashShape, + makeNatAmountShape, +} from '../type-guards.js'; /** * @import {FungibleTokenPacketData} from '@agoric/cosmic-proto/ibc/applications/transfer/v2/packet.js'; @@ -18,7 +22,7 @@ import { EvmHashShape, makeNatAmountShape } from '../type-guards.js'; * @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} from '../types.js'; + * @import {NobleAddress, LiquidityPoolKit, FeeConfig, EvmHash, LogFn, CctpTxEvidence} from '../types.js'; * @import {StatusManager} from './status-manager.js'; */ @@ -81,8 +85,9 @@ export const prepareSettler = ( makeAdvanceDetailsShape(USDC), M.boolean(), ).returns(), - forwardIfMinted: M.call( - ...Object.values(makeAdvanceDetailsShape(USDC)), + checkMintedEarly: M.call( + CctpTxEvidenceShape, + ChainAddressShape, ).returns(M.boolean()), }), self: M.interface('SettlerSelfI', { @@ -216,17 +221,13 @@ export const prepareSettler = ( ) { const { mintedEarly } = this.state; const { value: fullValue } = fullAmount; - // XXX i think this only contains Advancing txs - should this be a separate store? const key = makeMintedEarlyKey(forwardingAddress, fullValue); if (mintedEarly.has(key)) { mintedEarly.delete(key); + statusManager.advanceOutcomeForMintedEarly(txHash, success); if (success) { - // TODO: does not write `ADVANCED` to vstorage - // this is the "slow" experience, but we've already earmarked fees so disburse void this.facets.self.disburse(txHash, fullValue); } else { - // TODO: does not write `ADVANCE_FAILED` to vstorage - // if advance fails, attempt to forward (no fees) void this.facets.self.forward( txHash, fullValue, @@ -238,26 +239,27 @@ export const prepareSettler = ( } }, /** + * @param {CctpTxEvidence} evidence * @param {ChainAddress} destination - * @param {NobleAddress} forwardingAddress - * @param {Amount<'nat'>} fullAmount - * @param {EvmHash} txHash * @returns {boolean} * @throws {Error} if minted early, so advancer doesn't advance */ - forwardIfMinted(destination, forwardingAddress, fullAmount, txHash) { - const { value: fullValue } = fullAmount; - const key = makeMintedEarlyKey(forwardingAddress, fullValue); + checkMintedEarly(evidence, destination) { + const { + tx: { forwardingAddress, amount }, + txHash, + } = evidence; + const key = makeMintedEarlyKey(forwardingAddress, amount); const { mintedEarly } = this.state; if (mintedEarly.has(key)) { log( 'matched minted early key, initiating forward', forwardingAddress, - fullValue, + amount, ); mintedEarly.delete(key); - // TODO: does not write `OBSERVED` to vstorage - void this.facets.self.forward(txHash, fullValue, destination.value); + statusManager.advanceOutcomeForUnknownMint(evidence); + void this.facets.self.forward(txHash, amount, destination.value); return true; } return false; diff --git a/packages/fast-usdc/src/exos/status-manager.js b/packages/fast-usdc/src/exos/status-manager.js index 2e4c6a2a539..9a38e185d24 100644 --- a/packages/fast-usdc/src/exos/status-manager.js +++ b/packages/fast-usdc/src/exos/status-manager.js @@ -196,12 +196,12 @@ export const prepareStatusManager = ( 'Fast USDC Status Manager', M.interface('StatusManagerI', { // TODO: naming scheme for transition events - advance: M.call(CctpTxEvidenceShape).returns(M.undefined()), + advance: M.call(CctpTxEvidenceShape).returns(), advanceOutcome: M.call(M.string(), M.nat(), M.boolean()).returns(), - skipAdvance: M.call(CctpTxEvidenceShape, M.arrayOf(M.string())).returns( - M.undefined(), - ), - observe: M.call(CctpTxEvidenceShape).returns(M.undefined()), + skipAdvance: M.call(CctpTxEvidenceShape, M.arrayOf(M.string())).returns(), + advanceOutcomeForMintedEarly: M.call(EvmHashShape, M.boolean()).returns(), + advanceOutcomeForUnknownMint: M.call(CctpTxEvidenceShape).returns(), + observe: M.call(CctpTxEvidenceShape).returns(), hasBeenObserved: M.call(CctpTxEvidenceShape).returns(M.boolean()), deleteCompletedTxs: M.call().returns(M.undefined()), dequeueStatus: M.call(M.string(), M.bigint()).returns( @@ -216,7 +216,7 @@ export const prepareStatusManager = ( disbursed: M.call(EvmHashShape, AmountKeywordRecordShape).returns( M.undefined(), ), - forwarded: M.call(EvmHashShape, M.boolean()).returns(M.undefined()), + forwarded: M.call(EvmHashShape, M.boolean()).returns(), lookupPending: M.call(M.string(), M.bigint()).returns( M.arrayOf(PendingTxShape), ), @@ -266,6 +266,43 @@ export const prepareStatusManager = ( ); }, + /** + * If minted while advancing, publish a status update for the advance + * to vstorage. + * + * Does not add or amend `pendingSettleTxs` as this has + * already settled. + * + * @param {EvmHash} txHash + * @param {boolean} success whether the Transfer succeeded + */ + advanceOutcomeForMintedEarly(txHash, success) { + void publishTxnRecord( + txHash, + harden({ + status: success + ? PendingTxStatus.Advanced + : PendingTxStatus.AdvanceFailed, + }), + ); + }, + + /** + * If minted before observed and the evidence is eventually + * reported, publish the evidence without adding to `pendingSettleTxs` + * + * @param {CctpTxEvidence} evidence + */ + advanceOutcomeForUnknownMint(evidence) { + const { txHash } = evidence; + // unexpected path, since `hasBeenObserved` will be called before this + if (seenTxs.has(txHash)) { + throw makeError(`Transaction already seen: ${q(txHash)}`); + } + seenTxs.add(txHash); + publishEvidence(txHash, evidence); + }, + /** * Add a new transaction with OBSERVED status * @param {CctpTxEvidence} evidence diff --git a/packages/fast-usdc/test/exos/advancer.test.ts b/packages/fast-usdc/test/exos/advancer.test.ts index ac4e652e530..c5e53c0b70a 100644 --- a/packages/fast-usdc/test/exos/advancer.test.ts +++ b/packages/fast-usdc/test/exos/advancer.test.ts @@ -6,10 +6,10 @@ import { } from '@agoric/cosmic-proto/address-hooks.js'; import type { NatAmount } from '@agoric/ertp'; import { eventLoopIteration } from '@agoric/internal/src/testing-utils.js'; -import { denomHash } from '@agoric/orchestration'; +import { ChainAddressShape, denomHash } from '@agoric/orchestration'; import fetchedChainInfo from '@agoric/orchestration/src/fetched-chain-info.js'; import { type ZoeTools } from '@agoric/orchestration/src/utils/zoe-tools.js'; -import { Fail, q } from '@endo/errors'; +import { q } from '@endo/errors'; import { Far } from '@endo/pass-style'; import type { TestFn } from 'ava'; import { makeTracer } from '@agoric/internal'; @@ -34,6 +34,7 @@ import { prepareMockOrchAccounts, } from '../mocks.js'; import { commonSetup } from '../supports.js'; +import { CctpTxEvidenceShape } from '../../src/type-guards.js'; const trace = makeTracer('AdvancerTest', false); @@ -118,11 +119,9 @@ const createTestExtensions = (t, common: CommonSetup) => { notifyAdvancingResultCalls.push(args); }, // assume this never returns true for most tests - forwardIfMinted: (...args) => { - mustMatch( - harden(args), - harden([...Object.values(makeAdvanceDetailsShape(usdc.brand))]), - ); + checkMintedEarly: (evidence, destination) => { + mustMatch(harden(evidence), CctpTxEvidenceShape); + mustMatch(destination, ChainAddressShape); return false; }, }); @@ -674,7 +673,7 @@ test('rejects advances to unknown settlementAccount', async t => { ]); }); -test('no status update if `forwardIfMinted` returns true', async t => { +test('no status update if `checkMintedEarly` returns true', async t => { const { brands: { usdc }, bootstrap: { storage }, @@ -687,7 +686,7 @@ test('no status update if `forwardIfMinted` returns true', async t => { const mockNotifyF = Far('Settler Notify Facet', { notifyAdvancingResult: () => {}, - forwardIfMinted: (destination, forwardingAddress, fullAmount, txHash) => { + checkMintedEarly: (evidence, destination) => { return true; }, }); @@ -712,6 +711,6 @@ test('no status update if `forwardIfMinted` returns true', async t => { t.deepEqual(inspectLogs(), [ ['decoded EUD: dydx183dejcnmkka5dzcu9xw6mywq0p2m5peks28men'], - ['⚠️ minted before Observed'], + // no add'l logs as we return early ]); }); diff --git a/packages/fast-usdc/test/exos/settler.test.ts b/packages/fast-usdc/test/exos/settler.test.ts index a7352e79a19..4bed4311dae 100644 --- a/packages/fast-usdc/test/exos/settler.test.ts +++ b/packages/fast-usdc/test/exos/settler.test.ts @@ -180,12 +180,7 @@ const makeTestContext = async t => { const cctpTxEvidence = makeEvidence(evidence); const { destination, forwardingAddress, fullAmount, txHash } = makeNotifyInfo(cctpTxEvidence); - notifyFacet.forwardIfMinted( - destination, - forwardingAddress, - fullAmount, - txHash, - ); + notifyFacet.checkMintedEarly(cctpTxEvidence, destination); return cctpTxEvidence; }, }); @@ -542,7 +537,7 @@ test('Settlement for unknown transaction (minted early)', async t => { accounts.settlement.transferVResolver.resolve(undefined); await eventLoopIteration(); t.deepEqual(storage.getDeserialized(`fun.txns.${evidence.txHash}`), [ - /// TODO with no observed / evidence, does this break reporting reqs? + { evidence, status: 'OBSERVED' }, { status: 'FORWARDED' }, ]); }); @@ -598,7 +593,7 @@ test('Settlement for Advancing transaction (advance succeeds)', async t => { t.deepEqual(storage.getDeserialized(`fun.txns.${cctpTxEvidence.txHash}`), [ { evidence: cctpTxEvidence, status: 'OBSERVED' }, { status: 'ADVANCING' }, - // { status: 'ADVANCED' }, TODO: not reported by notifyAdvancingResult + { status: 'ADVANCED' }, { split: expectedSplit, status: 'DISBURSED' }, ]); }); @@ -664,7 +659,7 @@ test('Settlement for Advancing transaction (advance fails)', async t => { t.deepEqual(storage.getDeserialized(`fun.txns.${cctpTxEvidence.txHash}`), [ { evidence: cctpTxEvidence, status: 'OBSERVED' }, { status: 'ADVANCING' }, - // { status: 'ADVANCE_FAILED' }, TODO: not reported by notifyAdvancingResult + { status: 'ADVANCE_FAILED' }, { status: 'FORWARDED' }, ]); }); diff --git a/packages/fast-usdc/test/fast-usdc.contract.test.ts b/packages/fast-usdc/test/fast-usdc.contract.test.ts index a8b0f3c4ece..a7e48149671 100644 --- a/packages/fast-usdc/test/fast-usdc.contract.test.ts +++ b/packages/fast-usdc/test/fast-usdc.contract.test.ts @@ -918,7 +918,7 @@ test.serial('Settlement for unknown transaction (operator down)', async t => { await eventLoopIteration(); t.deepEqual(storage.getDeserialized(`fun.txns.${sent.txHash}`), [ - // { evidence: sent, status: 'OBSERVED' }, // no OBSERVED state recorded + { evidence: sent, status: 'OBSERVED' }, { status: 'FORWARDED' }, ]); }); @@ -959,7 +959,7 @@ test.serial('mint received while ADVANCING', async t => { t.deepEqual(storage.getDeserialized(`fun.txns.${sent.txHash}`), [ { evidence: sent, status: 'OBSERVED' }, { status: 'ADVANCING' }, - // { status: 'ADVANCED' }, TODO: not reported by notifyAdvancingResult + { status: 'ADVANCED' }, { split, status: 'DISBURSED' }, ]); });