Skip to content

feat: l2 token approval flow #2260

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
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
68 changes: 68 additions & 0 deletions src/adapter/BaseChainAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import { BaseBridgeAdapter, BridgeTransactionDetails } from "./bridges/BaseBridg
import { OutstandingTransfers } from "../interfaces";
import WETH_ABI from "../common/abi/Weth.json";
import { BaseL2BridgeAdapter } from "./l2Bridges/BaseL2BridgeAdapter";
import { ExpandedERC20 } from "@across-protocol/contracts";

export type SupportedL1Token = EvmAddress;
export type SupportedTokenSymbol = string;
Expand Down Expand Up @@ -123,6 +124,73 @@ export class BaseChainAdapter {
}

async checkTokenApprovals(l1Tokens: EvmAddress[]): Promise<void> {
await Promise.all([this.checkL1TokenApprovals(l1Tokens), this.checkL2TokenApprovals(l1Tokens)]);
}

async checkL2TokenApprovals(l1Tokens: EvmAddress[]): Promise<void> {
const tokensToApprove: { token: ExpandedERC20; bridges: Address[] }[] = [];
const unavailableTokens: EvmAddress[] = [];

await Promise.all(
l1Tokens.map(async (l1Token) => {
const l1TokenAddress = l1Token.toAddress();
const l2Bridge = this.l2Bridges[l1TokenAddress];

if (!l2Bridge || !this.isSupportedToken(l1Token)) {
unavailableTokens.push(l1Token);
return;
}

const requiredApprovals = l2Bridge.requiredTokenApprovals();

if (requiredApprovals.length === 0) {
return;
}

await Promise.all(
requiredApprovals.map(async ({ token: tokenAddress, bridge: bridgeAddress }) => {
const erc20 = ERC20.connect(tokenAddress.toAddress(), this.getSigner(this.chainId));

const senderAddress = EvmAddress.from(await erc20.signer.getAddress());

/*
TODO:
No cache ops for now as those are aimed at L1 tokens only. If I change cache key, would we need a DB migration of some sort?
I can of course just introduce a new key for L2 token approvals, but it seems redundant. Instead, the key could be smth like:
`bridgeAllowance_${chainId}_${token}_${userAddress}_targetContract:${targetContract}`;
*/
const allowance = await erc20.allowance(senderAddress.toAddress(), bridgeAddress.toAddress());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This comment relates to cache operations in a similar L1 token approval fn.


if (!aboveAllowanceThreshold(allowance)) {
const existingTokenIdx = tokensToApprove.findIndex((item) => item.token.address === erc20.address);

if (existingTokenIdx >= 0) {
tokensToApprove[existingTokenIdx].bridges.push(bridgeAddress);
} else {
tokensToApprove.push({ token: erc20, bridges: [bridgeAddress] });
}
}
})
);
})
);

if (unavailableTokens.length > 0) {
this.log("Some tokens do not have a bridge contract for L2 -> L1 bridging", {
unavailableTokens: unavailableTokens.map((token) => token.toAddress()),
});
}

if (tokensToApprove.length === 0) {
this.log("No L2 token bridge approvals needed", { l1Tokens: l1Tokens.map((token) => token.toAddress()) });
return;
}

const mrkdwn = await approveTokens(tokensToApprove, this.chainId, this.hubChainId, this.logger);
this.log("Approved L2 bridge tokens! 💰", { mrkdwn }, "info");
}

