From 13ec536217c69923b9d5e99f109162e6401b8ed0 Mon Sep 17 00:00:00 2001 From: nicholaspai <9457025+nicholaspai@users.noreply.github.com> Date: Sun, 19 May 2024 17:10:02 -0400 Subject: [PATCH 1/7] fix(Finalizer): Dedup transaction hash list when creating list of transactions to query for CCTP l2->l1 transactions (#1535) --- src/finalizer/utils/arbitrum.ts | 2 +- src/finalizer/utils/cctp/l1ToL2.ts | 4 +--- src/finalizer/utils/cctp/l2ToL1.ts | 27 +++++++++++++++++++-------- src/finalizer/utils/opStack.ts | 2 +- src/finalizer/utils/polygon.ts | 2 +- src/finalizer/utils/zkSync.ts | 2 +- 6 files changed, 24 insertions(+), 15 deletions(-) diff --git a/src/finalizer/utils/arbitrum.ts b/src/finalizer/utils/arbitrum.ts index 2f7e178d6..c6c013129 100644 --- a/src/finalizer/utils/arbitrum.ts +++ b/src/finalizer/utils/arbitrum.ts @@ -30,7 +30,7 @@ export async function arbitrumOneFinalizer( // Arbitrum takes 7 days to finalize withdrawals, so don't look up events younger than that. const redis = await getRedisCache(logger); const [fromBlock, toBlock] = await Promise.all([ - getBlockForTimestamp(chainId, getCurrentTime() - 9 * 60 * 60 * 24, undefined, redis), + getBlockForTimestamp(chainId, getCurrentTime() - 14 * 60 * 60 * 24, undefined, redis), getBlockForTimestamp(chainId, getCurrentTime() - 7 * 60 * 60 * 24, undefined, redis), ]); logger.debug({ diff --git a/src/finalizer/utils/cctp/l1ToL2.ts b/src/finalizer/utils/cctp/l1ToL2.ts index 3f012a557..e9769f398 100644 --- a/src/finalizer/utils/cctp/l1ToL2.ts +++ b/src/finalizer/utils/cctp/l1ToL2.ts @@ -32,9 +32,7 @@ export async function cctpL1toL2Finalizer( spokePoolClient: SpokePoolClient, l1ToL2AddressesToFinalize: string[] ): Promise { - // Let's just assume for now CCTP transfers don't take longer than 1 day and can - // happen very quickly. - const lookback = getCurrentTime() - 60 * 60 * 24; + const lookback = getCurrentTime() - 60 * 60 * 24 * 7; const redis = await getRedisCache(logger); const fromBlock = await getBlockForTimestamp(hubPoolClient.chainId, lookback, undefined, redis); logger.debug({ diff --git a/src/finalizer/utils/cctp/l2ToL1.ts b/src/finalizer/utils/cctp/l2ToL1.ts index 878340ddc..b7f0bd535 100644 --- a/src/finalizer/utils/cctp/l2ToL1.ts +++ b/src/finalizer/utils/cctp/l2ToL1.ts @@ -6,7 +6,9 @@ import { CONTRACT_ADDRESSES, Multicall2Call, chainIdsToCctpDomains } from "../.. import { Contract, Signer, + TOKEN_SYMBOLS_MAP, assert, + compareAddressesSimple, getBlockForTimestamp, getCurrentTime, getNetworkName, @@ -24,9 +26,7 @@ export async function cctpL2toL1Finalizer( hubPoolClient: HubPoolClient, spokePoolClient: SpokePoolClient ): Promise { - // Let's just assume for now CCTP transfers don't take longer than 1 day and can - // happen very quickly. - const lookback = getCurrentTime() - 60 * 60 * 24; + const lookback = getCurrentTime() - 60 * 60 * 24 * 7; const redis = await getRedisCache(logger); const fromBlock = await getBlockForTimestamp(hubPoolClient.chainId, lookback, undefined, redis); logger.debug({ @@ -67,14 +67,25 @@ async function resolveRelatedTxnReceipts( targetDestinationChainId: number, latestBlockToFinalize: number ): Promise { + const sourceChainId = client.chainId; + // Dedup the txnReceipt list because there might be multiple tokens bridged events in the same txn hash. + + const uniqueTxnHashes = new Set(); + client + .getTokensBridged() + .filter( + (bridgeEvent) => + bridgeEvent.blockNumber >= latestBlockToFinalize && + compareAddressesSimple(bridgeEvent.l2TokenAddress, TOKEN_SYMBOLS_MAP._USDC.addresses[sourceChainId]) + ) + .forEach((bridgeEvent) => uniqueTxnHashes.add(bridgeEvent.transactionHash)); + // Resolve the receipts to all collected txns const txnReceipts = await Promise.all( - client - .getTokensBridged() - .filter((bridgeEvent) => bridgeEvent.blockNumber >= latestBlockToFinalize) - .map((bridgeEvent) => client.spokePool.provider.getTransactionReceipt(bridgeEvent.transactionHash)) + Array.from(uniqueTxnHashes).map((hash) => client.spokePool.provider.getTransactionReceipt(hash)) ); - return resolveCCTPRelatedTxns(txnReceipts, client.chainId, targetDestinationChainId); + + return resolveCCTPRelatedTxns(txnReceipts, sourceChainId, targetDestinationChainId); } /** diff --git a/src/finalizer/utils/opStack.ts b/src/finalizer/utils/opStack.ts index 1865bde3c..60602f01a 100644 --- a/src/finalizer/utils/opStack.ts +++ b/src/finalizer/utils/opStack.ts @@ -53,8 +53,8 @@ export async function opStackFinalizer( // - Don't try to withdraw tokens that are not past the 7 day challenge period const redis = await getRedisCache(logger); const [earliestBlockToFinalize, latestBlockToProve] = await Promise.all([ + getBlockForTimestamp(chainId, getCurrentTime() - 14 * 60 * 60 * 24, undefined, redis), getBlockForTimestamp(chainId, getCurrentTime() - 7 * 60 * 60 * 24, undefined, redis), - getBlockForTimestamp(chainId, getCurrentTime() - 60 * 60 * 24, undefined, redis), ]); const { recentTokensBridgedEvents = [], olderTokensBridgedEvents = [] } = groupBy( spokePoolClient.getTokensBridged(), diff --git a/src/finalizer/utils/polygon.ts b/src/finalizer/utils/polygon.ts index 5b6212278..4a616dcc5 100644 --- a/src/finalizer/utils/polygon.ts +++ b/src/finalizer/utils/polygon.ts @@ -46,7 +46,7 @@ export async function polygonFinalizer( const { chainId } = spokePoolClient; const posClient = await getPosClient(signer); - const lookback = getCurrentTime() - 60 * 60 * 24; + const lookback = getCurrentTime() - 60 * 60 * 24 * 7; const redis = await getRedisCache(logger); const fromBlock = await getBlockForTimestamp(chainId, lookback, undefined, redis); diff --git a/src/finalizer/utils/zkSync.ts b/src/finalizer/utils/zkSync.ts index 7326f6f15..88305d9c5 100644 --- a/src/finalizer/utils/zkSync.ts +++ b/src/finalizer/utils/zkSync.ts @@ -47,7 +47,7 @@ export async function zkSyncFinalizer( // older than 2 days and earlier than 1 day. const redis = await getRedisCache(logger); const [fromBlock, toBlock] = await Promise.all([ - getBlockForTimestamp(l2ChainId, getCurrentTime() - 2 * 60 * 60 * 24, undefined, redis), + getBlockForTimestamp(l2ChainId, getCurrentTime() - 8 * 60 * 60 * 24, undefined, redis), getBlockForTimestamp(l2ChainId, getCurrentTime() - 1 * 60 * 60 * 24, undefined, redis), ]); logger.debug({ From 2af75df694f805f960eeaaa3172acd14fcc22ad7 Mon Sep 17 00:00:00 2001 From: "James Morris, MS" <96435344+james-a-morris@users.noreply.github.com> Date: Mon, 20 May 2024 10:53:28 -0400 Subject: [PATCH 2/7] fix(finalizer): polygon (#1538) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(finalizer): polygon * improve(finalizer): Filter USDC from arbitrum and op stack finalizers (#1539) * fix(Finalizer): Dedup transaction hash list when creating list of transactions to query for CCTP l2->l1 transactions We’re not de-duping the set of txn receipts to query for CCTP l2 to l1 deposits, but we do this in the l1 to l2 direciton. What’s unique for L2 to L1 is we look for SpokePool.TokensBridged() events but we forget there might be multiple of these in the same block * improve(finalizer): Increase margin of error for finalizer to cover for multiple day downtime Currently the finalizer is optimized for run-time speed and tries to look over as short of an event search window as possible. However, this provides very little margin for error in case the finalizer goes down for multiple days. * Update l2ToL1.ts * add arbitrum/optimism filters * Update l2ToL1.ts * Update arbitrum.ts --------- Co-authored-by: nicholaspai <9457025+nicholaspai@users.noreply.github.com> --- src/finalizer/utils/arbitrum.ts | 29 +++++++++++++-------- src/finalizer/utils/linea/l1ToL2.ts | 4 +-- src/finalizer/utils/linea/l2ToL1.ts | 5 ++-- src/finalizer/utils/opStack.ts | 40 +++++++++++++++++++---------- src/finalizer/utils/polygon.ts | 8 ++++++ 5 files changed, 56 insertions(+), 30 deletions(-) diff --git a/src/finalizer/utils/arbitrum.ts b/src/finalizer/utils/arbitrum.ts index c6c013129..e3e93f3e4 100644 --- a/src/finalizer/utils/arbitrum.ts +++ b/src/finalizer/utils/arbitrum.ts @@ -11,6 +11,8 @@ import { getRedisCache, getBlockForTimestamp, getL1TokenInfo, + compareAddressesSimple, + TOKEN_SYMBOLS_MAP, } from "../../utils"; import { TokensBridged } from "../../interfaces"; import { HubPoolClient, SpokePoolClient } from "../../clients"; @@ -139,17 +141,24 @@ async function getAllMessageStatuses( // This is important for bridge transactions containing multiple events. const logIndexesForMessage = getUniqueLogIndex(tokensBridged); return ( - await Promise.all( - tokensBridged.map((e, i) => getMessageOutboxStatusAndProof(logger, e, mainnetSigner, logIndexesForMessage[i])) + ( + await Promise.all( + tokensBridged.map((e, i) => getMessageOutboxStatusAndProof(logger, e, mainnetSigner, logIndexesForMessage[i])) + ) ) - ) - .map((result, i) => { - return { - ...result, - info: tokensBridged[i], - }; - }) - .filter((result) => result.message !== undefined); + .map((result, i) => { + return { + ...result, + info: tokensBridged[i], + }; + }) + // USDC withdrawals for Arbitrum should be finalized via the CCTP Finalizer. + .filter( + (result) => + result.message !== undefined && + !compareAddressesSimple(result.info.l2TokenAddress, TOKEN_SYMBOLS_MAP["_USDC"].addresses[CHAIN_ID]) + ) + ); } async function getMessageOutboxStatusAndProof( diff --git a/src/finalizer/utils/linea/l1ToL2.ts b/src/finalizer/utils/linea/l1ToL2.ts index c2575ccc7..18760fe4e 100644 --- a/src/finalizer/utils/linea/l1ToL2.ts +++ b/src/finalizer/utils/linea/l1ToL2.ts @@ -40,9 +40,7 @@ export async function lineaL1ToL2Finalizer( ); // Optimize block range for querying Linea's MessageSent events on L1. - // We want to conservatively query for events that are between 0 and 24 hours old - // because Linea L1->L2 messages are claimable after ~20 mins. - const { fromBlock, toBlock } = await getBlockRangeByHoursOffsets(l1ChainId, 24, 0); + const { fromBlock, toBlock } = await getBlockRangeByHoursOffsets(l1ChainId, 24 * 7, 0); logger.debug({ at: "Finalizer#LineaL1ToL2Finalizer", message: "Linea MessageSent event filter", diff --git a/src/finalizer/utils/linea/l2ToL1.ts b/src/finalizer/utils/linea/l2ToL1.ts index f5affc139..6649b7ba7 100644 --- a/src/finalizer/utils/linea/l2ToL1.ts +++ b/src/finalizer/utils/linea/l2ToL1.ts @@ -27,9 +27,8 @@ export async function lineaL2ToL1Finalizer( const getMessagesWithStatusByTxHash = makeGetMessagesWithStatusByTxHash(l2Contract, l1ClaimingService); // Optimize block range for querying relevant source events on L2. - // We want to conservatively query for events that are between 8 and 72 hours old - // because Linea L2->L1 messages are claimable after 6 - 32 hours - const { fromBlock, toBlock } = await getBlockRangeByHoursOffsets(l2ChainId, 72, 8); + // Linea L2->L1 messages are claimable after 6 - 32 hours + const { fromBlock, toBlock } = await getBlockRangeByHoursOffsets(l2ChainId, 24 * 8, 6); logger.debug({ at: "Finalizer#LineaL2ToL1Finalizer", message: "Linea TokensBridged event filter", diff --git a/src/finalizer/utils/opStack.ts b/src/finalizer/utils/opStack.ts index 60602f01a..adaad61ad 100644 --- a/src/finalizer/utils/opStack.ts +++ b/src/finalizer/utils/opStack.ts @@ -5,7 +5,9 @@ import { HubPoolClient, SpokePoolClient } from "../../clients"; import { TokensBridged } from "../../interfaces"; import { BigNumber, + CHAIN_IDs, chainIsOPStack, + compareAddressesSimple, convertFromWei, getBlockForTimestamp, getCachedProvider, @@ -16,6 +18,7 @@ import { getUniqueLogIndex, groupObjectCountsByProp, Signer, + TOKEN_SYMBOLS_MAP, winston, } from "../../utils"; import { Multicall2Call } from "../../common"; @@ -129,22 +132,31 @@ async function getCrossChainMessages( const logIndexesForMessage = getUniqueLogIndex(tokensBridged); return ( - await Promise.all( - tokensBridged.map( - async (l2Event, i) => - ( - await crossChainMessenger.getMessagesByTransaction(l2Event.transactionHash, { - direction: optimismSDK.MessageDirection.L2_TO_L1, - }) - )[logIndexesForMessage[i]] + ( + await Promise.all( + tokensBridged.map( + async (l2Event, i) => + ( + await crossChainMessenger.getMessagesByTransaction(l2Event.transactionHash, { + direction: optimismSDK.MessageDirection.L2_TO_L1, + }) + )[logIndexesForMessage[i]] + ) ) ) - ).map((message, i) => { - return { - message, - event: tokensBridged[i], - }; - }); + .map((message, i) => { + return { + message, + event: tokensBridged[i], + }; + }) + // USDC withdrawals for Base and Optimism should be finalized via the CCTP Finalizer. + .filter( + (e) => + !compareAddressesSimple(e.event.l2TokenAddress, TOKEN_SYMBOLS_MAP["_USDC"].addresses[_chainId]) || + !(_chainId === CHAIN_IDs.BASE || _chainId === CHAIN_IDs.OPTIMISM) + ) + ); } async function getMessageStatuses( diff --git a/src/finalizer/utils/polygon.ts b/src/finalizer/utils/polygon.ts index 4a616dcc5..c8bf962d4 100644 --- a/src/finalizer/utils/polygon.ts +++ b/src/finalizer/utils/polygon.ts @@ -13,6 +13,8 @@ import { getRedisCache, getBlockForTimestamp, getL1TokenInfo, + compareAddressesSimple, + TOKEN_SYMBOLS_MAP, } from "../../utils"; import { EthersError, TokensBridged } from "../../interfaces"; import { HubPoolClient, SpokePoolClient } from "../../clients"; @@ -110,6 +112,12 @@ async function getFinalizableTransactions( const exitStatus = await Promise.all( checkpointedTokensBridged.map(async (_, i) => { const payload = payloads[i]; + const { chainId, l2TokenAddress } = tokensBridged[i]; + + if (compareAddressesSimple(l2TokenAddress, TOKEN_SYMBOLS_MAP._USDC.addresses[chainId])) { + return { status: "NON_CANONICAL_BRIDGE" }; + } + try { // If we can estimate gas for exit transaction call, then we can exit the burn tx, otherwise its likely // been processed. Note this will capture mislabel some exit txns that fail for other reasons as "exit From 066733b1d56174a89d3c4564c781cea8cc930b83 Mon Sep 17 00:00:00 2001 From: nicholaspai <9457025+nicholaspai@users.noreply.github.com> Date: Mon, 20 May 2024 13:00:26 -0400 Subject: [PATCH 3/7] feat(Relayer): Consider all eligible repayment chains if top one is unprofitable (#1426) * feat(Relayer): Consider all eligible repayment chains if top one is unprofitable Currently the algorithm is to consider destination chain only if top preferred chain is unprofitable and not equal to destination chain This PR changes the algo to have `determineRefundChain` to return all eligible repayment chain so that the relayer considers them all before falling back to destination chain, assuming that destination chain wasn't already considered * lint * Update Relayer.ts * Update Relayer.ts * Re add back tests * fix tests * Use net, not gross relayer fee pct in logs * refactor and run profitablity loop i parallel * Update Relayer.ts * Update InventoryClient.ts * Update InventoryClient.RefundChain.ts * Update InventoryClient.RefundChain.ts * Update InventoryClient.RefundChain.ts * Update InventoryClient.RefundChain.ts * Update InventoryClient.RefundChain.ts * Update InventoryClient.RefundChain.ts --- src/clients/InventoryClient.ts | 49 ++--- src/clients/ProfitClient.ts | 25 ++- src/relayer/Relayer.ts | 210 +++++++++++++-------- test/InventoryClient.RefundChain.ts | 77 +++++--- test/ProfitClient.ConsiderProfitability.ts | 9 +- test/mocks/MockInventoryClient.ts | 4 +- 6 files changed, 235 insertions(+), 139 deletions(-) diff --git a/src/clients/InventoryClient.ts b/src/clients/InventoryClient.ts index f05cb4cb8..58065ced2 100644 --- a/src/clients/InventoryClient.ts +++ b/src/clients/InventoryClient.ts @@ -383,26 +383,29 @@ export class InventoryClient { return isInputTokenUSDC && isOutputTokenBridgedUSDC; } - // Work out where a relay should be refunded to optimally manage the bots inventory. If the inventory management logic - // not enabled then return funds on the chain the deposit was filled on Else, use the following algorithm for each - // of the origin and destination chain: - // a) Find the chain virtual balance (current balance + pending relays + pending refunds) minus current shortfall. - // b) Find the cumulative virtual balance, including the total refunds on all chains and excluding current shortfall. - // c) Consider the size of a and b post relay (i.e after the relay is paid and all current transfers are settled what - // will the balances be on the target chain and the overall cumulative balance). - // d) Use c to compute what the post relay post current in-flight transactions allocation would be. Compare this - // number to the target threshold and: - // If this number is less than the target for the destination chain + rebalance then select destination chain. We - // slightly prefer destination to origin chain to support relayer capital efficiency. - // Else, if this number is less than the target for the origin chain + rebalance then select origin - // chain. - // Else, take repayment on the Hub chain for ease of transferring out of L1 to any L2. - async determineRefundChainId(deposit: V3Deposit, l1Token?: string): Promise { + /* + * Return all eligible repayment chains for a deposit. If inventory management is enabled, then this function will + * only choose chains where the post-relay balance allocation for a potential repayment chain is under the maximum + * allowed allocation on that chain. Origin, Destination, and HubChains are always evaluated as potential + * repayment chains in addition to "Slow Withdrawal chains" such as Base, Optimism and Arbitrum for which + * taking repayment would reduce HubPool utilization. Post-relay allocation percentages take into + * account pending cross-chain inventory-management transfers, upcoming bundle refunds, token shortfalls + * needed to cover other unfilled deposits in addition to current token balances. Slow withdrawal chains are only + * selected if the SpokePool's running balance for that chain is over the system's desired target. + * @dev The HubChain is always evaluated as a fallback option if the inventory management is enabled and all other + * chains are over-allocated. + * @dev If inventory management is disabled, then destinationChain is used as a default. + * @param deposit Deposit to determine repayment chains for. + * @param l1Token L1Token linked with deposited inputToken and repayement chain refund token. + * @returns list of chain IDs that are possible repayment chains for the deposit, sorted from highest + * to lowest priority. + */ + async determineRefundChainId(deposit: V3Deposit, l1Token?: string): Promise { const { originChainId, destinationChainId, inputToken, outputToken, outputAmount, inputAmount } = deposit; const hubChainId = this.hubPoolClient.chainId; if (!this.isInventoryManagementEnabled()) { - return destinationChainId; + return [destinationChainId]; } // The InventoryClient assumes 1:1 equivalency between input and output tokens. At the moment there is no support @@ -469,6 +472,7 @@ export class InventoryClient { chainsToEvaluate.push(originChainId); } + const eligibleRefundChains: number[] = []; // At this point, all chains to evaluate have defined token configs and are sorted in order of // highest priority to take repayment on, assuming the chain is under-allocated. for (const _chain of chainsToEvaluate) { @@ -535,15 +539,16 @@ export class InventoryClient { } ); if (expectedPostRelayAllocation.lte(thresholdPct)) { - return _chain; + eligibleRefundChains.push(_chain); } } - // None of the chain allocation percentages are lower than their target so take - // repayment on the hub chain by default. The caller has also set a token config so they are not expecting - // repayments to default to destination chain. If caller wanted repayments to default to destination - // chain, then they should not set a token config. - return hubChainId; + // Always add hubChain as a fallback option if inventory management is enabled. If none of the chainsToEvaluate + // were selected, then this function will return just the hub chain as a fallback option. + if (!eligibleRefundChains.includes(hubChainId)) { + eligibleRefundChains.push(hubChainId); + } + return eligibleRefundChains; } /** diff --git a/src/clients/ProfitClient.ts b/src/clients/ProfitClient.ts index 7ed094444..c53e904fd 100644 --- a/src/clients/ProfitClient.ts +++ b/src/clients/ProfitClient.ts @@ -431,17 +431,22 @@ export class ProfitClient { return fillAmount.mul(tokenPriceInUsd).div(bn10.pow(l1TokenInfo.decimals)); } - async getFillProfitability(deposit: V3Deposit, lpFeePct: BigNumber, l1Token: L1Token): Promise { + async getFillProfitability( + deposit: V3Deposit, + lpFeePct: BigNumber, + l1Token: L1Token, + repaymentChainId: number + ): Promise { const minRelayerFeePct = this.minRelayerFeePct(l1Token.symbol, deposit.originChainId, deposit.destinationChainId); const fill = await this.calculateFillProfitability(deposit, lpFeePct, minRelayerFeePct); if (!fill.profitable || this.debugProfitability) { - const { depositId, originChainId } = deposit; + const { depositId } = deposit; const profitable = fill.profitable ? "profitable" : "unprofitable"; this.logger.debug({ at: "ProfitClient#getFillProfitability", - message: `${l1Token.symbol} v3 deposit ${depositId} on chain ${originChainId} is ${profitable}`, + message: `${l1Token.symbol} v3 deposit ${depositId} with repayment on ${repaymentChainId} is ${profitable}`, deposit, inputTokenPriceUsd: formatEther(fill.inputTokenPriceUsd), inputTokenAmountUsd: formatEther(fill.inputAmountUsd), @@ -470,17 +475,19 @@ export class ProfitClient { async isFillProfitable( deposit: V3Deposit, lpFeePct: BigNumber, - l1Token: L1Token - ): Promise> { + l1Token: L1Token, + repaymentChainId: number + ): Promise> { let profitable = false; - let grossRelayerFeePct = bnZero; + let netRelayerFeePct = bnZero; let nativeGasCost = uint256Max; let tokenGasCost = uint256Max; try { - ({ profitable, grossRelayerFeePct, nativeGasCost, tokenGasCost } = await this.getFillProfitability( + ({ profitable, netRelayerFeePct, nativeGasCost, tokenGasCost } = await this.getFillProfitability( deposit, lpFeePct, - l1Token + l1Token, + repaymentChainId )); } catch (err) { this.logger.debug({ @@ -495,7 +502,7 @@ export class ProfitClient { profitable: profitable || (this.isTestnet && nativeGasCost.lt(uint256Max)), nativeGasCost, tokenGasCost, - grossRelayerFeePct, + netRelayerFeePct, }; } diff --git a/src/relayer/Relayer.ts b/src/relayer/Relayer.ts index d12326b90..7a73c6a16 100644 --- a/src/relayer/Relayer.ts +++ b/src/relayer/Relayer.ts @@ -26,6 +26,12 @@ const UNPROFITABLE_DEPOSIT_NOTICE_PERIOD = 60 * 60; // 1 hour type RepaymentFee = { paymentChainId: number; lpFeePct: BigNumber }; type BatchLPFees = { [depositKey: string]: RepaymentFee[] }; +type RepaymentChainProfitability = { + gasLimit: BigNumber; + gasCost: BigNumber; + relayerFeePct: BigNumber; + lpFeePct: BigNumber; +}; export class Relayer { public readonly relayerAddress: string; @@ -330,13 +336,12 @@ export class Relayer { const l1Token = hubPoolClient.getL1TokenInfoForL2Token(inputToken, originChainId); const selfRelay = [depositor, recipient].every((address) => address === this.relayerAddress); if (tokenClient.hasBalanceForFill(deposit) && !selfRelay) { - const { - repaymentChainId, - realizedLpFeePct, - relayerFeePct, - gasLimit: _gasLimit, - gasCost, - } = await this.resolveRepaymentChain(deposit, l1Token, lpFees); + const { repaymentChainId, repaymentChainProfitability } = await this.resolveRepaymentChain( + deposit, + l1Token, + lpFees + ); + const { relayerFeePct, gasCost, gasLimit: _gasLimit, lpFeePct: realizedLpFeePct } = repaymentChainProfitability; if (isDefined(repaymentChainId)) { const gasLimit = isMessageEmpty(resolveDepositMessage(deposit)) ? undefined : _gasLimit; this.fillRelay(deposit, repaymentChainId, realizedLpFeePct, gasLimit); @@ -545,7 +550,7 @@ export class Relayer { const { spokePoolClients, multiCallerClient } = this.clients; this.logger.debug({ at: "Relayer::fillRelay", - message: "Filling v3 deposit.", + message: `Filling v3 deposit ${deposit.depositId} with repayment on ${repaymentChainId}.`, deposit, repaymentChainId, realizedLpFeePct, @@ -573,16 +578,23 @@ export class Relayer { multiCallerClient.enqueueTransaction({ contract, chainId, method, args, gasLimit, message, mrkdwn }); } + /** + * @notice Returns repayment chain choice for deposit given repayment fees and the hubPoolToken associated with the + * deposit inputToken. + * @param deposit + * @param hubPoolToken L1 token object associated with the deposit inputToken. + * @param repaymentFees + * @returns repaymentChainId is defined if and only if a profitable repayment chain is found. + * @returns repaymentChainProfitability contains the profitability data of the repaymentChainId if it is defined + * or the profitability data of the most preferred repayment chain otherwise. + */ protected async resolveRepaymentChain( deposit: V3DepositWithBlock, hubPoolToken: L1Token, repaymentFees: RepaymentFee[] ): Promise<{ - gasLimit: BigNumber; repaymentChainId?: number; - realizedLpFeePct: BigNumber; - relayerFeePct: BigNumber; - gasCost: BigNumber; + repaymentChainProfitability: RepaymentChainProfitability; }> { const { inventoryClient, profitClient } = this.clients; const { depositId, originChainId, destinationChainId, inputAmount, outputAmount, transactionHash } = deposit; @@ -590,54 +602,128 @@ export class Relayer { const destinationChain = getNetworkName(destinationChainId); const start = performance.now(); - const preferredChainId = await inventoryClient.determineRefundChainId(deposit, hubPoolToken.address); + const preferredChainIds = await inventoryClient.determineRefundChainId(deposit, hubPoolToken.address); + assert(preferredChainIds.length > 0, `No preferred repayment chains found for deposit ${depositId}.`); this.logger.debug({ at: "Relayer::resolveRepaymentChain", - message: `Determined preferred repayment chain ${preferredChainId} for deposit from ${originChain} to ${destinationChain} in ${ + message: `Determined eligible repayment chains ${JSON.stringify( + preferredChainIds + )} for deposit ${depositId} from ${originChain} to ${destinationChain} in ${ Math.round(performance.now() - start) / 1000 }s.`, }); - const repaymentFee = repaymentFees?.find(({ paymentChainId }) => paymentChainId === preferredChainId); - assert(isDefined(repaymentFee)); - const { lpFeePct } = repaymentFee; - - const { - profitable, - nativeGasCost: gasLimit, - tokenGasCost: gasCost, - grossRelayerFeePct: relayerFeePct, // gross relayer fee is equal to total fee minus the lp fee. - } = await profitClient.isFillProfitable(deposit, lpFeePct, hubPoolToken); - // If preferred chain is different from the destination chain and the preferred chain - // is not profitable, then check if the destination chain is profitable. + const _repaymentFees = preferredChainIds.map((chainId) => + repaymentFees.find(({ paymentChainId }) => paymentChainId === chainId) + ); + const lpFeePcts = _repaymentFees.map(({ lpFeePct }) => lpFeePct); + + // For each eligible repayment chain, compute profitability and pick the one that is profitable. If none are + // profitable, then finally check the destination chain even if its not a preferred repayment chain. The idea + // here is that depositors are receiving quoted lp fees from the API that assumes repayment on the destination + // chain, so we should honor all repayments on the destination chain if it's profitable, even if it doesn't + // fit within our inventory management. + + const getRepaymentChainProfitability = async ( + preferredChainId: number, + lpFeePct: BigNumber + ): Promise<{ profitable: boolean; gasLimit: BigNumber; gasCost: BigNumber; relayerFeePct: BigNumber }> => { + const { + profitable, + nativeGasCost: gasLimit, + tokenGasCost: gasCost, + netRelayerFeePct: relayerFeePct, // net relayer fee is equal to total fee minus the lp fee. + } = await profitClient.isFillProfitable(deposit, lpFeePct, hubPoolToken, preferredChainId); + return { + profitable, + gasLimit, + gasCost, + relayerFeePct, + }; + }; + + const repaymentChainProfitabilities = await Promise.all( + preferredChainIds.map(async (preferredChainId, i) => { + const lpFeePct = lpFeePcts[i]; + assert(isDefined(lpFeePct), `Missing lp fee pct for chain potential repayment chain ${preferredChainId}`); + return getRepaymentChainProfitability(preferredChainId, lpFeePcts[i]); + }) + ); + const profitableRepaymentChainIds = preferredChainIds.filter((_, i) => repaymentChainProfitabilities[i].profitable); + + // @dev preferredChainId will not be defined until a chain is found to be profitable. + let preferredChain: number | undefined = undefined; + + // @dev The following internal function should be the only one used to set `preferredChain` above. + const getProfitabilityDataForPreferredChainIndex = (preferredChainIndex: number): RepaymentChainProfitability => { + const lpFeePct = lpFeePcts[preferredChainIndex]; + const { gasLimit, gasCost, relayerFeePct } = repaymentChainProfitabilities[preferredChainIndex]; + return { + gasLimit, + gasCost, + relayerFeePct, + lpFeePct, + }; + }; + let profitabilityData: RepaymentChainProfitability = getProfitabilityDataForPreferredChainIndex(0); + + // If there are any profitable repayment chains, then set preferred chain to the first one since the preferred + // chains are given to us by the InventoryClient sorted in priority order. + + if (profitableRepaymentChainIds.length > 0) { + preferredChain = profitableRepaymentChainIds[0]; + const preferredChainIndex = preferredChainIds.indexOf(preferredChain); + profitabilityData = getProfitabilityDataForPreferredChainIndex(preferredChainIndex); + this.logger.debug({ + at: "Relayer::resolveRepaymentChain", + message: `Selected preferred repayment chain ${preferredChain} for deposit ${depositId}, #${ + preferredChainIndex + 1 + } in eligible chains ${JSON.stringify(preferredChainIds)} list.`, + profitableRepaymentChainIds, + }); + } + + // If none of the preferred chains are profitable and they also don't include the destination chain, + // then check if the destination chain is profitable. // This assumes that the depositor is getting quotes from the /suggested-fees endpoint // in the frontend-v2 repo which assumes that repayment is the destination chain. If this is profitable, then // go ahead and use the preferred chain as repayment and log the lp fee delta. This is a temporary solution // so that depositors can continue to quote lp fees assuming repayment is on the destination chain until - // we come up with a smarter profitability check. - if (!profitable && preferredChainId !== destinationChainId) { + // we come up with a smarter fee quoting algorithm that takes into account relayer inventory management more + // accurately. + if (!isDefined(preferredChain) && !preferredChainIds.includes(destinationChainId)) { this.logger.debug({ at: "Relayer::resolveRepaymentChain", - message: `Preferred chain ${preferredChainId} is not profitable. Checking destination chain ${destinationChainId} profitability.`, + message: `Preferred chains ${JSON.stringify( + preferredChainIds + )} are not profitable. Checking destination chain ${destinationChainId} profitability.`, deposit: { originChain, depositId, destinationChain, transactionHash }, }); + // Evaluate destination chain profitability to see if we can reset preferred chain. const { lpFeePct: destinationChainLpFeePct } = repaymentFees.find( ({ paymentChainId }) => paymentChainId === destinationChainId ); - assert(isDefined(lpFeePct)); - + assert(isDefined(destinationChainLpFeePct)); const fallbackProfitability = await profitClient.isFillProfitable( deposit, destinationChainLpFeePct, - hubPoolToken + hubPoolToken, + destinationChainId ); + + // If destination chain is profitable, then use the top preferred chain as a favor to the depositor + // but log that we might be taking a loss. This is to not penalize an honest depositor who set their + // fees according to the API that assumes destination chain repayment. if (fallbackProfitability.profitable) { + preferredChain = preferredChainIds[0]; + const deltaRelayerFee = profitabilityData.relayerFeePct.sub(fallbackProfitability.netRelayerFeePct); // This is the delta in the gross relayer fee. If negative, then the destination chain would have had a higher // gross relayer fee, and therefore represents a virtual loss to the relayer. However, the relayer is // maintaining its inventory allocation by sticking to its preferred repayment chain. - const deltaRelayerFee = relayerFeePct.sub(fallbackProfitability.grossRelayerFeePct); this.logger[this.config.sendingRelaysEnabled ? "info" : "debug"]({ at: "Relayer::resolveRepaymentChain", - message: `🦦 Taking repayment for filling deposit ${depositId} on preferred chain ${preferredChainId} is unprofitable but taking repayment on destination chain ${destinationChainId} is profitable. Electing to take repayment on preferred chain as favor to depositor who assumed repayment on destination chain in their quote. Delta in gross relayer fee: ${formatFeePct( + message: `🦦 Taking repayment for filling deposit ${depositId} on preferred chains ${JSON.stringify( + preferredChainIds + )} is unprofitable but taking repayment on destination chain ${destinationChainId} is profitable. Electing to take repayment on top preferred chain ${preferredChain} as favor to depositor who assumed repayment on destination chain in their quote. Delta in net relayer fee: ${formatFeePct( deltaRelayerFee )}%`, deposit: { @@ -646,33 +732,24 @@ export class Relayer { token: hubPoolToken.symbol, txnHash: blockExplorerLink(transactionHash, originChainId), }, - preferredChain: getNetworkName(preferredChainId), - preferredChainLpFeePct: `${formatFeePct(lpFeePct)}%`, + preferredChain: getNetworkName(preferredChain), + preferredChainLpFeePct: `${formatFeePct(profitabilityData.lpFeePct)}%`, destinationChainLpFeePct: `${formatFeePct(destinationChainLpFeePct)}%`, // The delta will cut into the gross relayer fee. If negative, then taking the repayment on destination chain // would have been more profitable to the relayer because the lp fee would have been lower. - deltaLpFeePct: `${formatFeePct(destinationChainLpFeePct.sub(lpFeePct))}%`, + deltaLpFeePct: `${formatFeePct(destinationChainLpFeePct.sub(profitabilityData.lpFeePct))}%`, // relayer fee is the gross relayer fee using the destination chain lp fee: inputAmount - outputAmount - lpFee. - preferredChainRelayerFeePct: `${formatFeePct(relayerFeePct)}%`, - destinationChainRelayerFeePct: `${formatFeePct(fallbackProfitability.grossRelayerFeePct)}%`, + preferredChainRelayerFeePct: `${formatFeePct(profitabilityData.relayerFeePct)}%`, + destinationChainRelayerFeePct: `${formatFeePct(fallbackProfitability.netRelayerFeePct)}%`, deltaRelayerFee: `${formatFeePct(deltaRelayerFee)}%`, }); - - // We've checked that the user set the output amount honestly and assumed that the payment would be on - // destination chain, therefore we will fill them using the original preferred chain to maintain - // inventory assumptions and also quote the original relayer fee pct. - return { - repaymentChainId: preferredChainId, - realizedLpFeePct: lpFeePct, - relayerFeePct, - gasCost, - gasLimit, - }; } else { // If preferred chain is not profitable and neither is fallback, then return the original profitability result. this.logger.debug({ at: "Relayer::resolveRepaymentChain", - message: `Taking repayment on destination chain ${destinationChainId} would also not be profitable.`, + message: `Taking repayment for deposit ${depositId} with preferred chains ${JSON.stringify( + preferredChainIds + )} on destination chain ${destinationChainId} would also not be profitable.`, deposit: { originChain, depositId, @@ -682,37 +759,18 @@ export class Relayer { inputAmount, outputAmount, }, - preferredChain: getNetworkName(preferredChainId), - preferredChainLpFeePct: `${formatFeePct(lpFeePct)}%`, + preferredChain: getNetworkName(preferredChainIds[0]), + preferredChainLpFeePct: `${formatFeePct(profitabilityData.lpFeePct)}%`, destinationChainLpFeePct: `${formatFeePct(destinationChainLpFeePct)}%`, - preferredChainRelayerFeePct: `${formatFeePct(relayerFeePct)}%`, - destinationChainRelayerFeePct: `${formatFeePct(fallbackProfitability.grossRelayerFeePct)}%`, + preferredChainRelayerFeePct: `${formatFeePct(profitabilityData.relayerFeePct)}%`, + destinationChainRelayerFeePct: `${formatFeePct(fallbackProfitability.netRelayerFeePct)}%`, }); } } - this.logger.debug({ - at: "Relayer::resolveRepaymentChain", - message: `Preferred chain ${preferredChainId} is${profitable ? "" : " not"} profitable.`, - deposit: { - originChain, - depositId, - destinationChain, - transactionHash, - token: hubPoolToken.symbol, - inputAmount, - outputAmount, - }, - preferredChainLpFeePct: `${formatFeePct(lpFeePct)}%`, - preferredChainRelayerFeePct: `${formatFeePct(relayerFeePct)}%`, - }); - return { - repaymentChainId: profitable ? preferredChainId : undefined, - realizedLpFeePct: lpFeePct, - relayerFeePct, - gasCost, - gasLimit, + repaymentChainProfitability: profitabilityData, + repaymentChainId: preferredChain, }; } diff --git a/test/InventoryClient.RefundChain.ts b/test/InventoryClient.RefundChain.ts index cf6b3000a..36534c0b0 100644 --- a/test/InventoryClient.RefundChain.ts +++ b/test/InventoryClient.RefundChain.ts @@ -13,6 +13,7 @@ import { toBNWei, toWei, winston, + spyLogIncludes, } from "./utils"; import { ConfigStoreClient, InventoryClient } from "../src/clients"; // Tested @@ -160,7 +161,7 @@ describe("InventoryClient: Refund chain selection", async function () { // above the threshold of 12 and so the bot should choose to be refunded on L1. sampleDepositData.inputAmount = toWei(1); sampleDepositData.outputAmount = await computeOutputAmount(sampleDepositData); - expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.equal(MAINNET); + expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.deep.equal([MAINNET]); expect(lastSpyLogIncludes(spy, 'expectedPostRelayAllocation":"136690647482014388"')).to.be.true; // (20-1)/(140-1)=0.136 // Now consider a case where the relayer is filling a marginally larger relay of size 5 WETH. Now the post relay @@ -168,7 +169,7 @@ describe("InventoryClient: Refund chain selection", async function () { // choose to refund on the L2. sampleDepositData.inputAmount = toWei(5); sampleDepositData.outputAmount = await computeOutputAmount(sampleDepositData); - expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.equal(OPTIMISM); + expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.deep.equal([OPTIMISM, MAINNET]); expect(lastSpyLogIncludes(spy, 'expectedPostRelayAllocation":"111111111111111111"')).to.be.true; // (20-5)/(140-5)=0.11 // Now consider a bigger relay that should force refunds on the L2 chain. Set the relay size to 10 WETH. now post @@ -176,7 +177,7 @@ describe("InventoryClient: Refund chain selection", async function () { // set the refund on L2. sampleDepositData.inputAmount = toWei(10); sampleDepositData.outputAmount = await computeOutputAmount(sampleDepositData); - expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.equal(OPTIMISM); + expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.deep.equal([OPTIMISM, MAINNET]); expect(lastSpyLogIncludes(spy, 'expectedPostRelayAllocation":"76923076923076923"')).to.be.true; // (20-10)/(140-10)=0.076 }); @@ -215,7 +216,7 @@ describe("InventoryClient: Refund chain selection", async function () { sampleDepositData.outputToken = l2TokensForWeth[ARBITRUM]; sampleDepositData.inputAmount = toWei(1.69); sampleDepositData.outputAmount = await computeOutputAmount(sampleDepositData); - expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.equal(ARBITRUM); + expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.deep.equal([ARBITRUM, MAINNET]); expect(lastSpyLogIncludes(spy, 'chainShortfall":"15000000000000000000"')).to.be.true; expect(lastSpyLogIncludes(spy, 'chainVirtualBalance":"24800000000000000000"')).to.be.true; // (10+14.8)=24.8 @@ -233,7 +234,7 @@ describe("InventoryClient: Refund chain selection", async function () { // relay allocation is 4.8/120 = 0.04. This is below the threshold of 0.05 so the bot should refund on the target. sampleDepositData.inputAmount = toWei(5); sampleDepositData.outputAmount = await computeOutputAmount(sampleDepositData); - expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.equal(ARBITRUM); + expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.deep.equal([ARBITRUM, MAINNET]); // Check only the final step in the computation. expect(lastSpyLogIncludes(spy, 'expectedPostRelayAllocation":"40000000000000000"')).to.be.true; // 4.8/120 = 0.04 @@ -248,7 +249,7 @@ describe("InventoryClient: Refund chain selection", async function () { l2TokensForWeth[ARBITRUM], initialAllocation[ARBITRUM][mainnetWeth].add(toWei(10)) ); - expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.equal(MAINNET); + expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.deep.equal([MAINNET]); }); it("Correctly decides where to refund based on upcoming refunds", async function () { @@ -270,21 +271,22 @@ describe("InventoryClient: Refund chain selection", async function () { // L1 token and destination chain ID, otherwise it won't be counted in upcoming // refunds. hubPoolClient.setEnableAllL2Tokens(true); - expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.equal(MAINNET); + expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.deep.equal([MAINNET]); expect(lastSpyLogIncludes(spy, 'expectedPostRelayAllocation":"166666666666666666"')).to.be.true; // (20-5)/(140-5)=0.11 // If we set this to false in this test, the destination chain will be default used since the refund data // will be ignored. hubPoolClient.setEnableAllL2Tokens(false); - expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.equal(OPTIMISM); + expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.deep.equal([OPTIMISM, MAINNET]); }); it("Correctly throws when Deposit tokens are not equivalent", async function () { sampleDepositData.inputAmount = toWei(5); sampleDepositData.outputAmount = await computeOutputAmount(sampleDepositData); - expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.equal( - sampleDepositData.destinationChainId - ); + expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.deep.equal([ + sampleDepositData.destinationChainId, + 1, + ]); sampleDepositData.outputToken = ZERO_ADDRESS; const srcChain = getNetworkName(sampleDepositData.originChainId); @@ -313,9 +315,9 @@ describe("InventoryClient: Refund chain selection", async function () { false, // simMode false // prioritizeUtilization ); - expect(await _inventoryClient.determineRefundChainId(sampleDepositData)).to.equal( - sampleDepositData.destinationChainId - ); + expect(await _inventoryClient.determineRefundChainId(sampleDepositData)).to.deep.equal([ + sampleDepositData.destinationChainId, + ]); }); it("includes origin, destination in repayment chain list", async function () { const possibleRepaymentChains = inventoryClient.getPossibleRepaymentChainIds(sampleDepositData); @@ -356,8 +358,12 @@ describe("InventoryClient: Refund chain selection", async function () { // Relayer should choose to refund on destination over origin if both are under allocated sampleDepositData.inputAmount = toWei(5); sampleDepositData.outputAmount = await computeOutputAmount(sampleDepositData); - expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.equal(OPTIMISM); - expect(lastSpyLogIncludes(spy, 'expectedPostRelayAllocation":"111940298507462686"')).to.be.true; + expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.deep.equal([ + OPTIMISM, + POLYGON, + MAINNET, + ]); + expect(spyLogIncludes(spy, -2, 'expectedPostRelayAllocation":"111940298507462686"')).to.be.true; }); it("Origin chain allocation does not depend on subtracting from numerator", async function () { // Post relay allocation does not subtract anything from chain virtual balance, unlike @@ -374,7 +380,7 @@ describe("InventoryClient: Refund chain selection", async function () { // Relayer should default to hub chain. sampleDepositData.inputAmount = toWei(10); sampleDepositData.outputAmount = await computeOutputAmount(sampleDepositData); - expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.equal(MAINNET); + expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.deep.equal([MAINNET]); expect(lastSpyLogIncludes(spy, 'expectedPostRelayAllocation":"71428571428571428"')).to.be.true; }); it("Origin allocation is below target", async function () { @@ -389,7 +395,7 @@ describe("InventoryClient: Refund chain selection", async function () { // Relayer should choose to refund origin since destination isn't an option. sampleDepositData.inputAmount = toWei(5); sampleDepositData.outputAmount = await computeOutputAmount(sampleDepositData); - expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.equal(POLYGON); + expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.deep.equal([POLYGON, MAINNET]); expect(lastSpyLogIncludes(spy, 'expectedPostRelayAllocation":"35714285714285714"')).to.be.true; }); it("Origin allocation depends on outstanding transfers", async function () { @@ -404,7 +410,7 @@ describe("InventoryClient: Refund chain selection", async function () { // Relayer should choose to refund origin since destination isn't an option. sampleDepositData.inputAmount = toWei(5); sampleDepositData.outputAmount = await computeOutputAmount(sampleDepositData); - expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.equal(POLYGON); + expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.deep.equal([POLYGON, MAINNET]); // Now add outstanding transfers to Polygon that make the allocation above the target. Note that this // increases cumulative balance a bit. @@ -415,7 +421,7 @@ describe("InventoryClient: Refund chain selection", async function () { // Optimism (destination chain): (30-5)/(160-5)=16.1% > 12% // Polygon (origin chain): (15)/(160-5)=9.6% > 7% // Relayer should now default to hub chain. - expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.equal(MAINNET); + expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.deep.equal([MAINNET]); expect(lastSpyLogIncludes(spy, 'expectedPostRelayAllocation":"100000000000000000"')).to.be.true; }); it("Origin allocation depends on short falls", async function () { @@ -430,7 +436,7 @@ describe("InventoryClient: Refund chain selection", async function () { // Optimism (destination chain): (25-5)/(145-5)=14.3% > 12% // Polygon (origin chain): (0)/(145-5)=0% < 7% // Relayer should still use origin chain - expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.equal(POLYGON); + expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.deep.equal([POLYGON, MAINNET]); expect(lastSpyLogIncludes(spy, 'expectedPostRelayAllocation":"0"')).to.be.true; // (20-5)/(140-5)=0.11 }); it("Origin allocation depends on upcoming refunds", async function () { @@ -458,7 +464,7 @@ describe("InventoryClient: Refund chain selection", async function () { // Optimism (destination chain): (30-5)/(155-5)=16.7% > 12% // Polygon (origin chain): (10)/(155-5)=6.7% > 7% // Relayer should still pick origin chain but compute a different allocation. - expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.equal(POLYGON); + expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.deep.equal([POLYGON, MAINNET]); expect(lastSpyLogIncludes(spy, 'expectedPostRelayAllocation":"68965517241379310"')).to.be.true; }); it("includes origin, destination and hub chain in repayment chain list", async function () { @@ -519,13 +525,22 @@ describe("InventoryClient: Refund chain selection", async function () { it("selects slow withdrawal chain with excess running balance and under relayer allocation", async function () { // Initial allocations are all under allocated so the first slow withdrawal chain should be selected since it has // the highest overage. - expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.equal(ARBITRUM); + expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.deep.equal([ + ARBITRUM, + OPTIMISM, + POLYGON, + MAINNET, + ]); // If we instead drop the excess on Arbitrum to 0, then we should take repayment on // the next slow withdrawal chain. excessRunningBalances[ARBITRUM] = toWei("0"); (inventoryClient as MockInventoryClient).setExcessRunningBalances(mainnetWeth, excessRunningBalances); - expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.equal(OPTIMISM); + expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.deep.equal([ + OPTIMISM, + POLYGON, + MAINNET, + ]); }); it("includes slow withdrawal chains in possible repayment chain list", async function () { const possibleRepaymentChains = inventoryClient.getPossibleRepaymentChainIds(sampleDepositData); @@ -593,25 +608,31 @@ describe("InventoryClient: Refund chain selection", async function () { .forEach((chainId) => expect(tokenClient.getBalance(chainId, nativeUSDC[chainId]).eq(bnZero)).to.be.true); // All chains are at target balance; cumulative balance will go down but repaymentToken balances on all chains are unaffected. - expect(await inventoryClient.determineRefundChainId(sampleDepositData, mainnetUsdc)).to.equal(MAINNET); + expect(await inventoryClient.determineRefundChainId(sampleDepositData, mainnetUsdc)).to.deep.equal([MAINNET]); expect(lastSpyLogIncludes(spy, 'expectedPostRelayAllocation":"71942446043165467"')).to.be.true; // (10000-0)/(14000-100)=0.71942 // Even when the output amount is equal to the destination's entire balance, take repayment on mainnet. sampleDepositData.outputAmount = inventoryClient.getBalanceOnChain(OPTIMISM, mainnetUsdc); - expect(await inventoryClient.determineRefundChainId(sampleDepositData, mainnetUsdc)).to.equal(MAINNET); + expect(await inventoryClient.determineRefundChainId(sampleDepositData, mainnetUsdc)).to.deep.equal([MAINNET]); expect(lastSpyLogIncludes(spy, 'expectedPostRelayAllocation":"83333333333333333"')).to.be.true; // (10000-0)/(14000-2000)=0.8333 // Drop the relayer's repaymentToken balance on Optimism. Repayment chain should now be Optimism. let balance = tokenClient.getBalance(OPTIMISM, bridgedUSDC[OPTIMISM]); tokenClient.setTokenData(OPTIMISM, bridgedUSDC[OPTIMISM], bnZero); - expect(await inventoryClient.determineRefundChainId(sampleDepositData, mainnetUsdc)).to.equal(OPTIMISM); + expect(await inventoryClient.determineRefundChainId(sampleDepositData, mainnetUsdc)).to.deep.equal([ + OPTIMISM, + MAINNET, + ]); // Restore the Optimism balance and drop the Arbitrum balance. Repayment chain should now be Arbitrum. tokenClient.setTokenData(OPTIMISM, bridgedUSDC[OPTIMISM], balance); balance = tokenClient.getBalance(ARBITRUM, bridgedUSDC[ARBITRUM]); tokenClient.setTokenData(ARBITRUM, bridgedUSDC[ARBITRUM], bnZero); - expect(await inventoryClient.determineRefundChainId(sampleDepositData, mainnetUsdc)).to.equal(ARBITRUM); + expect(await inventoryClient.determineRefundChainId(sampleDepositData, mainnetUsdc)).to.deep.equal([ + ARBITRUM, + MAINNET, + ]); }); }); }); diff --git a/test/ProfitClient.ConsiderProfitability.ts b/test/ProfitClient.ConsiderProfitability.ts index 5cb4f4423..b12b33480 100644 --- a/test/ProfitClient.ConsiderProfitability.ts +++ b/test/ProfitClient.ConsiderProfitability.ts @@ -359,7 +359,7 @@ describe("ProfitClient: Consider relay profit", () => { netRelayerFeeUsd: formatEther(expected.netRelayerFeeUsd), }); - const { profitable } = await profitClient.isFillProfitable(deposit, lpFeePct, token); + const { profitable } = await profitClient.isFillProfitable(deposit, lpFeePct, token, destinationChainId); expect(profitable).to.equal(expected.profitable); } } @@ -429,7 +429,12 @@ describe("ProfitClient: Consider relay profit", () => { netRelayerFeeUsd: formatEther(expected.netRelayerFeeUsd), }); - const { profitable } = await profitClient.isFillProfitable(deposit, effectiveLpFeePct, token); + const { profitable } = await profitClient.isFillProfitable( + deposit, + effectiveLpFeePct, + token, + destinationChainId + ); expect(profitable).to.equal(expected.profitable); } } diff --git a/test/mocks/MockInventoryClient.ts b/test/mocks/MockInventoryClient.ts index 74da30567..a2333df2a 100644 --- a/test/mocks/MockInventoryClient.ts +++ b/test/mocks/MockInventoryClient.ts @@ -33,8 +33,8 @@ export class MockInventoryClient extends InventoryClient { } // eslint-disable-next-line @typescript-eslint/no-unused-vars - override async determineRefundChainId(_deposit: Deposit): Promise { - return this.inventoryConfig === null ? 1 : super.determineRefundChainId(_deposit); + override async determineRefundChainId(_deposit: Deposit): Promise { + return this.inventoryConfig === null ? [1] : super.determineRefundChainId(_deposit); } setExcessRunningBalances(l1Token: string, balances: { [chainId: number]: BigNumber }): void { From d15f9aae7e162dfa92a887ac18ebc422a460060f Mon Sep 17 00:00:00 2001 From: nicholaspai <9457025+nicholaspai@users.noreply.github.com> Date: Mon, 20 May 2024 14:42:14 -0400 Subject: [PATCH 4/7] fix(reporter): Accumulate L2 token balances mapped to same L1 token address (#1533) * fix(reporter): Accumulate L2 token balances mapped to same L1 token address Currently we are only querying L2 tokens mapped to l1 tokens via PoolRebalanceRoutes, which means we are not reporting Bridged USDC balances * fix test --- src/monitor/Monitor.ts | 40 +++++++++++++++++++++++++++++++--------- test/Monitor.ts | 40 +++++++++++++++++++++++++++++++++++++--- 2 files changed, 68 insertions(+), 12 deletions(-) diff --git a/src/monitor/Monitor.ts b/src/monitor/Monitor.ts index 17e121887..4f2cd5514 100644 --- a/src/monitor/Monitor.ts +++ b/src/monitor/Monitor.ts @@ -27,6 +27,7 @@ import { toBN, toBNWei, winston, + TOKEN_SYMBOLS_MAP, } from "../utils"; import { MonitorClients, updateMonitorClients } from "./MonitorClientHelper"; @@ -265,18 +266,13 @@ export class Monitor { // Update current balances of all tokens on each supported chain for each relayer. async updateCurrentRelayerBalances(relayerBalanceReport: RelayerBalanceReport): Promise { const { hubPoolClient } = this.clients; - const l1Tokens = hubPoolClient.getL1Tokens(); + const _l1Tokens = hubPoolClient.getL1Tokens(); for (const relayer of this.monitorConfig.monitoredRelayers) { for (const chainId of this.monitorChains) { - const l2ToL1Tokens = Object.fromEntries( - l1Tokens - .filter(({ address: l1Token }) => hubPoolClient.l2TokenEnabledForL1Token(l1Token, chainId)) - .map((l1Token) => { - const l2Token = hubPoolClient.getL2TokenForL1TokenAtBlock(l1Token.address, chainId); - return [l2Token, l1Token]; - }) + const l1Tokens = _l1Tokens.filter(({ address: l1Token }) => + hubPoolClient.l2TokenEnabledForL1Token(l1Token, chainId) ); - + const l2ToL1Tokens = this.getL2ToL1TokenMap(l1Tokens, chainId); const l2TokenAddresses = Object.keys(l2ToL1Tokens); const tokenBalances = await this._getBalances( l2TokenAddresses.map((address) => ({ @@ -300,6 +296,32 @@ export class Monitor { } } + // Returns a dictionary of L2 token addresses on this chain to their mapped L1 token info. For example, this + // will return a dictionary for Optimism including WETH, WBTC, USDC, USDC.e, USDT entries where the key is + // the token's Optimism address and the value is the equivalent L1 token info. + protected getL2ToL1TokenMap(l1Tokens: L1Token[], chainId: number): { [l2TokenAddress: string]: L1Token } { + return Object.fromEntries( + l1Tokens + .map((l1Token) => { + // @dev l2TokenSymbols is a list of all keys in TOKEN_SYMBOLS_MAP where the hub chain address is equal to the + // l1 token address. + const l2TokenSymbols = Object.entries(TOKEN_SYMBOLS_MAP) + .filter( + ([, { addresses }]) => + addresses[this.clients.hubPoolClient.chainId]?.toLowerCase() === l1Token.address.toLowerCase() + ) + .map(([symbol]) => symbol); + + // Create an entry for all L2 tokens that share a symbol with the L1 token. This includes tokens + // like USDC which has multiple L2 tokens mapped to the same L1 token for a given chain ID. + return l2TokenSymbols + .filter((symbol) => TOKEN_SYMBOLS_MAP[symbol].addresses[chainId] !== undefined) + .map((symbol) => [TOKEN_SYMBOLS_MAP[symbol].addresses[chainId], l1Token]); + }) + .flat() + ); + } + async checkBalances(): Promise { const { monitoredBalances } = this.monitorConfig; const balances = await this._getBalances(monitoredBalances); diff --git a/test/Monitor.ts b/test/Monitor.ts index 6dd54a5da..0cd353100 100644 --- a/test/Monitor.ts +++ b/test/Monitor.ts @@ -10,7 +10,7 @@ import { import { CrossChainTransferClient } from "../src/clients/bridges"; import { spokePoolClientsToProviders } from "../src/common"; import { Dataworker } from "../src/dataworker/Dataworker"; -import { BalanceType, V3DepositWithBlock } from "../src/interfaces"; +import { BalanceType, L1Token, V3DepositWithBlock } from "../src/interfaces"; import { ALL_CHAINS_NAME, Monitor, REBALANCE_FINALIZE_GRACE_PERIOD } from "../src/monitor/Monitor"; import { MonitorConfig } from "../src/monitor/MonitorConfig"; import { MAX_UINT_VAL, getNetworkName, toBN } from "../src/utils"; @@ -31,6 +31,20 @@ import { toBNWei, } from "./utils"; +type TokenMap = { [l2TokenAddress: string]: L1Token }; + +class TestMonitor extends Monitor { + private overriddenTokenMap: { [chainId: number]: TokenMap } = {}; + + setL2ToL1TokenMap(chainId: number, map: TokenMap): void { + this.overriddenTokenMap[chainId] = map; + } + // Override internal function that calls into externally defined and hard-coded TOKEN_SYMBOLS_MAP. + protected getL2ToL1TokenMap(l1Tokens: L1Token[], chainId): TokenMap { + return this.overriddenTokenMap[chainId] ?? super.getL2ToL1TokenMap(l1Tokens, chainId); + } +} + describe("Monitor", async function () { const TEST_NETWORK_NAMES = ["Hardhat1", "Hardhat2", "unknown", ALL_CHAINS_NAME]; let l1Token: Contract, l2Token: Contract, erc20_2: Contract; @@ -154,7 +168,7 @@ describe("Monitor", async function () { adapterManager = new MockAdapterManager(null, null, null, null); adapterManager.setSupportedChains(chainIds); crossChainTransferClient = new CrossChainTransferClient(spyLogger, chainIds, adapterManager); - monitorInstance = new Monitor(spyLogger, monitorConfig, { + monitorInstance = new TestMonitor(spyLogger, monitorConfig, { bundleDataClient, configStoreClient, multiCallerClient, @@ -163,7 +177,27 @@ describe("Monitor", async function () { tokenTransferClient, crossChainTransferClient, }); - + (monitorInstance as TestMonitor).setL2ToL1TokenMap(originChainId, { + [l2Token.address]: { + symbol: "L1Token1", + address: l1Token.address, + decimals: 18, + }, + }); + (monitorInstance as TestMonitor).setL2ToL1TokenMap(destinationChainId, { + [erc20_2.address]: { + symbol: "L1Token1", + address: l1Token.address, + decimals: 18, + }, + }); + (monitorInstance as TestMonitor).setL2ToL1TokenMap(hubPoolClient.chainId, { + [l1Token.address]: { + symbol: "L1Token1", + address: l1Token.address, + decimals: 18, + }, + }); await updateAllClients(); }); From 9131cefc009199111d445855c5cb594de8c933a4 Mon Sep 17 00:00:00 2001 From: nicholaspai <9457025+nicholaspai@users.noreply.github.com> Date: Tue, 21 May 2024 11:05:29 -0400 Subject: [PATCH 5/7] fix(CCTPFinalizer): Set `fromBlock` from correct `chainId` (#1544) Only Base has a block height lower than Ethereum's so we're missing all of the Base CCTP withdrawals --- src/finalizer/utils/cctp/l2ToL1.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/finalizer/utils/cctp/l2ToL1.ts b/src/finalizer/utils/cctp/l2ToL1.ts index b7f0bd535..9e85835c0 100644 --- a/src/finalizer/utils/cctp/l2ToL1.ts +++ b/src/finalizer/utils/cctp/l2ToL1.ts @@ -28,7 +28,7 @@ export async function cctpL2toL1Finalizer( ): Promise { const lookback = getCurrentTime() - 60 * 60 * 24 * 7; const redis = await getRedisCache(logger); - const fromBlock = await getBlockForTimestamp(hubPoolClient.chainId, lookback, undefined, redis); + const fromBlock = await getBlockForTimestamp(spokePoolClient.chainId, lookback, undefined, redis); logger.debug({ at: `Finalizer#CCTPL2ToL1Finalizer:${spokePoolClient.chainId}`, message: `MessageSent event filter for ${getNetworkName(spokePoolClient.chainId)} to L1`, From cd3428ef881b8ac22ba128096489216301933f04 Mon Sep 17 00:00:00 2001 From: nicholaspai <9457025+nicholaspai@users.noreply.github.com> Date: Tue, 21 May 2024 12:15:05 -0400 Subject: [PATCH 6/7] improve(reporter): Add special USDC.e entry (#1542) * improve(reporter): Add special USDC.e entry Adds some special logic to break out USDC.e balances from USDC. L1 USDC is counted with Native USDC balance. This is required to be a special case because the balance report uses the L1 token symbol mapped to all L2 tokens as the key, whereas USDC.e and Native USDC on L2s both map to a single USDC token, so we need to invent a "USDC.e" virtual L1 token to keep the structure of the report intact. A more scalable change would have to change the report structure fundamentally. * Update Monitor.ts --- src/monitor/Monitor.ts | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/src/monitor/Monitor.ts b/src/monitor/Monitor.ts index 4f2cd5514..3da81d23a 100644 --- a/src/monitor/Monitor.ts +++ b/src/monitor/Monitor.ts @@ -28,6 +28,7 @@ import { toBNWei, winston, TOKEN_SYMBOLS_MAP, + compareAddressesSimple, } from "../utils"; import { MonitorClients, updateMonitorClients } from "./MonitorClientHelper"; @@ -216,7 +217,16 @@ export class Monitor { async reportRelayerBalances(): Promise { const relayers = this.monitorConfig.monitoredRelayers; - const allL1Tokens = this.clients.hubPoolClient.getL1Tokens(); + const allL1Tokens = [...this.clients.hubPoolClient.getL1Tokens()]; // @dev deep clone since we modify the + // array below and we don't want to modify the HubPoolClient's version + // @dev Handle special case for L1 USDC which is mapped to two L2 tokens on some chains, so we can more easily + // see L2 Bridged USDC balance versus Native USDC. Add USDC.e right after the USDC element. + const indexOfUsdc = allL1Tokens.findIndex(({ symbol }) => symbol === "USDC"); + allL1Tokens.splice(indexOfUsdc, 0, { + symbol: "USDC.e", + address: TOKEN_SYMBOLS_MAP["USDC.e"].addresses[this.clients.hubPoolClient.chainId], + decimals: 6, + }); const chainIds = this.monitorChains; const allChainNames = chainIds.map(getNetworkName).concat([ALL_CHAINS_NAME]); const reports = this.initializeBalanceReports(relayers, allL1Tokens, allChainNames); @@ -284,9 +294,23 @@ export class Monitor { for (let i = 0; i < l2TokenAddresses.length; i++) { const tokenInfo = l2ToL1Tokens[l2TokenAddresses[i]]; + let l1TokenSymbol = tokenInfo.symbol; + + // @dev Handle special case for USDC so we can see Bridged USDC and Native USDC balances split out. + // HubChain USDC balance will be grouped with Native USDC balance arbitrarily. + const l2TokenAddress = l2TokenAddresses[i]; + if ( + l1TokenSymbol === "USDC" && + chainId !== hubPoolClient.chainId && + (compareAddressesSimple(TOKEN_SYMBOLS_MAP["USDC.e"].addresses[chainId], l2TokenAddress) || + compareAddressesSimple(TOKEN_SYMBOLS_MAP["USDbC"].addresses[chainId], l2TokenAddress)) + ) { + l1TokenSymbol = "USDC.e"; + } + this.updateRelayerBalanceTable( relayerBalanceReport[relayer], - tokenInfo.symbol, + l1TokenSymbol, getNetworkName(chainId), BalanceType.CURRENT, tokenBalances[i] From c67b9e76d8959263d82a6a20bb435dce83537180 Mon Sep 17 00:00:00 2001 From: Paul <108695806+pxrl@users.noreply.github.com> Date: Tue, 21 May 2024 18:52:42 +0200 Subject: [PATCH 7/7] improve(indexer): More debug logging (#1532) In pursuit of a transient error in production. It may be related to 5xx errors on the WebSocketProvider. --- src/libexec/RelayerSpokePoolIndexer.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/libexec/RelayerSpokePoolIndexer.ts b/src/libexec/RelayerSpokePoolIndexer.ts index 10f25db00..14cb34871 100644 --- a/src/libexec/RelayerSpokePoolIndexer.ts +++ b/src/libexec/RelayerSpokePoolIndexer.ts @@ -286,6 +286,20 @@ async function run(argv: string[]): Promise { try { providers = getWSProviders(chainId, quorum); assert(providers.length > 0, `Insufficient providers for ${chain} (required ${quorum} by quorum)`); + providers.forEach((provider) => { + provider.on("error", (err) => + logger.debug({ at: "RelayerSpokePoolIndexer::run", message: `Caught ${chain} provider error.`, err }) + ); + + provider.on("close", () => { + logger.debug({ + at: "RelayerSpokePoolIndexer::run", + message: `${chain} provider connection closed.`, + provider: getOriginFromURL(provider.connection.url), + }); + }); + }); + logger.debug({ at: "RelayerSpokePoolIndexer::run", message: `Starting ${chain} listener.`, events, opts }); await listen(eventMgr, spokePool, events, providers, opts); } catch (err) {