Agoric / agoric-sdk

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

orchestration facade returns promises backed by vows #9449

Closed turadg closed 2 months ago

turadg commented 4 months ago

For each of these:

### Files to convert
- [x] exos/chain-account-kit.js (PC)
- [x] exos/chain-hub.js (TA)
- [x] exos/cosmos-orchestration-account.js (PC)
- [x] exos/icq-connection-kit.js (PC)
- [x] exos/local-chain-facade.js (TA)
- [x] exos/local-orchestration-account.js
- [x] exos/orchestrator.js (PC)
- [x] exos/remote-chain-facade.js (TA/PC)
- [x] timerUtils
- [x] withdrawFromSeat
- [x] agoricNames helper
- [x] service.js (~vat-orchestration, where we have makeAccount)
- [x] types (TA) (mainly in facades, see comment in this ticket)
- [x] chain-hub (currently uses heapVowTools)
dckc commented 3 months ago

@erights and I went over the client code in sendAnywhere.contract.js. We found a few things and sketched fixes...

diff --git a/packages/orchestration/src/examples/sendAnywhere.contract.js b/packages/orchestration/src/examples/sendAnywhere.contract.js
index ed49472ce..d37ae1a49 100644
--- a/packages/orchestration/src/examples/sendAnywhere.contract.js
+++ b/packages/orchestration/src/examples/sendAnywhere.contract.js
@@ -71,7 +71,7 @@ export const start = async (zcf, privateArgs, baggage) => {
     ...orchPowers,
   });

-  let contractAccount;
+  const accountCell = {};

   const findBrandInVBank = async brand => {
     const assets = await E(
@@ -85,9 +85,9 @@ export const start = async (zcf, privateArgs, baggage) => {
   /** @type {OfferHandler} */
   const sendIt = orchestrate(
     'sendIt',
-    { zcf },
+    { zcf, findBrandInVBank, accountCell },
     // eslint-disable-next-line no-shadow -- this `zcf` is enclosed in a membrane
-    async (orch, { zcf }, seat, offerArgs) => {
+    async (orch, { zcf, findBrandInVBank, accountCell }, seat, offerArgs) => {
       mustMatch(
         offerArgs,
         harden({ chainName: M.scalar(), destAddr: M.string() }),
@@ -98,10 +98,11 @@ export const start = async (zcf, privateArgs, baggage) => {
       const { denom } = await findBrandInVBank(amt.brand);
       const chain = await orch.getChain(chainName);

-      // XXX ok to use a heap var crossing the membrane scope this way?
-      if (!contractAccount) {
+      let contractAccount = accountCell.account;
+      if (contractAccount === undefined) {
         const agoricChain = await orch.getChain('agoric');
         contractAccount = await agoricChain.makeAccount();
+        accountCell.account = contractAccount;
       }

       const info = await chain.getChainInfo();
mhofman commented 3 months ago
+        accountCell.account = contractAccount;

The arguments must be durable compatible, which implies hardening. Mutating arguments like this is not supported, you'd need a method to get/set that internal state

erights commented 3 months ago

you'd need a method to get/set that internal state

Yeah, after the call with @dckc, I think a “slight” rewrite of the cell puts it in a category similar to the exo state object:

let contractAcctInternal;
const accountCell = harden({
  get account() { return contractAcctInternal; },
  set account(newValue) { contractAcctInternal = newValue; },
});

This can be made less awkward with a helper. It is only similar to the state object in that the state object need additional mechanisms. But this form of endowment does not need any further mechanism than what we’d need to do to support state objects.

Note that this is only intended to go through the endowments path, like functions and zcf, that may not themselves be Passable. The endowment mechanism has to wrap them with durables on the host side, and with unwrapping emulators on the guest side.

mhofman commented 3 months ago

One thing to be careful of is that functions that synchronously return a vow should be careful not to throw synchronously. I raised this in https://github.com/Agoric/agoric-sdk/pull/9454#discussion_r1626898694

0xpatrickdev commented 3 months ago

With https://github.com/Agoric/agoric-sdk/pull/9591, it seems the only remaining items are: