Skip to content

Commit

Permalink
feat: zoeTools.localTransfer error paths (#9902)
Browse files Browse the repository at this point in the history
closes: #9925
refs: #9193

## Description
- This PR primarily hardens the `zoeTools` implementation for orchestrated async-flows by:
  - ensuring payments handled in `zoeTools.localTransfer` are recovered to the original seat in the event of failure or partial failure
  - providing a `zoeTools.withdrawFromSeat` retriable function to facilitate withdrawing funds from a LocalChainAccount to a User seat. Ensures all changes are rolled back in the event of partial failure.
   - using `asVow` instead of `retriable` since these operations are not idempotent
- Creates `src/fixtures/zoe-tools.contract.js` to facilitate testing error paths
- To facilitate the change, `vowTools.allVowsSettled` was implemented in `@agoric/vow`. See #10077 which this PR depends on

### Security Considerations
Improves safety around Payment handling in async-flow contracts in `@agoric/orchestration`

### Scaling Considerations
n/a

### Documentation Considerations
Updates jsdoc strings for `localTransfer` and `withdrawToSeat`. As maintainer notes for considerations around `asVow` and promptness.

### Testing Considerations
Includes tests with different failure paths previously unaccounted for.

### Upgrade Considerations
Unreleased code that will be part of an npm release
  • Loading branch information
mergify[bot] authored Sep 19, 2024
2 parents 6ee57ba + ea73ef7 commit 3e9ff43
Show file tree
Hide file tree
Showing 19 changed files with 1,075 additions and 68 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ const contract = async (
const orchFns = orchestrateAll(flows, {
zcf,
contractState,
localTransfer: zoeTools.localTransfer,
zoeTools,
});

const publicFacet = zone.exo(
Expand Down
31 changes: 20 additions & 11 deletions packages/orchestration/src/examples/send-anywhere.flows.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { NonNullish } from '@agoric/internal';
import { makeError, q } from '@endo/errors';
import { M, mustMatch } from '@endo/patterns';

/**
* @import {GuestOf} from '@agoric/async-flow';
* @import {GuestInterface} from '@agoric/async-flow';
* @import {ZoeTools} from '../utils/zoe-tools.js';
* @import {Orchestrator, LocalAccountMethods, OrchestrationAccountI, OrchestrationFlow} from '../types.js';
*/
Expand All @@ -17,13 +18,13 @@ const { entries } = Object;
* @param {Orchestrator} orch
* @param {object} ctx
* @param {{ localAccount?: OrchestrationAccountI & LocalAccountMethods }} ctx.contractState
* @param {GuestOf<ZoeTools['localTransfer']>} ctx.localTransfer
* @param {GuestInterface<ZoeTools>} ctx.zoeTools
* @param {ZCFSeat} seat
* @param {{ chainName: string; destAddr: string }} offerArgs
*/
export const sendIt = async (
orch,
{ contractState, localTransfer },
{ contractState, zoeTools: { localTransfer, withdrawToSeat } },
seat,
offerArgs,
) => {
Expand Down Expand Up @@ -51,14 +52,22 @@ export const sendIt = async (

await localTransfer(seat, contractState.localAccount, give);

await contractState.localAccount.transfer(
{ denom, value: amt.value },
{
value: destAddr,
encoding: 'bech32',
chainId,
},
);
try {
await contractState.localAccount.transfer(
{ denom, value: amt.value },
{
value: destAddr,
encoding: 'bech32',
chainId,
},
);
} catch (e) {
await withdrawToSeat(contractState.localAccount, seat, give);
const errorMsg = `IBC Transfer failed ${q(e)}`;
seat.exit(errorMsg);
throw makeError(errorMsg);
}

seat.exit();
};
harden(sendIt);
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
*/

import { mustMatch } from '@endo/patterns';
import { makeError } from '@endo/errors';
import { makeError, q } from '@endo/errors';
import { makeTracer } from '@agoric/internal';
import { ChainAddressShape } from '../typeGuards.js';

Expand Down Expand Up @@ -76,9 +76,10 @@ export const depositAndDelegate = async (
try {
await contractState.localAccount.transfer(give.Stake, address);
} catch (cause) {
// TODO, put funds back on user seat and exit
// https://github.com/Agoric/agoric-sdk/issues/9925
throw makeError('ibc transfer failed', undefined, { cause });
await zoeTools.withdrawToSeat(contractState.localAccount, seat, give);
const errMsg = makeError(`ibc transfer failed ${q(cause)}`);
seat.exit(errMsg);
throw errMsg;
}
seat.exit();
await account.delegate(validator, give.Stake);
Expand Down
4 changes: 1 addition & 3 deletions packages/orchestration/src/utils/start-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ export const provideOrchestration = (
orchestration: zone.subZone('orchestration'),
/** system names for vows */
vows: zone.subZone('vows'),
/** system names for zoe */
zoe: zone.subZone('zoe'),
/** contract-provided names, and subzones */
contract: zone.subZone('contract'),
};
Expand All @@ -76,7 +74,7 @@ export const provideOrchestration = (

const chainHub = makeChainHub(agoricNames, vowTools);

const zoeTools = makeZoeTools(zones.zoe, { zcf, vowTools });
const zoeTools = makeZoeTools(zcf, vowTools);

const { makeRecorderKit } = prepareRecorderKitMakers(baggage, marshaller);
const makeLocalOrchestrationAccountKit = prepareLocalOrchestrationAccountKit(
Expand Down
167 changes: 135 additions & 32 deletions packages/orchestration/src/utils/zoe-tools.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,31 @@
import { Fail } from '@endo/errors';
import { atomicTransfer } from '@agoric/zoe/src/contractSupport/index.js';
/**
* @file Helper functions for transferring payments between a LocalChainAccount
* and a ZCFSeat.
*
* Maintainers: This exists as an endowment for orchestrated async-flows so we
* can make use of E and promises. The logic for recovering partial failures
* is also an added convenience for developers.
*
* Functions are written using `asVow` and non-resumable promises as we expect
* each invocation to resolve promptly - there are no timers or interchain
* network calls.
*
* A promise resolved promptly is currently safe from being severed by an
* upgrade because we only trigger vat upgrades as the result of network
* input.
*/

import { makeError, q, Fail } from '@endo/errors';
import { depositToSeat } from '@agoric/zoe/src/contractSupport/index.js';
import { E } from '@endo/far';

const { assign, keys, values } = Object;

/**
* @import {HostOf} from '@agoric/async-flow';
* @import {InvitationMakers} from '@agoric/smart-wallet/src/types.js';
* @import {ResolvedPublicTopic} from '@agoric/zoe/src/contractSupport/topics.js';
* @import {Vow, VowTools} from '@agoric/vow';
* @import {Zone} from '@agoric/zone';
* @import {OrchestrationAccount} from '../orchestration-api.js'
* @import {VowTools} from '@agoric/vow';
* @import {LocalAccountMethods} from '../types.js';
*/

Expand All @@ -24,60 +42,145 @@ import { E } from '@endo/far';
* @typedef {(
* srcSeat: ZCFSeat,
* localAccount: LocalAccountMethods,
* give: AmountKeywordRecord,
* amounts: AmountKeywordRecord,
* ) => Promise<void>} LocalTransfer
*/

/**
* @param {Zone} zone
* @param {{ zcf: ZCF; vowTools: VowTools }} io
* @typedef {(
* localAccount: LocalAccountMethods,
* destSeat: ZCFSeat,
* amounts: AmountKeywordRecord,
* ) => Promise<void>} WithdrawToSeat
*/

/**
* @param {ZCF} zcf
* @param {VowTools} vowTools
*/
export const makeZoeTools = (zone, { zcf, vowTools }) => {
export const makeZoeTools = (zcf, { when, allVows, allSettled, asVow }) => {
/**
* Transfer the `give` a seat to a local account.
* Transfer the `amounts` from `srcSeat` to `localAccount`. If any of the
* deposits fail, everything will be rolled back to the `srcSeat`. Supports
* multiple items in the `amounts` {@link AmountKeywordRecord}.
*
* @type {HostOf<LocalTransfer>}
*/
const localTransfer = vowTools.retriable(
zone,
'localTransfer',
/**
* @type {LocalTransfer}
*/
async (srcSeat, localAccount, give) => {
const localTransfer = (srcSeat, localAccount, amounts) =>
asVow(async () => {
!srcSeat.hasExited() || Fail`The seat cannot have exited.`;
const { zcfSeat: tempSeat, userSeat: userSeatP } = zcf.makeEmptySeatKit();
const userSeat = await userSeatP;
atomicTransfer(zcf, srcSeat, tempSeat, give);
zcf.atomicRearrange(harden([[srcSeat, tempSeat, amounts]]));
tempSeat.exit();
// TODO get the userSeat into baggage so it's at least recoverable
// TODO (#9541) get the userSeat into baggage so it's at least recoverable
// const userSeat = await subzone.makeOnce(
// 'localTransferHelper',
// async () => {
// const { zcfSeat: tempSeat, userSeat: userSeatP } =
// zcf.makeEmptySeatKit();
// const uSeat = await userSeatP;
// // TODO how do I store in the place for this retriable?
// atomicTransfer(zcf, srcSeat, tempSeat, give);
// atomicTransfer(zcf, srcSeat, tempSeat, amounts);
// tempSeat.exit();
// return uSeat;
// },
// );

// Now all the `give` are accessible, so we can move them to the localAccount`
// Now all the `amounts` are accessible, so we can move them to the localAccount
const payments = await Promise.all(
keys(amounts).map(kw => E(userSeat).getPayout(kw)),
);
const settleDeposits = await when(
allSettled(payments.map(pmt => E(localAccount).deposit(pmt))),
);
// if any of the deposits to localAccount failed, unwind all of the allocations
if (settleDeposits.find(x => x.status === 'rejected')) {
const amts = values(amounts);
const errors = [];
// withdraw the successfully deposited payments
const paymentsOrWithdrawVs = settleDeposits.map((x, i) => {
if (x.status === 'rejected') {
errors.push(x.reason);
return payments[i];
}
return E(localAccount).withdraw(amts[i]);
});

const depositVs = Object.entries(give).map(async ([kw, _amount]) => {
const pmt = await E(userSeat).getPayout(kw);
// TODO arrange recovery on upgrade of pmt?
return localAccount.deposit(pmt);
});
await vowTools.when(vowTools.allVows(depositVs));
// TODO remove userSeat from baggage
// TODO reject non-vbank issuers
// TODO recover failed deposits
},
);
// return all payments to the srcSeat
const paymentsToReturn = await when(allVows(paymentsOrWithdrawVs));
const paymentKwr = harden(
keys(amounts).reduce(
(kwr, kw, i) => assign(kwr, { [kw]: paymentsToReturn[i] }),
{},
),
);
const depositResponse = await depositToSeat(
zcf,
srcSeat,
amounts,
paymentKwr,
);
console.debug(depositResponse);
throw makeError(`One or more deposits failed ${q(errors)}`);
}
// TODO #9541 remove userSeat from baggage
});

/**
* Transfer the `amounts` from a `localAccount` to the `recipientSeat`. If any
* of the withdrawals fail, everything will be rolled back to the
* `srcLocalAccount`. Supports multiple items in the `amounts`
* {@link PaymentKeywordRecord}
*
* @type {HostOf<WithdrawToSeat>}
*/
const withdrawToSeat = (localAccount, destSeat, amounts) =>
asVow(async () => {
!destSeat.hasExited() || Fail`The seat cannot have exited.`;

const settledWithdrawals = await when(
allSettled(values(amounts).map(amt => E(localAccount).withdraw(amt))),
);

// if any of the withdrawals were rejected, unwind the successful ones
if (settledWithdrawals.find(x => x.status === 'rejected')) {
const returnPaymentVs = [];
const errors = [];
for (const result of settledWithdrawals) {
if (result.status === 'fulfilled') {
returnPaymentVs.push(E(localAccount).deposit(result.value));
} else {
errors.push(result.reason);
}
}
await when(allVows(returnPaymentVs));
throw makeError(`One or more withdrawals failed ${q(errors)}`);
}
// successfully withdrew payments from srcLocalAccount, deposit to recipientSeat
const paymentKwr = harden(
keys(amounts).reduce(
(acc, kw, i) =>
assign(acc, {
[kw]: /** @type {{ value: Amount }[]} */ (settledWithdrawals)[i]
.value,
}),
{},
),
);
const depositResponse = await depositToSeat(
zcf,
destSeat,
amounts,
paymentKwr,
);
console.debug(depositResponse);
});

return harden({
localTransfer,
withdrawToSeat,
});
};

/** @typedef {ReturnType<typeof makeZoeTools>} ZoeTools */
Loading

0 comments on commit 3e9ff43

Please sign in to comment.