async checkL1TokenApprovals(l1Tokens: EvmAddress[]): Promise<void> {
const unavailableTokens: EvmAddress[] = [];
// Approve tokens to bridges. This includes the tokens we want to send over a bridge as well as the custom gas tokens
// each bridge supports (if applicable).
Expand Down
9 changes: 9 additions & 0 deletions src/adapter/l2Bridges/BaseL2BridgeAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,13 @@ export abstract class BaseL2BridgeAdapter {
fromAddress: Address,
l2Token: Address
): Promise<BigNumber>;

/*
TODO:
Returning `EvmAddress`es for now, as upstream BaseChainAdapter impl. will only try to do evm-style approvals
*/
// Bridges that require specific approvals should override this method.
public requiredTokenApprovals(): { token: EvmAddress; bridge: EvmAddress }[] {
return [];
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think returning EvmAddresses is fine here

}
29 changes: 10 additions & 19 deletions src/adapter/l2Bridges/HyperlaneXERC20Bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import {
HYPERLANE_FEE_CAP_OVERRIDES,
} from "../../common";
import { AugmentedTransaction } from "../../clients/TransactionClient";
import ERC20_ABI from "../../common/abi/MinimalERC20.json";

export class HyperlaneXERC20BridgeL2 extends BaseL2BridgeAdapter {
readonly l2Token: EvmAddress;
Expand Down Expand Up @@ -91,20 +90,6 @@ export class HyperlaneXERC20BridgeL2 extends BaseL2BridgeAdapter {
const { decimals, symbol } = getTokenInfo(l2Token.toAddress(), this.l2chainId);
const formatter = createFormatFunction(2, 4, false, decimals);

const erc20 = new Contract(l2Token.toAddress(), ERC20_ABI, this.l2Signer);
const approvalTxn: AugmentedTransaction = {
contract: erc20,
chainId: this.l2chainId,
method: "approve",
unpermissioned: false,
nonMulticall: true,
args: [this.l2Bridge.address, amount],
message: `✅ Approve Hyperlane ${symbol} for withdrawal`,
mrkdwn: `Approve ${formatter(amount.toString())} ${symbol} for withdrawal via Hyperlane router ${
this.l2Bridge.address
} on ${getNetworkName(this.l2chainId)}`,
};

const fee: BigNumber = await this.l2Bridge.quoteGasPayment(this.destinationDomainId);
const feeCap = HYPERLANE_FEE_CAP_OVERRIDES[this.l2chainId] ?? HYPERLANE_DEFAULT_FEE_CAP;
assert(
Expand All @@ -120,9 +105,6 @@ export class HyperlaneXERC20BridgeL2 extends BaseL2BridgeAdapter {
method: "transferRemote",
unpermissioned: false,
nonMulticall: true,
// TODO: `canFailInSimulation` and `gasLimit` are set for now because of current approval flow (see tx above). If we approve these contracts in advance, we'll be able to remove these constraints
canFailInSimulation: true,
gasLimit: BigNumber.from(600000),
args: [this.destinationDomainId, toAddress.toBytes32(), amount],
value: fee,
message: `🎰 Withdrew Hyperlane xERC20 ${symbol} to L1`,
Expand All @@ -131,7 +113,7 @@ export class HyperlaneXERC20BridgeL2 extends BaseL2BridgeAdapter {
)} to L1 via Hyperlane`,
};

return [approvalTxn, withdrawTxn];
return [withdrawTxn];
}

async getL2PendingWithdrawalAmount(
Expand Down Expand Up @@ -186,4 +168,13 @@ export class HyperlaneXERC20BridgeL2 extends BaseL2BridgeAdapter {

return outstandingWithdrawalAmount;
}

public override requiredTokenApprovals(): { token: EvmAddress; bridge: EvmAddress }[] {
return [
{
token: this.l2Token,
bridge: EvmAddress.from(this.l2Bridge.address),
},
];
}
}
23 changes: 13 additions & 10 deletions src/adapter/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,24 +29,27 @@ export function aboveAllowanceThreshold(allowance: BigNumber): boolean {

export async function approveTokens(
tokens: { token: ExpandedERC20; bridges: Address[] }[],
chainId: number,
approvalChainId: number,
hubChainId: number,
logger: winston.Logger
): Promise<string> {
const bridges = tokens.flatMap(({ token, bridges }) => bridges.map((bridge) => ({ token, bridge })));
const approvalMarkdwn = await mapAsync(bridges, async ({ token: l1Token, bridge }) => {
const approvalMarkdwn = await mapAsync(bridges, async ({ token, bridge }) => {
const txs = [];
if (TOKEN_APPROVALS_TO_FIRST_ZERO[hubChainId]?.includes(l1Token.address)) {
txs.push(await runTransaction(logger, l1Token, "approve", [bridge.toAddress(), bnZero]));
if (approvalChainId == hubChainId) {
if (TOKEN_APPROVALS_TO_FIRST_ZERO[hubChainId]?.includes(token.address)) {
txs.push(await runTransaction(logger, token, "approve", [bridge.toAddress(), bnZero]));
}
}
txs.push(await runTransaction(logger, l1Token, "approve", [bridge.toAddress(), MAX_SAFE_ALLOWANCE]));
txs.push(await runTransaction(logger, token, "approve", [bridge.toAddress(), MAX_SAFE_ALLOWANCE]));
const receipts = await Promise.all(txs.map((tx) => tx.wait()));
const hubNetwork = getNetworkName(hubChainId);
const spokeNetwork = getNetworkName(chainId);
const networkName = getNetworkName(approvalChainId);

let internalMrkdwn =
` - Approved canonical ${spokeNetwork} token bridge ${blockExplorerLink(bridge.toAddress(), hubChainId)} ` +
`to spend ${await l1Token.symbol()} ${blockExplorerLink(l1Token.address, hubChainId)} on ${hubNetwork}.` +
`tx: ${blockExplorerLink(receipts.at(-1).transactionHash, hubChainId)}`;
` - Approved token bridge ${blockExplorerLink(bridge.toAddress(), approvalChainId)} ` +
`to spend ${await token.symbol()} ${blockExplorerLink(token.address, approvalChainId)} on ${networkName}.` +
`tx: ${blockExplorerLink(receipts.at(-1).transactionHash, approvalChainId)}`;

if (receipts.length > 1) {
internalMrkdwn += ` tx (to zero approval first): ${blockExplorerLink(receipts[0].transactionHash, hubChainId)}`;
}
Expand Down
7 changes: 4 additions & 3 deletions src/clients/InventoryClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ export class InventoryClient {
protected getRemoteTokenForL1Token(l1Token: string, chainId: number | string): string | undefined {
return chainId === this.hubPoolClient.chainId
? l1Token
: getRemoteTokenForL1Token(l1Token, chainId, this.hubPoolClient);
: getRemoteTokenForL1Token(l1Token, chainId, this.hubPoolClient.chainId);
}

/**
Expand Down Expand Up @@ -1466,13 +1466,14 @@ export class InventoryClient {
return runTransaction(this.logger, l2Weth, "withdraw", [amount]);
}

async setL1TokenApprovals(): Promise<void> {
async setTokenApprovals(): Promise<void> {
if (!this.isInventoryManagementEnabled()) {
return;
}
const l1Tokens = this.getL1Tokens();
this.log("Checking token approvals", { l1Tokens });
await this.adapterManager.setL1TokenApprovals(l1Tokens);

await this.adapterManager.setTokenApprovals(l1Tokens);
}

async wrapL2EthIfAboveThreshold(): Promise<void> {
Expand Down
2 changes: 1 addition & 1 deletion src/clients/ProfitClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ export class ProfitClient {
const outputToken =
destinationChainId === hubPoolClient.chainId
? hubToken
: getRemoteTokenForL1Token(hubToken, destinationChainId, this.hubPoolClient);
: getRemoteTokenForL1Token(hubToken, destinationChainId, this.hubPoolClient.chainId);
assert(isDefined(outputToken), `Chain ${destinationChainId} SpokePool is not configured for ${symbol}`);

const deposit = { ...sampleDeposit, destinationChainId, outputToken };
Expand Down
2 changes: 1 addition & 1 deletion src/clients/TokenClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ export class TokenClient {
.map(({ symbol, address }) => {
let tokenAddrs: string[] = [];
try {
const spokePoolToken = getRemoteTokenForL1Token(address, chainId, this.hubPoolClient);
const spokePoolToken = getRemoteTokenForL1Token(address, chainId, this.hubPoolClient.chainId);
tokenAddrs.push(spokePoolToken);
} catch {
// No known deployment for this token on the SpokePool.
Expand Down
6 changes: 3 additions & 3 deletions src/clients/bridges/AdapterManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ export class AdapterManager {
try {
// That the line below is critical. if the hubpoolClient returns the wrong destination token for the L1 token then
// the bot can irrecoverably send the wrong token to the chain and loose money. It should crash if this is detected.
const l2TokenForL1Token = getRemoteTokenForL1Token(l1Token, chainId, this.hubPoolClient);
const l2TokenForL1Token = getRemoteTokenForL1Token(l1Token, chainId, this.hubPoolClient.chainId);
if (!l2TokenForL1Token) {
throw new Error(`No L2 token found for L1 token ${l1Token} on chain ${chainId}`);
}
Expand All @@ -277,7 +277,7 @@ export class AdapterManager {
}
}

async setL1TokenApprovals(l1Tokens: string[]): Promise<void> {
async setTokenApprovals(l1Tokens: string[]): Promise<void> {
// Each of these calls must happen sequentially or we'll have collisions within the TransactionUtil. This should
// be refactored in a follow on PR to separate out by nonce increment by making the transaction util stateful.
for (const chainId of this.supportedChains()) {
Expand All @@ -292,7 +292,7 @@ export class AdapterManager {
}

l2TokenExistForL1Token(l1Token: string, l2ChainId: number): boolean {
return isDefined(getRemoteTokenForL1Token(l1Token, l2ChainId, this.hubPoolClient));
return isDefined(getRemoteTokenForL1Token(l1Token, l2ChainId, this.hubPoolClient.chainId));
}

// eslint-disable-next-line @typescript-eslint/no-empty-function
Expand Down
2 changes: 1 addition & 1 deletion src/monitor/Monitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1128,7 +1128,7 @@ export class Monitor {
protected getRemoteTokenForL1Token(l1Token: string, chainId: number | string): string | undefined {
return chainId === this.clients.hubPoolClient.chainId
? l1Token
: getRemoteTokenForL1Token(l1Token, chainId, this.clients.hubPoolClient);
: getRemoteTokenForL1Token(l1Token, chainId, this.clients.hubPoolClient.chainId);
}

private updateRelayerBalanceTable(
Expand Down
2 changes: 1 addition & 1 deletion src/relayer/Relayer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export class Relayer {
}

if (this.config.sendingRebalancesEnabled && this.config.sendingTransactionsEnabled) {
await inventoryClient.setL1TokenApprovals();
await inventoryClient.setTokenApprovals();
}

this.logger.debug({
Expand Down
2 changes: 1 addition & 1 deletion src/utils/AddressUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export function getTranslatedTokenAddress(
}
// Native USDC or not USDC, we can just look up in the token map directly.
if (isNativeUsdc || !compareAddressesSimple(l1Token, TOKEN_SYMBOLS_MAP.USDC.addresses[hubChainId])) {
return getRemoteTokenForL1Token(l1Token, l2ChainId, { chainId: hubChainId });
return getRemoteTokenForL1Token(l1Token, l2ChainId, hubChainId);
}
// Handle USDC special case where there could be multiple versions of USDC on an L2: Bridged or Native
const bridgedUsdcMapping = Object.values(TOKEN_SYMBOLS_MAP).find(
Expand Down
9 changes: 4 additions & 5 deletions src/utils/TokenUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { constants, utils } from "@across-protocol/sdk";
import { CONTRACT_ADDRESSES } from "../common";
import { BigNumberish } from "./BNUtils";
import { formatUnits } from "./SDKUtils";
import { HubPoolClient } from "../clients";
import { isDefined } from "./TypeGuards";

const { ZERO_ADDRESS } = constants;
Expand All @@ -12,17 +11,17 @@ export const { fetchTokenInfo, getL2TokenAddresses } = utils;

export function getRemoteTokenForL1Token(
l1Token: string,
chainId: number | string,
hubPoolClient: Pick<HubPoolClient, "chainId">
remoteChainId: number | string,
hubChainId: number
): string | undefined {
const tokenMapping = Object.values(TOKEN_SYMBOLS_MAP).find(
({ addresses }) => addresses[hubPoolClient.chainId] === l1Token && isDefined(addresses[chainId])
({ addresses }) => addresses[hubChainId] === l1Token && isDefined(addresses[remoteChainId])
);
if (!tokenMapping) {
return undefined;
}
const l1TokenSymbol = TOKEN_EQUIVALENCE_REMAPPING[tokenMapping.symbol] ?? tokenMapping.symbol;
return TOKEN_SYMBOLS_MAP[l1TokenSymbol]?.addresses[chainId] ?? tokenMapping.addresses[chainId];
return TOKEN_SYMBOLS_MAP[l1TokenSymbol]?.addresses[remoteChainId] ?? tokenMapping.addresses[remoteChainId];
}

export function getNativeTokenAddressForChain(chainId: number): string {
Expand Down