feat: approve gas token for custom token registration #477
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
spsjvc
reviewed
Jun 7, 2024
| const estimatedGas = BigNumber.from(60_000).mul(maxFeePerGas) | ||
|
|
||
| if (isL2NetworkWithCustomFeeToken()) { | ||
| const approvalTx = await ERC20__factory.connect( |
There was a problem hiding this comment.
This code should be inside approveGasToken and getApproveGasTokenRequest of AdminErc20Bridger
src/lib/assetBridger/erc20Bridger.ts
Outdated
Comment on lines
898
to
906
| const gEstimator = new L1ToL2MessageGasEstimator(l2Provider) | ||
|
|
||
| // increase by 500% | ||
| const maxFeePerGas = (await gEstimator.estimateMaxFeePerGas()).mul(6) | ||
|
|
||
| // hardcode gas limit to 60k | ||
| const estimatedGas = BigNumber.from(60_000).mul(maxFeePerGas) | ||
|
|
||
| if (allowance.lt(estimatedGas)) { |
There was a problem hiding this comment.
you don't neeed to use L1ToL2MessageGasEstimator + let's use percentIncrease (we can copy it over from L1ToL2MessageGasEstimator https://github.com/OffchainLabs/arbitrum-sdk/blob/main/src/lib/message/L1ToL2MessageGasEstimator.ts#L76)
Suggested change
| const gEstimator = new L1ToL2MessageGasEstimator(l2Provider) | |
| // increase by 500% | |
| const maxFeePerGas = (await gEstimator.estimateMaxFeePerGas()).mul(6) | |
| // hardcode gas limit to 60k | |
| const estimatedGas = BigNumber.from(60_000).mul(maxFeePerGas) | |
| if (allowance.lt(estimatedGas)) { | |
| const maxFeePerGasOnL2 = (await l2Provider.getFeeData()).maxFeePerGas | |
| const maxFeePerGasOnL2WithBuffer = percentIncrease(maxFeePerGasOnL2, 500) | |
| // hardcode gas limit to 60k | |
| const estimatedGasFee = BigNumber.from(60_000).mul(maxFeePerGasOnL2WithBuffer) | |
| if (allowance.lt(estimatedGasFee)) { |
TucksonDev
reviewed
Jun 11, 2024
src/lib/assetBridger/erc20Bridger.ts
Outdated
| return num.add(num.mul(increase).div(100)) | ||
| } | ||
|
|
||
| public getApproveTokenRequestForCustomTokenRegistration( |
There was a problem hiding this comment.
Why do we need this method as well as getApproveGasTokenRequestForCustomTokenRegistration?
spsjvc
reviewed
Jun 12, 2024
src/lib/assetBridger/erc20Bridger.ts
Outdated
| return num.add(num.mul(increase).div(100)) | ||
| } | ||
|
|
||
| public getApproveGasTokenRequestForCustomTokenRegistration( |
There was a problem hiding this comment.
Suggested change
| public getApproveGasTokenRequestForCustomTokenRegistration( | |
| public getApproveGasTokenForCustomTokenRegistrationRequest( |
Comment on lines
229
to
242
| const maxFeePerGasOnL2 = (await l2Signer.provider!.getFeeData()).maxFeePerGas | ||
| const maxFeePerGasOnL2WithBuffer = maxFeePerGasOnL2?.mul(7) | ||
| // hardcode gas limit to 60k | ||
| const estimatedGasFee = BigNumber.from(60_000).mul( | ||
| maxFeePerGasOnL2WithBuffer! | ||
| ) | ||
|
|
||
| if (isL2NetworkWithCustomFeeToken()) { | ||
| await adminErc20Bridger.approveGasTokenForCustomTokenRegistration({ | ||
| l1Signer, | ||
| amount: estimatedGasFee, | ||
| erc20L1Address: l1CustomToken.address, | ||
| }) | ||
| } |
There was a problem hiding this comment.
for test simplicity we can just use max
Suggested change
| const maxFeePerGasOnL2 = (await l2Signer.provider!.getFeeData()).maxFeePerGas | |
| const maxFeePerGasOnL2WithBuffer = maxFeePerGasOnL2?.mul(7) | |
| // hardcode gas limit to 60k | |
| const estimatedGasFee = BigNumber.from(60_000).mul( | |
| maxFeePerGasOnL2WithBuffer! | |
| ) | |
| if (isL2NetworkWithCustomFeeToken()) { | |
| await adminErc20Bridger.approveGasTokenForCustomTokenRegistration({ | |
| l1Signer, | |
| amount: estimatedGasFee, | |
| erc20L1Address: l1CustomToken.address, | |
| }) | |
| } | |
| if (isL2NetworkWithCustomFeeToken()) { | |
| await adminErc20Bridger.approveGasTokenForCustomTokenRegistration({ | |
| l1Signer, | |
| erc20L1Address: l1CustomToken.address, | |
| }) | |
| } |
…bs/arbitrum-sdk into register-token-approval
spsjvc
changed the title
registerCustomToken for custom gas token chains
spsjvc
approved these changes
Jun 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes FS-315