Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] improve(HubPoolClient): Don't throw on token resolution failure #643

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 20 additions & 11 deletions src/clients/BundleDataClient/utils/DataworkerUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
fixedPointAdjustment,
count2DDictionaryValues,
count3DDictionaryValues,
isDefined,
} from "../../../utils";
import {
addLastRunningBalance,
Expand Down Expand Up @@ -160,8 +161,10 @@ export function _buildPoolRebalanceRoot(
mainnetBundleEndBlock
);

updateRunningBalance(runningBalances, repaymentChainId, l1TokenCounterpart, totalRefundAmount);
updateRunningBalance(realizedLpFees, repaymentChainId, l1TokenCounterpart, totalRealizedLpFee);
if (isDefined(l1TokenCounterpart)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving a comment here since this is just a temporary fix to let the code compile. I'm pretty sure this is a bad idea to do since if l1TokenCounterpart is undefined, then we probably do want to throw an error. (Maybe we should just assert that here).

updateRunningBalance(runningBalances, repaymentChainId, l1TokenCounterpart, totalRefundAmount);
updateRunningBalance(realizedLpFees, repaymentChainId, l1TokenCounterpart, totalRealizedLpFee);
}
}
);
});
Expand All @@ -182,10 +185,12 @@ export function _buildPoolRebalanceRoot(
destinationChainId,
mainnetBundleEndBlock
);
const lpFee = deposit.lpFeePct.mul(deposit.inputAmount).div(fixedPointAdjustment);
updateRunningBalance(runningBalances, destinationChainId, l1TokenCounterpart, deposit.inputAmount.sub(lpFee));
// Slow fill LP fees are accounted for when the slow fill executes and a V3FilledRelay is emitted. i.e. when
// the slow fill execution is included in bundleFillsV3.
if (isDefined(l1TokenCounterpart)) {
const lpFee = deposit.lpFeePct.mul(deposit.inputAmount).div(fixedPointAdjustment);
updateRunningBalance(runningBalances, destinationChainId, l1TokenCounterpart, deposit.inputAmount.sub(lpFee));
// Slow fill LP fees are accounted for when the slow fill executes and a V3FilledRelay is emitted. i.e. when
// the slow fill execution is included in bundleFillsV3.
}
});
});
});
Expand All @@ -206,10 +211,12 @@ export function _buildPoolRebalanceRoot(
destinationChainId,
mainnetBundleEndBlock
);
const lpFee = deposit.lpFeePct.mul(deposit.inputAmount).div(fixedPointAdjustment);
updateRunningBalance(runningBalances, destinationChainId, l1TokenCounterpart, lpFee.sub(deposit.inputAmount));
// Slow fills don't add to lpFees, only when the slow fill is executed and a V3FilledRelay is emitted, so
// we don't need to subtract it here. Moreover, the HubPoole expects bundleLpFees to be > 0.
if (isDefined(l1TokenCounterpart)) {
const lpFee = deposit.lpFeePct.mul(deposit.inputAmount).div(fixedPointAdjustment);
updateRunningBalance(runningBalances, destinationChainId, l1TokenCounterpart, lpFee.sub(deposit.inputAmount));
// Slow fills don't add to lpFees, only when the slow fill is executed and a V3FilledRelay is emitted, so
// we don't need to subtract it here. Moreover, the HubPoole expects bundleLpFees to be > 0.
}
});
});
});
Expand Down Expand Up @@ -242,7 +249,9 @@ export function _buildPoolRebalanceRoot(
originChainId,
mainnetBundleEndBlock
);
updateRunningBalance(runningBalances, originChainId, l1TokenCounterpart, deposit.inputAmount);
if (isDefined(l1TokenCounterpart)) {
updateRunningBalance(runningBalances, originChainId, l1TokenCounterpart, deposit.inputAmount);
}
});
});
});
Expand Down
8 changes: 5 additions & 3 deletions src/clients/BundleDataClient/utils/FillUtils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Fill } from "../../../interfaces";
import { getBlockRangeForChain, isSlowFill } from "../../../utils";
import { getBlockRangeForChain, isSlowFill, assert, isDefined } from "../../../utils";
import { HubPoolClient } from "../../HubPoolClient";

export function getRefundInformationFromFill(
Expand Down Expand Up @@ -32,13 +32,15 @@ export function getRefundInformationFromFill(
fill.inputToken,
fill.originChainId,
endBlockForMainnet
);
)!;

