From 6d93548fc222807798cab5bd0e9dd0da5edcc5b1 Mon Sep 17 00:00:00 2001 From: Turadg Aleahmad Date: Wed, 17 Jul 2024 13:57:52 -0700 Subject: [PATCH 1/8] chore: type-coverage --update --- packages/async-flow/package.json | 2 +- packages/orchestration/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/async-flow/package.json b/packages/async-flow/package.json index 00cacb143c3..eeb9ae506de 100644 --- a/packages/async-flow/package.json +++ b/packages/async-flow/package.json @@ -61,6 +61,6 @@ "workerThreads": false }, "typeCoverage": { - "atLeast": 76.94 + "atLeast": 77.01 } } diff --git a/packages/orchestration/package.json b/packages/orchestration/package.json index e8bfeb7c7f5..d2c37ec1d18 100644 --- a/packages/orchestration/package.json +++ b/packages/orchestration/package.json @@ -92,6 +92,6 @@ "access": "public" }, "typeCoverage": { - "atLeast": 98.09 + "atLeast": 98.05 } } From 9d001a50153a25b249430b78f0d530ca1f410b56 Mon Sep 17 00:00:00 2001 From: Turadg Aleahmad Date: Wed, 17 Jul 2024 13:56:59 -0700 Subject: [PATCH 2/8] chore(types): free HostAsyncFuncWrapper params --- packages/async-flow/src/types.d.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/async-flow/src/types.d.ts b/packages/async-flow/src/types.d.ts index 85907144acd..114ae65aa51 100644 --- a/packages/async-flow/src/types.d.ts +++ b/packages/async-flow/src/types.d.ts @@ -31,7 +31,9 @@ export type GuestAsyncFunc = ( ...activationArgs: Guest[] ) => Guest>; -export type HostAsyncFuncWrapper = (...activationArgs: Host[]) => HostVow; +export type HostAsyncFuncWrapper = ( + ...activationArgs: Host[] +) => HostVow; /** * The function from the host as it will be available in the guest. From d283cdd44266618a2a8ed088f740351fbcc281af Mon Sep 17 00:00:00 2001 From: Turadg Aleahmad Date: Thu, 18 Jul 2024 09:28:14 -0700 Subject: [PATCH 3/8] refactor: rename sendAnywhere.flows.js --- packages/orchestration/src/examples/sendAnywhere.contract.js | 2 +- .../examples/{sendAnywhereFlows.js => sendAnywhere.flows.js} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename packages/orchestration/src/examples/{sendAnywhereFlows.js => sendAnywhere.flows.js} (100%) diff --git a/packages/orchestration/src/examples/sendAnywhere.contract.js b/packages/orchestration/src/examples/sendAnywhere.contract.js index 709e4818c41..c6a96c57abb 100644 --- a/packages/orchestration/src/examples/sendAnywhere.contract.js +++ b/packages/orchestration/src/examples/sendAnywhere.contract.js @@ -5,7 +5,7 @@ import { Fail } from '@endo/errors'; import { E } from '@endo/far'; import { M } from '@endo/patterns'; import { withOrchestration } from '../utils/start-helper.js'; -import { orchestrationFns } from './sendAnywhereFlows.js'; +import { orchestrationFns } from './sendAnywhere.flows.js'; import { prepareChainHubAdmin } from '../exos/chain-hub-admin.js'; /** diff --git a/packages/orchestration/src/examples/sendAnywhereFlows.js b/packages/orchestration/src/examples/sendAnywhere.flows.js similarity index 100% rename from packages/orchestration/src/examples/sendAnywhereFlows.js rename to packages/orchestration/src/examples/sendAnywhere.flows.js From cf4ce90da0bf89d1ed179ddef46b60e596d752c8 Mon Sep 17 00:00:00 2001 From: Turadg Aleahmad Date: Wed, 17 Jul 2024 13:39:36 -0700 Subject: [PATCH 4/8] refactor: '.flows' module convention --- .../src/examples/sendAnywhere.contract.js | 4 +- .../src/examples/sendAnywhere.flows.js | 92 +++++++++---------- 2 files changed, 46 insertions(+), 50 deletions(-) diff --git a/packages/orchestration/src/examples/sendAnywhere.contract.js b/packages/orchestration/src/examples/sendAnywhere.contract.js index c6a96c57abb..ba635f22dc9 100644 --- a/packages/orchestration/src/examples/sendAnywhere.contract.js +++ b/packages/orchestration/src/examples/sendAnywhere.contract.js @@ -5,7 +5,7 @@ import { Fail } from '@endo/errors'; import { E } from '@endo/far'; import { M } from '@endo/patterns'; import { withOrchestration } from '../utils/start-helper.js'; -import { orchestrationFns } from './sendAnywhere.flows.js'; +import * as flows from './sendAnywhere.flows.js'; import { prepareChainHubAdmin } from '../exos/chain-hub-admin.js'; /** @@ -76,7 +76,7 @@ const contract = async ( ); // orchestrate uses the names on orchestrationFns to do a "prepare" of the associated behavior - const orchFns = orchestrateAll(orchestrationFns, { + const orchFns = orchestrateAll(flows, { zcf, contractState, localTransfer: zoeTools.localTransfer, diff --git a/packages/orchestration/src/examples/sendAnywhere.flows.js b/packages/orchestration/src/examples/sendAnywhere.flows.js index db9405d5efc..9db65cb9ff8 100644 --- a/packages/orchestration/src/examples/sendAnywhere.flows.js +++ b/packages/orchestration/src/examples/sendAnywhere.flows.js @@ -12,51 +12,47 @@ const { entries } = Object; // in guest file (the orchestration functions) // the second argument is all the endowments provided -export const orchestrationFns = harden({ - /** - * @param {Orchestrator} orch - * @param {object} ctx - * @param {{ account?: OrchestrationAccountI & LocalAccountMethods }} ctx.contractState - * @param {GuestOf} ctx.localTransfer - * @param {(brand: Brand) => Promise} ctx.findBrandInVBank - * @param {ZCFSeat} seat - * @param {{ chainName: string; destAddr: string }} offerArgs - */ - async sendIt( - orch, - { contractState, localTransfer, findBrandInVBank }, - seat, - offerArgs, - ) { - mustMatch( - offerArgs, - harden({ chainName: M.scalar(), destAddr: M.string() }), - ); - const { chainName, destAddr } = offerArgs; - // NOTE the proposal shape ensures that the `give` is a single asset - const { give } = seat.getProposal(); - const [[_kw, amt]] = entries(give); - const { denom } = await findBrandInVBank(amt.brand); - const chain = await orch.getChain(chainName); - - if (!contractState.account) { - const agoricChain = await orch.getChain('agoric'); - contractState.account = await agoricChain.makeAccount(); - } - - const info = await chain.getChainInfo(); - const { chainId } = info; - assert(typeof chainId === 'string', 'bad chainId'); - - await localTransfer(seat, contractState.account, give); - - await contractState.account.transfer( - { denom, value: amt.value }, - { - value: destAddr, - encoding: 'bech32', - chainId, - }, - ); - }, -}); +/** + * @param {Orchestrator} orch + * @param {object} ctx + * @param {{ account?: OrchestrationAccountI & LocalAccountMethods }} ctx.contractState + * @param {GuestOf} ctx.localTransfer + * @param {(brand: Brand) => Promise} ctx.findBrandInVBank + * @param {ZCFSeat} seat + * @param {{ chainName: string; destAddr: string }} offerArgs + */ +export async function sendIt( + orch, + { contractState, localTransfer, findBrandInVBank }, + seat, + offerArgs, +) { + mustMatch(offerArgs, harden({ chainName: M.scalar(), destAddr: M.string() })); + const { chainName, destAddr } = offerArgs; + // NOTE the proposal shape ensures that the `give` is a single asset + const { give } = seat.getProposal(); + const [[_kw, amt]] = entries(give); + const { denom } = await findBrandInVBank(amt.brand); + const chain = await orch.getChain(chainName); + + if (!contractState.account) { + const agoricChain = await orch.getChain('agoric'); + contractState.account = await agoricChain.makeAccount(); + } + + const info = await chain.getChainInfo(); + const { chainId } = info; + assert(typeof chainId === 'string', 'bad chainId'); + + await localTransfer(seat, contractState.account, give); + + await contractState.account.transfer( + { denom, value: amt.value }, + { + value: destAddr, + encoding: 'bech32', + chainId, + }, + ); +} +harden(sendIt); From a74b32bffecc69ddf9947727f3c2b6702869b242 Mon Sep 17 00:00:00 2001 From: Turadg Aleahmad Date: Wed, 17 Jul 2024 14:19:09 -0700 Subject: [PATCH 5/8] feat: OrchestrationFlow interface --- .../src/examples/auto-stake-it.contract.js | 3 ++- .../orchestration/src/examples/basic-flows.contract.js | 4 +++- .../orchestration/src/examples/sendAnywhere.flows.js | 3 ++- .../orchestration/src/examples/swapExample.contract.js | 3 ++- .../src/examples/unbondExample.contract.js | 3 ++- packages/orchestration/src/facade.js | 10 +++------- packages/orchestration/src/orchestration-api.ts | 5 +++++ 7 files changed, 19 insertions(+), 12 deletions(-) diff --git a/packages/orchestration/src/examples/auto-stake-it.contract.js b/packages/orchestration/src/examples/auto-stake-it.contract.js index 1098b24011a..cc97565d0ba 100644 --- a/packages/orchestration/src/examples/auto-stake-it.contract.js +++ b/packages/orchestration/src/examples/auto-stake-it.contract.js @@ -17,7 +17,7 @@ import { preparePortfolioHolder } from '../exos/portfolio-holder-kit.js'; * @import {Remote} from '@agoric/vow'; * @import {Zone} from '@agoric/zone'; * @import {GuestInterface} from '@agoric/async-flow'; - * @import {CosmosValidatorAddress, Orchestrator, CosmosInterchainService, Denom, OrchestrationAccount, StakingAccountActions} from '@agoric/orchestration'; + * @import {CosmosValidatorAddress, Orchestrator, CosmosInterchainService, Denom, OrchestrationAccount, StakingAccountActions, OrchestrationFlow} from '@agoric/orchestration'; * @import {MakeStakingTap} from './auto-stake-it-tap-kit.js'; * @import {MakePortfolioHolder} from '../exos/portfolio-holder-kit.js'; * @import {ChainHub} from '../exos/chain-hub.js'; @@ -35,6 +35,7 @@ import { preparePortfolioHolder } from '../exos/portfolio-holder-kit.js'; */ /** + * @satisfies {OrchestrationFlow} * @param {Orchestrator} orch * @param {{ * makeStakingTap: MakeStakingTap; diff --git a/packages/orchestration/src/examples/basic-flows.contract.js b/packages/orchestration/src/examples/basic-flows.contract.js index 78928843cba..bbc428b6058 100644 --- a/packages/orchestration/src/examples/basic-flows.contract.js +++ b/packages/orchestration/src/examples/basic-flows.contract.js @@ -9,7 +9,7 @@ import { preparePortfolioHolder } from '../exos/portfolio-holder-kit.js'; /** * @import {Zone} from '@agoric/zone'; - * @import {OrchestrationAccount, Orchestrator} from '@agoric/orchestration'; + * @import {OrchestrationAccount, OrchestrationFlow, Orchestrator} from '@agoric/orchestration'; * @import {ResolvedPublicTopic} from '@agoric/zoe/src/contractSupport/topics.js'; * @import {OrchestrationPowers} from '../utils/start-helper.js'; * @import {MakePortfolioHolder} from '../exos/portfolio-holder-kit.js'; @@ -20,6 +20,7 @@ import { preparePortfolioHolder } from '../exos/portfolio-holder-kit.js'; * Create an account on a Cosmos chain and return a continuing offer with * invitations makers for Delegate, WithdrawRewards, Transfer, etc. * + * @satisfies {OrchestrationFlow} * @param {Orchestrator} orch * @param {undefined} _ctx * @param {ZCFSeat} seat @@ -39,6 +40,7 @@ const makeOrchAccountHandler = async (orch, _ctx, seat, { chainName }) => { * Calls to the underlying invitationMakers are proxied through the * `MakeInvitation` invitation maker. * + * @satisfies {OrchestrationFlow} * @param {Orchestrator} orch * @param {MakePortfolioHolder} makePortfolioHolder * @param {ZCFSeat} seat diff --git a/packages/orchestration/src/examples/sendAnywhere.flows.js b/packages/orchestration/src/examples/sendAnywhere.flows.js index 9db65cb9ff8..cdd44865a51 100644 --- a/packages/orchestration/src/examples/sendAnywhere.flows.js +++ b/packages/orchestration/src/examples/sendAnywhere.flows.js @@ -4,7 +4,7 @@ import { M, mustMatch } from '@endo/patterns'; * @import {GuestOf} from '@agoric/async-flow'; * @import {VBankAssetDetail} from '@agoric/vats/tools/board-utils.js'; * @import {ZoeTools} from '../utils/zoe-tools.js'; - * @import {Orchestrator, OrchestrationAccount, LocalAccountMethods, OrchestrationAccountI} from '../types.js'; + * @import {Orchestrator, LocalAccountMethods, OrchestrationAccountI, OrchestrationFlow} from '../types.js'; */ const { entries } = Object; @@ -13,6 +13,7 @@ const { entries } = Object; // the second argument is all the endowments provided /** + * @satisfies {OrchestrationFlow} * @param {Orchestrator} orch * @param {object} ctx * @param {{ account?: OrchestrationAccountI & LocalAccountMethods }} ctx.contractState diff --git a/packages/orchestration/src/examples/swapExample.contract.js b/packages/orchestration/src/examples/swapExample.contract.js index 1415d3b23e9..187a210b194 100644 --- a/packages/orchestration/src/examples/swapExample.contract.js +++ b/packages/orchestration/src/examples/swapExample.contract.js @@ -6,7 +6,7 @@ import { withOrchestration } from '../utils/start-helper.js'; /** * @import {LocalTransfer} from '../utils/zoe-tools.js'; - * @import {Orchestrator, CosmosValidatorAddress} from '../types.js' + * @import {Orchestrator, CosmosValidatorAddress, OrchestrationFlow} from '../types.js' * @import {TimerService} from '@agoric/time'; * @import {LocalChain} from '@agoric/vats/src/localchain.js'; * @import {Remote} from '@agoric/internal'; @@ -17,6 +17,7 @@ import { withOrchestration } from '../utils/start-helper.js'; */ /** + * @satisfies {OrchestrationFlow} * @param {Orchestrator} orch * @param {object} ctx * @param {LocalTransfer} ctx.localTransfer diff --git a/packages/orchestration/src/examples/unbondExample.contract.js b/packages/orchestration/src/examples/unbondExample.contract.js index 17efa3af2be..8752dc1ff4e 100644 --- a/packages/orchestration/src/examples/unbondExample.contract.js +++ b/packages/orchestration/src/examples/unbondExample.contract.js @@ -2,7 +2,7 @@ import { M } from '@endo/patterns'; import { withOrchestration } from '../utils/start-helper.js'; /** - * @import {Orchestrator, IcaAccount, CosmosValidatorAddress} from '../types.js' + * @import {Orchestrator, OrchestrationFlow} from '../types.js' * @import {TimerService} from '@agoric/time'; * @import {LocalChain} from '@agoric/vats/src/localchain.js'; * @import {NameHub} from '@agoric/vats'; @@ -13,6 +13,7 @@ import { withOrchestration } from '../utils/start-helper.js'; */ /** + * @satisfies {OrchestrationFlow} * @param {Orchestrator} orch * @param {object} ctx * @param {ZCF} ctx.zcf diff --git a/packages/orchestration/src/facade.js b/packages/orchestration/src/facade.js index f4cfa08aeef..a147c7e0e52 100644 --- a/packages/orchestration/src/facade.js +++ b/packages/orchestration/src/facade.js @@ -11,13 +11,13 @@ import { assertAllDefined } from '@agoric/internal'; * @import {HostOrchestrator} from './exos/orchestrator.js'; * @import {Remote} from '@agoric/internal'; * @import {CosmosInterchainService} from './exos/cosmos-interchain-service.js'; - * @import {Chain, ChainInfo, CosmosChainInfo, IBCConnectionInfo, OrchestrationAccount, Orchestrator} from './types.js'; + * @import {Chain, ChainInfo, CosmosChainInfo, IBCConnectionInfo, OrchestrationAccount, OrchestrationFlow, Orchestrator} from './types.js'; */ /** * For a given guest passed to orchestrate(), return the host-side form. * - * @template {(orc: Orchestrator, ctx: any, ...args: any[]) => Promise} GF + * @template {OrchestrationFlow} GF * @typedef {GF extends ( * orc: Orchestrator, * ctx: any, @@ -106,11 +106,7 @@ export const makeOrchestrationFacade = ({ * * @template HC - host context * @template {{ - * [durableName: string]: ( - * orc: Orchestrator, - * ctx: GuestInterface, - * ...args: any[] - * ) => Promise; + * [durableName: string]: OrchestrationFlow>; * }} GFM * guest fn map * @param {GFM} guestFns diff --git a/packages/orchestration/src/orchestration-api.ts b/packages/orchestration/src/orchestration-api.ts index 4c7998857a7..ee2201f63af 100644 --- a/packages/orchestration/src/orchestration-api.ts +++ b/packages/orchestration/src/orchestration-api.ts @@ -9,6 +9,7 @@ import type { CurrentWalletRecord } from '@agoric/smart-wallet/src/smartWallet.j import type { Timestamp } from '@agoric/time'; import type { LocalChainAccount } from '@agoric/vats/src/localchain.js'; import type { ResolvedPublicTopic } from '@agoric/zoe/src/contractSupport/topics.js'; +import type { Passable } from '@endo/marshal'; import type { ChainInfo, CosmosChainAccountMethods, @@ -192,6 +193,10 @@ export interface OrchestrationAccountI { getPublicTopics: () => Promise>>; } +export interface OrchestrationFlow { + (orc: Orchestrator, ctx: CT, ...args: Passable[]): Promise; +} + /** * Internal structure for TransferMsgs. * The type must be able to express transfers across different chains and transports. From 05b7ba1479e8998076a5c601a7c1ab524899975c Mon Sep 17 00:00:00 2001 From: Turadg Aleahmad Date: Wed, 17 Jul 2024 14:23:21 -0700 Subject: [PATCH 6/8] test: linting flows --- packages/eslint-config/eslint-config.cjs | 19 ++++++++++++++ .../orchestration/test/examples/bad.flows.js | 25 +++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 packages/orchestration/test/examples/bad.flows.js diff --git a/packages/eslint-config/eslint-config.cjs b/packages/eslint-config/eslint-config.cjs index aed217ac02b..ade8bf3c55c 100644 --- a/packages/eslint-config/eslint-config.cjs +++ b/packages/eslint-config/eslint-config.cjs @@ -1,4 +1,16 @@ /* eslint-env node */ + +const orchestrationFlowRestrictions = [ + { + selector: "Identifier[name='heapVowE']", + message: 'Eventual send is not yet supported within an orchestration flow', + }, + { + selector: "Identifier[name='E']", + message: 'Eventual send is not yet supported within an orchestration flow', + }, +]; + module.exports = { extends: [ 'airbnb-base', @@ -95,5 +107,12 @@ module.exports = { 'no-unused-vars': 'off', }, }, + { + // Orchestration flows + files: ['**/*.flows.js'], + rules: { + 'no-restricted-syntax': ['error', ...orchestrationFlowRestrictions], + }, + }, ], }; diff --git a/packages/orchestration/test/examples/bad.flows.js b/packages/orchestration/test/examples/bad.flows.js new file mode 100644 index 00000000000..5a095b57101 --- /dev/null +++ b/packages/orchestration/test/examples/bad.flows.js @@ -0,0 +1,25 @@ +/** + * @file Module with linting errors, to verify the linting config detects them. + * This assumes `reportUnusedDisableDirectives` is enabled in the local + * config. + */ + +// TODO error on exports that: +// - aren't hardened (probably a new rule in @endo/eslint-plugin ) +// - don't satisfy orchestration flow type + +// eslint-disable-next-line no-restricted-syntax -- intentional for test +import { E } from '@endo/far'; + +export function notFlow() { + console.log('This function is not a flow'); +} + +export async function notHardened() { + console.log('This function is the most minimal flow, but it’s not hardened'); +} + +export async function usesE(orch, { someEref }) { + // eslint-disable-next-line no-restricted-syntax -- intentional for test + await E(someEref).foo(); +} From 371fa942c91f3847ff5afce81ac8ecd5dc7ed50d Mon Sep 17 00:00:00 2001 From: Turadg Aleahmad Date: Thu, 18 Jul 2024 10:31:00 -0700 Subject: [PATCH 7/8] chore(types): use HostForGuest consistently --- packages/orchestration/src/facade.js | 13 ++++--------- packages/orchestration/test/types.test-d.ts | 3 +++ 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/packages/orchestration/src/facade.js b/packages/orchestration/src/facade.js index a147c7e0e52..f366b411369 100644 --- a/packages/orchestration/src/facade.js +++ b/packages/orchestration/src/facade.js @@ -66,18 +66,13 @@ export const makeOrchestrationFacade = ({ const { prepareEndowment, asyncFlow, adminAsyncFlow } = asyncFlowTools; /** - * @template GR - return type * @template HC - host context - * @template {any[]} GA - guest args + * @template {OrchestrationFlow>} GF guest fn * @param {string} durableName - the orchestration flow identity in the zone * (to resume across upgrades) * @param {HC} hostCtx - values to pass through the async flow membrane - * @param {( - * guestOrc: Orchestrator, - * guestCtx: GuestInterface, - * ...args: GA - * ) => Promise} guestFn - * @returns {(...args: HostArgs) => Vow} + * @param {GF} guestFn + * @returns {HostForGuest} */ const orchestrate = (durableName, hostCtx, guestFn) => { const subZone = zone.subZone(durableName); @@ -92,7 +87,7 @@ export const makeOrchestrationFacade = ({ const hostFn = asyncFlow(subZone, 'asyncFlow', guestFn); // cast because return could be arbitrary subtype - const orcFn = /** @type {(...args: HostArgs) => Vow} */ ( + const orcFn = /** @type {HostForGuest} */ ( (...args) => hostFn(wrappedOrc, wrappedCtx, ...args) ); diff --git a/packages/orchestration/test/types.test-d.ts b/packages/orchestration/test/types.test-d.ts index 4c55ba315e6..92f9c8de758 100644 --- a/packages/orchestration/test/types.test-d.ts +++ b/packages/orchestration/test/types.test-d.ts @@ -144,7 +144,10 @@ expectNotType(chainAddr); ) => Promise.resolve(num); { const h = facade.orchestrate('name', undefined, slowEcho); + // TODO keep the return type as Vow + expectType<(num: number) => Vow>(h); expectType>(h(42)); + // @ts-expect-error literal not carried, widened to number expectType>(h(42)); } From 1a91e7bb9df8880a1ccaa3f5143a55dd580fa653 Mon Sep 17 00:00:00 2001 From: Turadg Aleahmad Date: Thu, 18 Jul 2024 10:45:23 -0700 Subject: [PATCH 8/8] docs: flow modules convention --- packages/orchestration/README.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/packages/orchestration/README.md b/packages/orchestration/README.md index 3050f95c7da..a25fcb9a31e 100644 --- a/packages/orchestration/README.md +++ b/packages/orchestration/README.md @@ -3,3 +3,18 @@ Build feature-rich applications that can orchestrate assets and services across the interchain. Usage examples can be found under [src/examples](https://github.com/Agoric/agoric-sdk/tree/master/packages/orchestration/src/examples). They are exported for integration testing with other packages. + +## Orchestration flows + +Flows to orchestrate are regular Javascript functions but have some constraints to fulfill the requirements of resumability after termination of the enclosing vat. Some requirements for each orchestration flow: +- must not close over any values that could change between invocations +- must satisfy the `OrchestrationFlow` interface +- must be hardened +- must not use `E()` (eventual send) + +The call to `orchestrate` using a flow function in reincarnations of the vat must have the same `durableName` as before. To help enforce these constraints, we recommend: + +- keeping flows in a `.flows.js` module +- importing them all with `import * as flows` to get a single object keyed by the export name +- using `orchestrateAll` to treat each export name as the `durableName` of the flow +- adopting `@agoric/eslint-config` that has rules to help detect problems