Merge pull request #136 from bvotteler/fix-close-api-promise-connections

Chore: Replace new instances of ApiPromises with (re)using interBtcApi instance

.github/workflows/coverage.yaml

@@ -1,6 +1,9 @@
 name: 'coverage'
 
 on:
+  push:
+    branches:
+      - master
   pull_request_target:
     branches:
       - master

package.json

@@ -1,7 +1,7 @@
 {
     "name": "interbtc-indexer",
     "private": "true",
-    "version": "0.16.5",
+    "version": "0.16.6",
     "description": "GraphQL server and Substrate indexer for the interBTC parachain",
     "author": "",
     "license": "ISC",
@@ -65,6 +65,9 @@
     "jest" :{
         "preset": "ts-jest",
         "testEnvironment": "node",
+        "modulePathIgnorePatterns": [
+            "<rootDir>/lib/"
+        ],
         "collectCoverageFrom": [
             "<rootDir>/src/**/*.ts*"
         ],

src/mappings/_utils.test.ts

@@ -0,0 +1,211 @@
+import { ForeignAsset as LibForeignAsset } from "@interlay/interbtc-api";
+import { cacheForeignAssets, getForeignAsset, symbolFromCurrency, testHelpers } from "./_utils";
+import { ForeignAsset, NativeToken, Token } from "../model";
+
+// mocking getForeignAsset and getForeignAssets in the lib (interBtcApi)
+const libGetForeignAssetsMock = jest.fn();
+const libGetForeignAssetMock = jest.fn();
+const mockInterBtcApi = {
+    assetRegistry: {
+        getForeignAssets: () => libGetForeignAssetsMock(),
+        getForeignAsset: (id: number) => libGetForeignAssetMock(id)
+    }
+};
+
+// "default" good result mock implementation, should be set inside tests, too (will be reset between tests)
+const getInterBtcApiMock = jest.fn().mockImplementation(() => Promise.resolve(mockInterBtcApi));
+jest.mock("./utils/interBtcApi", () => {
+    const originalModule = jest.requireActual("./utils/interBtcApi");
+
+    return {
+        __esModule: true,
+        ...originalModule,
+        getInterBtcApi: () => getInterBtcApiMock()
+    }
+});
+
+describe("_utils", () => {
+    // fake asset to play with
+    const fakeAsset = {
+        foreignAsset: {
+            id: 42,
+            coingeckoId: "foo"
+        },
+        name: "FooCoin",
+        ticker:"FOO",
+        decimals: 7
+    };
+
+    afterAll(() => {
+        jest.resetAllMocks();
+    });
+
+    describe("getForeignAsset", () => {
+        beforeEach(() => {
+            // clean out cache
+            testHelpers.getForeignAssetsCache().clear();
+        });
+
+        afterEach(() => {
+            libGetForeignAssetMock.mockReset();
+            getInterBtcApiMock.mockReset();
+        });
+
+        it("should fetch value from cache first", async () => {
+            libGetForeignAssetMock.mockImplementation(() => Promise.resolve(fakeAsset));
+            getInterBtcApiMock.mockImplementation(() => Promise.resolve(mockInterBtcApi));
+
+            // relies on cache to have been passed as reference
+            testHelpers.getForeignAssetsCache().set(fakeAsset.foreignAsset.id, fakeAsset);
+
+            const actualAsset = await getForeignAsset(fakeAsset.foreignAsset.id);
+            expect(actualAsset).toBe(fakeAsset);
+            expect(getInterBtcApiMock).not.toHaveBeenCalled();
+            expect(libGetForeignAssetMock).not.toHaveBeenCalled();
+        });
+
+        it("should fetch from lib and add to cache", async () => {
+            libGetForeignAssetMock.mockImplementation(() => Promise.resolve(fakeAsset));
+            getInterBtcApiMock.mockImplementation(() => Promise.resolve(mockInterBtcApi));
+
+            // clean out cache
+            testHelpers.getForeignAssetsCache().clear();
+
+            const actualAsset = await getForeignAsset(fakeAsset.foreignAsset.id);
+            expect(getInterBtcApiMock).toHaveBeenCalledTimes(1);
+            expect(libGetForeignAssetMock).toHaveBeenCalledTimes(1);
+
+            expect(actualAsset).toBe(fakeAsset);
+
+            const cacheNow = testHelpers.getForeignAssetsCache();
+            expect(cacheNow.size).toBe(1);
+            expect(cacheNow.has(fakeAsset.foreignAsset.id)).toBe(true);
+        });
+
+        it("should reject if getInterBtcApi rejects", async () => {
+            getInterBtcApiMock.mockImplementation(() => Promise.reject(Error("soz lol")));
+
+            await expect(getForeignAsset(fakeAsset.foreignAsset.id)).rejects.toThrow("soz lol");
+            expect(getInterBtcApiMock).toHaveBeenCalledTimes(1);
+            expect(libGetForeignAssetMock).not.toHaveBeenCalled();
+        });
+
+        it("should reject if assetRegistry.getForeignAssets rejects", async () => {
+            getInterBtcApiMock.mockImplementation(() => Promise.resolve(mockInterBtcApi));
+            libGetForeignAssetMock.mockImplementation(() => Promise.reject(Error("computer says no")));
+
+            await expect(getForeignAsset(fakeAsset.foreignAsset.id)).rejects.toThrow("computer says no");
+            expect(getInterBtcApiMock).toHaveBeenCalledTimes(1);
+            expect(libGetForeignAssetMock).toHaveBeenCalledTimes(1);
+        });
+    });
+
+    describe("cacheForeignAssets", () => {
+        afterEach(() => {
+            libGetForeignAssetsMock.mockReset();
+            getInterBtcApiMock.mockReset();
+        });
+
+        it("should get all foreign assets from lib as expected", async () => {
+            const fakeAssets = [fakeAsset];
+            libGetForeignAssetsMock.mockImplementation(() => Promise.resolve(fakeAssets));
+            getInterBtcApiMock.mockImplementation(() => Promise.resolve(mockInterBtcApi));
+
+            await cacheForeignAssets();
+            const actualCache = testHelpers.getForeignAssetsCache();
+
+            expect(getInterBtcApiMock).toHaveBeenCalledTimes(1);
+            expect(libGetForeignAssetsMock).toHaveBeenCalledTimes(1);
+            expect(actualCache.has(fakeAsset.foreignAsset.id)).toBe(true);
+            expect(actualCache.get(fakeAsset.foreignAsset.id)).toBe(fakeAsset);
+        });
+
+        it("should reject if getInterBtcApi rejects", async () => {
+            getInterBtcApiMock.mockImplementation(() => Promise.reject(Error("nope")));
+
+            await expect(cacheForeignAssets()).rejects.toThrow("nope");
+            expect(getInterBtcApiMock).toHaveBeenCalledTimes(1);
+            expect(libGetForeignAssetsMock).not.toHaveBeenCalled();
+        });
+
+        it("should reject if assetRegistry.getForeignAssets rejects", async () => {
+            libGetForeignAssetsMock.mockImplementation(() => Promise.reject(Error("no assets for you")));
+            getInterBtcApiMock.mockImplementation(() => Promise.resolve(mockInterBtcApi));
+
+            await expect(cacheForeignAssets()).rejects.toThrow("no assets for you");
+            expect(getInterBtcApiMock).toHaveBeenCalledTimes(1);
+            expect(libGetForeignAssetsMock).toHaveBeenCalledTimes(1);
+        });
+
+        it("should set usdt asset id if found", async () => {
+            const oldUsdtAssetId = testHelpers.getUsdtAssetId() | 0;
+
+            const fakeUsdtAsset = {
+                foreignAsset: {
+                    id: oldUsdtAssetId + 13,
+                    coingeckoId: "usdt"
+                },
+                name: "Tether USD",
+                ticker:"USDT",
+                decimals: 6
+            };
+            const fakeAssets = [
+                fakeAsset,
+                fakeUsdtAsset,
+            ];
+
+            libGetForeignAssetsMock.mockImplementation(() => Promise.resolve(fakeAssets));
+            getInterBtcApiMock.mockImplementation(() => Promise.resolve(mockInterBtcApi));
+
+            await cacheForeignAssets();
+
+            const newUsdtAssetId = testHelpers.getUsdtAssetId();
+            expect(newUsdtAssetId).toBe(fakeUsdtAsset.foreignAsset.id);
+        });
+
+        it("should not set usdt asset id if not found", async () => {
+            const oldUsdtAssetId = testHelpers.getUsdtAssetId();
+
+            const fakeAssets = [
+                fakeAsset,
+            ];
+
+            libGetForeignAssetsMock.mockImplementation(() => Promise.resolve(fakeAssets));
+            getInterBtcApiMock.mockImplementation(() => Promise.resolve(mockInterBtcApi));
+
+            await cacheForeignAssets();
+
+            const newUsdtAssetId = testHelpers.getUsdtAssetId();
+            expect(newUsdtAssetId).toBe(oldUsdtAssetId);
+        });
+    });
+
+    describe("symbolFromCurrency", () => {
+        it("should return token name for native token", async () => {
+            const testNativeToken = new NativeToken({token: Token.KINT});
+            const actualSymbol = await symbolFromCurrency(testNativeToken);
+
+            expect(actualSymbol).toBe(Token.KINT);
+        });
+
+        it("should return ticker for foreign asset", async () => {
+            const testAssetId = fakeAsset.foreignAsset.id;
+            // prepare cache for lookup
+            testHelpers.getForeignAssetsCache().set(testAssetId, fakeAsset);
+
+            const testForeignAsset = new ForeignAsset({asset: testAssetId});
+            const actualSymbol = await symbolFromCurrency(testForeignAsset);
+            expect(actualSymbol).toBe(fakeAsset.ticker);
+        });
+
+        it("should return unknown for unhandled currency type", async () => {
+            const badTestCurrency = {
+                isTypeOf: "definitely not a valid type"
+            };
+
+            const actualSymbol = await symbolFromCurrency(badTestCurrency as any);
+            expect(actualSymbol).toBe("UNKNOWN");
+        });
+
+    });
+});

src/mappings/_utils.ts

@@ -1,17 +1,18 @@
 import { CurrencyExt, CurrencyIdentifier, currencyIdToMonetaryCurrency, FIXEDI128_SCALING_FACTOR, newCollateralBTCExchangeRate, newMonetaryAmount, StandardPooledTokenIdentifier } from "@interlay/interbtc-api";
-import { Bitcoin, ExchangeRate, InterBtc, Interlay, KBtc, Kintsugi, Kusama, Polkadot } from "@interlay/monetary-js";
-import { ApiPromise, WsProvider } from '@polkadot/api';
+import { Bitcoin, ExchangeRate } from "@interlay/monetary-js";
 import { BigDecimal } from "@subsquid/big-decimal";
 import { Store } from "@subsquid/typeorm-store";
 import { Big, BigSource } from "big.js";
 import * as process from "process";
 import { LessThanOrEqual, Like } from "typeorm";
 import { Currency, ForeignAsset, Height, Issue, OracleUpdate, OracleUpdateType, Redeem, Token, Vault } from "../model";
-import { Ctx, getInterBtcApi } from "../processor";
+import { Ctx } from "../processor";
+import { getInterBtcApi } from "./utils/interBtcApi";
 import { VaultId as VaultIdV1021000 } from "../types/v1021000";
 import { VaultId as VaultIdV15 } from "../types/v15";
 import { VaultId as VaultIdV6 } from "../types/v6";
 import { encodeLegacyVaultId, encodeVaultId } from "./encoding";
+import { ForeignAsset as LibForeignAsset } from "@interlay/interbtc-api";
 
 export type eventArgs = {
     event: { args: true };
@@ -73,48 +74,45 @@ type AssetMetadata = {
     symbol: string;
 }
 
-// This function uses the storage API to obtain the details directly from the
-// WSS RPC provider for the correct chain
-const cache: { [id: number]: AssetMetadata } = {};
+// simple cache for foreign assets by id
+const cache: Map<number, LibForeignAsset> = new Map();
 let usdtAssetId: number;
 
-export async function cacheForeignAsset(): Promise<void> {
-    try {
-        const wsProvider = new WsProvider(process.env.CHAIN_ENDPOINT);
-        const api = await ApiPromise.create({ provider: wsProvider, noInitWarn: true });
-        const assets = await api.query.assetRegistry.metadata.entries();
-        assets.forEach(([key, details]) => {
-            const id:number = Number(key.args[0]);
-            const assetDetails = details.toHuman() as AssetMetadata;
-            cache[id] = assetDetails;
-            if(assetDetails.symbol==='USDT') usdtAssetId = id;
-          });        
-    } catch (error) {
-        console.error(`Error getting foreign asset metadata: ${error}`);
-        throw error;
-    }
-}
+export async function cacheForeignAssets(): Promise<void> {
+    const interBtcApi = await getInterBtcApi();
+    const foreignAssets = await interBtcApi.assetRegistry.getForeignAssets();
+    foreignAssets.forEach((asset) => {
+        const id = asset.foreignAsset.id;
 
+        cache.set(id, asset);
 
-export async function getForeignAsset(id: number): Promise<AssetMetadata> {
-    if (id in cache) {
-        return cache[id];
-    }
-    try {
-        const wsProvider = new WsProvider(process.env.CHAIN_ENDPOINT);
-        const api = await ApiPromise.create({ provider: wsProvider, noInitWarn: true });
-        const assets = await api.query.assetRegistry.metadata(id);
-        const assetsJSON = assets.toHuman();
-        const metadata = assetsJSON as AssetMetadata;
-        console.debug(`Foreign Asset (${id}): ${JSON.stringify(metadata)}`);
-        cache[id] = metadata;
-        return metadata;
-    } catch (error) {
-        console.error(`Error getting foreign asset metadata: ${error}`);
-        throw error;
+        if(asset.ticker === 'USDT') {
+            usdtAssetId = id;
+        } 
+    });
+}
+
+export async function getForeignAsset(id: number): Promise<LibForeignAsset> {
+    if (cache.has(id)) {
+        return cache.get(id)!;
     }
+
+    const interBtcApi = await getInterBtcApi();
+    const asset = await interBtcApi.assetRegistry.getForeignAsset(id);
+
+    cache.set(id,asset);
+
+    return asset;
 }
 
+/**
+ * Helper methods to facilitate testing, use at own risk
+ */
+export const testHelpers = {
+    getForeignAssetsCache: () => cache,
+    getUsdtAssetId: () => usdtAssetId
+};
+
 /* This function takes a currency object (could be native, could be foreign) and
 an amount (in the smallest unit, e.g. Planck) and returns a human friendly string
 with a reasonable accuracy (6 digits after the decimal point for BTC and 2 for
@@ -155,13 +153,12 @@ export function divideByTenToTheNth(amount: bigint, n: number): number {
 }
 
 export async function symbolFromCurrency(currency: Currency): Promise<string> {
-    let amountFriendly: number;
     switch(currency.isTypeOf) {
         case 'NativeToken':
             return currency.token;
         case 'ForeignAsset':
             const details = await getForeignAsset(currency.asset)
-            return details.symbol;
+            return details.ticker;
         default:
             return `UNKNOWN`;
     }