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

Increase erc20 xcm gas cost #2408

Merged
merged 3 commits into from
Jul 25, 2023
Merged

Increase erc20 xcm gas cost #2408

merged 3 commits into from
Jul 25, 2023

Conversation

librelois
Copy link
Collaborator

@librelois librelois commented Jul 24, 2023

What does it do?

The cost of an erc20-xcm transfer is necessarily static, as XCM does not support dynamic costs for assets transfer.

Historically, we fixed the constant to 80,000 , we based our decision on the fact that the ERC20 standard implementation of openzepellin use only around 33 000 gas, we therefore considered at the time that 80 000 was already a very large number, we realize that we need more gas.

But with the introduction of the GMP precompile, which uses wormhole wrapped contracts with ercv20-xcm, we realize that we need more gas (around 140 000).

Until we find a solution in the medium term to make the cost dynamic, this PR increases the constant to 200 000 (the same value used by Acala).

⚠️ breaking changes ⚠️

❗ Increase the fees of erc20-xcm

What important points reviewers should know?

Is there something left for follow-up PRs?

What alternative implementations were considered?

Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?

What value does it bring to the blockchain users?

@librelois librelois added B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D3-trivial PR contains trivial changes in a runtime directory that do not require an audit breaking Needs to be mentioned in breaking changes labels Jul 24, 2023
@github-actions
Copy link
Contributor

Coverage generated "Mon Jul 24 14:36:23 UTC 2023":
https://d3ifz9vhxc2wtb.cloudfront.net/pulls/2408/html/index.html

Master coverage: 87.40%
Pull coverage: 87.41%

@librelois librelois mentioned this pull request Jul 25, 2023
16 tasks
@librelois librelois merged commit 61647d8 into master Jul 25, 2023
24 of 26 checks passed
@librelois librelois deleted the elois-fix-erc20-xcm-gas-cost branch July 25, 2023 14:25
librelois added a commit that referenced this pull request Jul 25, 2023
* improve erc20 amount conversion

* increase Erc20XcmBridgeTransferGasLimit from 80,000 to 200,000

* fix ts tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes breaking Needs to be mentioned in breaking changes D3-trivial PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants