Agoric / agoric-sdk

monorepo for the Agoric Javascript smart contract platform
Apache License 2.0
326 stars 205 forks source link

feat(async-flow): endowments #9566

Closed erights closed 3 months ago

erights commented 3 months ago

closes: #XXXX refs: https://github.com/Agoric/agoric-sdk/issues/9449 https://github.com/Agoric/agoric-sdk/pull/9521 https://github.com/Agoric/agoric-sdk/issues/9304 https://github.com/Agoric/agoric-sdk/issues/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 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

  getChain: M.callWhen(M.string()).returns(ChainInfoShape),

and the second line indicates where the guest code called it

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.

cloudflare-workers-and-pages[bot] commented 3 months ago

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: c4a252b
Status: ✅  Deploy successful!
Preview URL: https://f7b41959.agoric-sdk.pages.dev
Branch Preview URL: https://markm-endowment-asyncflow.agoric-sdk.pages.dev

View logs

erights commented 3 months ago

swapExample and unbondExample were the remaining example contracts calling orchestrate, so I hoisted their guest function as well. Unlike sendAnywhere, these seemed to hoist cleanly.

erights commented 3 months ago

The existing failures are Error: Promise returned by test never resolved, which looks like a bug in my code, not in the contracts being integration tested. Still trying to track that down.

erights commented 3 months ago

The existing failures are Error: Promise returned by test never resolved, which looks like a bug in my code, not in the contracts being integration tested. Still trying to track that down.

That problem fixed