Repository navigation
refactor(walletConnectToDapps): resolve the dApp gas limit in the simulation hook - #12748
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (10)
💤 Files with no reviewable changes (2)
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughWalletConnect transaction signing now uses separate gas-limit and fee-data queries. The signing UI passes supplied gas limits to the simulation hook, and transaction confirmation uses its effective limit. Advanced parameters display nonce and recommended gas values as placeholders. Request handling uses a truthy custom nonce. ChangesWalletConnect transaction gas and nonce handling
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant SigningUI as WalletConnect signing UI
participant SimulationHook as useSimulateEvmTransaction
participant EVMAdapter as EVM adapter
participant Tenderly as Tenderly
SigningUI->>SimulationHook: Pass transaction and optional gas limit
par Gas data
SimulationHook->>EVMAdapter: Request gas-limit estimate and fee data
EVMAdapter-->>SimulationHook: Return estimate and fee data
and Simulation
SimulationHook->>Tenderly: Simulate transaction
Tenderly-->>SimulationHook: Return gas_used
end
SimulationHook-->>SigningUI: Return recommended limit, effective limit, and fee
Suggested reviewers: Merge Risk: ⚪ Minimal · up to No identified issue blocks merging: a failed gas estimate can use Tenderly’s gas value, and an entered zero nonce is preserved. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. I’m a rabbit with a gas-limit chart, Comment |
…ulation hook One Tenderly simulation per transaction instead of two. Gas prices come from a polled getGasFeeData query and the gas limit from a speed-independent getGasLimit query, so changing speed refetches nothing. The hook resolves the effective gas limit (dApp or user value, else the buffered estimate) and the footer passes it to onConfirm, replacing the effect that wrote the estimate into the form and the gate that read it back. Confirm shows loading while the estimate it needs is in flight, and a dApp gas of 0 no longer counts as a supplied value. The current nonce is shown as the placeholder instead of being written into the form, so isUserDefinedNonce goes away and the form only ever holds what the user typed. The duplicate ConfirmData type, the unused gas form field and the simulator's dead feeData input are removed, and the displayed fee is priced on the gas limit that gets signed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
7a682b4 to
7423a5f
Compare
Description
Second of two PRs restructuring EVM gas handling for the WalletConnect dApp flow. Stacked on #12747, which provides
getGasLimit; retarget todeveloponce that merges.Follow-up to the #12743 hotfix, which papered over a lost form update. This removes the pattern that caused it.
Hook
getGasFeeDataquery and the gas limit from a speed-independentgetGasLimitquery, so changing speed refetches nothing andspeedis no longer in any query key.Footer and menu
onConfirm. The speed menu's effect that wrote the estimate into the form, and the footer gate that read it back, are gone.Advanced parameters
isUserDefinedNoncegoes away and the form only ever holds what the user typed. The handler signs with the form nonce when present, else the built transaction's nonce, same as before.Cleanup
ConfirmDatatype merged intoCustomTransactionData; unusedgasform field removed.feeDatainput removed, dead since the second simulation went away.transaction.gasis no longer sent asNaNwhen a dApp omits it.Known gap, pre-existing
When eth_estimateGas fails and Tenderly returns nothing, Confirm is disabled with no explanation. The Advanced Parameters gas-limit field unblocks it. Worth a ticket for an inline message.
Issue (if applicable)
closes #
Risk
Medium. WalletConnect dApp transaction confirmation UI and the data passed to signing. The signing handler itself changes only in how it picks the nonce (form value if typed, else built nonce), which matches the previous effective behaviour.
eth_sendTransactionandeth_signTransactionconfirmation.Testing
Engineering
pnpm vitest run src/plugins/walletConnectToDappsgas.Operations
Screenshots (if applicable)
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes