From cfbef6c12afb18b4838089865bc57a89173907a5 Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Wed, 20 Mar 2024 11:50:24 -0400 Subject: [PATCH 01/10] improve(TokenClient): Cache allowance and bondToken eth_call data These values should rarely change. Follows strategy implemented in #1282 in the BaseAdapter (inventory manager) --- src/clients/TokenClient.ts | 72 +++++++++++++++++++++++++++++++------- 1 file changed, 60 insertions(+), 12 deletions(-) diff --git a/src/clients/TokenClient.ts b/src/clients/TokenClient.ts index 72a91ec4f..1aeba9e8a 100644 --- a/src/clients/TokenClient.ts +++ b/src/clients/TokenClient.ts @@ -15,12 +15,17 @@ import { runTransaction, toBN, winston, + getRedisCache, } from "../utils"; type TokenDataType = { [chainId: number]: { [token: string]: { balance: BigNumber; allowance: BigNumber } } }; type TokenShortfallType = { [chainId: number]: { [token: string]: { deposits: number[]; totalRequirement: BigNumber } }; }; +type FetchTokenDataReturnType = { + tokenData: Record; + chainId: number; +}; export class TokenClient { tokenData: TokenDataType = {}; @@ -180,11 +185,31 @@ export class TokenClient { async update(): Promise { this.logger.debug({ at: "TokenBalanceClient", message: "Updating TokenBalance client" }); - - const [balanceInfo, bondToken] = await Promise.all([ - Promise.all(Object.values(this.spokePoolClients).map((spokePoolClient) => this.fetchTokenData(spokePoolClient))), - this.hubPoolClient.hubPool.bondToken(), - ]); + const redis = await getRedisCache(this.logger); + let bondToken: string; + let balanceInfo: FetchTokenDataReturnType[]; + if (redis) { + const cachedBondToken = await redis.get(this.getBondTokenCacheKey()); + if (cachedBondToken !== null) { + bondToken = cachedBondToken; + } + } + if (!bondToken) { + [balanceInfo, bondToken] = await Promise.all([ + Promise.all( + Object.values(this.spokePoolClients).map((spokePoolClient) => this.fetchTokenData(spokePoolClient)) + ), + this.hubPoolClient.hubPool.bondToken(), + ]); + if (redis) { + // Save allowance in cache with no TTL as this should never change. + await redis.set(this.getBondTokenCacheKey(), bondToken); + } + } else { + balanceInfo = await Promise.all( + Object.values(this.spokePoolClients).map((spokePoolClient) => this.fetchTokenData(spokePoolClient)) + ); + } this.bondToken = new Contract(bondToken, ERC20.abi, this.hubPoolClient.hubPool.signer); @@ -210,10 +235,20 @@ export class TokenClient { this.logger.debug({ at: "TokenBalanceClient", message: "TokenBalance client updated!", balanceData }); } - async fetchTokenData(spokePoolClient: SpokePoolClient): Promise<{ - tokenData: Record; - chainId: number; - }> { + async getAllowanceCacheKey(spokePoolClient: SpokePoolClient, originToken: string): Promise { + return `l2TokenAllowance_${ + spokePoolClient.chainId + }_${originToken}_${await spokePoolClient.spokePool.signer.getAddress()}_targetContract:${ + spokePoolClient.spokePool.address + }`; + } + + getBondTokenCacheKey(): string { + return `bondToken_${this.hubPoolClient.hubPool.address}`; + } + + async fetchTokenData(spokePoolClient: SpokePoolClient): Promise { + const redis = await getRedisCache(this.logger); const tokens = spokePoolClient .getAllOriginTokens() .map((address) => new Contract(address, ERC20.abi, spokePoolClient.spokePool.signer)); @@ -223,9 +258,22 @@ export class TokenClient { await Promise.all( tokens.map(async (token) => { const balance: BigNumber = await token.balanceOf(this.relayerAddress, { blockTag }); - const allowance: BigNumber = await token.allowance(this.relayerAddress, spokePoolClient.spokePool.address, { - blockTag, - }); + let allowance: BigNumber; + if (redis) { + const result = await redis.get(await this.getAllowanceCacheKey(spokePoolClient, token.address)); + if (result !== null) { + allowance = toBN(result); + } + } + if (!allowance) { + allowance = await token.allowance(this.relayerAddress, spokePoolClient.spokePool.address, { + blockTag, + }); + if (redis) { + // Save allowance in cache with no TTL as these should never decrement. + await redis.set(await this.getAllowanceCacheKey(spokePoolClient, token.address), MAX_SAFE_ALLOWANCE); + } + } return [token.address, { balance, allowance }]; }) From 2b2b325ac83c502cb9ccd3b4f6b6610d191379ed Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Thu, 21 Mar 2024 19:37:39 -0400 Subject: [PATCH 02/10] Update TokenClient.ts --- src/clients/TokenClient.ts | 34 ++++++++++------------------------ 1 file changed, 10 insertions(+), 24 deletions(-) diff --git a/src/clients/TokenClient.ts b/src/clients/TokenClient.ts index 1aeba9e8a..bb68ea193 100644 --- a/src/clients/TokenClient.ts +++ b/src/clients/TokenClient.ts @@ -185,31 +185,17 @@ export class TokenClient { async update(): Promise { this.logger.debug({ at: "TokenBalanceClient", message: "Updating TokenBalance client" }); - const redis = await getRedisCache(this.logger); - let bondToken: string; - let balanceInfo: FetchTokenDataReturnType[]; - if (redis) { - const cachedBondToken = await redis.get(this.getBondTokenCacheKey()); - if (cachedBondToken !== null) { - bondToken = cachedBondToken; - } - } - if (!bondToken) { - [balanceInfo, bondToken] = await Promise.all([ - Promise.all( - Object.values(this.spokePoolClients).map((spokePoolClient) => this.fetchTokenData(spokePoolClient)) + const [balanceInfo, bondToken] = await Promise.all([ + Promise.all(Object.values(this.spokePoolClients).map((spokePoolClient) => this.fetchTokenData(spokePoolClient))), + // Make eth_call using nearest 100_000th block to take advantage of eth_call caching when blockTag is + // specified. This is unexpected to change so we are ok to rarely increase this block tag. + this.hubPoolClient.hubPool.bondToken({ + blockTag: Math.min( + this.hubPoolClient.deploymentBlock, + Math.round(this.hubPoolClient.latestBlockSearched / 100_000) * 100_000 ), - this.hubPoolClient.hubPool.bondToken(), - ]); - if (redis) { - // Save allowance in cache with no TTL as this should never change. - await redis.set(this.getBondTokenCacheKey(), bondToken); - } - } else { - balanceInfo = await Promise.all( - Object.values(this.spokePoolClients).map((spokePoolClient) => this.fetchTokenData(spokePoolClient)) - ); - } + }), + ]); this.bondToken = new Contract(bondToken, ERC20.abi, this.hubPoolClient.hubPool.signer); From c1703fb0eca15cd255f7cf239d0de2d961f81051 Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Thu, 21 Mar 2024 19:38:28 -0400 Subject: [PATCH 03/10] Update TokenClient.ts --- src/clients/TokenClient.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/clients/TokenClient.ts b/src/clients/TokenClient.ts index bb68ea193..1d22ee3b3 100644 --- a/src/clients/TokenClient.ts +++ b/src/clients/TokenClient.ts @@ -185,6 +185,7 @@ export class TokenClient { async update(): Promise { this.logger.debug({ at: "TokenBalanceClient", message: "Updating TokenBalance client" }); + const [balanceInfo, bondToken] = await Promise.all([ Promise.all(Object.values(this.spokePoolClients).map((spokePoolClient) => this.fetchTokenData(spokePoolClient))), // Make eth_call using nearest 100_000th block to take advantage of eth_call caching when blockTag is @@ -229,10 +230,6 @@ export class TokenClient { }`; } - getBondTokenCacheKey(): string { - return `bondToken_${this.hubPoolClient.hubPool.address}`; - } - async fetchTokenData(spokePoolClient: SpokePoolClient): Promise { const redis = await getRedisCache(this.logger); const tokens = spokePoolClient From d8b0f00554c9aa26a7def4a22ec61aca3f9c71a5 Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Thu, 21 Mar 2024 20:03:05 -0400 Subject: [PATCH 04/10] Update TokenClient.ts --- src/clients/TokenClient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/clients/TokenClient.ts b/src/clients/TokenClient.ts index 1d22ee3b3..6e0c2ac2a 100644 --- a/src/clients/TokenClient.ts +++ b/src/clients/TokenClient.ts @@ -191,7 +191,7 @@ export class TokenClient { // Make eth_call using nearest 100_000th block to take advantage of eth_call caching when blockTag is // specified. This is unexpected to change so we are ok to rarely increase this block tag. this.hubPoolClient.hubPool.bondToken({ - blockTag: Math.min( + blockTag: Math.max( this.hubPoolClient.deploymentBlock, Math.round(this.hubPoolClient.latestBlockSearched / 100_000) * 100_000 ), From 930791e5a96575cacda39dc4f402fe090ed406be Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Thu, 21 Mar 2024 20:26:01 -0400 Subject: [PATCH 05/10] Revert "Update TokenClient.ts" This reverts commit d8b0f00554c9aa26a7def4a22ec61aca3f9c71a5. --- src/clients/TokenClient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/clients/TokenClient.ts b/src/clients/TokenClient.ts index 6e0c2ac2a..1d22ee3b3 100644 --- a/src/clients/TokenClient.ts +++ b/src/clients/TokenClient.ts @@ -191,7 +191,7 @@ export class TokenClient { // Make eth_call using nearest 100_000th block to take advantage of eth_call caching when blockTag is // specified. This is unexpected to change so we are ok to rarely increase this block tag. this.hubPoolClient.hubPool.bondToken({ - blockTag: Math.max( + blockTag: Math.min( this.hubPoolClient.deploymentBlock, Math.round(this.hubPoolClient.latestBlockSearched / 100_000) * 100_000 ), From 835c9127a22a8865270b3ba1201a18ef4a85883b Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Thu, 21 Mar 2024 20:26:05 -0400 Subject: [PATCH 06/10] Revert "Update TokenClient.ts" This reverts commit c1703fb0eca15cd255f7cf239d0de2d961f81051. --- src/clients/TokenClient.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/clients/TokenClient.ts b/src/clients/TokenClient.ts index 1d22ee3b3..bb68ea193 100644 --- a/src/clients/TokenClient.ts +++ b/src/clients/TokenClient.ts @@ -185,7 +185,6 @@ export class TokenClient { async update(): Promise { this.logger.debug({ at: "TokenBalanceClient", message: "Updating TokenBalance client" }); - const [balanceInfo, bondToken] = await Promise.all([ Promise.all(Object.values(this.spokePoolClients).map((spokePoolClient) => this.fetchTokenData(spokePoolClient))), // Make eth_call using nearest 100_000th block to take advantage of eth_call caching when blockTag is @@ -230,6 +229,10 @@ export class TokenClient { }`; } + getBondTokenCacheKey(): string { + return `bondToken_${this.hubPoolClient.hubPool.address}`; + } + async fetchTokenData(spokePoolClient: SpokePoolClient): Promise { const redis = await getRedisCache(this.logger); const tokens = spokePoolClient From 0fa9ac0d7352c25944a15d85e2900e7b6b4d176a Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Thu, 21 Mar 2024 20:26:08 -0400 Subject: [PATCH 07/10] Revert "Update TokenClient.ts" This reverts commit 2b2b325ac83c502cb9ccd3b4f6b6610d191379ed. --- src/clients/TokenClient.ts | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/src/clients/TokenClient.ts b/src/clients/TokenClient.ts index bb68ea193..1aeba9e8a 100644 --- a/src/clients/TokenClient.ts +++ b/src/clients/TokenClient.ts @@ -185,17 +185,31 @@ export class TokenClient { async update(): Promise { this.logger.debug({ at: "TokenBalanceClient", message: "Updating TokenBalance client" }); - const [balanceInfo, bondToken] = await Promise.all([ - Promise.all(Object.values(this.spokePoolClients).map((spokePoolClient) => this.fetchTokenData(spokePoolClient))), - // Make eth_call using nearest 100_000th block to take advantage of eth_call caching when blockTag is - // specified. This is unexpected to change so we are ok to rarely increase this block tag. - this.hubPoolClient.hubPool.bondToken({ - blockTag: Math.min( - this.hubPoolClient.deploymentBlock, - Math.round(this.hubPoolClient.latestBlockSearched / 100_000) * 100_000 + const redis = await getRedisCache(this.logger); + let bondToken: string; + let balanceInfo: FetchTokenDataReturnType[]; + if (redis) { + const cachedBondToken = await redis.get(this.getBondTokenCacheKey()); + if (cachedBondToken !== null) { + bondToken = cachedBondToken; + } + } + if (!bondToken) { + [balanceInfo, bondToken] = await Promise.all([ + Promise.all( + Object.values(this.spokePoolClients).map((spokePoolClient) => this.fetchTokenData(spokePoolClient)) ), - }), - ]); + this.hubPoolClient.hubPool.bondToken(), + ]); + if (redis) { + // Save allowance in cache with no TTL as this should never change. + await redis.set(this.getBondTokenCacheKey(), bondToken); + } + } else { + balanceInfo = await Promise.all( + Object.values(this.spokePoolClients).map((spokePoolClient) => this.fetchTokenData(spokePoolClient)) + ); + } this.bondToken = new Contract(bondToken, ERC20.abi, this.hubPoolClient.hubPool.signer); From a472e4321c53e8d4ac4d36a55e0756a51e2d4f4e Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Thu, 21 Mar 2024 20:33:55 -0400 Subject: [PATCH 08/10] Refactor because using hard-coded blockTag breaks tests While fixing the block tag to the nearest 100_000th block when querying bondToken is nice, it breaks tests and I don't think they're worth fixing. The main motivation for taking advantage of the eth_call specific-blockTag caching is to make the code cleaner but I think this commit's refactor is nice too --- src/clients/TokenClient.ts | 109 ++++++++++++++++++------------------- 1 file changed, 54 insertions(+), 55 deletions(-) diff --git a/src/clients/TokenClient.ts b/src/clients/TokenClient.ts index 1aeba9e8a..0d7c2f222 100644 --- a/src/clients/TokenClient.ts +++ b/src/clients/TokenClient.ts @@ -185,31 +185,11 @@ export class TokenClient { async update(): Promise { this.logger.debug({ at: "TokenBalanceClient", message: "Updating TokenBalance client" }); - const redis = await getRedisCache(this.logger); - let bondToken: string; - let balanceInfo: FetchTokenDataReturnType[]; - if (redis) { - const cachedBondToken = await redis.get(this.getBondTokenCacheKey()); - if (cachedBondToken !== null) { - bondToken = cachedBondToken; - } - } - if (!bondToken) { - [balanceInfo, bondToken] = await Promise.all([ - Promise.all( - Object.values(this.spokePoolClients).map((spokePoolClient) => this.fetchTokenData(spokePoolClient)) - ), - this.hubPoolClient.hubPool.bondToken(), - ]); - if (redis) { - // Save allowance in cache with no TTL as this should never change. - await redis.set(this.getBondTokenCacheKey(), bondToken); - } - } else { - balanceInfo = await Promise.all( - Object.values(this.spokePoolClients).map((spokePoolClient) => this.fetchTokenData(spokePoolClient)) - ); - } + + const [balanceInfo, bondToken] = await Promise.all([ + Promise.all(Object.values(this.spokePoolClients).map((spokePoolClient) => this.fetchTokenData(spokePoolClient))), + this._getBondToken(), + ]); this.bondToken = new Contract(bondToken, ERC20.abi, this.hubPoolClient.hubPool.signer); @@ -235,20 +215,7 @@ export class TokenClient { this.logger.debug({ at: "TokenBalanceClient", message: "TokenBalance client updated!", balanceData }); } - async getAllowanceCacheKey(spokePoolClient: SpokePoolClient, originToken: string): Promise { - return `l2TokenAllowance_${ - spokePoolClient.chainId - }_${originToken}_${await spokePoolClient.spokePool.signer.getAddress()}_targetContract:${ - spokePoolClient.spokePool.address - }`; - } - - getBondTokenCacheKey(): string { - return `bondToken_${this.hubPoolClient.hubPool.address}`; - } - async fetchTokenData(spokePoolClient: SpokePoolClient): Promise { - const redis = await getRedisCache(this.logger); const tokens = spokePoolClient .getAllOriginTokens() .map((address) => new Contract(address, ERC20.abi, spokePoolClient.spokePool.signer)); @@ -258,23 +225,7 @@ export class TokenClient { await Promise.all( tokens.map(async (token) => { const balance: BigNumber = await token.balanceOf(this.relayerAddress, { blockTag }); - let allowance: BigNumber; - if (redis) { - const result = await redis.get(await this.getAllowanceCacheKey(spokePoolClient, token.address)); - if (result !== null) { - allowance = toBN(result); - } - } - if (!allowance) { - allowance = await token.allowance(this.relayerAddress, spokePoolClient.spokePool.address, { - blockTag, - }); - if (redis) { - // Save allowance in cache with no TTL as these should never decrement. - await redis.set(await this.getAllowanceCacheKey(spokePoolClient, token.address), MAX_SAFE_ALLOWANCE); - } - } - + const allowance = await this._getAllowance(spokePoolClient, token, blockTag); return [token.address, { balance, allowance }]; }) ) @@ -283,6 +234,54 @@ export class TokenClient { return { tokenData, chainId: spokePoolClient.chainId }; } + private async _getAllowanceCacheKey(spokePoolClient: SpokePoolClient, originToken: string): Promise { + return `l2TokenAllowance_${ + spokePoolClient.chainId + }_${originToken}_${await spokePoolClient.spokePool.signer.getAddress()}_targetContract:${ + spokePoolClient.spokePool.address + }`; + } + + private async _getAllowance( + spokePoolClient: SpokePoolClient, + token: Contract, + blockTag: number | "latest" + ): Promise { + const redis = await getRedisCache(this.logger); + let allowance: BigNumber; + if (redis) { + const result = await redis.get(await this._getAllowanceCacheKey(spokePoolClient, token.address)); + if (result !== null) { + allowance = toBN(result); + } + } + if (!allowance) { + allowance = await token.allowance(this.relayerAddress, spokePoolClient.spokePool.address, { + blockTag, + }); + if (redis) { + // Save allowance in cache with no TTL as these should never decrement. + await redis.set(await this._getAllowanceCacheKey(spokePoolClient, token.address), MAX_SAFE_ALLOWANCE); + } + } + return allowance; + } + + _getBondTokenCacheKey(): string { + return `bondToken_${this.hubPoolClient.hubPool.address}`; + } + + private async _getBondToken(): Promise { + const redis = await getRedisCache(this.logger); + if (redis) { + const cachedBondToken = await redis.get(this._getBondTokenCacheKey()); + if (cachedBondToken !== null) { + return cachedBondToken; + } + } + return await this.hubPoolClient.hubPool.bondToken(); + } + private _hasTokenPairData(chainId: number, token: string) { const hasData = !!this.tokenData?.[chainId]?.[token]; if (!hasData) { From 06011b6933baef0f365e2296f9b9c0495bdb538f Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Thu, 21 Mar 2024 20:37:18 -0400 Subject: [PATCH 09/10] Update TokenClient.ts --- src/clients/TokenClient.ts | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/clients/TokenClient.ts b/src/clients/TokenClient.ts index 0d7c2f222..748a884db 100644 --- a/src/clients/TokenClient.ts +++ b/src/clients/TokenClient.ts @@ -22,10 +22,6 @@ type TokenDataType = { [chainId: number]: { [token: string]: { balance: BigNumbe type TokenShortfallType = { [chainId: number]: { [token: string]: { deposits: number[]; totalRequirement: BigNumber } }; }; -type FetchTokenDataReturnType = { - tokenData: Record; - chainId: number; -}; export class TokenClient { tokenData: TokenDataType = {}; @@ -215,7 +211,10 @@ export class TokenClient { this.logger.debug({ at: "TokenBalanceClient", message: "TokenBalance client updated!", balanceData }); } - async fetchTokenData(spokePoolClient: SpokePoolClient): Promise { + async fetchTokenData(spokePoolClient: SpokePoolClient): Promise<{ + tokenData: Record; + chainId: number; + }> { const tokens = spokePoolClient .getAllOriginTokens() .map((address) => new Contract(address, ERC20.abi, spokePoolClient.spokePool.signer)); @@ -248,21 +247,18 @@ export class TokenClient { blockTag: number | "latest" ): Promise { const redis = await getRedisCache(this.logger); - let allowance: BigNumber; if (redis) { const result = await redis.get(await this._getAllowanceCacheKey(spokePoolClient, token.address)); if (result !== null) { - allowance = toBN(result); + return toBN(result); } } - if (!allowance) { - allowance = await token.allowance(this.relayerAddress, spokePoolClient.spokePool.address, { - blockTag, - }); - if (redis) { - // Save allowance in cache with no TTL as these should never decrement. - await redis.set(await this._getAllowanceCacheKey(spokePoolClient, token.address), MAX_SAFE_ALLOWANCE); - } + const allowance: BigNumber = await token.allowance(this.relayerAddress, spokePoolClient.spokePool.address, { + blockTag, + }); + if (redis) { + // Save allowance in cache with no TTL as these should never decrement. + await redis.set(await this._getAllowanceCacheKey(spokePoolClient, token.address), MAX_SAFE_ALLOWANCE); } return allowance; } @@ -279,7 +275,12 @@ export class TokenClient { return cachedBondToken; } } - return await this.hubPoolClient.hubPool.bondToken(); + const bondToken: string = await this.hubPoolClient.hubPool.bondToken(); + if (redis) { + // Save allowance in cache with no TTL as this should never change. + await redis.set(this._getBondTokenCacheKey(), bondToken); + } + return bondToken; } private _hasTokenPairData(chainId: number, token: string) { From 28058f398426f2da599beeead0c5e36fdb4d835b Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Fri, 22 Mar 2024 07:52:42 -0400 Subject: [PATCH 10/10] check for MAX_SAFE_ALLOWANCE --- src/clients/TokenClient.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/clients/TokenClient.ts b/src/clients/TokenClient.ts index 748a884db..e0b102afa 100644 --- a/src/clients/TokenClient.ts +++ b/src/clients/TokenClient.ts @@ -256,8 +256,8 @@ export class TokenClient { const allowance: BigNumber = await token.allowance(this.relayerAddress, spokePoolClient.spokePool.address, { blockTag, }); - if (redis) { - // Save allowance in cache with no TTL as these should never decrement. + if (allowance.gte(MAX_SAFE_ALLOWANCE) && redis) { + // Save allowance in cache with no TTL as these should be exhausted. await redis.set(await this._getAllowanceCacheKey(spokePoolClient, token.address), MAX_SAFE_ALLOWANCE); } return allowance; @@ -277,7 +277,8 @@ export class TokenClient { } const bondToken: string = await this.hubPoolClient.hubPool.bondToken(); if (redis) { - // Save allowance in cache with no TTL as this should never change. + // The bond token should not change, and using the wrong bond token will be immediately obvious, so cache with + // infinite TTL. await redis.set(this._getBondTokenCacheKey(), bondToken); } return bondToken;