feat!: getAsset and getDenomInfo require holdingChainName param

- since denoms are only unique to a chain and all chains, require `holdingChainName` parameter for asset info lookups
Agoric · Nov 29, 2024 · d43db44 · d43db44
1 parent 49cdf6e
commit d43db44
Show file tree

Hide file tree

Showing 13 changed files with 85 additions and 48 deletions.
diff --git a/packages/fast-usdc/test/snapshots/fast-usdc.contract.test.ts.md b/packages/fast-usdc/test/snapshots/fast-usdc.contract.test.ts.md
@@ -539,22 +539,22 @@ Generated by [AVA](https://avajs.dev).
           },
         },
         denom: {
-          'ibc/498A0751C798A0D9A389AA3691123DADA57DAA4FE165D5C75894505B876BA6E4': {
-            baseDenom: 'uusdc',
-            baseName: 'noble',
-            chainName: 'osmosis',
-          },
-          'ibc/FE98AAD68F02F03565E9FA39A5E627946699B2B07115889ED812D8BA639576A9': {
+          'agoric:ibc/FE98AAD68F02F03565E9FA39A5E627946699B2B07115889ED812D8BA639576A9': {
             baseDenom: 'uusdc',
             baseName: 'noble',
             brand: Object @Alleged: USDC brand {},
             chainName: 'agoric',
           },
-          uusdc: {
+          'noble:uusdc': {
             baseDenom: 'uusdc',
             baseName: 'noble',
             chainName: 'noble',
           },
+          'osmosis:ibc/498A0751C798A0D9A389AA3691123DADA57DAA4FE165D5C75894505B876BA6E4': {
+            baseDenom: 'uusdc',
+            baseName: 'noble',
+            chainName: 'osmosis',
+          },
         },
         lookupChainInfo_kindHandle: 'Alleged: kind',
         lookupChainsAndConnection_kindHandle: 'Alleged: kind',

diff --git a/packages/fast-usdc/test/snapshots/fast-usdc.contract.test.ts.snap b/packages/fast-usdc/test/snapshots/fast-usdc.contract.test.ts.snap
diff --git a/packages/orchestration/src/exos/chain-hub.js b/packages/orchestration/src/exos/chain-hub.js
@@ -206,7 +206,9 @@ const ChainHubI = M.interface('ChainHub', {
   getConnectionInfo: M.call(ChainIdArgShape, ChainIdArgShape).returns(VowShape),
   getChainsAndConnection: M.call(M.string(), M.string()).returns(VowShape),
   registerAsset: M.call(M.string(), DenomDetailShape).returns(),
-  getAsset: M.call(M.string()).returns(M.or(DenomDetailShape, M.undefined())),
+  getAsset: M.call(M.string(), M.string()).returns(
+    M.or(DenomDetailShape, M.undefined()),
+  ),
   getDenom: M.call(BrandShape).returns(M.or(M.string(), M.undefined())),
   makeChainAddress: M.call(M.string()).returns(ChainAddressShape),
   makeTransferRoute: M.call(ChainAddressShape, DenomAmountShape, M.string())
@@ -254,6 +256,14 @@ export const makeChainHub = (zone, agoricNames, vowTools) => {
     valueShape: M.string(),
   });
 
+  /**
+   * @param {Denom} denom - on the holding chain, whose name is given in
+   *   `detail.chainName`
+   * @param {DenomDetail['chainName']} holdingChainName
+   */
+  const makeDenomKey = (denom, holdingChainName) =>
+    `${holdingChainName}:${denom}`;
+
   const lookupChainInfo = vowTools.retryable(
     zone,
     'lookupChainInfo',
@@ -438,20 +448,28 @@ export const makeChainHub = (zone, agoricNames, vowTools) => {
         Fail`must register chain ${q(chainName)} first`;
       chainInfos.has(baseName) ||
         Fail`must register chain ${q(baseName)} first`;
-      denomDetails.init(denom, detail);
+
+      const denomKey = makeDenomKey(denom, detail.chainName);
+      denomDetails.has(denomKey) &&
+        Fail`already registered ${q(denom)} on ${q(chainName)}`;
+      denomDetails.init(denomKey, detail);
       if (detail.brand) {
+        chainName === 'agoric' ||
+          Fail`brands only registerable for agoric-held assets`;
         brandDenoms.init(detail.brand, denom);
       }
     },
     /**
      * Retrieve holding, issuing chain names etc. for a denom.
      *
      * @param {Denom} denom
+     * @param {string} holdingChainName - the chainName the denom is held on
      * @returns {DenomDetail | undefined}
      */
-    getAsset(denom) {
-      if (denomDetails.has(denom)) {
-        return denomDetails.get(denom);
+    getAsset(denom, holdingChainName) {
+      const denomKey = makeDenomKey(denom, holdingChainName);
+      if (denomDetails.has(denomKey)) {
+        return denomDetails.get(denomKey);
       }
       return undefined;
     },
@@ -502,11 +520,16 @@ export const makeChainHub = (zone, agoricNames, vowTools) => {
       chainInfos.has(holdingChainName) ||
         Fail`chain info not found for holding chain: ${q(holdingChainName)}`;
 
-      const denomDetail = chainHub.getAsset(denomAmount.denom);
+      const denomDetail = chainHub.getAsset(
+        denomAmount.denom,
+        holdingChainName,
+      );
       denomDetail ||
-        Fail`no denom detail for: ${q(denomAmount.denom)}. ensure it is registered in chainHub.`;
+        Fail`no denom detail for: ${q(denomAmount.denom)} on ${q(holdingChainName)}. ensure it is registered in chainHub.`;
 
       const { baseName, chainName } = /** @type {DenomDetail} */ (denomDetail);
+
+      // currently unreachable since assets are registered with holdingChainName
       chainName === holdingChainName ||
         Fail`cannot transfer asset ${q(denomAmount.denom)}. held on ${q(chainName)} not ${q(holdingChainName)}.`;
 

diff --git a/packages/orchestration/src/exos/local-orchestration-account.js b/packages/orchestration/src/exos/local-orchestration-account.js
@@ -503,7 +503,7 @@ export const prepareLocalOrchestrationAccountKit = (
           return asVow(() => {
             const [brand, denom] =
               typeof denomArg === 'string'
-                ? [chainHub.getAsset(denomArg)?.brand, denomArg]
+                ? [chainHub.getAsset(denomArg, 'agoric')?.brand, denomArg]
                 : [denomArg, chainHub.getDenom(denomArg)];
 
             if (!denom) {

diff --git a/packages/orchestration/src/exos/orchestrator.js b/packages/orchestration/src/exos/orchestrator.js
@@ -34,7 +34,7 @@ const trace = makeTracer('Orchestrator');
 /** @see {Orchestrator} */
 export const OrchestratorI = M.interface('Orchestrator', {
   getChain: M.call(M.string()).returns(Vow$(ChainInfoShape)),
-  getDenomInfo: M.call(DenomShape).returns(DenomInfoShape),
+  getDenomInfo: M.call(DenomShape, M.string()).returns(DenomInfoShape),
   asAmount: M.call(DenomAmountShape).returns(AmountShape),
 });
 
@@ -137,9 +137,13 @@ const prepareOrchestratorKit = (
             return vow;
           });
         },
+        // TODO
+        // - we have `DenomInfo` and `DenomDetail` types - why both?
+        // - should this move to the chain Object? exist at all given `chainHub.getAsset()`?
+        // - impossible to get pass `getAsset` if chains aren't registered, so the `chainByName.has()` checks don't do anything
         /** @type {HostOf<Orchestrator['getDenomInfo']>} */
-        getDenomInfo(denom) {
-          const denomDetail = chainHub.getAsset(denom);
+        getDenomInfo(denom, holdingChainName) {
+          const denomDetail = chainHub.getAsset(denom, holdingChainName);
           if (!denomDetail) throw Fail`No denom detail for ${q(denom)}`;
           const { chainName, baseName, baseDenom, brand } = denomDetail;
           chainByName.has(chainName) ||

diff --git a/packages/orchestration/src/orchestration-api.ts b/packages/orchestration/src/orchestration-api.ts
@@ -146,6 +146,7 @@ export interface Orchestrator {
     IssuingChain extends keyof KnownChains,
   >(
     denom: Denom,
+    holdingChainName: HoldingChain,
   ) => DenomInfo<HoldingChain, IssuingChain>;
 
   /**

diff --git a/packages/orchestration/test/examples/snapshots/send-anywhere.test.ts.md b/packages/orchestration/test/examples/snapshots/send-anywhere.test.ts.md
@@ -1114,33 +1114,33 @@ Generated by [AVA](https://avajs.dev).
           },
         },
         denom: {
-          'ibc/8E27BA2D5493AF5636760E354E46004562C46AB7EC0CC4C1CA14E9E20E2545B5': {
-            baseDenom: 'uusdc',
-            baseName: 'noble',
-            chainName: 'dydx',
-          },
-          'ibc/BA313C4A19DFBF943586C0387E6B11286F9E416B4DD27574E6909CABE0E342FA': {
+          'agoric:ibc/BA313C4A19DFBF943586C0387E6B11286F9E416B4DD27574E6909CABE0E342FA': {
             baseDenom: 'uatom',
             baseName: 'cosmoshub',
             chainName: 'agoric',
           },
-          'ibc/FE98AAD68F02F03565E9FA39A5E627946699B2B07115889ED812D8BA639576A9': {
+          'agoric:ibc/FE98AAD68F02F03565E9FA39A5E627946699B2B07115889ED812D8BA639576A9': {
             baseDenom: 'uusdc',
             baseName: 'noble',
             chainName: 'agoric',
           },
-          ubld: {
+          'agoric:ubld': {
             baseDenom: 'ubld',
             baseName: 'agoric',
             brand: Object @Alleged: BLD brand {},
             chainName: 'agoric',
           },
-          uist: {
+          'agoric:uist': {
             baseDenom: 'uist',
             baseName: 'agoric',
             brand: Object @Alleged: IST brand {},
             chainName: 'agoric',
           },
+          'dydx:ibc/8E27BA2D5493AF5636760E354E46004562C46AB7EC0CC4C1CA14E9E20E2545B5': {
+            baseDenom: 'uusdc',
+            baseName: 'noble',
+            chainName: 'dydx',
+          },
         },
         lookupChainInfo_kindHandle: 'Alleged: kind',
         lookupChainsAndConnection_kindHandle: 'Alleged: kind',

diff --git a/packages/orchestration/test/examples/snapshots/send-anywhere.test.ts.snap b/packages/orchestration/test/examples/snapshots/send-anywhere.test.ts.snap
diff --git a/packages/orchestration/test/examples/snapshots/staking-combinations.test.ts.md b/packages/orchestration/test/examples/snapshots/staking-combinations.test.ts.md
@@ -98,7 +98,7 @@ Generated by [AVA](https://avajs.dev).
           },
         },
         denom: {
-          ubld: {
+          'agoric:ubld': {
             baseDenom: 'ubld',
             baseName: 'agoric',
             brand: Object @Alleged: BLD brand {},

diff --git a/packages/orchestration/test/examples/snapshots/staking-combinations.test.ts.snap b/packages/orchestration/test/examples/snapshots/staking-combinations.test.ts.snap
diff --git a/packages/orchestration/test/exos/chain-hub.test.ts b/packages/orchestration/test/exos/chain-hub.test.ts
@@ -86,28 +86,28 @@ test('denom info support via getAsset and getDenom', async t => {
   const denom = 'utok1';
   const info1: CosmosChainInfo = {
     bech32Prefix: 'chain',
-    chainId: 'chain1',
+    chainId: 'agoric',
     stakingTokens: [{ denom }],
   };
   const tok1 = withAmountUtils(makeIssuerKit('Tok1'));
 
-  chainHub.registerChain('chain1', info1);
+  chainHub.registerChain('agoric', info1);
   const info = {
-    chainName: 'chain1',
-    baseName: 'chain1',
+    chainName: 'agoric',
+    baseName: 'agoric',
     baseDenom: denom,
     brand: tok1.brand,
   };
   chainHub.registerAsset('utok1', info);
 
   t.deepEqual(
-    chainHub.getAsset('utok1'),
+    chainHub.getAsset('utok1', 'agoric'),
     info,
     'getAsset(denom) returns denom info',
   );
 
   t.is(
-    chainHub.getAsset('utok404'),
+    chainHub.getAsset('utok404', 'agoric'),
     undefined,
     'getAsset returns undefined when denom not registered',
   );
@@ -145,7 +145,7 @@ test('toward asset info in agoricNames (#9572)', async t => {
   registerAssets(chainHub, 'cosmoshub', details);
 
   {
-    const actual = chainHub.getAsset('uatom');
+    const actual = chainHub.getAsset('uatom', 'cosmoshub');
     t.deepEqual(actual, {
       chainName: 'cosmoshub',
       baseName: 'cosmoshub',
@@ -156,6 +156,7 @@ test('toward asset info in agoricNames (#9572)', async t => {
   {
     const actual = chainHub.getAsset(
       'ibc/F04D72CF9B5D9C849BB278B691CDFA2241813327430EC9CDC83F8F4CA4CDC2B0',
+      'cosmoshub',
     );
     t.deepEqual(actual, {
       chainName: 'cosmoshub',
@@ -446,7 +447,7 @@ test('makeTransferRoute - no asset info', t => {
       ),
     {
       message:
-        'no denom detail for: "uist". ensure it is registered in chainHub.',
+        'no denom detail for: "uist" on "agoric". ensure it is registered in chainHub.',
     },
   );
 
@@ -459,7 +460,7 @@ test('makeTransferRoute - no asset info', t => {
       ),
     {
       message:
-        'no denom detail for: "ibc/FE98AAD68F02F03565E9FA39A5E627946699B2B07115889ED812D8BA639576A9". ensure it is registered in chainHub.',
+        'no denom detail for: "ibc/FE98AAD68F02F03565E9FA39A5E627946699B2B07115889ED812D8BA639576A9" on "agoric". ensure it is registered in chainHub.',
     },
   );
 });
@@ -553,7 +554,7 @@ test('makeTransferRoute - asset not on holding chain', t => {
       ),
     {
       message:
-        'cannot transfer asset "ibc/FE98AAD68F02F03565E9FA39A5E627946699B2B07115889ED812D8BA639576A9". held on "agoric" not "osmosis".',
+        'no denom detail for: "ibc/FE98AAD68F02F03565E9FA39A5E627946699B2B07115889ED812D8BA639576A9" on "osmosis". ensure it is registered in chainHub.',
     },
   );
 });

diff --git a/packages/orchestration/test/facade-durability.test.ts b/packages/orchestration/test/facade-durability.test.ts
@@ -184,16 +184,16 @@ test('asset / denom info', async t => {
   const { chainHub, orchestrate } = orchKit;
 
   chainHub.registerChain('agoric', fetchedChainInfo.agoric);
-  chainHub.registerChain(mockChainInfo.chainId, mockChainInfo);
+  chainHub.registerChain('mock', mockChainInfo);
   chainHub.registerConnection(
     'agoric-3',
     mockChainInfo.chainId,
     mockChainConnection,
   );
 
   chainHub.registerAsset('utoken1', {
-    chainName: mockChainInfo.chainId,
-    baseName: mockChainInfo.chainId,
+    chainName: 'mock',
+    baseName: 'mock',
     baseDenom: 'utoken1',
   });
 
@@ -203,7 +203,7 @@ test('asset / denom info', async t => {
   t.log(`utoken1 over ${channelId}: ${agDenom}`);
   chainHub.registerAsset(agDenom, {
     chainName: 'agoric',
-    baseName: mockChainInfo.chainId,
+    baseName: 'mock',
     baseDenom: 'utoken1',
     brand,
   });
@@ -213,10 +213,14 @@ test('asset / denom info', async t => {
     { brand },
     // eslint-disable-next-line no-shadow
     async (orc, { brand }) => {
-      const c1 = await orc.getChain(mockChainInfo.chainId);
+      const c1 = await orc.getChain('mock');
 
       {
-        const actual = orc.getDenomInfo('utoken1');
+        const actual = orc.getDenomInfo(
+          'utoken1',
+          // @ts-expect-error 'mock' not a KnownChain
+          'mock',
+        );
         console.log('actual', actual);
         const info = await actual.chain.getChainInfo();
         t.deepEqual(info, mockChainInfo);
@@ -230,12 +234,12 @@ test('asset / denom info', async t => {
       }
 
       const agP = orc.getChain('agoric');
-      t.throws(() => orc.getDenomInfo(agDenom), {
+      t.throws(() => orc.getDenomInfo(agDenom, 'agoric'), {
         message: /^wait until getChain\("agoric"\) completes/,
       });
       const ag = await agP;
       {
-        const actual = orc.getDenomInfo(agDenom);
+        const actual = orc.getDenomInfo(agDenom, 'agoric');
 
         t.deepEqual(actual, {
           chain: ag,
@@ -258,7 +262,11 @@ test('asset / denom info', async t => {
   });
 
   const missingGetChain = orchestrate('missing getChain', {}, async orc => {
-    const actual = orc.getDenomInfo('utoken2');
+    const actual = orc.getDenomInfo(
+      'utoken2',
+      // @ts-expect-error 'mock' not a KnownChain
+      'anotherChain',
+    );
   });
 
   await t.throwsAsync(vt.when(missingGetChain()), {

diff --git a/packages/orchestration/test/types.test-d.ts b/packages/orchestration/test/types.test-d.ts
@@ -110,7 +110,7 @@ expectNotType<CosmosValidatorAddress>(chainAddr);
   expectNotType<() => Promise<number>>(vowFn);
 
   const getDenomInfo: HostOf<Orchestrator['getDenomInfo']> = null as any;
-  const chainHostOf = getDenomInfo('uatom').chain;
+  const chainHostOf = getDenomInfo('uatom', 'cosmoshub').chain;
   expectType<Vow<any>>(chainHostOf.getChainInfo());
 }