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

feat: add support for custom gas token #310

Merged
merged 216 commits into from
Feb 14, 2024
Merged

feat: add support for custom gas token #310

merged 216 commits into from
Feb 14, 2024

Conversation

spsjvc
Copy link
Member

@spsjvc spsjvc commented Jul 11, 2023

What's Changed

L2Network

  • Added a new property nativeToken

AssetBridger

  • Added a new property nativeToken
  • Added a new property nativeTokenIsEth which is a boolean that represents whether the Arbitrum chain that is being used with the bridger uses ETH as the native token.

EthBridger

Added getApproveGasTokenRequest and approveGasToken to EthBridger, which help approve the custom gas token to be spent by the Inbox. When adding support for a custom gas token chain, the user only needs to call this before doing the regular deposit, while the rest of the flow stays the same. The bridger object will pick between 2 separate code paths depending on the value of nativeTokenIsEth.

These methods will throw if used on a chain that uses ETH as the gas token.

Erc20Bridger

Added getApproveGasTokenRequest and approveGasToken to EthBridger, which help approve the custom gas token to be spent by the appropriate Gateway. When adding support for a custom gas token chain, the user only needs to call this before doing the regular deposit, while the rest of the flow stays the same. The bridger object will pick between 2 separate code paths depending on the value of nativeTokenIsEth.

These methods will throw if used on a chain that uses ETH as the gas token.

L1ToL2MessageCreator

Updated getTicketCreationRequest to support custom gas token chains and have different calldata in that case.

@cla-bot cla-bot bot added the cla-signed label Jul 11, 2023
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@@ -696,6 +788,11 @@ export class Erc20Bridger extends AssetBridger<
// in the future we want to do proper estimation here
/* eslint-disable @typescript-eslint/no-unused-vars */
estimateL1GasLimit: async (l1Provider: Provider) => {
if (!this.nativeTokenIsEth) {
// measured 172867 - add some padding
return BigNumber.from(200000)
Copy link
Member

Choose a reason for hiding this comment

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

this probably won't work for L3->L2 withdrawal

@@ -706,7 +803,7 @@ export class Erc20Bridger extends AssetBridger<
const isWeth = await this.isWethGateway(l1GatewayAddress, l1Provider)

// measured 157421 - add some padding
return isWeth ? BigNumber.from(180000) : BigNumber.from(160000)
return isWeth ? BigNumber.from(190000) : BigNumber.from(160000)
Copy link
Member

Choose a reason for hiding this comment

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

this probably won't work for L3->L2 withdrawal

Copy link
Member

@gzeoneth gzeoneth left a comment

Choose a reason for hiding this comment

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

LGTM
recommend to add a comment regarding the hardcoded gas: the correct way to do is use NodeInterace.gasEstimateL1Component to calculate the L1 component + hardcoded L2 component

@spsjvc spsjvc merged commit ec05c84 into main Feb 14, 2024
24 checks passed
@spsjvc spsjvc deleted the feat-custom-fee-token branch February 14, 2024 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants