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

asyncFlow packaging should provide endowments, meta-operations #9304

Closed
Tracked by #9281
erights opened this issue Apr 28, 2024 · 5 comments
Closed
Tracked by #9281

asyncFlow packaging should provide endowments, meta-operations #9304

erights opened this issue Apr 28, 2024 · 5 comments
Assignees
@mhofman
Labels
asyncFlow related to membrane-based replay and upgrade of async functions bug Something isn't working

Comments

@erights
Copy link
Member

erights commented Apr 28, 2024

See #9097

This does not affect the low level asyncFlow abstraction itself. But such conveniences must appear at the abstraction level immediately above it.

@mhofman , I'll leave this for you to describe, since you're already writing about this.
@erights erights added bug Something isn't working asyncFlow related to membrane-based replay and upgrade of async functions labels Apr 28, 2024
This was referenced Apr 30, 2024
Closed
Open
@aj-agoric aj-agoric assigned iomekam and unassigned erights May 13, 2024
@erights
Copy link
Member Author

erights commented May 17, 2024 •
edited
Loading

For example, should add a meta operation for asking about the incarnation number of the next log state, that does not log the query, but would always give the same answer if asked at the same point. This would permit a successor to ask when its predecessor did not, and vice versa. (Suggested by @dtribble I think)

@iomekam iomekam assigned mhofman and unassigned iomekam Jun 10, 2024
@erights erights mentioned this issue Jun 23, 2024
Merged
mergify bot pushed a commit that referenced this issue Jun 25, 2024
@erights
feat(async-flow): endowments (#9566)
4390d8c 
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.
@mhofman
Copy link
Member

mhofman commented Sep 17, 2024

This was essentially done in #9566

@mhofman mhofman closed this as completed Sep 17, 2024
@mhofman
Copy link
Member

mhofman commented Sep 17, 2024

Some tests may be missing, which is tracked by #9933

@mhofman mhofman mentioned this issue Sep 17, 2024
Open
@mhofman
Copy link
Member

mhofman commented Sep 17, 2024

Tracking meta operations at #10103

@mhofman mhofman mentioned this issue Sep 18, 2024
Open
@mhofman
Copy link
Member

mhofman commented Sep 18, 2024

Issue to review endowments: #10104

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mhofman mhofman

Labels
asyncFlow related to membrane-based replay and upgrade of async functions bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mhofman @erights @iomekam