Fix orch.getChain() race (#10225)

closes: #10219

best reviewed commit-by-commit

## Description
Stores a structure in the `chainByName` store with the vow or settled value, allowing `getChain` to return the vow for the pending lookup, and `getDenomInfo` to continue synchronously returning if the chain info is available.

To avoid confusion, this also moves the `chainByName` map store definition inside `prepareOrchestratorKit`

### Security Considerations
Nothing new, but `getDenomInfo` basically allows probing synchronously for the result of a `getChain` lookup.

### Scaling Considerations
None

### Documentation Considerations
None

### Testing Considerations
Added unit tests covering this race

### Upgrade Considerations
Contracts can picked up this fix as part of a new orchestration NPM package.

packages/orchestration/src/exos/orchestrator.js

@@ -47,7 +47,6 @@ export const OrchestratorI = M.interface('Orchestrator', {
  *   asyncFlowTools: AsyncFlowTools;
  *   chainHub: ChainHub;
  *   localchain: Remote<LocalChain>;
- *   chainByName: MapStore<string, HostInterface<Chain>>;
  *   makeRecorderKit: MakeRecorderKit;
  *   makeLocalChainFacade: MakeLocalChainFacade;
  *   makeRemoteChainFacade: MakeRemoteChainFacade;
@@ -63,13 +62,20 @@ const prepareOrchestratorKit = (
   {
     chainHub,
     localchain,
-    chainByName,
     makeLocalChainFacade,
     makeRemoteChainFacade,
     vowTools: { watch, asVow },
   },
-) =>
-  zone.exoClassKit(
+) => {
+  /**
+   * @template T
+   * @typedef {{ vow: Vow<T>; pending: true } | { value: T; pending: false }} MaybePendingValue
+   */
+
+  /** @type {MapStore<string, MaybePendingValue<HostInterface<Chain>>>} */
+  const chainByName = zone.mapStore('chainName');
+
+  return zone.exoClassKit(
     'Orchestrator',
     {
       orchestrator: OrchestratorI,
@@ -97,7 +103,7 @@ const prepareOrchestratorKit = (
          */
         onFulfilled(agoricChainInfo) {
           const it = makeLocalChainFacade(agoricChainInfo);
-          chainByName.init('agoric', it);
+          chainByName.set('agoric', harden({ value: it, pending: false }));
           return it;
         },
       },
@@ -115,27 +121,32 @@ const prepareOrchestratorKit = (
          */
         onFulfilled([_agoricChainInfo, remoteChainInfo, connectionInfo], name) {
           const it = makeRemoteChainFacade(remoteChainInfo, connectionInfo);
-          chainByName.init(name, it);
+          chainByName.set(name, harden({ value: it, pending: false }));
           return it;
         },
       },
       orchestrator: {
         /** @type {HostOf<Orchestrator['getChain']>} */
         getChain(name) {
-          if (chainByName.has(name)) {
-            return asVow(() => chainByName.get(name));
-          }
-          if (name === 'agoric') {
-            return watch(
-              chainHub.getChainInfo('agoric'),
-              this.facets.makeLocalChainFacadeWatcher,
-            );
-          }
-          return watch(
-            chainHub.getChainsAndConnection('agoric', name),
-            this.facets.makeRemoteChainFacadeWatcher,
-            name,
-          );
+          return asVow(() => {
+            if (chainByName.has(name)) {
+              const maybeChain = chainByName.get(name);
+              return maybeChain.pending ? maybeChain.vow : maybeChain.value;
+            }
+            const vow =
+              name === 'agoric'
+                ? watch(
+                    chainHub.getChainInfo('agoric'),
+                    this.facets.makeLocalChainFacadeWatcher,
+                  )
+                : watch(
+                    chainHub.getChainsAndConnection('agoric', name),
+                    this.facets.makeRemoteChainFacadeWatcher,
+                    name,
+                  );
+            chainByName.init(name, harden({ vow, pending: true }));
+            return vow;
+          });
         },
         makeLocalAccount() {
           return watch(E(localchain).makeAccount());
@@ -147,17 +158,26 @@ const prepareOrchestratorKit = (
           const { chainName, baseName, baseDenom, brand } = denomDetail;
           chainByName.has(chainName) ||
             Fail`use getChain(${q(chainName)}) before getDenomInfo(${q(denom)})`;
-          const chain = chainByName.get(chainName);
+          const maybeChain = chainByName.get(chainName);
+          if (maybeChain.pending) {
+            throw Fail`wait until getChain(${q(chainName)}) completes before getDenomInfo(${q(denom)})`;
+          }
+          const chain = maybeChain.value;
           chainByName.has(baseName) ||
             Fail`use getChain(${q(baseName)}) before getDenomInfo(${q(denom)})`;
-          const base = chainByName.get(baseName);
+          const maybeBase = chainByName.get(baseName);
+          if (maybeBase.pending) {
+            throw Fail`wait until getChain(${q(baseName)}) completes before getDenomInfo(${q(denom)})`;
+          }
+          const base = maybeBase.value;
           return harden({ chain, base, brand, baseDenom });
         },
         /** @type {HostOf<Orchestrator['asAmount']>} */
         asAmount: () => Fail`not yet implemented`,
       },
     },
   );
+};
 harden(prepareOrchestratorKit);
 
 /**
@@ -166,7 +186,6 @@ harden(prepareOrchestratorKit);
  *   asyncFlowTools: AsyncFlowTools;
  *   chainHub: ChainHub;
  *   localchain: Remote<LocalChain>;
- *   chainByName: MapStore<string, HostInterface<Chain>>;
  *   makeRecorderKit: MakeRecorderKit;
  *   makeLocalChainFacade: MakeLocalChainFacade;
  *   makeRemoteChainFacade: MakeRemoteChainFacade;

packages/orchestration/src/utils/start-helper.js

@@ -129,13 +129,10 @@ export const provideOrchestration = (
     vowTools,
   });
 
-  const chainByName = zones.orchestration.mapStore('chainName');
-
   const makeOrchestrator = prepareOrchestrator(zones.orchestration, {
     asyncFlowTools,
     chainHub,
     localchain: remotePowers.localchain,
-    chainByName,
     makeRecorderKit,
     makeLocalChainFacade,
     makeRemoteChainFacade,

packages/orchestration/test/examples/snapshots/send-anywhere.test.ts.md

@@ -82,7 +82,16 @@ Generated by [AVA](https://avajs.dev).
         LocalChainFacade_kindHandle: 'Alleged: kind',
         Orchestrator_kindHandle: 'Alleged: kind',
         RemoteChainFacade_kindHandle: 'Alleged: kind',
-        chainName: {},
+        chainName: {
+          agoric: {
+            pending: true,
+            vow: Object @Vow {
+              payload: {
+                vowV0: Object @Alleged: VowInternalsKit vowV0 {},
+              },
+            },
+          },
+        },
         ibcTools: {
           IBCTransferSenderKit_kindHandle: 'Alleged: kind',
           ibcResultWatcher_kindHandle: 'Alleged: kind',

packages/orchestration/test/examples/snapshots/staking-combinations.test.ts.md

@@ -196,8 +196,14 @@ Generated by [AVA](https://avajs.dev).
         Orchestrator_kindHandle: 'Alleged: kind',
         RemoteChainFacade_kindHandle: 'Alleged: kind',
         chainName: {
-          agoric: 'Alleged: LocalChainFacade public',
-          cosmoshub: 'Alleged: RemoteChainFacade public',
+          agoric: {
+            pending: false,
+            value: Object @Alleged: LocalChainFacade public {},
+          },
+          cosmoshub: {
+            pending: false,
+            value: Object @Alleged: RemoteChainFacade public {},
+          },
         },
         ibcTools: {
           IBCTransferSenderKit_kindHandle: 'Alleged: kind',

packages/orchestration/test/examples/snapshots/unbond.contract.test.ts.md

@@ -146,8 +146,14 @@ Generated by [AVA](https://avajs.dev).
         Orchestrator_kindHandle: 'Alleged: kind',
         RemoteChainFacade_kindHandle: 'Alleged: kind',
         chainName: {
-          osmosis: 'Alleged: RemoteChainFacade public',
-          stride: 'Alleged: RemoteChainFacade public',
+          osmosis: {
+            pending: false,
+            value: Object @Alleged: RemoteChainFacade public {},
+          },
+          stride: {
+            pending: false,
+            value: Object @Alleged: RemoteChainFacade public {},
+          },
         },
         ibcTools: {
           IBCTransferSenderKit_kindHandle: 'Alleged: kind',

packages/orchestration/test/facade-durability.test.ts

@@ -127,6 +127,55 @@ test.serial('faulty chain info', async t => {
   });
 });
 
+test.serial('racy chain info', async t => {
+  const { facadeServices, commonPrivateArgs } = await commonSetup(t);
+
+  // XXX relax again
+  reincarnate({ relaxDurabilityRules: true });
+  const { zcf } = await setupZCFTest();
+
+  // After setupZCFTest because this disables relaxDurabilityRules
+  // which breaks Zoe test setup's fakeVatAdmin
+  const zone = provideDurableZone('test');
+  const vt = prepareSwingsetVowTools(zone);
+
+  const orchKit = provideOrchestration(
+    zcf,
+    zone.mapStore('test'),
+    {
+      agoricNames: facadeServices.agoricNames,
+      timerService: facadeServices.timerService,
+      storageNode: commonPrivateArgs.storageNode,
+      orchestrationService: facadeServices.orchestrationService,
+      localchain: facadeServices.localchain,
+    },
+    commonPrivateArgs.marshaller,
+  );
+
+  const { chainHub, orchestrate } = orchKit;
+
+  chainHub.registerChain('mock', mockChainInfo);
+  chainHub.registerConnection(
+    'agoric-3',
+    mockChainInfo.chainId,
+    mockChainConnection,
+  );
+
+  const raceGetChain = orchestrate('race', {}, async orc => {
+    return Promise.all([orc.getChain('mock'), orc.getChain('mock')]);
+  });
+
+  const resultP = vt.when(raceGetChain());
+  await t.notThrowsAsync(resultP);
+  const result = await resultP;
+  t.is(result[0], result[1], 'same chain facade');
+  const chainInfos = await vt.when(
+    vt.allVows([result[0].getChainInfo(), result[1].getChainInfo()]),
+  );
+  t.deepEqual(await chainInfos[0], mockChainInfo);
+  t.deepEqual(await chainInfos[1], mockChainInfo);
+});
+
 test.serial('asset / denom info', async t => {
   const { facadeServices, commonPrivateArgs } = await commonSetup(t);
 
@@ -195,7 +244,11 @@ test.serial('asset / denom info', async t => {
         });
       }
 
-      const ag = await orc.getChain('agoric');
+      const agP = orc.getChain('agoric');
+      t.throws(() => orc.getDenomInfo(agDenom), {
+        message: /^wait until getChain\("agoric"\) completes/,
+      });
+      const ag = await agP;
       {
         const actual = orc.getDenomInfo(agDenom);