feat(orchestration): ZCFTools #10057

dckc · 2024-09-10T20:35:30Z

closes: #9773

Description

Provide selected ZCF APIs for use in orchestration flows. For example: use vows for resumable promises.

Security Considerations

nothing new

Scaling Considerations

n/a

Documentation Considerations

reference docs for attenuated ZCF

Testing Considerations

unit tests
a bit of integration with one of the examples

Upgrade Considerations

n/a

turadg

This needs to also update the orchestration facades to replace any full ZCF in a context to the attenuated one. Once that's done some of the existing example tests will cover this.

cloudflare-workers-and-pages · 2024-09-10T21:35:50Z

Deploying agoric-sdk with Cloudflare Pages

Latest commit:	`d1e234f`
Status:	✅ Deploy successful!
Preview URL:	https://4eeeceb2.agoric-sdk.pages.dev
Branch Preview URL:	https://9773-attenuated-zcf.agoric-sdk.pages.dev

View logs

mhofman · 2024-09-11T02:58:46Z

A durable exo needs to be explicitly created from a heap zcf object.

I'm not quite sure how to do that. Maybe pass in zcf at prepare time? Is this thing supposed to be a singleton?

Yes, a singleton durable exo can be created whose behavior wraps over the singleton zcf object provided to the contract start function.

mhofman · 2024-09-11T03:02:06Z

Do we establish that by inspection? Is it testable?

Good question. I'm actually wondering if patterns could help here. Effectively the new objects have to implement all previous methods, accepting at least the same arguments that were previously accepted. The return values must have at least the same fields that were previously present. TypeScript types might be able to help here, as this is effectively the definition of assignability.

packages/orchestration/src/facade.js

packages/orchestration/test/examples/snapshots/send-anywhere.test.ts.md

packages/orchestration/src/exos/zcf-for-flow.js

mhofman · 2024-09-11T03:18:02Z

packages/orchestration/src/facade.js

@@ -64,6 +65,8 @@ export const makeOrchestrationFacade = ({

  const { prepareEndowment, asyncFlow } = asyncFlowTools;

+  const zcfLtd = prepareZcfForFlows(zcf, zone, vowTools);


I think this needs to be called by provideOrchestration using zones.orchestration (aka the system orchestration zone), and returned as one of the orchestration tools.

What's the impact on contracts?

I started making this change... it's straightforward enough... but it means that all the contracts don't use the attenuated ZCF unless we change them to opt-in. Is that by design?

How does this impact the 1st arg to the withOrchestration callback (usually contract)? Do we keep the normal zcf there plus the attenuated zcf in the tools?

const contract = async (zcf, _privateArgs, zone, tools) => { ... } export const start = withOrchestration(contract);

turadg · 2024-09-12T23:29:12Z

packages/orchestration/src/exos/zcf-for-flow.js

+
+import { M } from '@endo/patterns';
+
+export const ZcfI = M.interface(


this is different than ZcfI exported from @agoric/zoe. Let's have a distinct name.

I'd suggest ZcfForFlowsI but that's quite clunky. But that goes for the whole Exo name imo.

Would FlowsZcf, FlowsZcfI and flowsZcf suffice? JSdoc could resolve any ambiguity

One advantage of zcfForFlows is that it starts with zcf, and makes autocomplete simpler.

sorting with ZCF seems worthwhile. going with ZcfForFlowsI

turadg · 2024-09-12T23:30:48Z

packages/orchestration/src/exos/zcf-for-flow.js

+    /**
+     * @deprecated
+     * @type {ZCF['reallocate']}
+     */
+    reallocate(seat1, seat2, ...seatRest) {
+      return zcf.reallocate(seat1, seat2, ...seatRest);
+    },


no need to propagate this function on the chopping block

Suggested change

/**

* @deprecated

* @type {ZCF['reallocate']}

*/

reallocate(seat1, seat2, ...seatRest) {

return zcf.reallocate(seat1, seat2, ...seatRest);

},

ok. it was specified in #9773, but I'm dropping it.

turadg · 2024-09-12T23:31:29Z

packages/orchestration/src/facade.js

@@ -78,6 +78,11 @@ export const makeOrchestrationFacade = ({
    const [wrappedCtx] = prepareEndowment(subZone, 'endowments', [hostCtx]);
    const hostFn = asyncFlow(subZone, 'asyncFlow', guestFn);

+    deepMapObject(
+      wrappedCtx,
+      val => val === zcf && assert.fail('do not pass zcf'),


Should the message point towards using zcfForFlows too?

turadg · 2024-09-12T23:32:14Z

packages/orchestration/test/exos/zcf-for-flows.test.ts

+
+test.before('set up context', async t => (t.context = await makeTestContext()));
+
+test('yes as is: atomicRearrange(), ... getTerms()', async t => {


took me a bit to understand the test name. consider,

removed

unchanged

changed:

changed:

packages/orchestration/test/exos/zcf-for-flows.test.ts

mhofman

Just a small request on the makeInvitation guard. I hadn't realized why it had this shape in the first place.

mhofman · 2024-09-13T03:48:22Z

packages/orchestration/src/exos/zcf-for-flow.js

+
+import { M } from '@endo/patterns';
+
+export const ZcfI = M.interface(


One advantage of zcfForFlows is that it starts with zcf, and makes autocomplete simpler.

mhofman · 2024-09-13T03:50:19Z

packages/orchestration/src/facade.js

@@ -78,6 +78,11 @@ export const makeOrchestrationFacade = ({
    const [wrappedCtx] = prepareEndowment(subZone, 'endowments', [hostCtx]);
    const hostFn = asyncFlow(subZone, 'asyncFlow', guestFn);

+    deepMapObject(
+      wrappedCtx,
+      val => val === zcf && assert.fail('do not pass zcf'),


Should the message point towards using zcfForFlows too?

packages/orchestration/src/types.ts

mhofman · 2024-09-13T04:04:42Z

packages/orchestration/test/exos/zcf-for-flows.test.ts

+test('makeInvitation: non-passable handler', async t => {
+  const { zoe, zcf, zcfLtd, vt } = t.context;
+
+  const toTradeVow = zcfLtd.makeInvitation(_seat => {}, 'trade');
+
+  const toTrade = await vt.when(toTradeVow);
+  const amt = await E(E(zoe).getInvitationIssuer()).getAmountOf(toTrade);
+  t.like(amt, { value: [{ description: 'trade' }] });
+});


This is unnecessary to test for as it's not explicitly supported. Only passable arguments without promises are supported by the async flow membrane, the attenuated zcf should never see such a thing. Let's actually make the guard stricter than the original zcf and test that it refuses a function handler.

mhofman · 2024-09-13T04:07:25Z

packages/orchestration/src/exos/zcf-for-flow.js

+export const ZcfI = M.interface(
+  'ZCF',
+  {
+    makeInvitation: M.call(M.raw(), M.string())


Given that the membrane does not support non-passable arguments, let's restrict to a handler in remotable form ?

Suggested change

makeInvitation: M.call(M.raw(), M.string())

makeInvitation: M.call(M.remotable('OfferHandler'), M.string())

dckc · 2024-09-13T15:27:57Z

packages/orchestration/src/examples/unbond.contract.js

+  // TODO: figure out Guarded<ZcfForFlows> vs. ZcfForFlows
+  const { unbondAndLiquidStake } = orchestrateAll(flows, { zcfForFlows });


@turadg I'm still interested in help with the types here.

I took a look and the best fix isn't obvious to me. Some of the problem may be going from guest to host and back.

I would try defining the interface as a Guest interface (akin to ZCF) and making that what the flow takes. Then define the Exo in terms of that using HostOf.

ok, I did something like that. Now it seems to be griping about brands etc. that come from getTerms() -- their isMyIssuer method returns a promise, and it wants vows for those. Clues?

mhofman

LGTM

packages/orchestration/src/types.ts

turadg · 2024-09-13T21:24:37Z

packages/orchestration/src/exos/zcf-for-flow.js

@@ -26,6 +27,7 @@ export const ZcfForFlowsI = M.interface(
 * @param {VowTools} vowTools
 */
 export const prepareZcfForFlows = (zcf, zone, vowTools) => {
+  /** @satisfies {HostInterface<ZcfForFlows>} */


👌

some of the errors now are with the Host interface exposing methods that return promises. Should we excise those?

getTerms().issuers[0].isMyIssuer(): Promise<boolean> makeEmptySeatKit(...).zcfSeat.getSubscriber(...): Promise<Subscriber<Allocation>>

I think otherwise they'll need durable watchers to become vows.

Ugh yeah these will not be usable inside the membrane

I'm inclined to drop getTerms() -- I don't see any need to delay calling it until a flow has started.

excising getSubscriber means wrapping zcfSeat. I suppose that's reasonably straightforward.

Wait I'm confused, zcfSeat is a local or remote object?

The type error does point to some runtime impact.

I just don't know how to remove methods from brands while preserving their identity, which is kinda the whole point of a brand

I see. I think we can just remove them from the types so that they don't appear to be available. If someone goes and uses a method that the docs and static types say isn't there, they shouldn't be surprised that it fails. (This doesn't apply to when we need to attenuate powers, but that's not the case here. If the method worked we'd gladly include it.)

I think we can just remove them from the types so that they don't appear to be available.

That was my original thought but the problem is that passing such an object back as argument where a zcf seat is expected would cause a type error because the object wouldn't be compatible

The attenuated ZCF should just be different from the ZCF. It just doesn't get past as an argument

@dtribble the problem is not ZCF but zcfSeat in the result of makeEmptySeatKit. Does that need to be passed back anywhere? Should some of its methods be usable from the flow?

turadg · 2024-09-19T20:38:01Z

packages/orchestration/src/facade.js

@@ -78,6 +78,13 @@ export const makeOrchestrationFacade = ({
    const [wrappedCtx] = prepareEndowment(subZone, 'endowments', [hostCtx]);
    const hostFn = asyncFlow(subZone, 'asyncFlow', guestFn);

+    deepMapObject(


nb: the usage here somewhat abuses the "map" idea. Kind of a deepAssert or visitObject

packages/orchestration/src/types.ts

turadg · 2024-09-19T20:42:42Z

packages/orchestration/src/types.ts

+export type ZcfTools<CT = Record<string, unknown>> = Pick<
+  ZCF<CT>,
+  'atomicRearrange' | 'assertUniqueKeyword'
+> & {
+  makeInvitation: <R, A = undefined>(
+    offerHandler: OfferHandler<ERef<R>, A>,
+    description: string,
+    customDetails?: object,
+    proposalShape?: Pattern,
+  ) => Promise<Invitation<R, A>>;
+};


let's use an actual interface. This doesn't need any parameterization because CT (Custom Terms) will not be available.

Suggested change

export type ZcfTools<CT = Record<string, unknown>> = Pick<

ZCF<CT>,

'atomicRearrange' | 'assertUniqueKeyword'

> & {

makeInvitation: <R, A = undefined>(

offerHandler: OfferHandler<ERef<R>, A>,

description: string,

customDetails?: object,

proposalShape?: Pattern,

) => Promise<Invitation<R, A>>;

};

export interface ZcfTools {

assertUniqueKeyword: ZCF['assertUniqueKeyword'];

atomicRearrange: ZCF['atomicRearrange'];

makeInvitation: ZCF['makeInvitation'];

}

Ah yeah, an explicit interface like this makes more sense for when we start deviating

packages/orchestration/src/utils/zcf-tools.js

packages/orchestration/test/fixtures/zcfTester.contract.js

packages/orchestration/src/types.ts

mhofman · 2024-09-20T02:38:27Z

packages/orchestration/src/types.ts

+export type ZcfTools<CT = Record<string, unknown>> = Pick<
+  ZCF<CT>,
+  'atomicRearrange' | 'assertUniqueKeyword'
+> & {
+  makeInvitation: <R, A = undefined>(
+    offerHandler: OfferHandler<ERef<R>, A>,
+    description: string,
+    customDetails?: object,
+    proposalShape?: Pattern,
+  ) => Promise<Invitation<R, A>>;
+};


Ah yeah, an explicit interface like this makes more sense for when we start deviating

packages/orchestration/test/examples/snapshots/unbond.contract.test.ts.md

- unit tests - zcfTester copied from @agoric/zoe - don't pass zcf thru context in examples - restrict makeInvitation handler to passable - regenerate baggage snapshots

dckc requested review from turadg and mhofman September 10, 2024 20:35

turadg reviewed Sep 10, 2024

View reviewed changes

mhofman requested changes Sep 11, 2024

View reviewed changes

dckc force-pushed the 9773-attenuated-zcf branch 3 times, most recently from 25a1716 to 0e3215c Compare September 12, 2024 22:28

dckc marked this pull request as ready for review September 12, 2024 22:28

dckc requested review from turadg and mhofman September 12, 2024 22:28

dckc force-pushed the 9773-attenuated-zcf branch from 0e3215c to cd7e632 Compare September 12, 2024 22:32

turadg reviewed Sep 12, 2024

View reviewed changes

mhofman reviewed Sep 13, 2024

View reviewed changes

dckc mentioned this pull request Sep 13, 2024

import @agoric/orchestration fails to find types.js #10086

Open

dckc commented Sep 13, 2024

View reviewed changes

dckc requested review from mhofman and turadg September 13, 2024 15:28

mhofman approved these changes Sep 13, 2024

View reviewed changes

turadg reviewed Sep 13, 2024

View reviewed changes

dckc mentioned this pull request Sep 19, 2024

Variant of ZCF for orchestrated flows #9773

Open

dckc force-pushed the 9773-attenuated-zcf branch 2 times, most recently from fec4ab1 to 81b621a Compare September 19, 2024 18:35

dckc requested review from mhofman and turadg September 19, 2024 18:38

dckc changed the title ~~feat(orchestration): attenuated ZCF~~ feat(orchestration): ZCFTools Sep 19, 2024

turadg reviewed Sep 19, 2024

View reviewed changes

mhofman reviewed Sep 20, 2024

View reviewed changes

feat(orchestration): ZcfTools for use in flows

d1e234f

- unit tests - zcfTester copied from @agoric/zoe - don't pass zcf thru context in examples - restrict makeInvitation handler to passable - regenerate baggage snapshots

dckc force-pushed the 9773-attenuated-zcf branch from 1f9acb0 to d1e234f Compare September 20, 2024 21:19

dckc requested review from turadg and mhofman September 20, 2024 21:32

turadg approved these changes Sep 20, 2024

View reviewed changes

dckc added the automerge:rebase Automatically rebase updates, then merge label Sep 20, 2024

mhofman approved these changes Sep 20, 2024

View reviewed changes

		@@ -64,6 +65,8 @@ export const makeOrchestrationFacade = ({

		const { prepareEndowment, asyncFlow } = asyncFlowTools;

		const zcfLtd = prepareZcfForFlows(zcf, zone, vowTools);


		import { M } from '@endo/patterns';

		export const ZcfI = M.interface(


		test.before('set up context', async t => (t.context = await makeTestContext()));

		test('yes as is: atomicRearrange(), ... getTerms()', async t => {

	makeInvitation: M.call(M.raw(), M.string())
	makeInvitation: M.call(M.remotable('OfferHandler'), M.string())

		// TODO: figure out Guarded<ZcfForFlows> vs. ZcfForFlows
		const { unbondAndLiquidStake } = orchestrateAll(flows, { zcfForFlows });

feat(orchestration): ZCFTools #10057

Are you sure you want to change the base?

feat(orchestration): ZCFTools #10057

Conversation

dckc commented Sep 10, 2024 • edited Loading

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Upgrade Considerations

turadg left a comment

Choose a reason for hiding this comment

cloudflare-workers-and-pages bot commented Sep 10, 2024 • edited Loading

Deploying agoric-sdk with Cloudflare Pages

mhofman commented Sep 11, 2024

mhofman commented Sep 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhofman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhofman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dckc commented Sep 10, 2024 •

edited

Loading

cloudflare-workers-and-pages bot commented Sep 10, 2024 •

edited

Loading