From 1b71597eaa64f49a57b7ff44d2108ffb08064231 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C3=ADas=20Mart=C3=ADnez?= <131624652+mat1asm@users.noreply.github.com> Date: Tue, 29 Aug 2023 14:48:24 -0300 Subject: [PATCH] Wallet pool review (#94) * fixing linter issues and add github action * moves tests to own folder and adds tests for wallet manager.withWallet * some documentation and adding a counter for locks * normalize token vs address on sui --- .eslintrc.js | 1 + .github/workflow/test.yml | 11 --- .github/workflows/test.yml | 23 +++++ examples/package-lock.json | 2 +- examples/rebalancing.ts | 3 +- package-lock.json | 66 +++++++-------- package.json | 2 +- src/balances/evm/index.ts | 2 +- src/balances/sui.ts | 2 +- src/chain-wallet-manager.ts | 10 +-- src/grpc/client.ts | 2 +- .../proto/wallet-manager-grpc-service.proto | 2 +- src/grpc/service-impl.ts | 5 +- src/grpc/service.ts | 2 +- src/i-wallet-manager.ts | 6 +- src/prometheus-exporter.ts | 20 +++++ src/wallet-manager.ts | 38 +++++++-- src/wallets/base-wallet.ts | 5 +- src/wallets/evm/polygon.config.ts | 1 - src/wallets/index.ts | 3 +- src/wallets/sui/index.ts | 21 +++-- src/wallets/wallet-pool.ts | 4 +- {src => test}/balances/sui.test.ts | 2 +- test/wallet-manager.test.ts | 83 +++++++++++++++++++ {src => test}/wallets/sui/sui.test.ts | 20 +++-- tsconfig.json | 3 +- 26 files changed, 248 insertions(+), 91 deletions(-) delete mode 100644 .github/workflow/test.yml create mode 100644 .github/workflows/test.yml rename {src => test}/balances/sui.test.ts (95%) create mode 100644 test/wallet-manager.test.ts rename {src => test}/wallets/sui/sui.test.ts (88%) diff --git a/.eslintrc.js b/.eslintrc.js index 2687dcb..5cc0da9 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -2,6 +2,7 @@ module.exports = { env: { browser: true, es2021: true, + node: true }, extends: ["eslint:recommended", "plugin:@typescript-eslint/recommended"], overrides: [], diff --git a/.github/workflow/test.yml b/.github/workflow/test.yml deleted file mode 100644 index 2d8b5b6..0000000 --- a/.github/workflow/test.yml +++ /dev/null @@ -1,11 +0,0 @@ -name: Run tests -on: - push: - branches: ["main"] - pull_request: -jobs: - build: - runs-on: ubuntu-latest - steps: - - name: Run tests - run: npm test \ No newline at end of file diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..34eabf4 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,23 @@ +name: Run tests +on: + push: + branches: ["main"] + pull_request: +jobs: + build: + runs-on: ubuntu-latest + steps: + - name: Checkout repo + uses: actions/checkout@v3 + - uses: actions/setup-node@v3 + with: + node-version: 18 + cache: "npm" + cache-dependency-path: | + ./package-lock.json + - name: npm ci + run: npm ci + - name: typecheck + run: npm run build + - name: test + run: npm test \ No newline at end of file diff --git a/examples/package-lock.json b/examples/package-lock.json index 0315d72..a9c129e 100644 --- a/examples/package-lock.json +++ b/examples/package-lock.json @@ -14,7 +14,7 @@ }, "..": { "name": "@xlabs-xyz/wallet-monitor", - "version": "0.2.8", + "version": "0.2.14", "license": "MIT", "dependencies": { "@ethersproject/bignumber": "^5.7.0", diff --git a/examples/rebalancing.ts b/examples/rebalancing.ts index 08ef07f..bc7b67d 100644 --- a/examples/rebalancing.ts +++ b/examples/rebalancing.ts @@ -21,7 +21,8 @@ const options: WalletManagerOptions = { enabled: true, serve: true, port: 9091, - } + }, + failOnInvalidChain: true }; const allChainWallets: WalletManagerConfig = { diff --git a/package-lock.json b/package-lock.json index bfa7f82..c719e63 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1904,9 +1904,9 @@ } }, "node_modules/@mapbox/node-pre-gyp/node_modules/semver": { - "version": "7.5.1", - "resolved": "https://registry.npmjs.org/semver/-/semver-7.5.1.tgz", - "integrity": "sha512-Wvss5ivl8TMRZXXESstBA4uR5iXgEN/VC5/sOcuXdVLzcdkz4HWetIoRfG5gb5X+ij/G9rw9YoGn3QoQ8OCSpw==", + "version": "7.5.4", + "resolved": "https://registry.npmjs.org/semver/-/semver-7.5.4.tgz", + "integrity": "sha512-1bCSESV6Pv+i21Hvpxp3Dx+pSD8lIPt8uVjRrxAUt/nbswYc+tK6Y2btiULjd4+fnq15PX+nqQDC7Oft7WkwcA==", "dev": true, "dependencies": { "lru-cache": "^6.0.0" @@ -2669,9 +2669,9 @@ } }, "node_modules/@typescript-eslint/eslint-plugin/node_modules/semver": { - "version": "7.5.1", - "resolved": "https://registry.npmjs.org/semver/-/semver-7.5.1.tgz", - "integrity": "sha512-Wvss5ivl8TMRZXXESstBA4uR5iXgEN/VC5/sOcuXdVLzcdkz4HWetIoRfG5gb5X+ij/G9rw9YoGn3QoQ8OCSpw==", + "version": "7.5.4", + "resolved": "https://registry.npmjs.org/semver/-/semver-7.5.4.tgz", + "integrity": "sha512-1bCSESV6Pv+i21Hvpxp3Dx+pSD8lIPt8uVjRrxAUt/nbswYc+tK6Y2btiULjd4+fnq15PX+nqQDC7Oft7WkwcA==", "dev": true, "dependencies": { "lru-cache": "^6.0.0" @@ -2813,9 +2813,9 @@ } }, "node_modules/@typescript-eslint/typescript-estree/node_modules/semver": { - "version": "7.5.1", - "resolved": "https://registry.npmjs.org/semver/-/semver-7.5.1.tgz", - "integrity": "sha512-Wvss5ivl8TMRZXXESstBA4uR5iXgEN/VC5/sOcuXdVLzcdkz4HWetIoRfG5gb5X+ij/G9rw9YoGn3QoQ8OCSpw==", + "version": "7.5.4", + "resolved": "https://registry.npmjs.org/semver/-/semver-7.5.4.tgz", + "integrity": "sha512-1bCSESV6Pv+i21Hvpxp3Dx+pSD8lIPt8uVjRrxAUt/nbswYc+tK6Y2btiULjd4+fnq15PX+nqQDC7Oft7WkwcA==", "dev": true, "dependencies": { "lru-cache": "^6.0.0" @@ -2872,9 +2872,9 @@ } }, "node_modules/@typescript-eslint/utils/node_modules/semver": { - "version": "7.5.1", - "resolved": "https://registry.npmjs.org/semver/-/semver-7.5.1.tgz", - "integrity": "sha512-Wvss5ivl8TMRZXXESstBA4uR5iXgEN/VC5/sOcuXdVLzcdkz4HWetIoRfG5gb5X+ij/G9rw9YoGn3QoQ8OCSpw==", + "version": "7.5.4", + "resolved": "https://registry.npmjs.org/semver/-/semver-7.5.4.tgz", + "integrity": "sha512-1bCSESV6Pv+i21Hvpxp3Dx+pSD8lIPt8uVjRrxAUt/nbswYc+tK6Y2btiULjd4+fnq15PX+nqQDC7Oft7WkwcA==", "dev": true, "dependencies": { "lru-cache": "^6.0.0" @@ -7327,9 +7327,9 @@ } }, "node_modules/jest-snapshot/node_modules/semver": { - "version": "7.5.0", - "resolved": "https://registry.npmjs.org/semver/-/semver-7.5.0.tgz", - "integrity": "sha512-+XC0AD/R7Q2mPSRuy2Id0+CGTZ98+8f+KvwirxOKIEyid+XSx6HbC63p+O4IndTHuX5Z+JxQ0TghCkO5Cg/2HA==", + "version": "7.5.4", + "resolved": "https://registry.npmjs.org/semver/-/semver-7.5.4.tgz", + "integrity": "sha512-1bCSESV6Pv+i21Hvpxp3Dx+pSD8lIPt8uVjRrxAUt/nbswYc+tK6Y2btiULjd4+fnq15PX+nqQDC7Oft7WkwcA==", "dev": true, "dependencies": { "lru-cache": "^6.0.0" @@ -8387,9 +8387,9 @@ } }, "node_modules/protobufjs": { - "version": "7.2.3", - "resolved": "https://registry.npmjs.org/protobufjs/-/protobufjs-7.2.3.tgz", - "integrity": "sha512-TtpvOqwB5Gdz/PQmOjgsrGH1nHjAQVCN7JG4A6r1sXRWESL5rNMAiRcBQlCAdKxZcAbstExQePYG8xof/JVRgg==", + "version": "7.2.4", + "resolved": "https://registry.npmjs.org/protobufjs/-/protobufjs-7.2.4.tgz", + "integrity": "sha512-AT+RJgD2sH8phPmCf7OUZR8xGdcJRga4+1cOaXJ64hvcSkVhNcRHOwIxUatPH15+nj59WAGTDv3LSGZPEQbJaQ==", "hasInstallScript": true, "dependencies": { "@protobufjs/aspromise": "^1.1.2", @@ -8658,9 +8658,9 @@ "integrity": "sha512-cdwTTnqPu0Hyvf5in5asVdZocVDTNRmR7XEcJuIzMjJeSHybHl7vpB66AzwTaIg6CLSbtjcxc8fqcySfnTkccA==" }, "node_modules/semver": { - "version": "6.3.0", - "resolved": "https://registry.npmjs.org/semver/-/semver-6.3.0.tgz", - "integrity": "sha512-b39TBaTSfV6yBrapU89p5fKekE2m/NwnDocOVruQFS1/veMgdzuPcnOM34M6CwxW8jH/lxEa5rBoDeUwu5HHTw==", + "version": "6.3.1", + "resolved": "https://registry.npmjs.org/semver/-/semver-6.3.1.tgz", + "integrity": "sha512-BR7VvDCVHO+q2xBEWskxS6DJE1qRnb7DxzUrogb71CWoSficBxYsiAGd+Kl0mmq/MprG9yArRkyrQxTO6XjMzA==", "dev": true, "bin": { "semver": "bin/semver.js" @@ -9059,9 +9059,9 @@ } }, "node_modules/ts-jest/node_modules/semver": { - "version": "7.5.0", - "resolved": "https://registry.npmjs.org/semver/-/semver-7.5.0.tgz", - "integrity": "sha512-+XC0AD/R7Q2mPSRuy2Id0+CGTZ98+8f+KvwirxOKIEyid+XSx6HbC63p+O4IndTHuX5Z+JxQ0TghCkO5Cg/2HA==", + "version": "7.5.4", + "resolved": "https://registry.npmjs.org/semver/-/semver-7.5.4.tgz", + "integrity": "sha512-1bCSESV6Pv+i21Hvpxp3Dx+pSD8lIPt8uVjRrxAUt/nbswYc+tK6Y2btiULjd4+fnq15PX+nqQDC7Oft7WkwcA==", "dev": true, "dependencies": { "lru-cache": "^6.0.0" @@ -9156,9 +9156,9 @@ } }, "node_modules/ts-proto-descriptors/node_modules/protobufjs": { - "version": "6.11.3", - "resolved": "https://registry.npmjs.org/protobufjs/-/protobufjs-6.11.3.tgz", - "integrity": "sha512-xL96WDdCZYdU7Slin569tFX712BxsxslWwAfAhCYjQKGTq7dAU91Lomy6nLLhh/dyGhk/YH4TwTSRxTzhuHyZg==", + "version": "6.11.4", + "resolved": "https://registry.npmjs.org/protobufjs/-/protobufjs-6.11.4.tgz", + "integrity": "sha512-5kQWPaJHi1WoCpjTGszzQ32PG2F4+wRY6BmAT4Vfw56Q2FZ4YZzK20xUYQH4YkfehY1e6QSICrJquM6xXZNcrw==", "hasInstallScript": true, "dependencies": { "@protobufjs/aspromise": "^1.1.2", @@ -9181,9 +9181,9 @@ } }, "node_modules/ts-proto/node_modules/protobufjs": { - "version": "6.11.3", - "resolved": "https://registry.npmjs.org/protobufjs/-/protobufjs-6.11.3.tgz", - "integrity": "sha512-xL96WDdCZYdU7Slin569tFX712BxsxslWwAfAhCYjQKGTq7dAU91Lomy6nLLhh/dyGhk/YH4TwTSRxTzhuHyZg==", + "version": "6.11.4", + "resolved": "https://registry.npmjs.org/protobufjs/-/protobufjs-6.11.4.tgz", + "integrity": "sha512-5kQWPaJHi1WoCpjTGszzQ32PG2F4+wRY6BmAT4Vfw56Q2FZ4YZzK20xUYQH4YkfehY1e6QSICrJquM6xXZNcrw==", "hasInstallScript": true, "dependencies": { "@protobufjs/aspromise": "^1.1.2", @@ -9476,9 +9476,9 @@ } }, "node_modules/word-wrap": { - "version": "1.2.3", - "resolved": "https://registry.npmjs.org/word-wrap/-/word-wrap-1.2.3.tgz", - "integrity": "sha512-Hz/mrNwitNRh/HUAtM/VT/5VH+ygD6DV7mYKZAtHOrbs8U7lvPS6xf7EJKMF0uW1KJCl0H701g3ZGus+muE5vQ==", + "version": "1.2.5", + "resolved": "https://registry.npmjs.org/word-wrap/-/word-wrap-1.2.5.tgz", + "integrity": "sha512-BN22B5eaMMI9UMtjrGd5g5eCYPpCPDUy0FJXbYsaT5zYxjFOckS53SQDE3pWkVoWpHXVb3BrYcEN4Twa55B5cA==", "dev": true, "engines": { "node": ">=0.10.0" diff --git a/package.json b/package.json index ace7bb4..2e83dc7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@xlabs-xyz/wallet-monitor", - "version": "0.2.14", + "version": "0.2.15", "description": "A set of utilities to monitor blockchain wallets and react to them", "main": "lib/index.js", "types": "lib/index.d.ts", diff --git a/src/balances/evm/index.ts b/src/balances/evm/index.ts index 4acea81..8bc293e 100644 --- a/src/balances/evm/index.ts +++ b/src/balances/evm/index.ts @@ -37,7 +37,7 @@ export async function pullEvmTokenBalance( isNative: false, rawBalance: balance.toString(), } -}; +} export async function pullEvmNativeBalance( provider: ethers.providers.JsonRpcProvider, diff --git a/src/balances/sui.ts b/src/balances/sui.ts index 682f92e..91ad76d 100644 --- a/src/balances/sui.ts +++ b/src/balances/sui.ts @@ -124,7 +124,7 @@ function buildEd25519KeyPair(key: Buffer): Ed25519Keypair { } function buildSuiKeyPairFromPrivateKey(privateKey: string): Secp256k1Keypair | Ed25519Keypair { - let parsedKey = Buffer.from(privateKey, 'base64'); + const parsedKey = Buffer.from(privateKey, 'base64'); let key, buildKeyPair; if (parsedKey.length === PRIVATE_KEY_SIZE+1) { diff --git a/src/chain-wallet-manager.ts b/src/chain-wallet-manager.ts index be0e8f4..18848c6 100644 --- a/src/chain-wallet-manager.ts +++ b/src/chain-wallet-manager.ts @@ -132,7 +132,7 @@ export class ChainWalletManager { console.warn(`A monitoring run is already in progress for ${this.options.chainName}. Will skip run`); this.emitter.emit('skipped', { chainName: this.options.chainName, rawConfig: this.wallets }); return; - }; + } this.locked = true; @@ -158,7 +158,7 @@ export class ChainWalletManager { return acc; }, {} as WalletBalancesByAddress); - }; + } public on(event: string, listener: (...args: any[]) => void) { this.emitter.on(event, listener); @@ -190,7 +190,7 @@ export class ChainWalletManager { } public async acquireLock(opts?: WalletExecuteOptions) { - return this.walletToolbox.acquire(opts?.address, opts?.leaseTimeout) + return this.walletToolbox.acquire(opts?.address, opts?.waitToAcquireTimeout) } public async releaseLock(address: string) { @@ -198,7 +198,7 @@ export class ChainWalletManager { } // Returns a boolean indicating if a rebalance was executed - public async executeRebalanceIfNeeded(): Promise { + public async executeRebalanceIfNeeded(): Promise { if (this.rebalanceLocked) { this.logger.warn(`A rebalance is already in progress for ${this.options.chainName}. Will skip rebalance`); return false; @@ -236,7 +236,7 @@ export class ChainWalletManager { } receipts.push(receipt); - }; + } await this.refreshBalances(); diff --git a/src/grpc/client.ts b/src/grpc/client.ts index 29e8603..7141026 100644 --- a/src/grpc/client.ts +++ b/src/grpc/client.ts @@ -45,7 +45,7 @@ export class ClientWalletManager implements IClientWalletManager { const chainManager = this.managers[chainName]; if (!chainManager) throw new Error(`No wallets configured for chain: ${chainName}`); - const { address: acquiredAddress } = await this.walletManagerGRPCClient.acquireLock({chainName, address: opts?.address, leaseTimeout: opts?.leaseTimeout}) + const { address: acquiredAddress } = await this.walletManagerGRPCClient.acquireLock({chainName, address: opts?.address, leaseTimeout: opts?.leaseTimeout, acquireTimeout: opts?.waitToAcquireTimeout }) // FIXME // Dirty solution. We are doing as little work as possible to get the same expected WalletInterface after diff --git a/src/grpc/proto/wallet-manager-grpc-service.proto b/src/grpc/proto/wallet-manager-grpc-service.proto index 57e3335..3b2ef68 100644 --- a/src/grpc/proto/wallet-manager-grpc-service.proto +++ b/src/grpc/proto/wallet-manager-grpc-service.proto @@ -10,8 +10,8 @@ service WalletManagerGRPCService { message AcquireLockRequest { string chain_name = 1; optional string address = 2; - // FIXME: We are missing waitToAcquireTimeout. optional uint64 lease_timeout = 3; + optional uint64 acquire_timeout = 4; } message AcquireLockResponse { string address = 1; diff --git a/src/grpc/service-impl.ts b/src/grpc/service-impl.ts index 6c71137..16622e5 100644 --- a/src/grpc/service-impl.ts +++ b/src/grpc/service-impl.ts @@ -12,7 +12,10 @@ export class WalletManagerGRPCService implements WalletManagerGRPCServiceImpleme constructor(private underlyingWalletManager: IServiceWalletManager) {} async acquireLock(request: AcquireLockRequest, context: CallContext): Promise> { - const acquiredWallet = await this.underlyingWalletManager.acquireLock(request.chainName as ChainName, {address: request.address, leaseTimeout: request.leaseTimeout}) + const acquiredWallet = await this.underlyingWalletManager.acquireLock( + request.chainName as ChainName, + { address: request.address, leaseTimeout: request.leaseTimeout, waitToAcquireTimeout: request.acquireTimeout } + ); return {address: acquiredWallet.address}; } diff --git a/src/grpc/service.ts b/src/grpc/service.ts index bdc59a7..267ca4d 100644 --- a/src/grpc/service.ts +++ b/src/grpc/service.ts @@ -33,7 +33,7 @@ const walletManager = new WalletManager(fileConfig.config, fileConfig.options) const walletManagerGRPCService = new WalletManagerGRPCService(walletManager); -const server = createServer(); +const server = createServer(); // TODO: add observability for the GRPC server server.add(WalletManagerGRPCServiceDefinition, walletManagerGRPCService); server.listen(fileConfig.grpc.listenAddress + ':' + fileConfig.grpc.listenPort); diff --git a/src/i-wallet-manager.ts b/src/i-wallet-manager.ts index beaa42f..096cfeb 100644 --- a/src/i-wallet-manager.ts +++ b/src/i-wallet-manager.ts @@ -24,6 +24,6 @@ interface IWMBareLocks { releaseLock(chainName: ChainName, address: string): Promise } -export interface ILibraryWalletManager extends IWMContextManagedLocks {} -export interface IClientWalletManager extends IWMContextManagedLocks {} -export interface IServiceWalletManager extends IWMBareLocks {} +export type ILibraryWalletManager = IWMContextManagedLocks +export type IClientWalletManager = IWMContextManagedLocks +export type IServiceWalletManager = IWMBareLocks diff --git a/src/prometheus-exporter.ts b/src/prometheus-exporter.ts index d511ae2..d4902aa 100644 --- a/src/prometheus-exporter.ts +++ b/src/prometheus-exporter.ts @@ -50,6 +50,15 @@ function createRebalanceTransactionsCounter (registry: Registry, name: string) { }); } +function createCounter(registry: Registry, name: string, help: string, labels: string[]) { + return new Counter({ + name, + help, + labelNames: labels, + registers: [registry], + }); +} + export function createBalancesGauge(registry: Registry, gaugeName: string) { return new Gauge({ name: gaugeName, @@ -87,6 +96,8 @@ export class PrometheusExporter { private rebalanceExpenditureCounter: Counter; private rebalanceInstructionsCounter: Counter; private rebalanceTransactionsCounter: Counter; + private locksAcquiredCounter: Counter; + private prometheusPort: number; private prometheusPath: string; @@ -102,6 +113,7 @@ export class PrometheusExporter { this.rebalanceExpenditureCounter = createRebalanceExpenditureCounter(this.registry, 'wallet_monitor_rebalance_expenditure'); this.rebalanceInstructionsCounter = createRebalanceInstructionsCounter(this.registry, 'wallet_monitor_rebalance_instruction'); this.rebalanceTransactionsCounter = createRebalanceTransactionsCounter(this.registry, 'wallet_monitor_rebalance_transaction'); + this.locksAcquiredCounter = createCounter(this.registry, "locks_acquired_total", "Total number of acquired locks", ['chain_name', 'status']); } public getRegistry() { @@ -135,6 +147,14 @@ export class PrometheusExporter { this.rebalanceExpenditureCounter.labels(chainName, strategy).inc(totalExpenditure); } + public increaseAcquiredLocks(chainName: string) { + this.locksAcquiredCounter.labels(chainName, "success").inc(); + } + + public increaseAcquireLockFailure(chainName: string) { + this.locksAcquiredCounter.labels(chainName, "failed").inc(); + } + public async startMetricsServer(): Promise { this.app = await startMetricsServer(this.prometheusPort, this.prometheusPath, async () => { const metrics = await this.metrics() diff --git a/src/wallet-manager.ts b/src/wallet-manager.ts index 2d84c98..942daa0 100644 --- a/src/wallet-manager.ts +++ b/src/wallet-manager.ts @@ -17,7 +17,7 @@ import { WalletBalance, WalletConfigSchema, } from "./wallets"; -import { TransferRecepit } from "./wallets/base-wallet"; +import { TransferRecepit, WalletInterface } from "./wallets/base-wallet"; import { RebalanceInstruction } from "./rebalance-strategies"; export const WalletRebalancingConfigSchema = z.object({ @@ -135,8 +135,7 @@ export class WalletManager { rebalance: chainConfig.rebalance, walletOptions: chainConfig.chainConfig, balancePollInterval: options?.balancePollInterval, - // defaulted by Zod: - failOnInvalidTokens: options?.failOnInvalidTokens!, + failOnInvalidTokens: options?.failOnInvalidTokens ?? true, }; const chainManager = new ChainWalletManager( @@ -215,12 +214,19 @@ export class WalletManager { return this.exporter?.getRegistry(); } - public acquireLock(chainName: ChainName, opts?: WalletExecuteOptions) { + public acquireLock(chainName: ChainName, opts?: WalletExecuteOptions): Promise { const chainManager = this.managers[chainName]; if (!chainManager) throw new Error(`No wallets configured for chain: ${chainName}`); - return chainManager.acquireLock(opts); + return this.runGuarded( + () => chainManager.acquireLock(opts), + () => this.exporter?.increaseAcquiredLocks(chainName), + () => { + this.logger.error(`Failed to acquire lock for ${chainName}`); + this.exporter?.increaseAcquireLockFailure(chainName); + } + ); } public releaseLock(chainName: ChainName, address: string) { @@ -231,6 +237,15 @@ export class WalletManager { return chainManager.releaseLock(address); } + /** + * Guarantees wallet will only be used by caller in a single process setup. + * There is no enforcement of lease timeout, so you should have one defined by the fn param. + * If no lock is obtained befored specified waitToAcquireTimeout expires, an error will be thrown. + * + * @param chainName + * @param fn + * @param opts - leaseTimeout will be ignored + */ public async withWallet( chainName: ChainName, fn: WithWalletExecutor, @@ -239,7 +254,7 @@ export class WalletManager { const wallet = await this.acquireLock(chainName, opts); try { - return fn(wallet); + await fn(wallet); } catch (error) { this.logger.error(`Error while executing wallet function: ${error}`); throw error; @@ -265,4 +280,15 @@ export class WalletManager { return manager.getBalances(); } + + private async runGuarded(supplier: () => Promise, onSuccess: () => void, onFailure: () => void): Promise { + try { + const result = await supplier(); + onSuccess(); + return result; + } catch (e) { + onFailure(); + throw e; + } + } } diff --git a/src/wallets/base-wallet.ts b/src/wallets/base-wallet.ts index fca7ffb..494e59b 100644 --- a/src/wallets/base-wallet.ts +++ b/src/wallets/base-wallet.ts @@ -1,5 +1,4 @@ export interface WalletPool { - acquire(resourceId?: string): Promise; blockAndAcquire(blockTimeout: number, resourceId?: string): Promise; release(wallet: string): Promise; } @@ -147,8 +146,8 @@ export abstract class WalletToolbox { return balances; } - public async acquire(address?: string, leaseTimeout?: number): Promise { - const timeout = leaseTimeout || DEFAULT_WALLET_ACQUIRE_TIMEOUT; + public async acquire(address?: string, acquireTimeout?: number): Promise { + const timeout = acquireTimeout || DEFAULT_WALLET_ACQUIRE_TIMEOUT; // this.grpcClient.acquireWallet(address); const walletAddress = await this.walletPool.blockAndAcquire(timeout, address); diff --git a/src/wallets/evm/polygon.config.ts b/src/wallets/evm/polygon.config.ts index 91512bc..9501e67 100644 --- a/src/wallets/evm/polygon.config.ts +++ b/src/wallets/evm/polygon.config.ts @@ -1,4 +1,3 @@ -import { DEVNET } from '../index'; import { EvmDefaultConfigs } from './index'; export const POLYGON = 'polygon'; diff --git a/src/wallets/index.ts b/src/wallets/index.ts index 64b8ba3..c207b2f 100644 --- a/src/wallets/index.ts +++ b/src/wallets/index.ts @@ -57,7 +57,7 @@ export function isSolanaChain(chainName: ChainName): chainName is SolanaChainNam export function isSuiChain(chainName: ChainName): chainName is SuiChainName { return chainName in SUI_CHAINS; -}; +} export function createWalletToolbox( network: string, chainName: string, wallets: WalletConfig[], walletOptions: WalletOptions @@ -82,3 +82,4 @@ export function createWalletToolbox( throw new Error(`Unknown chain name ${chainName}`); } } + diff --git a/src/wallets/sui/index.ts b/src/wallets/sui/index.ts index 4f53fb3..12c7d0b 100644 --- a/src/wallets/sui/index.ts +++ b/src/wallets/sui/index.ts @@ -11,7 +11,6 @@ import { pullSuiNativeBalance, pullSuiTokenBalances, pullSuiTokenData, transferS import { SUI_CHAIN_CONFIG, SUI, - SUI_NATIVE_COIN_MODULE, SuiDefaultConfig, } from "./sui.config"; import { getSuiAddressFromPrivateKey } from "../../balances/sui"; @@ -99,21 +98,19 @@ export class SuiWalletToolbox extends WalletToolbox { throw new Error(`Invalid token address: ${token}`); } - const knownTokens = SUI_CHAIN_CONFIGS[this.chainName].knownTokens[this.network]; - - if (!(token.toUpperCase() in knownTokens)) { + if (!(this.isKnownToken(token))) { throw new Error(`Unknown token address: ${token}`); } return true; } public parseTokensConfig(tokens: string[], failOnInvalidTokens: boolean): string[] { - const knownTokens = SUI_CHAIN_CONFIGS[this.chainName].knownTokens[this.network]; + const knownTokens = this.getKnownTokens(); const validTokens: string[] = []; for (const token of tokens) { if (this.isValidNativeTokenAddress(token)) { validTokens.push(token); - } else if (token.toUpperCase() in knownTokens) { + } else if (this.isKnownToken(token)) { validTokens.push(token.toUpperCase()); } else if (failOnInvalidTokens) { throw new Error(`Invalid token address: ${token}`); @@ -122,9 +119,19 @@ export class SuiWalletToolbox extends WalletToolbox { return validTokens; } + private getKnownTokens(): Record { + return SUI_CHAIN_CONFIGS[this.chainName].knownTokens[this.network]; + } + + private isKnownToken(token: string): boolean { + return token.toUpperCase() in this.getKnownTokens(); + } + public async warmup() { const tokens = this.walletTokens(this.wallets); - await mapConcurrent(tokens, async (tokenAddress: string): Promise => { + await mapConcurrent(tokens, async (token: string): Promise => { + // token can be stored as symbol or address, so we normalize to address here + const tokenAddress = this.isKnownToken(token) ? this.getKnownTokens()[token] : token; this.tokenData[tokenAddress] = await pullSuiTokenData(this.provider, tokenAddress); }, 1); diff --git a/src/wallets/wallet-pool.ts b/src/wallets/wallet-pool.ts index ba8ee91..28fd0a0 100644 --- a/src/wallets/wallet-pool.ts +++ b/src/wallets/wallet-pool.ts @@ -1,4 +1,4 @@ -import { WalletInterface, WalletPool } from './base-wallet'; +import { WalletPool } from './base-wallet'; type Resource = { id: string; @@ -19,7 +19,7 @@ export class LocalWalletPool implements WalletPool { } } - public async acquire(resourceId?: string): Promise { + private async acquire(resourceId?: string): Promise { const resource = resourceId ? this.resources[resourceId] : Object.values(this.resources).find((resource) => !resource.locked); diff --git a/src/balances/sui.test.ts b/test/balances/sui.test.ts similarity index 95% rename from src/balances/sui.test.ts rename to test/balances/sui.test.ts index 9c13e78..d645331 100644 --- a/src/balances/sui.test.ts +++ b/test/balances/sui.test.ts @@ -16,7 +16,7 @@ describe("Test sui balance util", () => { ["-1", 6, "-0.000001"], // negative minimal precision for usdc [(2n ** 64n - 1n).toString(), 8, "184467440737.09551615"], // max uint64 ])("Test format units", (rawUnits: string, decimals: number, expectedFormattedValue: string) => { - let result = formatFixed(rawUnits, decimals); + const result = formatFixed(rawUnits, decimals); expect(result).toBe(expectedFormattedValue); }); }); \ No newline at end of file diff --git a/test/wallet-manager.test.ts b/test/wallet-manager.test.ts new file mode 100644 index 0000000..8652f16 --- /dev/null +++ b/test/wallet-manager.test.ts @@ -0,0 +1,83 @@ +import { describe, expect, afterEach, test } from "@jest/globals"; +import { WalletManager } from "../src/wallet-manager"; +import { ChainName } from "../src/wallets"; +import { WithWalletExecutor } from "../src/chain-wallet-manager"; +import { ETHEREUM } from "../src/wallets/evm/ethereum.config"; + +const ETH_ADDR = "0x80C67432656d59144cEFf962E8fAF8926599bCF8"; +let walletManager: WalletManager; + +describe("wallet-manager", () => { + afterEach(() => { + walletManager.stop(); + }); + + test("should execute sequentially per address", async () => { + const markers: number[] = []; + + givenAWalletManager(); + + await Promise.all([ + await whenWalletCalled(ETHEREUM, async () => { markers.push(0); }), + await whenWalletCalled(ETHEREUM, async () => { markers.push(1); }) + ]); + + expect(markers).toEqual([0, 1]); + }); + + test("should continue when something goes wrong", async () => { + const markers: number[] = []; + + givenAWalletManager(); + + await Promise.allSettled([ + whenWalletCalled(ETHEREUM, async () => { throw new Error("oops"); }), + whenWalletCalled(ETHEREUM, async () => { markers.push(1); }, 50) + ]); + + expect(markers).toEqual([1]); + }); + + test("should fail if timeout is reached", async () => { + const markers: number[] = []; + const waitToAcquireTimeout = 10; + + givenAWalletManager(); + + const settledWithWalletCalls = await Promise.allSettled([ + whenWalletCalled(ETHEREUM, async () => { + await delay(waitToAcquireTimeout * 2) + markers.push(0); + }, waitToAcquireTimeout), + whenWalletCalled(ETHEREUM, async () => { markers.push(1); }, waitToAcquireTimeout) + ]); + + expect(markers).toEqual([0]); + expect(settledWithWalletCalls[1].status).toBe("rejected"); + }, 20_000); + +}); + +const delay = (ms: number) => new Promise((resolve, reject) => setTimeout(resolve, ms)); + +const givenAWalletManager = (rebalance = false) => { + const cfg = { + [ETHEREUM]: { + rebalance: { + enabled: rebalance, + }, + wallets: [ + { + address: ETH_ADDR, + tokens: ["USDC"], + }, + ], + } + }; + + walletManager = new WalletManager(cfg); +}; + +const whenWalletCalled = (chain: ChainName, executor: WithWalletExecutor, waitToAcquireTimeout = 1000) => + walletManager.withWallet(chain, executor, { waitToAcquireTimeout }); + diff --git a/src/wallets/sui/sui.test.ts b/test/wallets/sui/sui.test.ts similarity index 88% rename from src/wallets/sui/sui.test.ts rename to test/wallets/sui/sui.test.ts index 6a39207..8dbd67c 100644 --- a/src/wallets/sui/sui.test.ts +++ b/test/wallets/sui/sui.test.ts @@ -1,12 +1,15 @@ import { describe, expect, jest, test } from '@jest/globals'; -import { SuiWalletOptions, SuiWalletToolbox } from "./"; -import { SUI, SUI_MAINNET } from "./sui.config"; -import { WalletConfig } from "../index"; +import { SuiWalletOptions } from "../../../src/wallets/sui"; + +import { createWalletToolbox, WalletConfig } from "../../../src/wallets"; + import * as winston from "winston"; -import { pullSuiTokenData } from "../../balances/sui"; +import { pullSuiTokenData } from "../../../src/balances/sui"; + + -jest.mock("../../balances/sui", () => ({ +jest.mock("../../../src/balances/sui", () => ({ pullSuiTokenBalances: jest.fn(() => [ { coinType: "0x5d4b302506645c37ff133b98c4b50a5ae14841659738d6d733d59d0d217a93bf::coin::COIN", @@ -59,6 +62,7 @@ describe("sui wallet tests", () => { options = { logger, nodeUrl: "", + failOnInvalidTokens: false }; }) test("Pull token balances", async () => { @@ -68,9 +72,9 @@ describe("sui wallet tests", () => { tokens: ["USDC", "PEPE", "SHIBA"], }, ]; - const wallet = new SuiWalletToolbox( - SUI_MAINNET, - SUI, + const wallet = createWalletToolbox( + "mainnet", + "sui", wallets, options ); diff --git a/tsconfig.json b/tsconfig.json index 1440e0c..4ea9266 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -25,6 +25,7 @@ "exclude": [ "node_modules", "lib/**/*", - "src/**/*.test.ts" + "src/**/*.test.ts", + "test" ] }