Skip to content

fix: replace hardcoded gas 21000 - #115

Merged
kstroobants merged 3 commits into
mainfrom
dxp-653-replace-hardcoded-gas-21000
Sep 11, 2025
Merged

kstroobants merged 3 commits into
mainfrom
dxp-653-replace-hardcoded-gas-21000

Conversation

@kstroobants

@kstroobants kstroobants commented Sep 11, 2025 •

Copy link
Copy Markdown
Contributor

Fixes #DXP-653

What

  • Refactored the createClient function in src/client/client.ts to first add transaction actions before contract actions, ensuring that contract actions can access transaction actions because estimateTransactionGas was not available.
  • Implemented gas estimation in _sendTransaction function in src/contracts/actions.ts to dynamically calculate gas, with a fallback to a default value if estimation fails.
  • Added a new method estimateTransactionGas in src/transactions/actions.ts to estimate gas using the eth_estimateGas method.
  • Updated GenLayerClient type in src/types/clients.ts to include the new estimateTransactionGas method.

Why

To provide a more accurate gas estimation for transactions.

Testing done

Tested the gas estimation feature to ensure it calculates gas correctly and falls back to a default value when necessary.

Decisions made

Decided to name method estimateTransactionGas because Viem has estimateGas already.

Checks

  • I have tested this code
  • I have reviewed my own PR
  • I have created an issue for this PR
  • I have set a descriptive PR title compliant with conventional commits

Reviewing tips

Review the implementation if the gas estimation method has the correct eth input params.

User facing release notes

Added dynamic gas estimation for transactions.

Summary by CodeRabbit

  • New Features

    • Added a transaction gas estimation API so you can preview required gas before sending.
    • Contract-initiated transactions now attempt dynamic gas estimation and fall back to a safe default if estimation fails.
  • Refactor

    • Reordered client action initialization to ensure transaction capabilities are available before contract features, with receipt handling applied last for consistent behavior.

@kstroobants kstroobants self-assigned this Sep 11, 2025
@coderabbitai

coderabbitai Bot commented Sep 11, 2025 •

Copy link
Copy Markdown

Walkthrough

Reorders client action composition to ensure transaction actions are available before contract actions; adds eth_estimateGas and exposes estimateTransactionGas in client types; uses it in contract transaction sending with a fallback on failure; consolidates imports in transaction decoders. No removed public signatures; one method added.

Changes

Cohort / File(s) Summary
Client action sequencing
src/client/client.ts
Rebuilds client in phases: basic → transactionActions (plus chain/wallet) → contractActions → receiptActions; updates comments and ordering to ensure contractActions can rely on transactionActions.
Gas estimation integration
src/contracts/actions.ts, src/transactions/actions.ts, src/types/clients.ts
Adds eth_estimateGas support and estimateTransactionGas API in types and transaction actions; contracts call it in _sendTransaction to compute estimatedGas, with a logged fallback (200_000n) on estimation failure, then proceeds to send/sign the transaction.
Import consolidation
src/transactions/decoders.ts
Merges multiple imports from ../types/transactions into a single consolidated import; no behavioral change.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant App as App
  participant Contract as ContractActions
  participant Client as GenLayerClient (tx+chain+wallet)
  participant Node as RPC Node

  App->>Contract: sendTransaction(params)
  Note over Contract: prepare encoded data & params
  Contract->>Client: estimateTransactionGas(txParams)
  Client->>Node: eth_estimateGas(txParams)
  alt estimate succeeds
    Node-->>Client: gas (hex)
    Client-->>Contract: gas (bigint)
  else estimate fails
    Node-->>Client: error
    Client-->>Contract: warning + fallback gas (200_000n)
  end
  Contract->>Client: eth_sendTransaction / sign+send (with gas)
  Client->>Node: sendTransaction RPC
  Node-->>Client: tx hash / receipt
  Client-->>App: result / receipt
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • danielrc888

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title "fix: replace hardcoded gas 21000" succinctly and accurately captures the primary change in the PR — replacing the 21000 hardcoded gas with dynamic gas estimation and wiring estimateTransactionGas into the client — and it follows conventional commit style, making it clear and relevant for reviewers scanning history.
Description Check ✅ Passed The PR description follows the repository template and is mostly complete: it includes "Fixes #DXP-653", a clear "What" section listing the client refactor and the new estimateTransactionGas behavior, a "Why" section, a "Testing done" note, "Decisions made", checklist items, reviewing tips, and user-facing release notes, so reviewers can understand intent and scope.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Poem

A rabbit nudges gas to test and try,
Estimates first before they fly.
Clients stacked in careful rows,
Contracts follow where logic goes.
Imports hopped and neat—now off we fly. 🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a8dad29 and bfaef3b.

📒 Files selected for processing (1)
  • src/contracts/actions.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/contracts/actions.ts
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dxp-653-replace-hardcoded-gas-21000

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]

This comment was marked as resolved.

@kstroobants
kstroobants merged commit 8eacd5d into main Sep 11, 2025
2 checks passed
@kstroobants
kstroobants deleted the dxp-653-replace-hardcoded-gas-21000 branch September 11, 2025 09:05
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.

2 participants