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

across-protocol · Jan 26, 2025 · 8f517ce · 8f517ce
1 parent 96139ff
commit 8f517ce
Show file tree

Hide file tree

Showing 8 changed files with 23 additions and 17 deletions.
diff --git a/src/clients/BundleDataClient/BundleDataClient.ts b/src/clients/BundleDataClient/BundleDataClient.ts
@@ -1311,7 +1311,11 @@ export class BundleDataClient {
   // spoke pool contract. However, this internal function is used to uniquely identify a bridging event
   // for speed since its easier to build a string from the event data than to hash it.
   private getRelayHashFromEvent(event: V3DepositWithBlock | V3FillWithBlock | SlowFillRequestWithBlock): string {
-    return `${event.depositor}-${event.recipient}-${event.exclusiveRelayer}-${event.inputToken}-${event.outputToken}-${event.inputAmount}-${event.outputAmount}-${event.originChainId}-${event.depositId}-${event.fillDeadline}-${event.exclusivityDeadline}-${event.message}-${event.destinationChainId}`;
+    return `${event.depositor}-${event.recipient}-${event.exclusiveRelayer}-${event.inputToken}-${event.outputToken}-${
+      event.inputAmount
+    }-${event.outputAmount}-${event.originChainId}-${event.depositId.toString()}-${event.fillDeadline}-${
+      event.exclusivityDeadline
+    }-${event.message}-${event.destinationChainId}`;
   }
 
   async getBundleBlockTimestamps(

diff --git a/src/clients/SpokePoolClient.ts b/src/clients/SpokePoolClient.ts
@@ -315,7 +315,7 @@ export class SpokePoolClient extends BaseAbstractClient {
 
     this.logger.debug({
       at: "SpokePoolClient::getDepositForFill",
-      message: `Rejected fill for ${getNetworkName(fill.originChainId)} deposit ${fill.depositId}.`,
+      message: `Rejected fill for ${getNetworkName(fill.originChainId)} deposit ${fill.depositId.toString()}.`,
       reason: match.reason,
       deposit,
       fill,
@@ -399,7 +399,7 @@ export class SpokePoolClient extends BaseAbstractClient {
    * @note This hash takes the form of: `${depositId}-${originChainId}`.
    */
   public getDepositHash(event: { depositId: BigNumber; originChainId: number }): string {
-    return `${event.depositId}-${event.originChainId}`;
+    return `${event.depositId.toString()}-${event.originChainId}`;
   }
 
   /**
@@ -816,7 +816,7 @@ export class SpokePoolClient extends BaseAbstractClient {
       const srcChain = getNetworkName(this.chainId);
       const dstChain = getNetworkName(destinationChainId);
       throw new Error(
-        `Could not find deposit ${depositId} for ${dstChain} fill` +
+        `Could not find deposit ${depositId.toString()} for ${dstChain} fill` +
           ` between ${srcChain} blocks [${searchBounds.low}, ${searchBounds.high}]`
       );
     }

diff --git a/src/clients/mocks/MockSpokePoolClient.ts b/src/clients/mocks/MockSpokePoolClient.ts
@@ -122,7 +122,7 @@ export class MockSpokePoolClient extends SpokePoolClient {
     const { blockNumber, transactionIndex } = deposit;
     let { depositId, depositor, destinationChainId, inputToken, inputAmount, outputToken, outputAmount } = deposit;
     depositId ??= this.numberOfDeposits;
-    assert(depositId.gte(this.numberOfDeposits), `${depositId} < ${this.numberOfDeposits}`);
+    assert(depositId.gte(this.numberOfDeposits), `${depositId.toString()} < ${this.numberOfDeposits}`);
     this.numberOfDeposits = depositId.add(bnOne);
 
     destinationChainId ??= random(1, 42161, false);

diff --git a/src/utils/CachingUtils.ts b/src/utils/CachingUtils.ts
@@ -52,5 +52,5 @@ export async function setDepositInCache(
  */
 export function getDepositKey(bridgeEvent: Deposit | Fill | SlowFillRequest): string {
   const relayHash = getRelayHashFromEvent(bridgeEvent);
-  return `deposit_${bridgeEvent.originChainId}_${bridgeEvent.depositId}_${relayHash}`;
+  return `deposit_${bridgeEvent.originChainId}_${bridgeEvent.depositId.toString()}_${relayHash}`;
 }
diff --git a/src/utils/DepositUtils.ts b/src/utils/DepositUtils.ts
@@ -56,7 +56,7 @@ export async function queryHistoricalDepositForFill(
     return {
       found: false,
       code: InvalidFill.DepositIdInvalid,
-      reason: `Deposit ID ${depositId} is outside of SpokePool bounds [${lowId},${highId}].`,
+      reason: `Deposit ID ${depositId.toString()} is outside of SpokePool bounds [${lowId},${highId}].`,
     };
   }
 
@@ -73,14 +73,14 @@ export async function queryHistoricalDepositForFill(
       return {
         found: false,
         code: InvalidFill.FillMismatch,
-        reason: `Fill for ${originChain} deposit ID ${depositId} is invalid (${match.reason}).`,
+        reason: `Fill for ${originChain} deposit ID ${depositId.toString()} is invalid (${match.reason}).`,
       };
     }
 
     return {
       found: false,
       code: InvalidFill.DepositIdNotFound,
-      reason: `${originChain} deposit ID ${depositId} not found in SpokePoolClient event buffer.`,
+      reason: `${originChain} deposit ID ${depositId.toString()} not found in SpokePoolClient event buffer.`,
     };
   }
 

diff --git a/src/utils/SpokeUtils.ts b/src/utils/SpokeUtils.ts
@@ -153,7 +153,7 @@ export async function getBlockRangeForDepositId(
     //                                     // targetId = 2 <- pass (does not trigger this error)
     //                                     // targetId = 3 <- pass (does not trigger this error)
     throw new Error(
-      `Target depositId is less than the initial low block (${targetDepositId} > ${lowestDepositIdInRange})`
+      `Target depositId is less than the initial low block (${targetDepositId.toString()} > ${lowestDepositIdInRange})`
     );
   }
 
@@ -280,7 +280,9 @@ export async function relayFillStatus(
 
   if (![FillStatus.Unfilled, FillStatus.RequestedSlowFill, FillStatus.Filled].includes(fillStatus)) {
     const { originChainId, depositId } = relayData;
-    throw new Error(`relayFillStatus: Unexpected fillStatus for ${originChainId} deposit ${depositId} (${fillStatus})`);
+    throw new Error(
+      `relayFillStatus: Unexpected fillStatus for ${originChainId} deposit ${depositId.toString()} (${fillStatus})`
+    );
   }
 
   return fillStatus;
@@ -369,7 +371,7 @@ export async function findFillBlock(
   if (initialFillStatus === FillStatus.Filled) {
     const { depositId, originChainId } = relayData;
     const [srcChain, dstChain] = [getNetworkName(originChainId), getNetworkName(destinationChainId)];
-    throw new Error(`${srcChain} deposit ${depositId} filled on ${dstChain} before block ${lowBlockNumber}`);
+    throw new Error(`${srcChain} deposit ${depositId.toString()} filled on ${dstChain} before block ${lowBlockNumber}`);
   }
 
   // Find the leftmost block where filledAmount equals the deposit amount.

diff --git a/test/SpokePoolClient.fills.ts b/test/SpokePoolClient.fills.ts
@@ -1,7 +1,7 @@
 import hre from "hardhat";
 import { SpokePoolClient } from "../src/clients";
 import { Deposit } from "../src/interfaces";
-import { bnOne, findFillBlock, getNetworkName } from "../src/utils";
+import { bnOne, bnZero, findFillBlock, getNetworkName } from "../src/utils";
 import { EMPTY_MESSAGE, ZERO_ADDRESS } from "../src/constants";
 import { originChainId, destinationChainId } from "./constants";
 import {
@@ -48,7 +48,7 @@ describe("SpokePoolClient: Fills", function () {
     const spokePoolTime = Number(await spokePool.getCurrentTime());
     const outputAmount = toBNWei(1);
     deposit = {
-      depositId: 0,
+      depositId: bnZero,
       originChainId,
       destinationChainId,
       depositor: depositor.address,
@@ -69,7 +69,7 @@ describe("SpokePoolClient: Fills", function () {
 
   it("Correctly fetches fill data single fill, single chain", async function () {
     await fillV3Relay(spokePool, deposit, relayer1);
-    await fillV3Relay(spokePool, { ...deposit, depositId: deposit.depositId + 1 }, relayer1);
+    await fillV3Relay(spokePool, { ...deposit, depositId: deposit.depositId.add(1) }, relayer1);
     await spokePoolClient.update();
     expect(spokePoolClient.getFills().length).to.equal(2);
   });
@@ -132,7 +132,7 @@ describe("SpokePoolClient: Fills", function () {
     const srcChain = getNetworkName(deposit.originChainId);
     await assertPromiseError(
       findFillBlock(spokePool, deposit, lateBlockNumber),
-      `${srcChain} deposit ${deposit.depositId} filled on `
+      `${srcChain} deposit ${deposit.depositId.toString()} filled on `
     );
 
     // Should assert if highBlock <= lowBlock.

diff --git a/test/relayFeeCalculator.test.ts b/test/relayFeeCalculator.test.ts
@@ -337,7 +337,7 @@ describe("RelayFeeCalculator: Composable Bridging", function () {
           outputToken: destErc20.address,
           recipient: testContract.address,
           quoteTimestamp: 1,
-          depositId: 1000000,
+          depositId: BigNumber.from(1000000),
           depositor: depositor.address,
           originChainId: 10,
           destinationChainId: 1,