Add safe buffer to Rootstock collective staking gas estimation#697
Conversation
|
""" WalkthroughThe changes introduce specialized gas estimation logic for staking transactions on Rootstock (RSK) mainnet and testnet. A new configuration and function are added to fetch recent staking transactions and adjust gas limits accordingly. The transaction finalization process is updated to use this logic for legacy staking transactions on RSK networks. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Transaction
participant GasUtils
User->>Transaction: finalizeTransaction(tx, ...)
Transaction->>GasUtils: safeGasForStaking(chainID, estimatedGas)
GasUtils->>ExternalAPI: Fetch recent staking transactions
GasUtils->>ExternalAPI: Fetch transaction details (for each staking tx)
GasUtils-->>Transaction: Return adjusted gas limit
Transaction-->>User: Finalized transaction with adjusted gas
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/extension/src/providers/ethereum/libs/transaction/gas-utils.ts (1)
198-242: Smart gas limit adjustment logic with proper fallbacksThe implementation intelligently analyzes recent successful staking transactions to determine appropriate gas limits. I particularly like:
- Fallback to original estimate if API requests fail
- Additional 30% buffer when gas usage is high (>80%)
- Using Math.max() to ensure we never go below the estimated gas
Consider adding error logging in the catch block to help with debugging:
- } catch { + } catch (error) { + console.error('Error fetching safe gas for staking:', error); return estimatedGas; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/extension/src/providers/ethereum/libs/transaction/gas-utils.ts(2 hunks)packages/extension/src/providers/ethereum/libs/transaction/index.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/extension/src/providers/ethereum/libs/transaction/index.ts (2)
packages/utils/src/index.ts (1)
numberToHex(46-46)packages/extension/src/providers/ethereum/libs/transaction/gas-utils.ts (2)
collectiveGasConfig(247-247)safeGasForStaking(246-246)
🔇 Additional comments (3)
packages/extension/src/providers/ethereum/libs/transaction/gas-utils.ts (1)
182-195: Well-structured configuration for RSK staking transactionsThe configuration is clean and properly organized with all necessary parameters for both RSK mainnet and testnet. The use of dynamic keys with
[rsk.chainID]is a good practice.packages/extension/src/providers/ethereum/libs/transaction/index.ts (2)
17-22: Required imports for RSK staking functionalityThe imports are correctly added to support the new gas estimation feature for RSK staking transactions.
112-118: Verify API service reliability and performance impactMaking API calls during transaction finalization could affect wallet responsiveness if the external APIs are slow or temporarily unavailable.
While the implementation has proper fallbacks when APIs fail, it's important to verify:
- The reliability and response time of the Blockscout APIs being used
- The impact on transaction preparation time for users
Consider implementing caching of recent staking transaction gas data to minimize API calls during high usage periods.
ahsan-javaiid
force-pushed
the
fix/collective-staking-fee
branch
from
2ad1a33 to
a3d14cf
Compare
|
Hi @kvhnuke |
kvhnuke
merged commit 13807d4
into
enkryptcom:devop/release-v-2-7-0
2 participants
Description
Enkrypt uses web3 estimateGas function to estimate gas for the transactions. estimateGas sometimes/ intermittently returns less gas estimations in case of staking contract calls. Due to this reason contract call goes out of gas. Here is example of such failed transaction which consumed 100% of gas and went out of gas during execution.
Reasons
Due to the nature of staking contract / upgradeable contract / OpenZeppelin Upgradeable Proxy patterns / storage / networks conditions makes estimateGas to reutrn very optimistic estimate which sometimes not enough for the contract execution.
We have also seen some custom gas estimation logic in enkrypt for other networks which involves swap operation and enkrypt is adding safe buffer to estimateGas to prevent the contract call going out of the gas here.
Testing
Using the successful stake tx from history to figure out the safe buffer for stake tx on top of estimateGas call. We tested staking with this estimation approach and would like it to be part of enkrypt team.
Motivation
Users are extensively using enkrypt with collective for staking and facing out of gas issue on stake contract call. This PR adds an safe buffer to gas estimated by web3 to prevent the stake contract call going out of gas.
Summary by CodeRabbit
New Features
Bug Fixes