From 3a0afe72058cf4577e5cb3c50b02ffebc328a84f 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 | 36 ++++++++------- packages/fast-usdc/src/exos/status-manager.js | 45 +++++++++++++++++-- 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(+), 70 deletions(-) diff --git a/packages/fast-usdc/src/exos/advancer.js b/packages/fast-usdc/src/exos/advancer.js index 5069705797b..0a28925007d 100644 --- a/packages/fast-usdc/src/exos/advancer.js +++ b/packages/fast-usdc/src/exos/advancer.js @@ -158,25 +158,15 @@ 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); @@ -200,7 +190,7 @@ export const prepareAdvancerKit = ( forwardingAddress: evidence.tx.forwardingAddress, fullAmount, tmpSeat, - txHash, + txHash: evidence.txHash, }); } catch (error) { log('Advancer error:', error); @@ -219,13 +209,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 }, @@ -290,11 +274,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 a2382cdc38a..ff20f4907db 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', { @@ -215,16 +220,14 @@ 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.advanceOutcomeForSettled(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, @@ -237,26 +240,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.notifyObserved(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 7944f68f07a..85f40557b5b 100644 --- a/packages/fast-usdc/src/exos/status-manager.js +++ b/packages/fast-usdc/src/exos/status-manager.js @@ -193,9 +193,11 @@ 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(), - observe: M.call(CctpTxEvidenceShape).returns(M.undefined()), + advanceOutcomeForSettled: M.call(EvmHashShape, M.boolean()).returns(), + notifyObserved: 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( @@ -210,7 +212,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), ), @@ -242,6 +244,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 + */ + advanceOutcomeForSettled(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 + */ + notifyObserved(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 8a23b4fe117..d314ae143fc 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; }, }); @@ -640,7 +639,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 }, @@ -653,7 +652,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; }, }); @@ -678,6 +677,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 8545d089441..3ab72f5ee55 100644 --- a/packages/fast-usdc/test/exos/settler.test.ts +++ b/packages/fast-usdc/test/exos/settler.test.ts @@ -172,12 +172,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; }, }); @@ -454,7 +449,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' }, ]); }); @@ -510,7 +505,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' }, ]); }); @@ -576,7 +571,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 6e339ee0cfa..4acb7c3d170 100644 --- a/packages/fast-usdc/test/fast-usdc.contract.test.ts +++ b/packages/fast-usdc/test/fast-usdc.contract.test.ts @@ -883,7 +883,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' }, ]); }); @@ -924,7 +924,7 @@ test.serial('mint received why 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' }, ]); });