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 Bifrost mainnet and testnet gas limit Issue #2710

Closed
wants to merge 0 commits into from

Conversation

shamshod01
Copy link
Contributor

Rabby's gas estimation logic currently applies a buffer of 1.5x on all networks, except for Moonbeam and Moonriver, where it uses a 2x buffer as the SAFE_GAS_LIMIT_RATIO.

However, on the Bifrost network, certain transactions are failing due to excessively high gas limits. These transactions are being rejected by the RPC node because the gas limit exceeds the allowed threshold for a single transaction.

To address this issue, it would be safer to use a 1.2x buffer on the Bifrost network instead.

Notably, this issue has not been observed in MetaMask, nor was it encountered when using Rabby on the Bifrost network under typical conditions.

Please let me know if you have any questions or need further clarification!

@vvvvvv1vvvvvv
Copy link
Member

any example for failed transaction?
MetaMask should use same ratio(1.5x) gasLimit, so if there is no issue with MetaMask, there should also no issue with Rabby

@shamshod01
Copy link
Contributor Author

@vvvvvv1vvvvvv
The issue is related to how gas limits are set at the dApp level.

In MetaMask, the gas limit provided by the dApp is used directly, without applying any multiplier, which allows the transaction to succeed.

In contrast, Rabby applies a 1.5x multiplier to the estimated gas limit. On the Bifrost network, where the maximum gas limits are capped (52 million on testnet and 43.3 million on mainnet), this multiplier can result in a gas limit exceeding the allowed threshold for a single transaction.

As a result, the transaction is rejected by the RPC node and doesn't even reach the transaction pool.

I cannot provide an example of a failed transaction since it fails at the validation stage before inclusion in the transaction pool. However, I am attaching screenshots from MetaMask and Rabby for clarity:

Image 1: MetaMask uses the exact gas limit value provided by the dApp.
Images 2-3: Rabby applies a 1.5x multiplier, causing the gas limit to exceed the chain's restrictions.
Please let me know if further clarification is needed!
1 - Screenshot 2025-01-07 at 10 09 22 AM

2 - Screenshot 2025-01-07 at 10 12 57 AM

3 - Screenshot 2025-01-07 at 10 13 10 AM

@shamshod01
Copy link
Contributor Author

@vvvvvv1vvvvvv Is there any update regarding this issue?

@vvvvvv1vvvvvv
Copy link
Member

vvvvvv1vvvvvv commented Jan 13, 2025

1.2x does not solve the problem, it just happens that your transaction will not exceed block.gasLimit at 1.2x. We should consider block.gasLimit when calculating gasLimit and make sure tx.gasLimit will not be higher than block.gasLimit. We will submit a PR to solve this problem later.

@heisenberg-2077 can you check this?

heisenberg-2077 added a commit that referenced this pull request Jan 13, 2025
@dnjscksdn98
Copy link

@vvvvvv1vvvvvv @heisenberg-2077

1099f73

For substrate based chains, such as polkadot or our own chain, the default setup that a single transaction can consume is 86% of the maximum block gas limit. For instance, if the gas limit is set to 60,000,000, it will be 52,000,000 of gas per tx. But in the commit that you've just pushed seems to be assuming that if it exceeds the block gas limit, it detours to use 95% of the limit.

recommendGasLimit = new BigNumber(blockGasLimit)
              .times(0.95)     ---> .times(0.86)
              .toFixed(0);

Is there an option that it can be specifically set to 86% for our chain?

@heisenberg-2077
Copy link
Contributor

heisenberg-2077 commented Jan 14, 2025

@vvvvvv1vvvvvv @heisenberg-2077

1099f73

For substrate based chains, such as polkadot or our own chain, the default setup that a single transaction can consume is 86% of the maximum block gas limit. For instance, if the gas limit is set to 60,000,000, it will be 52,000,000 of gas per tx. But in the commit that you've just pushed seems to be assuming that if it exceeds the block gas limit, it detours to use 95% of the limit.

recommendGasLimit = new BigNumber(blockGasLimit)
              .times(0.95)     ---> .times(0.86)
              .toFixed(0);

Is there an option that it can be specifically set to 86% for our chain?

@dnjscksdn98
https://github.com/RabbyHub/Rabby/pull/2726/files#diff-0928b4cf6bc62f13072f4d5481f2d0a9a910985b9768d60a9a04b1d1be274ea2R1179-R1181 You can modify this option

@shamshod01 shamshod01 closed this Jan 14, 2025
vvvvvv1vvvvvv pushed a commit that referenced this pull request Jan 14, 2025
* fix: gas limit issue in testnet, close #2710

* feat: buffer

* fix: gas

* chore: buffer list
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants