Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Variant of ZCF for orchestrated flows #9773

Open
mhofman opened this issue Jul 25, 2024 · 7 comments · May be fixed by #10057
Open

Variant of ZCF for orchestrated flows #9773

mhofman opened this issue Jul 25, 2024 · 7 comments · May be fixed by #10057
Assignees
@dckc
Labels
asyncFlow related to membrane-based replay and upgrade of async functions enhancement New feature or request needs-design

Comments

@mhofman
Copy link
Member

mhofman commented Jul 25, 2024

What is the Problem Being Solved?

ZCF currently has some APIs that are not compatible with the MVP async-flow membrane. Thankfully these APIs are not strictly required for the MVP orchestration.

Description of the Design

Expose a filtered zcf as endowment to orchestrated flows:

  • atomicRearrange - yes as is
  • reallocate - yes as is
  • assertUniqueKeyword - yes as is
  • saveIssuer - no
  • makeInvitation - yes - watch
  • shutdown - no
  • shutdownWithFailure - no
  • stopAcceptingOffers - no
  • makeZCFMint - yes - watch result
  • registerFeeMint - no
  • makeEmptySeatKit - yes - needs custom wrapping. remove UserSeat promise from result
  • getZoeService - no
  • getInvitationIssuer - no
  • getTerms - yes as is
  • getBrandForIssuer - no
  • getIssuerForBrand - no
  • getAssetKind - no
  • setTestJig - no
  • getInstance - no
  • setOfferFilter - no
  • getOfferFilter - no

The above changes the return signature of makeEmptySeatKit, so typing might be interesting.

How to plumb the replaced zcf in might be interesting as it's technically provided by the user as an explicit endowment when declaring the orchestrated flow. Maybe we could assume it would come in as a zcf property of the context, or simply recognize the zcf instance that should match the one used to prepare the orchestrate tools.

Security Considerations

None, this is purely an attenuation

Scaling Considerations

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

Test Plan

TBD

Upgrade Considerations

When allowing more functionalities through in the zcf endowment, the new shape should be compatible with the previous shape that remains potentially exposed to previous pending activations of the orchestrated flow.
@mhofman mhofman added enhancement New feature or request asyncFlow related to membrane-based replay and upgrade of async functions labels Jul 25, 2024
This was referenced Aug 1, 2024
Closed
Open
@mhofman
Copy link
Member Author

mhofman commented Aug 3, 2024

How to plumb the replaced zcf in might be interesting as it's technically provided by the user as an explicit endowment when declaring the orchestrated flow. Maybe we could assume it would come in as a zcf property of the context, or simply recognize the zcf instance that should match the one used to prepare the orchestrate tools.

It would likely be more future-proof to go with a replacer approach (#9823) as that would allow passing back zcf in arguments, and the original to be seen by the host instead of the explicit wrapper.

@mhofman
Copy link
Member Author

mhofman commented Sep 9, 2024

After discussing with @dtribble the other day we realized that the "zcf for orchestration" can simply be a separate object created as part of the orchestration tools, and that the endowments logic does not need to recognize the original zcf to substitute it, removing the dependency on #9823.

@dckc
Copy link
Member

dckc commented Sep 10, 2024 •
edited
Loading

Any suggestions on test plan? In particular, do we have something like setupZoeForTest but for ZCF? @Chris-Hibbert ?

p.s. depositTo.test.js seems like a workable model.

@Chris-Hibbert
Copy link
Contributor

What's the incompatibility with the membrane? I'm trying to figure out what determined what was included and what was excluded.

@dckc dckc linked a pull request Sep 10, 2024 that will close this issue
Open
3 tasks
@mhofman
Copy link
Member Author

mhofman commented Sep 11, 2024 •
edited
Loading

What's the incompatibility with the membrane?

Promises are not durably storable, and incompatible with the record keeping that async flow does.

I'm trying to figure out what determined what was included and what was excluded.

Any API returning a promise was either removed, or made to watch the result. Some APIs also seemed to not have much of a purpose for orchestrated flows (@dtribble made some of these decisions), or once other APIs were removed.

@Chris-Hibbert
Copy link
Contributor

getZoeService() provides access to some important services. It might be worth adding Zoe itself, or these methods: getInvitationIssuer(), getPublicFacet(), offer(). I'm presuming the point of many orchestration flows is weaving together use of multiple contracts.

@dckc
Copy link
Member

dckc commented Sep 19, 2024

Attenuating ZCF leads to attenuating ZcfSeat which leads to attenuating Brand, which interferes badly with identity.
#10057 (comment)

Adding needs-design.

@turadg turadg changed the title Attenuated ZCF for orchestrated flows Variant of ZCF for orchestrated flows Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dckc dckc

Labels
asyncFlow related to membrane-based replay and upgrade of async functions enhancement New feature or request needs-design
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(orchestration): ZCFTools Agoric/agoric-sdk
3 participants
@mhofman @dckc @Chris-Hibbert