From b95cffe35e068734b5ec7185c8c4546bee63aeb9 Mon Sep 17 00:00:00 2001 From: Aleksa Colovic Date: Thu, 19 Jun 2025 11:56:43 +0200 Subject: [PATCH 1/6] chore: Mark all potential chainId runtime errors --- src/clients/BundleDataClient/BundleDataClient.ts | 9 +++++++++ src/clients/HubPoolClient.ts | 2 ++ 2 files changed, 11 insertions(+) diff --git a/src/clients/BundleDataClient/BundleDataClient.ts b/src/clients/BundleDataClient/BundleDataClient.ts index 7ca90cfc3..5288f80a4 100644 --- a/src/clients/BundleDataClient/BundleDataClient.ts +++ b/src/clients/BundleDataClient/BundleDataClient.ts @@ -435,6 +435,8 @@ export class BundleDataClient { // and then query the FillStatus on-chain, but that might slow this function down too much. For now, we // will live with this expected inaccuracy as it should be small. The pre-fill would have to precede the deposit // by more than the caller's event lookback window which is expected to be unlikely. + + // @TODO This can throw an runtime error if chainId is wrong. const fillsToCount = this.spokePoolClients[chainId].getFills().filter((fill) => { if ( fill.blockNumber < blockRanges[chainIndex][0] || @@ -790,10 +792,12 @@ export class BundleDataClient { }; const _depositIsExpired = (deposit: DepositWithBlock): boolean => { + // @TODO This can throw an runtime error if chainId is wrong. return deposit.fillDeadline < bundleBlockTimestamps[deposit.destinationChainId][1]; }; const _getFillStatusForDeposit = (deposit: Deposit, queryBlock: number): Promise => { + // @TODO This can throw an runtime error if chainId is wrong. return spokePoolClients[deposit.destinationChainId].relayFillStatus( deposit, // We can assume that in production @@ -801,6 +805,8 @@ export class BundleDataClient { // hasn't queried. This is because this function will usually be called // in production with block ranges that were validated by // DataworkerUtils.blockRangesAreInvalidForSpokeClients. + + // @TODO This can throw an runtime error if chainId is wrong. Math.min(queryBlock, spokePoolClients[deposit.destinationChainId].latestHeightSearched) ); }; @@ -1413,6 +1419,8 @@ export class BundleDataClient { } const deposit = deposits[index]; const { destinationChainId } = deposit; + // @TODO The function getBlockRangeForChain can throw an error if chainId is wrong. + // Look at getBlockRangeForChain implementation. const destinationBlockRange = getBlockRangeForChain(blockRangesForChains, destinationChainId, chainIds); // Only look for deposits that were mined before this bundle and that are newly expired. @@ -1562,6 +1570,7 @@ export class BundleDataClient { const unknownReasonInvalidFills: FillWithBlock[] = []; bundleInvalidFillsV3.forEach((fill) => { + // @TODO This can throw an runtime error if chainId is wrong. const originClient = spokePoolClients[fill.originChainId]; const fullyMatchedDeposit = originClient.getDepositForFill(fill); if (!isDefined(fullyMatchedDeposit)) { diff --git a/src/clients/HubPoolClient.ts b/src/clients/HubPoolClient.ts index cfee7bba4..60951cf1e 100644 --- a/src/clients/HubPoolClient.ts +++ b/src/clients/HubPoolClient.ts @@ -218,6 +218,7 @@ export class HubPoolClient extends BaseAbstractClient { .filter((l1Token) => this.l2TokenEnabledForL1Token(l1Token, destinationChainId)) .map((l1Token) => { // Return all matching L2 token mappings that are equal to or earlier than the target block. + // @TODO This can throw an runtime error if chainId is wrong. return this.l1TokensToDestinationTokensWithBlock[l1Token][destinationChainId].filter( (mapping) => mapping.l2Token === l2Token && mapping.blockNumber <= latestHubBlock ); @@ -1115,6 +1116,7 @@ export class HubPoolClient extends BaseAbstractClient { if (chainIdIndex >= bundleEvaluationBlockNumbers.length) { return 0; } + // @TODO This can throw an runtime error if chainId is wrong. return bundleEvaluationBlockNumbers[chainIdIndex].toNumber(); } } From 0dca96ba2e03317a18b0dd17f8a2e90fba674058 Mon Sep 17 00:00:00 2001 From: Aleksa Colovic Date: Thu, 19 Jun 2025 12:23:57 +0200 Subject: [PATCH 2/6] Remove unecessary TODOs --- src/clients/BundleDataClient/BundleDataClient.ts | 1 - src/clients/HubPoolClient.ts | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/clients/BundleDataClient/BundleDataClient.ts b/src/clients/BundleDataClient/BundleDataClient.ts index 5288f80a4..b27cecd4f 100644 --- a/src/clients/BundleDataClient/BundleDataClient.ts +++ b/src/clients/BundleDataClient/BundleDataClient.ts @@ -436,7 +436,6 @@ export class BundleDataClient { // will live with this expected inaccuracy as it should be small. The pre-fill would have to precede the deposit // by more than the caller's event lookback window which is expected to be unlikely. - // @TODO This can throw an runtime error if chainId is wrong. const fillsToCount = this.spokePoolClients[chainId].getFills().filter((fill) => { if ( fill.blockNumber < blockRanges[chainIndex][0] || diff --git a/src/clients/HubPoolClient.ts b/src/clients/HubPoolClient.ts index 60951cf1e..1f8430b4d 100644 --- a/src/clients/HubPoolClient.ts +++ b/src/clients/HubPoolClient.ts @@ -218,7 +218,7 @@ export class HubPoolClient extends BaseAbstractClient { .filter((l1Token) => this.l2TokenEnabledForL1Token(l1Token, destinationChainId)) .map((l1Token) => { // Return all matching L2 token mappings that are equal to or earlier than the target block. - // @TODO This can throw an runtime error if chainId is wrong. + return this.l1TokensToDestinationTokensWithBlock[l1Token][destinationChainId].filter( (mapping) => mapping.l2Token === l2Token && mapping.blockNumber <= latestHubBlock ); @@ -1116,7 +1116,7 @@ export class HubPoolClient extends BaseAbstractClient { if (chainIdIndex >= bundleEvaluationBlockNumbers.length) { return 0; } - // @TODO This can throw an runtime error if chainId is wrong. + return bundleEvaluationBlockNumbers[chainIdIndex].toNumber(); } } From cd663d617ea5c40c8523ed93269d2d22adb375df Mon Sep 17 00:00:00 2001 From: Aleksa Colovic Date: Thu, 19 Jun 2025 12:25:11 +0200 Subject: [PATCH 3/6] Lint fix --- src/clients/BundleDataClient/BundleDataClient.ts | 2 +- src/clients/HubPoolClient.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/clients/BundleDataClient/BundleDataClient.ts b/src/clients/BundleDataClient/BundleDataClient.ts index b27cecd4f..1b474d948 100644 --- a/src/clients/BundleDataClient/BundleDataClient.ts +++ b/src/clients/BundleDataClient/BundleDataClient.ts @@ -1418,7 +1418,7 @@ export class BundleDataClient { } const deposit = deposits[index]; const { destinationChainId } = deposit; - // @TODO The function getBlockRangeForChain can throw an error if chainId is wrong. + // @TODO The function getBlockRangeForChain can throw an error if chainId is wrong. // Look at getBlockRangeForChain implementation. const destinationBlockRange = getBlockRangeForChain(blockRangesForChains, destinationChainId, chainIds); diff --git a/src/clients/HubPoolClient.ts b/src/clients/HubPoolClient.ts index 1f8430b4d..2acbccc5b 100644 --- a/src/clients/HubPoolClient.ts +++ b/src/clients/HubPoolClient.ts @@ -218,7 +218,7 @@ export class HubPoolClient extends BaseAbstractClient { .filter((l1Token) => this.l2TokenEnabledForL1Token(l1Token, destinationChainId)) .map((l1Token) => { // Return all matching L2 token mappings that are equal to or earlier than the target block. - + return this.l1TokensToDestinationTokensWithBlock[l1Token][destinationChainId].filter( (mapping) => mapping.l2Token === l2Token && mapping.blockNumber <= latestHubBlock ); @@ -1116,7 +1116,7 @@ export class HubPoolClient extends BaseAbstractClient { if (chainIdIndex >= bundleEvaluationBlockNumbers.length) { return 0; } - + return bundleEvaluationBlockNumbers[chainIdIndex].toNumber(); } } From e09a7768013ac5bacbc792ed88add1249c210eb8 Mon Sep 17 00:00:00 2001 From: Aleksa Colovic Date: Thu, 19 Jun 2025 12:44:31 +0200 Subject: [PATCH 4/6] Add more descriptive comment about _getFillStatusForDeposit --- src/clients/BundleDataClient/BundleDataClient.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/clients/BundleDataClient/BundleDataClient.ts b/src/clients/BundleDataClient/BundleDataClient.ts index 1b474d948..8b7da7f99 100644 --- a/src/clients/BundleDataClient/BundleDataClient.ts +++ b/src/clients/BundleDataClient/BundleDataClient.ts @@ -813,6 +813,7 @@ export class BundleDataClient { // Infer chain ID's to load from number of block ranges passed in. const allChainIds = blockRangesForChains .map((_blockRange, index) => chainIds[index]) + // Because of this check, there is a good chance that _getFillStatusForDeposit will not throw an error. .filter((chainId) => !_isChainDisabled(chainId) && spokePoolClients[chainId] !== undefined); allChainIds.forEach((chainId) => { const spokePoolClient = spokePoolClients[chainId]; From 7ab9d510c18f960c78cb9b20b0711c507c8788e1 Mon Sep 17 00:00:00 2001 From: Aleksa Colovic Date: Mon, 23 Jun 2025 12:08:03 +0200 Subject: [PATCH 5/6] Add better handling for invalid chainIds --- .../BundleDataClient/BundleDataClient.ts | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/clients/BundleDataClient/BundleDataClient.ts b/src/clients/BundleDataClient/BundleDataClient.ts index 8b7da7f99..f28e9a588 100644 --- a/src/clients/BundleDataClient/BundleDataClient.ts +++ b/src/clients/BundleDataClient/BundleDataClient.ts @@ -791,22 +791,26 @@ export class BundleDataClient { }; const _depositIsExpired = (deposit: DepositWithBlock): boolean => { - // @TODO This can throw an runtime error if chainId is wrong. - return deposit.fillDeadline < bundleBlockTimestamps[deposit.destinationChainId][1]; + const [, endTimestamp] = + bundleBlockTimestamps[deposit.destinationChainId] ?? bundleBlockTimestamps[this.clients.hubPoolClient.chainId]; + return deposit.fillDeadline < endTimestamp; }; const _getFillStatusForDeposit = (deposit: Deposit, queryBlock: number): Promise => { - // @TODO This can throw an runtime error if chainId is wrong. - return spokePoolClients[deposit.destinationChainId].relayFillStatus( + const spokePoolClient = spokePoolClients[deposit.destinationChainId]; + + if (!isDefined(spokePoolClient)) { + return Promise.resolve(FillStatus.Unfilled); + } + + return spokePoolClient.relayFillStatus( deposit, // We can assume that in production // the block to query is not one that the spoke pool client // hasn't queried. This is because this function will usually be called // in production with block ranges that were validated by // DataworkerUtils.blockRangesAreInvalidForSpokeClients. - - // @TODO This can throw an runtime error if chainId is wrong. - Math.min(queryBlock, spokePoolClients[deposit.destinationChainId].latestHeightSearched) + Math.min(queryBlock, spokePoolClient.latestHeightSearched) ); }; @@ -1572,7 +1576,7 @@ export class BundleDataClient { bundleInvalidFillsV3.forEach((fill) => { // @TODO This can throw an runtime error if chainId is wrong. const originClient = spokePoolClients[fill.originChainId]; - const fullyMatchedDeposit = originClient.getDepositForFill(fill); + const fullyMatchedDeposit = originClient?.getDepositForFill(fill); if (!isDefined(fullyMatchedDeposit)) { const partiallyMatchedDeposit = originClient.getDeposit(fill.depositId); if (isDefined(partiallyMatchedDeposit)) { From fda968434aa6356763658b92451f055fa0ddf1b6 Mon Sep 17 00:00:00 2001 From: Aleksa Colovic Date: Wed, 25 Jun 2025 12:27:12 +0200 Subject: [PATCH 6/6] Mark place for future imporvements --- src/clients/BundleDataClient/BundleDataClient.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/clients/BundleDataClient/BundleDataClient.ts b/src/clients/BundleDataClient/BundleDataClient.ts index f28e9a588..95e05e339 100644 --- a/src/clients/BundleDataClient/BundleDataClient.ts +++ b/src/clients/BundleDataClient/BundleDataClient.ts @@ -791,9 +791,12 @@ export class BundleDataClient { }; const _depositIsExpired = (deposit: DepositWithBlock): boolean => { - const [, endTimestamp] = - bundleBlockTimestamps[deposit.destinationChainId] ?? bundleBlockTimestamps[this.clients.hubPoolClient.chainId]; - return deposit.fillDeadline < endTimestamp; + // @TODO: We should be defaulting to the mainnet time in case destination SpokePool deployment is not available. + // Something like this: + // const [, endTimestamp] = + // bundleBlockTimestamps[deposit.destinationChainId] ?? bundleBlockTimestamps[this.clients.hubPoolClient.chainId]; + // return deposit.fillDeadline < endTimestamp; + return deposit.fillDeadline < bundleBlockTimestamps[deposit.destinationChainId][1]; }; const _getFillStatusForDeposit = (deposit: Deposit, queryBlock: number): Promise => { @@ -1574,7 +1577,6 @@ export class BundleDataClient { const unknownReasonInvalidFills: FillWithBlock[] = []; bundleInvalidFillsV3.forEach((fill) => { - // @TODO This can throw an runtime error if chainId is wrong. const originClient = spokePoolClients[fill.originChainId]; const fullyMatchedDeposit = originClient?.getDepositForFill(fill); if (!isDefined(fullyMatchedDeposit)) {