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

fix: correct gas estimations for eth custom destination #1956

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

brtkx
Copy link
Contributor

@brtkx brtkx commented Oct 2, 2024

ETH transfers to a custom destination address use retryables (https://github.com/OffchainLabs/arbitrum-sdk/blob/main/src/lib/assetBridger/ethBridger.ts#L339) so we need to also approve native currency gas in case for custom gas tokens.

To do that, we use approvalAmountIncrease option in bridge-sdk. We bump it with the required gas.

@cla-bot cla-bot bot added the cla-signed label Oct 2, 2024
Copy link

vercel bot commented Oct 2, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
arbitrum-token-bridge ✅ Ready (Inspect) Visit Preview Oct 31, 2024 3:30pm

@brtkx brtkx changed the title fix: eth transfers to custom destination correct gas estimations fix: eth transfers to custom dest correct gas estimations Oct 2, 2024
@brtkx brtkx changed the title fix: eth transfers to custom dest correct gas estimations fix: correct gas estimations for eth custom destination Oct 2, 2024
dewanshparashar
dewanshparashar previously approved these changes Oct 2, 2024
@@ -32,6 +33,40 @@ export class EthDepositStarter extends BridgeTransferStarter {
return this.ethBridger
}

private async getParentRetryableGas({
Copy link
Member

Choose a reason for hiding this comment

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

yep this feels much better

fionnachan
fionnachan previously approved these changes Oct 22, 2024
@@ -32,6 +33,40 @@ export class EthDepositStarter extends BridgeTransferStarter {
return this.ethBridger
}

private async getParentRetryableGas({
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
private async getParentRetryableGas({
private async getDepositRetryableFees({

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.

4 participants