improve: follow up fixes to treating depositId's internally as BN

I think the main change that might affect prod are changes in src/clients/SpokePoolClient
across-protocol · Jan 26, 2025 · 3e8c619 · 3e8c619
1 parent ad198e6
commit 3e8c619
Show file tree

Hide file tree

Showing 13 changed files with 48 additions and 43 deletions.
diff --git a/src/clients/InventoryClient.ts b/src/clients/InventoryClient.ts
@@ -436,7 +436,7 @@ export class InventoryClient {
     if (!this.validateOutputToken(deposit)) {
       const [srcChain, dstChain] = [getNetworkName(originChainId), getNetworkName(destinationChainId)];
       throw new Error(
-        `Unexpected ${dstChain} output token on ${srcChain} deposit ${deposit.depositId}` +
+        `Unexpected ${dstChain} output token on ${srcChain} deposit ${deposit.depositId.toString()}` +
           ` (${inputToken} != ${outputToken})`
       );
     }
@@ -572,7 +572,7 @@ export class InventoryClient {
       this.log(
         `Evaluated taking repayment on ${
           chainId === originChainId ? "origin" : chainId === destinationChainId ? "destination" : "slow withdrawal"
-        } chain ${chainId} for deposit ${deposit.depositId}: ${
+        } chain ${chainId} for deposit ${deposit.depositId.toString()}: ${
           expectedPostRelayAllocation.lte(effectiveTargetPct) ? "UNDERALLOCATED ✅" : "OVERALLOCATED ❌"
         }`,
         {

diff --git a/src/clients/ProfitClient.ts b/src/clients/ProfitClient.ts
@@ -447,7 +447,7 @@ export class ProfitClient {
 
       this.logger.debug({
         at: "ProfitClient#getFillProfitability",
-        message: `${l1Token.symbol} deposit ${depositId} with repayment on ${repaymentChainId} is ${profitable}`,
+        message: `${l1Token.symbol} deposit ${depositId.toString()} with repayment on ${repaymentChainId} is ${profitable}`,
         deposit,
         inputTokenPriceUsd: formatEther(fill.inputTokenPriceUsd),
         inputTokenAmountUsd: formatEther(fill.inputAmountUsd),

diff --git a/src/clients/SpokePoolClient.ts b/src/clients/SpokePoolClient.ts
@@ -4,7 +4,7 @@ import { Contract } from "ethers";
 import { clients, utils as sdkUtils } from "@across-protocol/sdk";
 import { Log } from "../interfaces";
 import { CHAIN_MAX_BLOCK_LOOKBACK, RELAYER_DEFAULT_SPOKEPOOL_INDEXER } from "../common/Constants";
-import { EventSearchConfig, getNetworkName, isDefined, MakeOptional, winston } from "../utils";
+import { bnZero, EventSearchConfig, getNetworkName, isDefined, MakeOptional, winston } from "../utils";
 import { EventsAddedMessage, EventRemovedMessage } from "../utils/SuperstructUtils";
 
 export type SpokePoolClient = clients.SpokePoolClient;
@@ -283,8 +283,8 @@ export class IndexedSpokePoolClient extends clients.SpokePoolClient {
     // Find the latest deposit Ids, and if there are no new events, fall back to already stored values.
     const fundsDeposited = eventsToQuery.indexOf("V3FundsDeposited");
     const [firstDepositId, latestDepositId] = [
-      events[fundsDeposited].at(0)?.args?.depositId ?? this.getDeposits().at(0) ?? 0,
-      events[fundsDeposited].at(-1)?.args?.depositId ?? this.getDeposits().at(-1) ?? 0,
+      events[fundsDeposited].at(0)?.args?.depositId ?? this.getDeposits().at(0) ?? bnZero,
+      events[fundsDeposited].at(-1)?.args?.depositId ?? this.getDeposits().at(-1) ?? bnZero,
     ];
 
     return {

diff --git a/src/dataworker/PoolRebalanceUtils.ts b/src/dataworker/PoolRebalanceUtils.ts
@@ -171,7 +171,7 @@ export function generateMarkdownForRootBundle(
     assert(
       inputAmount.gte(updatedOutputAmount),
       "Unexpected output amount for slow fill on" +
-        ` ${getNetworkName(leaf.relayData.originChainId)} depositId ${leaf.relayData.depositId}`
+        ` ${getNetworkName(leaf.relayData.originChainId)} depositId ${leaf.relayData.depositId.toString()}`
     );
 
     // @todo: When v2 types are removed, update the slowFill definition to be more precise about the member fields.

diff --git a/src/relayer/Relayer.ts b/src/relayer/Relayer.ts
@@ -240,7 +240,7 @@ export class Relayer {
       this.logger.debug({
         at: "Relayer::filterDeposit",
         message: `Skipping ${srcChain} deposit due to insufficient deposit confirmations.`,
-        depositId,
+        depositId: depositId.toString(),
         blockNumber,
         confirmations: latestBlockSearched - blockNumber,
         minConfirmations,
@@ -350,7 +350,7 @@ export class Relayer {
           message: "😱 Skipping deposit with greater unfilled amount than API suggested limit",
           limit,
           l1Token: l1Token.address,
-          depositId,
+          depositId: depositId.toString(),
           inputToken,
           inputAmount,
           originChainId,
@@ -611,9 +611,8 @@ export class Relayer {
     if (isDefined(this.pendingTxnReceipts[destinationChainId])) {
       this.logger.info({
         at: "Relayer::evaluateFill",
-        message: `${destChain} transaction queue has pending fills; skipping ${originChain} deposit ${depositId}...`,
+        message: `${destChain} transaction queue has pending fills; skipping ${originChain} deposit ${depositId.toString()}...`,
         originChainId,
-        depositId,
         transactionHash,
       });
       return;
@@ -623,8 +622,7 @@ export class Relayer {
     if (deposit.blockNumber > maxBlockNumber) {
       this.logger.debug({
         at: "Relayer::evaluateFill",
-        message: `Skipping ${originChain} deposit ${depositId} due to insufficient deposit confirmations.`,
-        depositId,
+        message: `Skipping ${originChain} deposit ${depositId.toString()} due to insufficient deposit confirmations.`,
         blockNumber: deposit.blockNumber,
         maxBlockNumber,
         transactionHash,
@@ -697,7 +695,7 @@ export class Relayer {
           const limits = this.fillLimits[originChainId].slice(limitIdx);
           this.logger.debug({
             at: "Relayer::evaluateFill",
-            message: `Skipping ${originChain} deposit ${depositId} due to anticipated origin chain overcommitment.`,
+            message: `Skipping ${originChain} deposit ${depositId.toString()} due to anticipated origin chain overcommitment.`,
             blockNumber,
             fillAmountUsd,
             limits,
@@ -723,7 +721,7 @@ export class Relayer {
           at: "Relayer::evaluateFill",
           message: "Skipping self-relay deposit originating from lite chain.",
           originChainId,
-          depositId,
+          depositId: depositId.toString(),
         });
         return;
       }
@@ -974,7 +972,7 @@ export class Relayer {
       // @todo (future) infer the updated outputAmount by zeroing the relayer fee in order to print the correct amount.
       return (
         `Requested slow fill 🐌 of ${outputAmount} ${symbol}` +
-        ` on ${dstChain} for ${srcChain} depositId ${depositId}.`
+        ` on ${dstChain} for ${srcChain} depositId ${depositId.toString()}.`
       );
     };
 
@@ -1001,7 +999,7 @@ export class Relayer {
     const { spokePoolClients } = this.clients;
     this.logger.debug({
       at: "Relayer::fillRelay",
-      message: `Filling v3 deposit ${deposit.depositId} with repayment on ${repaymentChainId}.`,
+      message: `Filling v3 deposit ${deposit.depositId.toString()} with repayment on ${repaymentChainId}.`,
       deposit,
       repaymentChainId,
       realizedLpFeePct,
@@ -1010,7 +1008,7 @@ export class Relayer {
     // If a deposit originates from a lite chain, then the repayment chain must be the origin chain.
     assert(
       !deposit.fromLiteChain || repaymentChainId === deposit.originChainId,
-      `Lite chain deposits must be filled on its origin chain (${deposit.originChainId}). Deposit Id: ${deposit.depositId}.`
+      `Lite chain deposits must be filled on its origin chain (${deposit.originChainId}). Deposit Id: ${deposit.depositId.toString()}.`
     );
 
     const [method, messageModifier, args] = !isDepositSpedUp(deposit)
@@ -1072,7 +1070,7 @@ export class Relayer {
         at: "Relayer::resolveRepaymentChain",
         message: deposit.fromLiteChain
           ? `Deposit ${depositId} originated from over-allocated lite chain ${originChain}`
-          : `Unable to identify a preferred repayment chain for ${originChain} deposit ${depositId}.`,
+          : `Unable to identify a preferred repayment chain for ${originChain} deposit ${depositId.toString()}.`,
         txn: blockExplorerLink(transactionHash, originChainId),
       });
       return {
@@ -1089,9 +1087,8 @@ export class Relayer {
     mark.stop({
       message: `Determined eligible repayment chains ${JSON.stringify(
         preferredChainIds
-      )} for deposit ${depositId} from ${originChain} to ${destinationChain}.`,
+      )} for deposit ${depositId.toString()} from ${originChain} to ${destinationChain}.`,
       preferredChainIds,
-      depositId,
       originChain,
       destinationChain,
     });
@@ -1168,7 +1165,7 @@ export class Relayer {
       profitabilityData = getProfitabilityDataForPreferredChainIndex(preferredChainIndex);
       this.logger.debug({
         at: "Relayer::resolveRepaymentChain",
-        message: `Selected preferred repayment chain ${preferredChain} for deposit ${depositId}, #${
+        message: `Selected preferred repayment chain ${preferredChain} for deposit ${depositId.toString()}, #${
           preferredChainIndex + 1
         } in eligible chains ${JSON.stringify(preferredChainIds)} list.`,
         profitableRepaymentChainIds,
@@ -1192,7 +1189,7 @@ export class Relayer {
         message: `Preferred chains ${JSON.stringify(
           preferredChainIds
         )} are not profitable. Checking destination chain ${destinationChainId} profitability.`,
-        deposit: { originChain, depositId, destinationChain, transactionHash },
+        deposit: { originChain, depositId: depositId.toString(), destinationChain, transactionHash },
       });
       // Evaluate destination chain profitability to see if we can reset preferred chain.
       const { lpFeePct: destinationChainLpFeePct } = repaymentFees.find(
@@ -1217,7 +1214,7 @@ export class Relayer {
         // maintaining its inventory allocation by sticking to its preferred repayment chain.
         this.logger[this.config.sendingRelaysEnabled ? "info" : "debug"]({
           at: "Relayer::resolveRepaymentChain",
-          message: `🦦 Taking repayment for filling deposit ${depositId} on preferred chains ${JSON.stringify(
+          message: `🦦 Taking repayment for filling deposit ${depositId.toString()} 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
@@ -1243,12 +1240,11 @@ export class Relayer {
         // 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 for deposit ${depositId} with preferred chains ${JSON.stringify(
+          message: `Taking repayment for deposit ${depositId.toString()} with preferred chains ${JSON.stringify(
             preferredChainIds
           )} on destination chain ${destinationChainId} would also not be profitable.`,
           deposit: {
             originChain,
-            depositId,
             destinationChain,
             transactionHash,
             token: hubPoolToken.symbol,
@@ -1363,7 +1359,7 @@ export class Relayer {
         const fromOverallocatedLiteChain = deposit.fromLiteChain && lpFeePct.eq(bnUint256Max);
         const depositFailedToSimulateWithMessage = !isMessageEmpty(deposit.message) && gasCost.eq(bnUint256Max);
         depositMrkdwn +=
-          `- DepositId ${deposit.depositId} (tx: ${depositblockExplorerLink})` +
+          `- DepositId ${deposit.depositId.toString()} (tx: ${depositblockExplorerLink})` +
           ` of input amount ${formattedInputAmount} ${inputSymbol}` +
           ` and output amount ${formattedOutputAmount} ${outputSymbol}` +
           ` from ${getNetworkName(originChainId)} to ${getNetworkName(destinationChainId)}` +
@@ -1422,7 +1418,7 @@ export class Relayer {
     const depositor = blockExplorerLink(deposit.depositor, deposit.originChainId);
     const inputAmount = createFormatFunction(2, 4, false, decimals)(deposit.inputAmount.toString());
 
-    let msg = `Relayed depositId ${deposit.depositId} from ${srcChain} to ${dstChain} of ${inputAmount} ${symbol}`;
+    let msg = `Relayed depositId ${deposit.depositId.toString()} from ${srcChain} to ${dstChain} of ${inputAmount} ${symbol}`;
     const realizedLpFeePct = formatFeePct(_realizedLpFeePct);
     const _totalFeePct = deposit.inputAmount
       .sub(deposit.outputAmount)

diff --git a/src/utils/RedisUtils.ts b/src/utils/RedisUtils.ts
@@ -134,7 +134,7 @@ export async function setRedisKey(
 }
 
 export function getRedisDepositKey(depositOrFill: Deposit | Fill): string {
-  return `deposit_${depositOrFill.originChainId}_${depositOrFill.depositId}`;
+  return `deposit_${depositOrFill.originChainId}_${depositOrFill.depositId.toString()}`;
 }
 
 export async function setDeposit(

diff --git a/test/Dataworker.executePoolRebalances.ts b/test/Dataworker.executePoolRebalances.ts
@@ -453,7 +453,7 @@ describe("Dataworker: Execute pool rebalances", async function () {
             originChainId: 10,
             depositor: randomAddress(),
             recipient: randomAddress(),
-            depositId: 0,
+            depositId: bnZero,
             inputToken: randomAddress(),
             inputAmount: slowFillAmount,
             outputToken: l1Token_1.address,

diff --git a/test/Dataworker.loadData.fill.ts b/test/Dataworker.loadData.fill.ts
@@ -576,7 +576,7 @@ describe("Dataworker: Load data used in all functions", async function () {
 
       // Send a fill now and force the bundle data client to query for the historical deposit.
       // However, send a fill that doesn't match with the above deposit. This should produce an invalid fill.
-      await fillV3Relay(spokePool_2, { ...depositObject, depositId: depositObject.depositId + 1 }, relayer);
+      await fillV3Relay(spokePool_2, { ...depositObject, depositId: depositObject.depositId.add(1) }, relayer);
       await updateAllClients();
       const fills = spokePoolClient_2.getFills();
       expect(fills.length).to.equal(1);
@@ -930,7 +930,7 @@ describe("Dataworker: Load data used in all functions", async function () {
       expect(convertToNumericStrings(refunds)).to.deep.equal(expectedRefunds);
 
       // Send an invalid fill and check it is not included.
-      await fillV3Relay(spokePool_2, { ...deposit1, depositId: deposit1.depositId + 1 }, relayer, repaymentChainId);
+      await fillV3Relay(spokePool_2, { ...deposit1, depositId: deposit1.depositId.add(1) }, relayer, repaymentChainId);
       await updateAllClients();
       expect(
         convertToNumericStrings(

diff --git a/test/Dataworker.loadData.slowFill.ts b/test/Dataworker.loadData.slowFill.ts
@@ -551,7 +551,7 @@ describe("BundleDataClient: Slow fill handling & validation", async function ()
       inputAmount: deposit.inputAmount.add(1),
       outputAmount: deposit.outputAmount.add(1),
       originChainId: destinationChainId,
-      depositId: deposit.depositId + 1,
+      depositId: deposit.depositId.add(1),
       fillDeadline: deposit.fillDeadline + 1,
       exclusivityDeadline: deposit.exclusivityDeadline + 1,
       message: randomAddress(),
@@ -623,7 +623,7 @@ describe("BundleDataClient: Slow fill handling & validation", async function ()
     expect(deposits.length).to.equal(0);
 
     // Send a slow fill request now and force the bundle data client to query for the historical deposit.
-    await requestSlowFill(spokePool_2, relayer, { ...depositObject, depositId: depositObject.depositId + 1 });
+    await requestSlowFill(spokePool_2, relayer, { ...depositObject, depositId: depositObject.depositId.add(1) });
     await updateAllClients();
     const requests = spokePoolClient_2.getSlowFillRequestsForOriginChain(originChainId);
     expect(requests.length).to.equal(1);

diff --git a/test/InventoryClient.RefundChain.ts b/test/InventoryClient.RefundChain.ts
@@ -138,7 +138,9 @@ describe("InventoryClient: Refund chain selection", async function () {
     beforeEach(async function () {
       const inputAmount = toBNWei(1);
       sampleDepositData = {
-        depositId: 0,
+        depositId: bnZero,
+        fromLiteChain: false,
+        toLiteChain: false,
         originChainId: MAINNET,
         destinationChainId: OPTIMISM,
         depositor: owner.address,
@@ -319,7 +321,9 @@ describe("InventoryClient: Refund chain selection", async function () {
     beforeEach(async function () {
       const inputAmount = toBNWei(1);
       sampleDepositData = {
-        depositId: 0,
+        depositId: bnZero,
+        fromLiteChain: false,
+        toLiteChain: false,
         originChainId: POLYGON,
         destinationChainId: OPTIMISM,
         depositor: owner.address,
@@ -489,7 +493,9 @@ describe("InventoryClient: Refund chain selection", async function () {
       (inventoryClient as MockInventoryClient).setBalanceOnChainForL1Token(undefined);
       const inputAmount = toBNWei(1);
       sampleDepositData = {
-        depositId: 0,
+        depositId: bnZero,
+        fromLiteChain: false,
+        toLiteChain: false,
         originChainId: POLYGON,
         destinationChainId: MAINNET,
         depositor: owner.address,
@@ -570,7 +576,9 @@ describe("InventoryClient: Refund chain selection", async function () {
 
       const inputAmount = toMegaWei(100);
       sampleDepositData = {
-        depositId: 0,
+        depositId: bnZero,
+        fromLiteChain: false,
+        toLiteChain: false,
         originChainId: ARBITRUM,
         destinationChainId: OPTIMISM,
         depositor: owner.address,

diff --git a/test/ProfitClient.ConsiderProfitability.ts b/test/ProfitClient.ConsiderProfitability.ts
@@ -43,7 +43,7 @@ describe("ProfitClient: Consider relay profit", () => {
   const gasFeePct = toBNWei(1).div(1e4);
   const v3DepositTemplate: Deposit = {
     originChainId,
-    depositId: 1,
+    depositId: BigNumber.from(1),
     destinationChainId,
     depositor: randomAddress(),
     recipient: randomAddress(),

diff --git a/test/Relayer.BasicFill.ts b/test/Relayer.BasicFill.ts
@@ -769,7 +769,7 @@ describe("Relayer: Check for Unfilled Deposits and Fill", async function () {
         originChainConfirmations[0].minConfirmations >
         spokePoolClient_2.latestBlockSearched - deposit1.blockNumber
       ) {
-        await fillV3Relay(spokePool_2, { ...deposit1, depositId: randomNumber(), outputAmount: bnZero }, relayer);
+        await fillV3Relay(spokePool_2, { ...deposit1, depositId: BigNumber.from(randomNumber()), outputAmount: bnZero }, relayer);
         await updateAllClients();
       }
       const originChainLimits = relayerInstance.computeOriginChainLimits(originChainId);
@@ -795,7 +795,7 @@ describe("Relayer: Check for Unfilled Deposits and Fill", async function () {
         originChainConfirmations[0].minConfirmations >
         spokePoolClient_2.latestBlockSearched - deposit2.blockNumber
       ) {
-        await fillV3Relay(spokePool_2, { ...deposit2, depositId: randomNumber(), outputAmount: bnZero }, relayer);
+        await fillV3Relay(spokePool_2, { ...deposit2, depositId: BigNumber.from(randomNumber()), outputAmount: bnZero }, relayer);
         await updateAllClients();
       }
 

diff --git a/test/TokenClient.TokenShortfall.ts b/test/TokenClient.TokenShortfall.ts
@@ -2,6 +2,7 @@ import { SpokePoolClient, TokenClient } from "../src/clients";
 import { MockConfigStoreClient, MockHubPoolClient } from "./mocks";
 import { originChainId, destinationChainId, ZERO_ADDRESS } from "./constants";
 import {
+  BigNumber,
   Contract,
   SignerWithAddress,
   createSpyLogger,
@@ -82,7 +83,7 @@ describe("TokenClient: Token shortfall", async function () {
     const balance = toBNWei(69);
     await erc20_2.mint(owner.address, balance);
     await updateAllClients();
-    const depositId = 1;
+    const depositId = BigNumber.from(1);
     let needed = toBNWei(420);
     let shortfall = needed.sub(balance);
     tokenClient.captureTokenShortfall(destinationChainId, erc20_2.address, depositId, toBNWei(420));
@@ -93,7 +94,7 @@ describe("TokenClient: Token shortfall", async function () {
     expect(tokenShortFallData.deposits).to.deep.equal([depositId]);
 
     // A subsequent shortfall deposit of 42 should add to the token shortfall and append the deposit id as 351+42 = 393.
-    const depositId2 = 2;
+    const depositId2 = BigNumber.from(2);
 
     tokenClient.captureTokenShortfall(destinationChainId, erc20_2.address, depositId2, toBNWei(42));
     needed = needed.add(toBNWei(42));