assert(isDefined(l1TokenCounterpart), "There must be an l1 token counterpart for a filled deposit");
const repaymentToken = hubPoolClient.getL2TokenForL1TokenAtBlock(
l1TokenCounterpart,
chainToSendRefundTo,
endBlockForMainnet
);
)!;
assert(isDefined(repaymentToken), "There must be defined repayment token for a filled deposit");
return {
chainToSendRefundTo,
repaymentToken,
Expand Down
6 changes: 4 additions & 2 deletions src/clients/BundleDataClient/utils/PoolRebalanceUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { MerkleTree } from "@across-protocol/contracts/dist/utils/MerkleTree";
import { RunningBalances, PoolRebalanceLeaf, Clients, SpokePoolTargetBalance } from "../../../interfaces";
import { SpokePoolClient } from "../../SpokePoolClient";
import { BigNumber } from "ethers";
import { bnZero, compareAddresses } from "../../../utils";
import { bnZero, compareAddresses, isDefined } from "../../../utils";
import { HubPoolClient } from "../../HubPoolClient";
import { V3DepositWithBlock } from "./shims";
import { AcrossConfigStoreClient } from "../../AcrossConfigStoreClient";
Expand Down Expand Up @@ -171,7 +171,9 @@ export function updateRunningBalanceForDeposit(
deposit.originChainId,
deposit.quoteBlockNumber
);
updateRunningBalance(runningBalances, deposit.originChainId, l1TokenCounterpart, updateAmount);
if (isDefined(l1TokenCounterpart)) {
updateRunningBalance(runningBalances, deposit.originChainId, l1TokenCounterpart, updateAmount);
}
}

export function constructPoolRebalanceLeaves(
Expand Down
85 changes: 48 additions & 37 deletions src/clients/HubPoolClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
fetchTokenInfo,
getCachedBlockForTimestamp,
getCurrentTime,
getNetworkName,
isDefined,
mapAsync,
paginatedEventQuery,
Expand Down Expand Up @@ -184,32 +183,23 @@
l1Token: string,
destinationChainId: number,
latestHubBlock = Number.MAX_SAFE_INTEGER
): string {
): string | undefined {
if (!this.l1TokensToDestinationTokensWithBlock?.[l1Token]?.[destinationChainId]) {
const chain = getNetworkName(destinationChainId);
const { symbol } = this.l1Tokens.find(({ address }) => address === l1Token) ?? { symbol: l1Token };
throw new Error(`Could not find SpokePool mapping for ${symbol} on ${chain} and L1 token ${l1Token}`);
return undefined;
}
// Find the last mapping published before the target block.
const l2Token: DestinationTokenWithBlock | undefined = sortEventsDescending(
this.l1TokensToDestinationTokensWithBlock[l1Token][destinationChainId]
).find((mapping: DestinationTokenWithBlock) => mapping.blockNumber <= latestHubBlock);
if (!l2Token) {
const chain = getNetworkName(destinationChainId);
const { symbol } = this.l1Tokens.find(({ address }) => address === l1Token) ?? { symbol: l1Token };
throw new Error(
`Could not find SpokePool mapping for ${symbol} on ${chain} at or before HubPool block ${latestHubBlock}!`
);
}
return l2Token.l2Token;
return l2Token?.l2Token;
}

// Returns the latest L1 token to use for an L2 token as of the input hub block.
getL1TokenForL2TokenAtBlock(
l2Token: string,
destinationChainId: number,
latestHubBlock = Number.MAX_SAFE_INTEGER
): string {
): string | undefined {
const l2Tokens = Object.keys(this.l1TokensToDestinationTokensWithBlock)
.filter((l1Token) => this.l2TokenEnabledForL1Token(l1Token, destinationChainId))
.map((l1Token) => {
Expand All @@ -219,14 +209,9 @@
);
})
.flat();
if (l2Tokens.length === 0) {
const chain = getNetworkName(destinationChainId);
throw new Error(
`Could not find HubPool mapping for ${l2Token} on ${chain} at or before HubPool block ${latestHubBlock}!`
);
}

// Find the last mapping published before the target block.
return sortEventsDescending(l2Tokens)[0].l1Token;
return sortEventsDescending(l2Tokens)[0]?.l1Token;
}

/**
Expand All @@ -236,7 +221,9 @@
* @param deposit Deposit event
* @param returns string L1 token counterpart for Deposit
*/
getL1TokenForDeposit(deposit: Pick<DepositWithBlock, "originChainId" | "inputToken" | "quoteBlockNumber">): string {
getL1TokenForDeposit(
deposit: Pick<DepositWithBlock, "originChainId" | "inputToken" | "quoteBlockNumber">
): string | undefined {
// L1-->L2 token mappings are set via PoolRebalanceRoutes which occur on mainnet,
// so we use the latest token mapping. This way if a very old deposit is filled, the relayer can use the
// latest L2 token mapping to find the L1 token counterpart.
Expand All @@ -247,16 +234,18 @@
* Returns the L2 token that should be used as a counterpart to a deposit event. For example, the caller
* might want to know what the refund token will be on l2ChainId for the deposit event.
* @param l2ChainId Chain where caller wants to get L2 token counterpart for
* @param event Deposit event
* @param event Deposit eventn
* @returns string L2 token counterpart on l2ChainId
*/
getL2TokenForDeposit(
deposit: Pick<DepositWithBlock, "originChainId" | "destinationChainId" | "inputToken" | "quoteBlockNumber">,
l2ChainId = deposit.destinationChainId
): string {
): string | undefined {
const l1Token = this.getL1TokenForDeposit(deposit);
// Use the latest hub block number to find the L2 token counterpart.
return this.getL2TokenForL1TokenAtBlock(l1Token, l2ChainId, deposit.quoteBlockNumber);
return isDefined(l1Token)
? this.getL2TokenForL1TokenAtBlock(l1Token, l2ChainId, deposit.quoteBlockNumber)
: undefined;
}

l2TokenEnabledForL1Token(l1Token: string, destinationChainId: number): boolean {
Expand Down Expand Up @@ -378,11 +367,23 @@
// Map SpokePool token addresses to HubPool token addresses.
// Note: Should only be accessed via `getHubPoolToken()` or `getHubPoolTokens()`.
const hubPoolTokens: { [k: string]: string } = {};
const getHubPoolToken = (deposit: LpFeeRequest, quoteBlockNumber: number): string => {
const getHubPoolToken = (deposit: LpFeeRequest, quoteBlockNumber: number): string | undefined => {
const tokenKey = `${deposit.originChainId}-${deposit.inputToken}`;
return (hubPoolTokens[tokenKey] ??= this.getL1TokenForDeposit({ ...deposit, quoteBlockNumber }));
if (isDefined(hubPoolTokens[tokenKey])) {
return hubPoolTokens[tokenKey];
}
const l1Token = this.getL1TokenForDeposit({ ...deposit, quoteBlockNumber });
if (!isDefined(l1Token)) {
this.logger.warn({
at: "HubPoolClient",
message: `Unable to determine an appropriate L1 Token for deposit ${deposit}`,
});
}
hubPoolTokens[tokenKey] = l1Token;

Check failure on line 382 in src/clients/HubPoolClient.ts

View workflow job for this annotation

GitHub Actions / Builds

Type 'string | undefined' is not assignable to type 'string'.

Check failure on line 382 in src/clients/HubPoolClient.ts

View workflow job for this annotation

GitHub Actions / Builds

Type 'string | undefined' is not assignable to type 'string'.

Check failure on line 382 in src/clients/HubPoolClient.ts

View workflow job for this annotation

GitHub Actions / Builds

Type 'string | undefined' is not assignable to type 'string'.

Check failure on line 382 in src/clients/HubPoolClient.ts

View workflow job for this annotation

GitHub Actions / Builds

Type 'string | undefined' is not assignable to type 'string'.

Check failure on line 382 in src/clients/HubPoolClient.ts

View workflow job for this annotation

GitHub Actions / Builds

Type 'string | undefined' is not assignable to type 'string'.

Check failure on line 382 in src/clients/HubPoolClient.ts

View workflow job for this annotation

GitHub Actions / Builds

Type 'string | undefined' is not assignable to type 'string'.

Check failure on line 382 in src/clients/HubPoolClient.ts

View workflow job for this annotation

GitHub Actions / Lint

Type 'string | undefined' is not assignable to type 'string'.

Check failure on line 382 in src/clients/HubPoolClient.ts

View workflow job for this annotation

GitHub Actions / Lint

Type 'string | undefined' is not assignable to type 'string'.

Check failure on line 382 in src/clients/HubPoolClient.ts

View workflow job for this annotation

GitHub Actions / Lint

Type 'string | undefined' is not assignable to type 'string'.

Check failure on line 382 in src/clients/HubPoolClient.ts

View workflow job for this annotation

GitHub Actions / Test

Type 'string | undefined' is not assignable to type 'string'.

Check failure on line 382 in src/clients/HubPoolClient.ts

View workflow job for this annotation

GitHub Actions / Test

Type 'string | undefined' is not assignable to type 'string'.

Check failure on line 382 in src/clients/HubPoolClient.ts

View workflow job for this annotation

GitHub Actions / Test

Type 'string | undefined' is not assignable to type 'string'.
return l1Token;
};
const getHubPoolTokens = (): string[] => dedupArray(Object.values(hubPoolTokens));
const getHubPoolTokens = (): string[] =>
dedupArray(Object.values(hubPoolTokens)).filter((token) => isDefined(token));

// Helper to resolve the unqiue hubPoolToken & quoteTimestamp mappings.
const resolveUniqueQuoteTimestamps = (deposit: LpFeeRequest): void => {
Expand All @@ -391,6 +392,9 @@
// Resolve the HubPool token address for this origin chainId/token pair, if it isn't already known.
const quoteBlockNumber = quoteBlocks[quoteTimestamp];
const hubPoolToken = getHubPoolToken(deposit, quoteBlockNumber);
if (!isDefined(hubPoolToken)) {
return;
}

// Append the quoteTimestamp for this HubPool token, if it isn't already enqueued.
utilizationTimestamps[hubPoolToken] ??= [];
Expand Down Expand Up @@ -431,11 +435,11 @@
const { originChainId, paymentChainId, inputAmount, quoteTimestamp } = deposit;
const quoteBlock = quoteBlocks[quoteTimestamp];

if (paymentChainId === undefined) {
const hubPoolToken = getHubPoolToken(deposit, quoteBlock);
if (!isDefined(paymentChainId) || !isDefined(hubPoolToken)) {
return { quoteBlock, realizedLpFeePct: bnZero };
}

const hubPoolToken = getHubPoolToken(deposit, quoteBlock);
const rateModel = this.configStoreClient.getRateModelForBlockNumber(
hubPoolToken,
originChainId,
Expand Down Expand Up @@ -495,13 +499,16 @@

getL1TokenInfoForL2Token(l2Token: string, chainId: number): L1Token | undefined {
const l1TokenCounterpart = this.getL1TokenForL2TokenAtBlock(l2Token, chainId, this.latestBlockSearched);
return this.getTokenInfoForL1Token(l1TokenCounterpart);
return isDefined(l1TokenCounterpart) ? this.getTokenInfoForL1Token(l1TokenCounterpart) : undefined;
}

getTokenInfoForDeposit(deposit: Deposit): L1Token | undefined {
return this.getTokenInfoForL1Token(
this.getL1TokenForL2TokenAtBlock(deposit.inputToken, deposit.originChainId, this.latestBlockSearched)
const l1Token = this.getL1TokenForL2TokenAtBlock(
deposit.inputToken,
deposit.originChainId,
this.latestBlockSearched
);
return isDefined(l1Token) ? this.getTokenInfoForL1Token(l1Token) : undefined;
}

getTokenInfo(chainId: number | string, tokenAddress: string): L1Token | undefined {
Expand All @@ -524,10 +531,14 @@
return false;
}

// Resolve both HubPool tokens back to a current SpokePool token and verify that they match.
const _tokenA = this.getL2TokenForL1TokenAtBlock(l1TokenA, chainIdA, hubPoolBlock);
const _tokenB = this.getL2TokenForL1TokenAtBlock(l1TokenB, chainIdB, hubPoolBlock);
return tokenA === _tokenA && tokenB === _tokenB;
if (isDefined(l1TokenA) && isDefined(l1TokenB)) {
// Resolve both HubPool tokens back to a current SpokePool token and verify that they match.
const _tokenA = this.getL2TokenForL1TokenAtBlock(l1TokenA, chainIdA, hubPoolBlock);
const _tokenB = this.getL2TokenForL1TokenAtBlock(l1TokenB, chainIdB, hubPoolBlock);
return tokenA === _tokenA && tokenB === _tokenB;
} else {
return false;
}
} catch {
return false; // One or both input tokens were not recognised.
}
Expand Down
3 changes: 2 additions & 1 deletion src/clients/SpokePoolClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,8 @@ export class SpokePoolClient extends BaseAbstractClient {
return ZERO_ADDRESS;
}

return this.hubPoolClient.getL2TokenForDeposit(deposit);
// If there is no l2 token for the deposit also return the zero address.
return this.hubPoolClient.getL2TokenForDeposit(deposit) ?? ZERO_ADDRESS;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/clients/mocks/MockHubPoolClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export class MockHubPoolClient extends HubPoolClient {
this.spokePoolTokens[l1Token][chainId] = l2Token;
}

getL1TokenForL2TokenAtBlock(l2Token: string, chainId: number, blockNumber: number): string {
getL1TokenForL2TokenAtBlock(l2Token: string, chainId: number, blockNumber: number): string | undefined {
const l1Token = Object.keys(this.spokePoolTokens).find(
(l1Token) => this.spokePoolTokens[l1Token]?.[chainId] === l2Token
);
Expand Down
Loading
Loading