From 12510e94355421108c06396b836df910835a998c Mon Sep 17 00:00:00 2001 From: 0xPatrick Date: Mon, 9 Dec 2024 13:58:33 -0500 Subject: [PATCH 1/5] chore: resolve `withdrawToSeat` TODO with comment --- packages/fast-usdc/src/exos/settler.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/fast-usdc/src/exos/settler.js b/packages/fast-usdc/src/exos/settler.js index 5d092dd87cb..b8952d8086a 100644 --- a/packages/fast-usdc/src/exos/settler.js +++ b/packages/fast-usdc/src/exos/settler.js @@ -237,13 +237,13 @@ export const prepareSettler = ( const split = calculateSplit(received); log('disbursing', split); - // TODO: what if this throws? - // arguably, it cannot. Even if deposits - // and notifications get out of order, - // we don't ever withdraw more than has been deposited. + // If this throws, which arguably can't occur since we don't ever + // withdraw more than has been deposited (as denoted by + // `FungibleTokenPacketData`), funds will remain in the + // `settlementAccount`. A remediation can occur in a future upgrade. await vowTools.when( withdrawToSeat( - // @ts-expect-error Vow vs. Promise stuff. TODO: is this OK??? + // @ts-expect-error LocalAccountMethods vs OrchestrationAccount settlementAccount, settlingSeat, harden({ In: received }), From d04c5eac94e1954456cd23e9006e9f4daabb3759 Mon Sep 17 00:00:00 2001 From: 0xPatrick Date: Thu, 12 Dec 2024 12:00:24 -0500 Subject: [PATCH 2/5] fix: do not stringify logs --- packages/fast-usdc/src/exos/advancer.js | 27 +++--- packages/fast-usdc/test/exos/advancer.test.ts | 84 ++++++++++++++----- .../src/exos/local-orchestration-account.js | 2 +- 3 files changed, 78 insertions(+), 35 deletions(-) diff --git a/packages/fast-usdc/src/exos/advancer.js b/packages/fast-usdc/src/exos/advancer.js index 5fc01abfa38..9608b9de1cb 100644 --- a/packages/fast-usdc/src/exos/advancer.js +++ b/packages/fast-usdc/src/exos/advancer.js @@ -3,7 +3,6 @@ import { assertAllDefined, makeTracer } from '@agoric/internal'; import { AnyNatAmountShape, ChainAddressShape } from '@agoric/orchestration'; import { pickFacet } from '@agoric/vat-data'; import { VowShape } from '@agoric/vow'; -import { q } from '@endo/errors'; import { E } from '@endo/far'; import { M } from '@endo/patterns'; import { @@ -153,6 +152,7 @@ export const prepareAdvancerKit = ( recipientAddress, EudParamShape, ); + log(`decoded EUD: ${EUD}`); // throws if the bech32 prefix is not found const destination = chainHub.makeChainAddress(EUD); @@ -182,8 +182,8 @@ export const prepareAdvancerKit = ( tmpSeat, txHash: evidence.txHash, }); - } catch (e) { - log('Advancer error:', q(e).toString()); + } catch (error) { + log('Advancer error:', error); statusManager.observe(evidence); } }, @@ -217,13 +217,13 @@ export const prepareAdvancerKit = ( */ onRejected(error, { tmpSeat }) { // TODO return seat allocation from ctx to LP? - log('🚨 advance deposit failed', q(error).toString()); - // TODO #10510 (comprehensive error testing) determine - // course of action here log( - 'TODO live payment on seat to return to LP', - q(tmpSeat).toString(), + '⚠️ deposit to localOrchAccount failed, attempting to return payment to LP', + error, ); + // TODO #10510 (comprehensive error testing) determine + // course of action here + log('TODO live payment on seat to return to LP', tmpSeat); }, }, transferHandler: { @@ -234,10 +234,11 @@ export const prepareAdvancerKit = ( onFulfilled(result, ctx) { const { notifyFacet } = this.state; const { advanceAmount, destination, ...detail } = ctx; - log( - 'Advance transfer fulfilled', - q({ advanceAmount, destination, result }).toString(), - ); + log('Advance transfer fulfilled', { + advanceAmount, + destination, + result, + }); // During development, due to a bug, this call threw. // The failure was silent (no diagnostics) due to: // - #10576 Vows do not report unhandled rejections @@ -252,7 +253,7 @@ export const prepareAdvancerKit = ( */ onRejected(error, ctx) { const { notifyFacet } = this.state; - log('Advance transfer rejected', q(error).toString()); + log('Advance transfer rejected', error); notifyFacet.notifyAdvancingResult(ctx, false); }, }, diff --git a/packages/fast-usdc/test/exos/advancer.test.ts b/packages/fast-usdc/test/exos/advancer.test.ts index 908880d3fb4..7c860d138d2 100644 --- a/packages/fast-usdc/test/exos/advancer.test.ts +++ b/packages/fast-usdc/test/exos/advancer.test.ts @@ -181,9 +181,23 @@ test('updates status to ADVANCING in happy path', async t => { 'ADVANCED status in happy path', ); - t.deepEqual(inspectLogs(0), [ - 'Advance transfer fulfilled', - '{"advanceAmount":{"brand":"[Alleged: USDC brand]","value":"[146999999n]"},"destination":{"chainId":"osmosis-1","encoding":"bech32","value":"osmo183dejcnmkka5dzcu9xw6mywq0p2m5peks28men"},"result":"[undefined]"}', + t.deepEqual(inspectLogs(), [ + ['decoded EUD: osmo183dejcnmkka5dzcu9xw6mywq0p2m5peks28men'], + [ + 'Advance transfer fulfilled', + { + advanceAmount: { + brand: usdc.brand, + value: 146999999n, + }, + destination: { + chainId: 'osmosis-1', + encoding: 'bech32', + value: 'osmo183dejcnmkka5dzcu9xw6mywq0p2m5peks28men', + }, + result: undefined, + }, + ], ]); // We expect to see an `Advanced` update, but that is now Settler's job. @@ -232,9 +246,15 @@ test('updates status to OBSERVED on insufficient pool funds', async t => { 'OBSERVED status on insufficient pool funds', ); - t.deepEqual(inspectLogs(0), [ - 'Advancer error:', - '"[Error: Cannot borrow. Requested {\\"brand\\":\\"[Alleged: USDC brand]\\",\\"value\\":\\"[294999999n]\\"} must be less than pool balance {\\"brand\\":\\"[Alleged: USDC brand]\\",\\"value\\":\\"[1n]\\"}.]"', + t.deepEqual(inspectLogs(), [ + ['decoded EUD: dydx183dejcnmkka5dzcu9xw6mywq0p2m5peks28men'], + [ + 'Advancer error:', + Error( + 'Cannot borrow. Requested {"brand":"[Alleged: USDC brand]","value":"[294999999n]"} ' + + 'must be less than pool balance {"brand":"[Alleged: USDC brand]","value":"[1n]"}.', + ), + ], ]); }); @@ -256,9 +276,12 @@ test('updates status to OBSERVED if makeChainAddress fails', async t => { 'OBSERVED status on makeChainAddress failure', ); - t.deepEqual(inspectLogs(0), [ - 'Advancer error:', - '"[Error: Chain info not found for bech32Prefix \\"random\\"]"', + t.deepEqual(inspectLogs(), [ + ['decoded EUD: random1addr'], + [ + 'Advancer error:', + Error('Chain info not found for bech32Prefix "random"'), + ], ]); }); @@ -289,9 +312,9 @@ test('calls notifyAdvancingResult (AdvancedFailed) on failed transfer', async t mockPoolAccount.transferVResolver.reject(new Error('simulated error')); await eventLoopIteration(); - t.deepEqual(inspectLogs(0), [ - 'Advance transfer rejected', - '"[Error: simulated error]"', + t.deepEqual(inspectLogs(), [ + ['decoded EUD: dydx183dejcnmkka5dzcu9xw6mywq0p2m5peks28men'], + ['Advance transfer rejected', Error('simulated error')], ]); // We expect to see an `AdvancedFailed` update, but that is now Settler's job. @@ -334,9 +357,13 @@ test('updates status to OBSERVED if pre-condition checks fail', async t => { 'tx is recorded as OBSERVED', ); - t.deepEqual(inspectLogs(0), [ - 'Advancer error:', - '"[Error: Unable to parse query params: \\"agoric16kv2g7snfc4q24vg3pjdlnnqgngtjpwtetd2h689nz09lcklvh5s8u37ek\\"]"', + t.deepEqual(inspectLogs(), [ + [ + 'Advancer error:', + Error( + 'Unable to parse query params: "agoric16kv2g7snfc4q24vg3pjdlnnqgngtjpwtetd2h689nz09lcklvh5s8u37ek"', + ), + ], ]); }); @@ -347,6 +374,7 @@ test('will not advance same txHash:chainId evidence twice', async t => { helpers: { inspectLogs }, mocks: { mockPoolAccount, resolveLocalTransferV }, }, + brands: { usdc }, } = t.context; const mockEvidence = MockCctpTxEvidences.AGORIC_PLUS_OSMO(); @@ -357,17 +385,31 @@ test('will not advance same txHash:chainId evidence twice', async t => { mockPoolAccount.transferVResolver.resolve(); await eventLoopIteration(); - t.deepEqual(inspectLogs(0), [ - 'Advance transfer fulfilled', - '{"advanceAmount":{"brand":"[Alleged: USDC brand]","value":"[146999999n]"},"destination":{"chainId":"osmosis-1","encoding":"bech32","value":"osmo183dejcnmkka5dzcu9xw6mywq0p2m5peks28men"},"result":"[undefined]"}', + t.deepEqual(inspectLogs(), [ + ['decoded EUD: osmo183dejcnmkka5dzcu9xw6mywq0p2m5peks28men'], + [ + 'Advance transfer fulfilled', + { + advanceAmount: { brand: usdc.brand, value: 146999999n }, + destination: { + chainId: 'osmosis-1', + encoding: 'bech32', + value: 'osmo183dejcnmkka5dzcu9xw6mywq0p2m5peks28men', + }, + result: undefined, + }, + ], ]); // Second attempt void advancer.handleTransactionEvent(mockEvidence); await eventLoopIteration(); - t.deepEqual(inspectLogs(1), [ - 'txHash already seen:', - '0xc81bc6105b60a234c7c50ac17816ebcd5561d366df8bf3be59ff387552761702', + const [, , ...remainingLogs] = inspectLogs(); + t.deepEqual(remainingLogs, [ + [ + 'txHash already seen:', + '0xc81bc6105b60a234c7c50ac17816ebcd5561d366df8bf3be59ff387552761702', + ], ]); }); diff --git a/packages/orchestration/src/exos/local-orchestration-account.js b/packages/orchestration/src/exos/local-orchestration-account.js index 362ac8950d7..41d151475fd 100644 --- a/packages/orchestration/src/exos/local-orchestration-account.js +++ b/packages/orchestration/src/exos/local-orchestration-account.js @@ -690,7 +690,7 @@ export const prepareLocalOrchestrationAccountKit = ( 'agoric', forwardOpts, ); - trace('got transfer route', q(route).toString()); + trace('got transfer route', route); // set a `timeoutTimestamp` if caller does not supply either `timeoutHeight` or `timeoutTimestamp` // TODO #9324 what's a reasonable default? currently 5 minutes From b7065e715a68c6b73a3e5325eb88211509d3b1e2 Mon Sep 17 00:00:00 2001 From: 0xPatrick Date: Thu, 12 Dec 2024 14:28:25 -0500 Subject: [PATCH 3/5] chore: return advance payment to LP if `zoeTools.localTransfer` fails --- packages/fast-usdc/src/exos/advancer.js | 23 ++- packages/fast-usdc/src/exos/liquidity-pool.js | 29 ++- packages/fast-usdc/test/exos/advancer.test.ts | 177 ++++++++++++++++-- 3 files changed, 204 insertions(+), 25 deletions(-) diff --git a/packages/fast-usdc/src/exos/advancer.js b/packages/fast-usdc/src/exos/advancer.js index 9608b9de1cb..cc1facdb9eb 100644 --- a/packages/fast-usdc/src/exos/advancer.js +++ b/packages/fast-usdc/src/exos/advancer.js @@ -212,18 +212,31 @@ export const prepareAdvancerKit = ( }); }, /** + * We do not expect this to be a common failure. it should only occur + * if USDC is not registered in vbank or the tmpSeat has less than + * `advanceAmount`. + * + * If we do hit this path, we return funds to the Liquidity Pool and + * notify of Advancing failure. + * * @param {Error} error * @param {AdvancerVowCtx & { tmpSeat: ZCFSeat }} ctx */ - onRejected(error, { tmpSeat }) { - // TODO return seat allocation from ctx to LP? + onRejected(error, { tmpSeat, advanceAmount, ...restCtx }) { log( '⚠️ deposit to localOrchAccount failed, attempting to return payment to LP', error, ); - // TODO #10510 (comprehensive error testing) determine - // course of action here - log('TODO live payment on seat to return to LP', tmpSeat); + try { + const { borrowerFacet, notifyFacet } = this.state; + notifyFacet.notifyAdvancingResult(restCtx, false); + borrowerFacet.returnToPool( + tmpSeat, + harden({ USDC: advanceAmount }), + ); + } catch (e) { + log('🚨 deposit to localOrchAccount failure recovery failed', e); + } }, }, transferHandler: { diff --git a/packages/fast-usdc/src/exos/liquidity-pool.js b/packages/fast-usdc/src/exos/liquidity-pool.js index d8b2991a4df..21a7f612dfd 100644 --- a/packages/fast-usdc/src/exos/liquidity-pool.js +++ b/packages/fast-usdc/src/exos/liquidity-pool.js @@ -28,7 +28,7 @@ import { * @import {PoolStats} from '../types.js'; */ -const { add, isEqual, makeEmpty } = AmountMath; +const { add, isEqual, isGTE, makeEmpty } = AmountMath; /** @param {Brand} brand */ const makeDust = brand => AmountMath.make(brand, 1n); @@ -88,6 +88,10 @@ export const prepareLiquidityPoolKit = (zone, zcf, USDC, tools) => { SeatShape, harden({ USDC: makeNatAmountShape(USDC, 1n) }), ).returns(), + returnToPool: M.call( + SeatShape, + harden({ USDC: makeNatAmountShape(USDC, 1n) }), + ).returns(), }), repayer: M.interface('repayer', { repay: M.call( @@ -178,7 +182,28 @@ export const prepareLiquidityPoolKit = (zone, zcf, USDC, tools) => { Object.assign(this.state, post); this.facets.external.publishPoolMetrics(); }, - // TODO method to repay failed `LOA.deposit()` + /** + * If something fails during advance, return funds to the pool. + * + * @param {ZCFSeat} borrowSeat + * @param {{ USDC: Amount<'nat'>}} amountKWR + */ + returnToPool(borrowSeat, amountKWR) { + const { zcfSeat: repaySeat } = zcf.makeEmptySeatKit(); + const returnAmounts = harden({ + Principal: amountKWR.USDC, + PoolFee: makeEmpty(USDC), + ContractFee: makeEmpty(USDC), + }); + const borrowSeatAllocation = borrowSeat.getCurrentAllocation(); + isGTE(borrowSeatAllocation.USDC, amountKWR.USDC) || + Fail`⚠️ borrowSeatAllocation ${q(borrowSeatAllocation)} less than amountKWR ${q(amountKWR)}`; + // arrange payments in a format repay is expecting + zcf.atomicRearrange( + harden([[borrowSeat, repaySeat, amountKWR, returnAmounts]]), + ); + return this.facets.repayer.repay(repaySeat, returnAmounts); + }, }, repayer: { /** diff --git a/packages/fast-usdc/test/exos/advancer.test.ts b/packages/fast-usdc/test/exos/advancer.test.ts index 7c860d138d2..b6323e586a7 100644 --- a/packages/fast-usdc/test/exos/advancer.test.ts +++ b/packages/fast-usdc/test/exos/advancer.test.ts @@ -6,7 +6,7 @@ import fetchedChainInfo from '@agoric/orchestration/src/fetched-chain-info.js'; import { Far } from '@endo/pass-style'; import type { NatAmount } from '@agoric/ertp'; import { type ZoeTools } from '@agoric/orchestration/src/utils/zoe-tools.js'; -import { q } from '@endo/errors'; +import { Fail, q } from '@endo/errors'; import { PendingTxStatus } from '../../src/constants.js'; import { prepareAdvancer } from '../../src/exos/advancer.js'; import type { SettlerKit } from '../../src/exos/settler.js'; @@ -20,6 +20,7 @@ import { makeTestLogger, prepareMockOrchAccounts, } from '../mocks.js'; +import type { LiquidityPoolKit } from '../../src/types.js'; const LOCAL_DENOM = `ibc/${denomHash({ denom: 'uusdc', @@ -62,6 +63,11 @@ const createTestExtensions = (t, common: CommonSetup) => { // pretend funds move from tmpSeat to poolAccount localTransferVK.resolver.resolve(); }; + const rejectLocalTransfeferV = () => { + localTransferVK.resolver.reject( + new Error('One or more deposits failed: simulated error'), + ); + }; const mockZoeTools = Far('MockZoeTools', { localTransfer(...args: Parameters) { console.log('ZoeTools.localTransfer called with', args); @@ -94,18 +100,17 @@ const createTestExtensions = (t, common: CommonSetup) => { }, }); + const mockBorrowerFacetCalls: { + borrow: Parameters[]; + returnToPool: Parameters[]; + } = { borrow: [], returnToPool: [] }; + const mockBorrowerF = Far('LiquidityPool Borrow Facet', { borrow: (seat: ZCFSeat, amounts: { USDC: NatAmount }) => { - console.log('LP.borrow called with', amounts); + mockBorrowerFacetCalls.borrow.push([seat, amounts]); }, - }); - - const mockBorrowerErrorF = Far('LiquidityPool Borrow Facet', { - borrow: (seat: ZCFSeat, amounts: { USDC: NatAmount }) => { - console.log('LP.borrow called with', amounts); - throw new Error( - `Cannot borrow. Requested ${q(amounts.USDC)} must be less than pool balance ${q(usdc.make(1n))}.`, - ); + returnToPool: (seat: ZCFSeat, amounts: { USDC: NatAmount }) => { + mockBorrowerFacetCalls.returnToPool.push([seat, amounts]); }, }); @@ -124,12 +129,13 @@ const createTestExtensions = (t, common: CommonSetup) => { helpers: { inspectLogs, inspectNotifyCalls: () => harden(notifyAdvancingResultCalls), + inspectBorrowerFacetCalls: () => harden(mockBorrowerFacetCalls), }, mocks: { ...mockAccounts, - mockBorrowerErrorF, mockNotifyF, resolveLocalTransferV, + rejectLocalTransfeferV, }, services: { advancer, @@ -220,17 +226,27 @@ test('updates status to ADVANCING in happy path', async t => { test('updates status to OBSERVED on insufficient pool funds', async t => { const { + brands: { usdc }, bootstrap: { storage }, extensions: { services: { makeAdvancer, statusManager }, helpers: { inspectLogs }, - mocks: { mockPoolAccount, mockBorrowerErrorF, mockNotifyF }, + mocks: { mockPoolAccount, mockNotifyF }, }, } = t.context; + const mockBorrowerFacet = Far('LiquidityPool Borrow Facet', { + borrow: (seat: ZCFSeat, amounts: { USDC: NatAmount }) => { + throw new Error( + `Cannot borrow. Requested ${q(amounts.USDC)} must be less than pool balance ${q(usdc.make(1n))}.`, + ); + }, + returnToPool: () => {}, // not expecting this to be called + }); + // make a new advancer that intentionally throws const advancer = makeAdvancer({ - borrowerFacet: mockBorrowerErrorF, + borrowerFacet: mockBorrowerFacet, notifyFacet: mockNotifyF, poolAccount: mockPoolAccount.account, intermediateRecipient, @@ -251,8 +267,7 @@ test('updates status to OBSERVED on insufficient pool funds', async t => { [ 'Advancer error:', Error( - 'Cannot borrow. Requested {"brand":"[Alleged: USDC brand]","value":"[294999999n]"} ' + - 'must be less than pool balance {"brand":"[Alleged: USDC brand]","value":"[1n]"}.', + `Cannot borrow. Requested ${q(usdc.make(294999999n))} must be less than pool balance ${q(usdc.make(1n))}.`, ), ], ]); @@ -413,6 +428,132 @@ test('will not advance same txHash:chainId evidence twice', async t => { ]); }); -test.todo( - '#10510 zoeTools.localTransfer fails to deposit borrowed USDC to LOA', -); +test('returns payment to LP if zoeTools.localTransfer fails', async t => { + const { + extensions: { + services: { advancer }, + helpers: { inspectLogs, inspectBorrowerFacetCalls, inspectNotifyCalls }, + mocks: { rejectLocalTransfeferV }, + }, + brands: { usdc }, + } = t.context; + const mockEvidence = MockCctpTxEvidences.AGORIC_PLUS_OSMO(); + + void advancer.handleTransactionEvent(mockEvidence); + rejectLocalTransfeferV(); + + await eventLoopIteration(); + + t.deepEqual( + inspectLogs(), + [ + ['decoded EUD: osmo183dejcnmkka5dzcu9xw6mywq0p2m5peks28men'], + [ + '⚠️ deposit to localOrchAccount failed, attempting to return payment to LP', + Error('One or more deposits failed: simulated error'), + ], + ], + 'contract logs report error', + ); + + const { borrow, returnToPool } = inspectBorrowerFacetCalls(); + + const expectedArguments = [ + Far('MockZCFSeat', {}), + { USDC: usdc.make(146999999n) }, // net of fees + ]; + + t.is(borrow.length, 1, 'borrow is called before zt.localTransfer fails'); + t.deepEqual(borrow[0], expectedArguments, 'borrow arguments match expected'); + + t.is( + returnToPool.length, + 1, + 'returnToPool is called after zt.localTransfer fails', + ); + t.deepEqual( + returnToPool[0], + expectedArguments, + 'same amount borrowed is returned to LP', + ); + + t.like( + inspectNotifyCalls(), + [ + [ + { + txHash: mockEvidence.txHash, + forwardingAddress: mockEvidence.tx.forwardingAddress, + }, + false, // indicates advance failed + ], + ], + 'Advancing tx is recorded as AdvanceFailed', + ); +}); + +test('alerts if `returnToPool` fallback fails', async t => { + const { + brands: { usdc }, + extensions: { + services: { makeAdvancer }, + helpers: { inspectLogs, inspectNotifyCalls }, + mocks: { mockPoolAccount, mockNotifyF, rejectLocalTransfeferV }, + }, + } = t.context; + + const mockBorrowerFacet = Far('LiquidityPool Borrow Facet', { + borrow: (seat: ZCFSeat, amounts: { USDC: NatAmount }) => { + // note: will not be tracked by `inspectBorrowerFacetCalls` + }, + returnToPool: (seat: ZCFSeat, amounts: { USDC: NatAmount }) => { + throw new Error( + `⚠️ borrowSeatAllocation ${q({ USDC: usdc.make(0n) })} less than amountKWR ${q(amounts)}`, + ); + }, + }); + + // make a new advancer that intentionally throws during returnToPool + const advancer = makeAdvancer({ + borrowerFacet: mockBorrowerFacet, + notifyFacet: mockNotifyF, + poolAccount: mockPoolAccount.account, + intermediateRecipient, + }); + + const mockEvidence = MockCctpTxEvidences.AGORIC_PLUS_OSMO(); + void advancer.handleTransactionEvent(mockEvidence); + rejectLocalTransfeferV(); + + await eventLoopIteration(); + + t.deepEqual(inspectLogs(), [ + ['decoded EUD: osmo183dejcnmkka5dzcu9xw6mywq0p2m5peks28men'], + [ + '⚠️ deposit to localOrchAccount failed, attempting to return payment to LP', + Error('One or more deposits failed: simulated error'), + ], + [ + '🚨 deposit to localOrchAccount failure recovery failed', + Error( + `⚠️ borrowSeatAllocation ${q({ USDC: usdc.make(0n) })} less than amountKWR ${q({ USDC: usdc.make(146999999n) })}`, + ), + ], + ]); + + await eventLoopIteration(); + + t.like( + inspectNotifyCalls(), + [ + [ + { + txHash: mockEvidence.txHash, + forwardingAddress: mockEvidence.tx.forwardingAddress, + }, + false, // indicates advance failed + ], + ], + 'Advancing tx is recorded as AdvanceFailed', + ); +}); From 81517e21746d3b6d7be1642a4e68a72643741e65 Mon Sep 17 00:00:00 2001 From: 0xPatrick Date: Thu, 12 Dec 2024 16:53:03 -0500 Subject: [PATCH 4/5] chore: do not call `shutdownWithFailure()` if `atomicRearrange()` fails - wait until #10684, where we can terminate an incarnation without terminating the contract - see https://github.com/Agoric/agoric-sdk/pull/10659#discussion_r1878631017 - note: try/catch/finally remains for `borrow()` and `deposit()` so we can exit seats --- packages/fast-usdc/src/exos/liquidity-pool.js | 58 ++++++++----------- 1 file changed, 23 insertions(+), 35 deletions(-) diff --git a/packages/fast-usdc/src/exos/liquidity-pool.js b/packages/fast-usdc/src/exos/liquidity-pool.js index 21a7f612dfd..9cdd37fee94 100644 --- a/packages/fast-usdc/src/exos/liquidity-pool.js +++ b/packages/fast-usdc/src/exos/liquidity-pool.js @@ -171,13 +171,8 @@ export const prepareLiquidityPoolKit = (zone, zcf, USDC, tools) => { ); // COMMIT POINT - try { - zcf.atomicRearrange(harden([[poolSeat, toSeat, amountKWR]])); - } catch (cause) { - const reason = Error('🚨 cannot commit borrow', { cause }); - console.error(reason.message, cause); - zcf.shutdownWithFailure(reason); - } + // UNTIL #10684: ability to terminate an incarnation w/o terminating the contract + zcf.atomicRearrange(harden([[poolSeat, toSeat, amountKWR]])); Object.assign(this.state, post); this.facets.external.publishPoolMetrics(); @@ -233,23 +228,18 @@ export const prepareLiquidityPoolKit = (zone, zcf, USDC, tools) => { const { ContractFee, ...rest } = amounts; // COMMIT POINT - try { - zcf.atomicRearrange( - harden([ - [ - fromSeat, - poolSeat, - rest, - { USDC: add(amounts.PoolFee, amounts.Principal) }, - ], - [fromSeat, feeSeat, { ContractFee }, { USDC: ContractFee }], - ]), - ); - } catch (cause) { - const reason = Error('🚨 cannot commit repay', { cause }); - console.error(reason.message, cause); - zcf.shutdownWithFailure(reason); - } + // UNTIL #10684: ability to terminate an incarnation w/o terminating the contract + zcf.atomicRearrange( + harden([ + [ + fromSeat, + poolSeat, + rest, + { USDC: add(amounts.PoolFee, amounts.Principal) }, + ], + [fromSeat, feeSeat, { ContractFee }, { USDC: ContractFee }], + ]), + ); Object.assign(this.state, post); this.facets.external.publishPoolMetrics(); @@ -284,9 +274,8 @@ export const prepareLiquidityPoolKit = (zone, zcf, USDC, tools) => { const post = depositCalc(shareWorth, proposal); // COMMIT POINT - + const mint = shareMint.mintGains(post.payouts); try { - const mint = shareMint.mintGains(post.payouts); this.state.shareWorth = post.shareWorth; zcf.atomicRearrange( harden([ @@ -296,12 +285,12 @@ export const prepareLiquidityPoolKit = (zone, zcf, USDC, tools) => { [mint, lp, post.payouts], ]), ); + } catch (cause) { + // UNTIL #10684: ability to terminate an incarnation w/o terminating the contract + throw new Error('🚨 cannot commit deposit', { cause }); + } finally { lp.exit(); mint.exit(); - } catch (cause) { - const reason = Error('🚨 cannot commit deposit', { cause }); - console.error(reason.message, cause); - zcf.shutdownWithFailure(reason); } external.publishPoolMetrics(); }, @@ -321,7 +310,6 @@ export const prepareLiquidityPoolKit = (zone, zcf, USDC, tools) => { const post = withdrawCalc(shareWorth, proposal); // COMMIT POINT - try { this.state.shareWorth = post.shareWorth; zcf.atomicRearrange( @@ -333,12 +321,12 @@ export const prepareLiquidityPoolKit = (zone, zcf, USDC, tools) => { ]), ); shareMint.burnLosses(proposal.give, burn); + } catch (cause) { + // UNTIL #10684: ability to terminate an incarnation w/o terminating the contract + throw new Error('🚨 cannot commit withdraw', { cause }); + } finally { lp.exit(); burn.exit(); - } catch (cause) { - const reason = Error('🚨 cannot commit withdraw', { cause }); - console.error(reason.message, cause); - zcf.shutdownWithFailure(reason); } external.publishPoolMetrics(); }, From 50c1bd13affc72375802a061dbf58ba84088fc0c Mon Sep 17 00:00:00 2001 From: 0xPatrick Date: Thu, 12 Dec 2024 17:08:58 -0500 Subject: [PATCH 5/5] refactor: `LP.borrowerFacet` expects `Amount`, not `AmountKWR` --- packages/fast-usdc/src/exos/advancer.js | 10 ++----- packages/fast-usdc/src/exos/liquidity-pool.js | 30 ++++++++----------- packages/fast-usdc/test/exos/advancer.test.ts | 22 +++++++------- 3 files changed, 26 insertions(+), 36 deletions(-) diff --git a/packages/fast-usdc/src/exos/advancer.js b/packages/fast-usdc/src/exos/advancer.js index cc1facdb9eb..4fc29f6c253 100644 --- a/packages/fast-usdc/src/exos/advancer.js +++ b/packages/fast-usdc/src/exos/advancer.js @@ -161,9 +161,8 @@ export const prepareAdvancerKit = ( const advanceAmount = feeTools.calculateAdvance(fullAmount); const { zcfSeat: tmpSeat } = zcf.makeEmptySeatKit(); - const amountKWR = harden({ USDC: advanceAmount }); // throws if the pool has insufficient funds - borrowerFacet.borrow(tmpSeat, amountKWR); + borrowerFacet.borrow(tmpSeat, advanceAmount); // this cannot throw since `.isSeen()` is called in the same turn statusManager.advance(evidence); @@ -172,7 +171,7 @@ export const prepareAdvancerKit = ( tmpSeat, // @ts-expect-error LocalAccountMethods vs OrchestrationAccount poolAccount, - amountKWR, + harden({ USDC: advanceAmount }), ); void watch(depositV, this.facets.depositHandler, { fullAmount, @@ -230,10 +229,7 @@ export const prepareAdvancerKit = ( try { const { borrowerFacet, notifyFacet } = this.state; notifyFacet.notifyAdvancingResult(restCtx, false); - borrowerFacet.returnToPool( - tmpSeat, - harden({ USDC: advanceAmount }), - ); + borrowerFacet.returnToPool(tmpSeat, advanceAmount); } catch (e) { log('🚨 deposit to localOrchAccount failure recovery failed', e); } diff --git a/packages/fast-usdc/src/exos/liquidity-pool.js b/packages/fast-usdc/src/exos/liquidity-pool.js index 9cdd37fee94..5df4c86bbf2 100644 --- a/packages/fast-usdc/src/exos/liquidity-pool.js +++ b/packages/fast-usdc/src/exos/liquidity-pool.js @@ -84,14 +84,8 @@ export const prepareLiquidityPoolKit = (zone, zcf, USDC, tools) => { 'Liquidity Pool', { borrower: M.interface('borrower', { - borrow: M.call( - SeatShape, - harden({ USDC: makeNatAmountShape(USDC, 1n) }), - ).returns(), - returnToPool: M.call( - SeatShape, - harden({ USDC: makeNatAmountShape(USDC, 1n) }), - ).returns(), + borrow: M.call(SeatShape, makeNatAmountShape(USDC, 1n)).returns(), + returnToPool: M.call(SeatShape, makeNatAmountShape(USDC, 1n)).returns(), }), repayer: M.interface('repayer', { repay: M.call( @@ -157,14 +151,14 @@ export const prepareLiquidityPoolKit = (zone, zcf, USDC, tools) => { borrower: { /** * @param {ZCFSeat} toSeat - * @param {{ USDC: Amount<'nat'>}} amountKWR + * @param {Amount<'nat'>} amount */ - borrow(toSeat, amountKWR) { + borrow(toSeat, amount) { const { encumberedBalance, poolSeat, poolStats } = this.state; // Validate amount is available in pool const post = borrowCalc( - amountKWR.USDC, + amount, poolSeat.getAmountAllocated('USDC', USDC), encumberedBalance, poolStats, @@ -172,7 +166,7 @@ export const prepareLiquidityPoolKit = (zone, zcf, USDC, tools) => { // COMMIT POINT // UNTIL #10684: ability to terminate an incarnation w/o terminating the contract - zcf.atomicRearrange(harden([[poolSeat, toSeat, amountKWR]])); + zcf.atomicRearrange(harden([[poolSeat, toSeat, { USDC: amount }]])); Object.assign(this.state, post); this.facets.external.publishPoolMetrics(); @@ -181,21 +175,21 @@ export const prepareLiquidityPoolKit = (zone, zcf, USDC, tools) => { * If something fails during advance, return funds to the pool. * * @param {ZCFSeat} borrowSeat - * @param {{ USDC: Amount<'nat'>}} amountKWR + * @param {Amount<'nat'>} amount */ - returnToPool(borrowSeat, amountKWR) { + returnToPool(borrowSeat, amount) { const { zcfSeat: repaySeat } = zcf.makeEmptySeatKit(); const returnAmounts = harden({ - Principal: amountKWR.USDC, + Principal: amount, PoolFee: makeEmpty(USDC), ContractFee: makeEmpty(USDC), }); const borrowSeatAllocation = borrowSeat.getCurrentAllocation(); - isGTE(borrowSeatAllocation.USDC, amountKWR.USDC) || - Fail`⚠️ borrowSeatAllocation ${q(borrowSeatAllocation)} less than amountKWR ${q(amountKWR)}`; + isGTE(borrowSeatAllocation.USDC, amount) || + Fail`⚠️ borrowSeatAllocation ${q(borrowSeatAllocation)} less than amountKWR ${q(amount)}`; // arrange payments in a format repay is expecting zcf.atomicRearrange( - harden([[borrowSeat, repaySeat, amountKWR, returnAmounts]]), + harden([[borrowSeat, repaySeat, { USDC: amount }, returnAmounts]]), ); return this.facets.repayer.repay(repaySeat, returnAmounts); }, diff --git a/packages/fast-usdc/test/exos/advancer.test.ts b/packages/fast-usdc/test/exos/advancer.test.ts index b6323e586a7..d724e4a1cf8 100644 --- a/packages/fast-usdc/test/exos/advancer.test.ts +++ b/packages/fast-usdc/test/exos/advancer.test.ts @@ -106,11 +106,11 @@ const createTestExtensions = (t, common: CommonSetup) => { } = { borrow: [], returnToPool: [] }; const mockBorrowerF = Far('LiquidityPool Borrow Facet', { - borrow: (seat: ZCFSeat, amounts: { USDC: NatAmount }) => { - mockBorrowerFacetCalls.borrow.push([seat, amounts]); + borrow: (seat: ZCFSeat, amount: NatAmount) => { + mockBorrowerFacetCalls.borrow.push([seat, amount]); }, - returnToPool: (seat: ZCFSeat, amounts: { USDC: NatAmount }) => { - mockBorrowerFacetCalls.returnToPool.push([seat, amounts]); + returnToPool: (seat: ZCFSeat, amount: NatAmount) => { + mockBorrowerFacetCalls.returnToPool.push([seat, amount]); }, }); @@ -236,9 +236,9 @@ test('updates status to OBSERVED on insufficient pool funds', async t => { } = t.context; const mockBorrowerFacet = Far('LiquidityPool Borrow Facet', { - borrow: (seat: ZCFSeat, amounts: { USDC: NatAmount }) => { + borrow: (seat: ZCFSeat, amount: NatAmount) => { throw new Error( - `Cannot borrow. Requested ${q(amounts.USDC)} must be less than pool balance ${q(usdc.make(1n))}.`, + `Cannot borrow. Requested ${q(amount)} must be less than pool balance ${q(usdc.make(1n))}.`, ); }, returnToPool: () => {}, // not expecting this to be called @@ -460,7 +460,7 @@ test('returns payment to LP if zoeTools.localTransfer fails', async t => { const expectedArguments = [ Far('MockZCFSeat', {}), - { USDC: usdc.make(146999999n) }, // net of fees + usdc.make(146999999n), // net of fees ]; t.is(borrow.length, 1, 'borrow is called before zt.localTransfer fails'); @@ -503,12 +503,12 @@ test('alerts if `returnToPool` fallback fails', async t => { } = t.context; const mockBorrowerFacet = Far('LiquidityPool Borrow Facet', { - borrow: (seat: ZCFSeat, amounts: { USDC: NatAmount }) => { + borrow: (seat: ZCFSeat, amount: NatAmount) => { // note: will not be tracked by `inspectBorrowerFacetCalls` }, - returnToPool: (seat: ZCFSeat, amounts: { USDC: NatAmount }) => { + returnToPool: (seat: ZCFSeat, amount: NatAmount) => { throw new Error( - `⚠️ borrowSeatAllocation ${q({ USDC: usdc.make(0n) })} less than amountKWR ${q(amounts)}`, + `⚠️ borrowSeatAllocation ${q({ USDC: usdc.make(0n) })} less than amountKWR ${q(amount)}`, ); }, }); @@ -536,7 +536,7 @@ test('alerts if `returnToPool` fallback fails', async t => { [ '🚨 deposit to localOrchAccount failure recovery failed', Error( - `⚠️ borrowSeatAllocation ${q({ USDC: usdc.make(0n) })} less than amountKWR ${q({ USDC: usdc.make(146999999n) })}`, + `⚠️ borrowSeatAllocation ${q({ USDC: usdc.make(0n) })} less than amountKWR ${q(usdc.make(146999999n))}`, ), ], ]);