Integrate asyncFlow with orchestration API (umbrella issue) #9281
Assignees
Labels
asyncFlow
related to membrane-based replay and upgrade of async functions
Comments
erights
added
the
asyncFlow
|
Added a task item: Vows support in Orchestration Service ( Context: OrchestrationService consumes Vows from |
|
To clarify the async-flow requirements, the whole chain up to the orchestration helpers running in the vat using async-flow needs to be upgradable. For other vats that means using and consuming vows. For the vat using async-flow, that means the host API (the orchestration helper and other context or arguments objects) need to return vows (and not promises for vows) |
mergify bot
added a commit
that referenced
this issue
Jun 18, 2024
refs: #9281 ## Description Orchestration API needs asyncTools. This makes it required for construction. That made the boilerplate in `start` quite large so this also adds a helper to do all the setup that any orchestration contract needs. ### Security Considerations none ### Scaling Considerations none ### Documentation Considerations Part of these example contracts ### Testing Considerations Existing coverage suffics ### Upgrade Considerations Not yet deployed
mergify bot
added a commit
that referenced
this issue
Jun 19, 2024
refs: #9281 ## Description AsyncFlow requires that everything passing the membrane is durable. This makes the facade objects durable to conform. Doing so for `localChainFacade` is deferred so we can get this into master sooner, to aid @erights 's #9521 . ### Security Considerations none ### Scaling Considerations Exo for each chain and each account ### Documentation Considerations none ### Testing Considerations Existing coverage ### Upgrade Considerations none, not yet deployed
This was referenced Jun 19, 2024
mergify bot
added a commit
that referenced
this issue
Jun 19, 2024
refs: #9281 ## Description AsyncFlow requires that everything passing the membrane is durable. This makes the facade objects durable to conform. Following #9529 ### Security Considerations none ### Scaling Considerations Exo for each chain and each account ### Documentation Considerations none ### Testing Considerations Existing coverage ### Upgrade Considerations not yet deployed
Merged
mergify bot
added a commit
that referenced
this issue
Jun 19, 2024
refs: #9281 ## Description AsyncFlow requires that everything passing the membrane is durable. This makes the ChainHub durable by making it an Exo. ### Security Considerations none ### Scaling Considerations It's a singleton, because there will only be one per vat. ### Documentation Considerations none ### Testing Considerations Existing coverage ### Upgrade Considerations Not yet deployed. Now that it's an Exo, we have to consider its state an upgradability. We expect to make a new one on each vat invocation. The only state it has is cache of AgoricNames and data supplied by the contract.
mergify bot
pushed a commit
that referenced
this issue
Jun 19, 2024
Staged on #9533 refs: #9281 ## Description The `zcf` object will effectively need to be passed through `orchestrate` as an endowment. Because zcf is not durable, or even an exo, we were originally planning to do it with a mechanism involving a standing durable object, and then wrap and unwrap it on either side of the membrane. But if `zcf` were durable, we wouldn't need all this complexity. It turns out, if this PR is correct, that making `zcf` durable is trivial. ### Security Considerations Making `zcf` into a durable exo also involves giving it an interface guard. The interface guard in the first commit of this PR makes a needed exception for `makeInvitation` and `setTestJig` because both of them accept non-passable parameters. The `defaultGuards: 'passable'` option means that all other methods default to a guard that merely enforces that all arguments and return results are passable. This does make `zcf` somewhat more defensive, but not much. Given this starting point, we can grow that `ZcfI` interface guard to do more explicit input validation of the other methods, which will help security, and make us less vulnerable to insufficient input validation in the zcf methods themselves. As we move more of the input validation to the method guards, we should be able to remove ad hoc input validation code in the method which has become redundant. Replacement of ad hoc input validation with declarative guard-based input validation should help security. I don't yet know whether I'll grow the `ZcfI` interface guard to have these explicit method guards in further commits to this PR or in later PR. ### Scaling Considerations The extra guard checks are potentially an issue, but we won't know until we profile. ### Documentation Considerations none ### Testing Considerations I need to understand `setTestJig` better. ### Upgrade Considerations Making `zcf` durable means that it has a durable identity that survives upgrade. As a durable exo singleton, it is stateless, meaning that it gets back all the state it needs during `prepareExo` as state that its methods capture (close over) rather than as exo instance state. This reflects naturally the initial intuition that the `zcf` endowment, being stateless, could just be represented to `asyncFlow` as a singleton standin, re-endowed during the prepare phase.
mergify bot
pushed a commit
that referenced
this issue
Jun 25, 2024
closes: #XXXX refs: #9449 #9521 #9304 #9281 ## Description Changed async-flow to support endowments. Changed `orchestrate` to use `asyncFlow` with endowments. Changed `sendAnywhere` example orchestration contract to be more compatible with this new `orchestrate`. The CI errors are all in the `orchestration` package. After some earlier iteration where orchestration failures indicated async-flow bugs, which I fixed, the remaining errors seem plausibly to be integration bugs on the orchestration side revealed by using this improved `orchestrate` function. If so, that satisfies the purpose of this PR -- to enable integration testing to reveal such errors. However, this leaves open the question of how to bring this PR to fruition despite these CI errors. In that iteration, the majority of errors were due to host-side promises, which we expected. To proceed with integration testing, I temporarily turned that case into a warning, by wrapping the host-side promise with a host-side vow. This stopgap measure is obviously fragile under upgrade. It would cause may upgrades to fail However, I have not investigated these CI errors enough to be at all confident that none of them are due to bugs in async-flow. For any of those, they should be fixed in this PR. ### Security Considerations nothing new ### Scaling Considerations none, given that total endowments are low cardinality. All these endowments are prepare-time per-function. There should not be any cardinality limit on the activations making use of these endowments. But like all other async-flow scaling issues, that remains to be tested. ### Documentation Considerations The endowment rules and taxonomy is interesting, and should be documented. ### Testing Considerations We get CI errors only from the `orchestration` package. Some of these may be the integration bugs we wanted this exercise to reveal. However, others may be async-flow bugs, which should have been caught by async-flow unit tests. The warning stopgap I mentioned above [appears in CI](https://github.com/Agoric/agoric-sdk/actions/runs/9637015639/job/26575694851?pr=9566#step:12:648) as, for example ``` Warning for now: vow expected, not promise Promise { <pending> } (Error#1) Error#1: where warning happened at makeGuestForHostVow (.../async-flow/src/replay-membrane.js:329:9) at eval (.../async-flow/src/convert.js:119:10) at innerConvert (.../async-flow/src/convert.js:63:8) at convertRecur (.../async-flow/src/convert.js:30:8) at convert (.../async-flow/src/convert.js:76:1) at performCall (.../async-flow/src/replay-membrane.js:137:1) at guestCallsHost (.../async-flow/src/replay-membrane.js:195:9) at In "getChain" method of (Orchestrator orchestrator) [as getChain] (.../async-flow/src/replay-membrane.js:282:8) at eval (.../orchestration/src/examples/unbondExample.contract.js:60:23) at eval (.../async-flow/src/async-flow.js:222:1) at Object.restart (.../async-flow/src/async-flow.js:222:30) at makeAsyncFlowKit (.../async-flow/src/async-flow.js:430:6) at asyncFlow_hostFlow (.../async-flow/src/async-flow.js:448:13) at orcFn (.../orchestration/src/facade.js:124:15) at eval (.../pass-style/src/make-far.js:224:31) ``` The relevant lines are ``` at In "getChain" method of (Orchestrator orchestrator) [as getChain] (.../async-flow/src/replay-membrane.js:282:8) at eval (.../orchestration/src/examples/unbondExample.contract.js:60:23) ``` where the first line indicates what method or method guard provided the inappropriate promise ```js getChain: M.callWhen(M.string()).returns(ChainInfoShape), ``` and the second line indicates where the guest code called it ```js const omni = await orch.getChain('omniflixhub'); ``` ### Upgrade Considerations The orchestration code in question cannot be truly upgrade safe until we see no more of these "vow expected, not promise" warnings. Even then, we should expect that async-flow as of this PR is ready for lots of testing, but not yet ready to run on the main chain with durable state expected to survive real upgrades.
ivanlei
changed the title
|
@mitdralla to sync up with Orchestration folks to see if anything else is missing in async-flow to fully enable orchestration MVP features. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
asyncFlowitself is a low-level helper. For smooth integration with orchestration API, we'll need additional wrappers and helper functions to use with exos and allow injecting closed-over state in the membrane used by asyncflow. We'll also need to address hazards and current incompatibilities.This task captures details needed for integration and will detail learnings as we scope them.
Tasks
The text was updated successfully, but these errors were encountered: