fix: Fix the `GasEstimateMessageGas` API
Summary of changes
Changes introduced in this pull request:
- Refactor the calculation to get the gas premium
- Fix the issues which was causing empty prices
- Add Fee config -
MaxFee0.07 FIL
Reference issue to close (if applicable)
Closes https://github.com/ChainSafe/forest/issues/5940
Other information and links
Change checklist
- [ ] I have performed a self-review of my own code,
- [ ] I have made corresponding changes to the documentation. All new code adheres to the team's documentation standards,
- [ ] I have added tests that prove my fix is effective or that my feature works (if possible),
- [ ] I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.
Summary by CodeRabbit
-
New Features
- Configurable default max fee (defaults to 0.07 FIL) applied across node and CLI.
- RPC message responses now include message + CID together.
- Message send options expose max_fee, msg_uuid, and maximize_fee_cap.
-
Improvements
- Tipset-aware gas estimation with blended premium calculation and explicit fee capping.
- ETH gas endpoints and wallet send flow updated for new estimators.
- Expanded TokenAmount arithmetic and percent-compare utilities.
-
Bug Fixes
- Fixed per-sender initialization during tipset message processing.
-
Tests
- Added cross-endpoint gas validation and refreshed RPC snapshots.
✏️ Tip: You can customize this high-level summary in your review settings.
Walkthrough
Flip per-sender gating in tipset message application; add configurable default max fee to ChainConfig; make gas estimation tipset-aware with median-premium computation and fee capping; return RPC messages flattened with their CID; expand MessageSendSpec; update wallet/tests/snapshots; extend TokenAmount arithmetic/traits.
Changes
| Cohort / File(s) | Summary |
|---|---|
Chain store: message gatingsrc/chain/store/chain_store.rs |
Invert per-block-header gating in messages_for_tipset: first-seen from now triggers actor state lookup/initialization (insert into applied/balances); subsequent messages for same sender follow sequencing and balance checks. |
Config / ChainConfig: default max feesrc/cli_shared/cli/config.rs, src/daemon/context.rs, src/networks/mod.rs |
Add FeeConfig { pub max_fee: TokenAmount } with Default (0.07 FIL); add default_max_fee: TokenAmount to ChainConfig; thread config.fee.max_fee into daemon ChainConfig init; network constructors initialize field to zero. |
RPC gas estimation: tipset-aware & cap logicsrc/rpc/methods/gas.rs, src/rpc/methods/eth.rs, src/rpc/types/mod.rs |
Make gas functions tipset-aware (accept ApiTipsetKey, load tipset); introduce GasMeta and compute_gas_premium; update estimate_gas_premium, estimate_fee_cap, estimate_message_gas signatures and flows; add cap_gas_fee; expose MessageSendSpec.max_fee, msg_uuid, maximize_fee_cap. |
RPC message shape: flattened with CIDsrc/rpc/methods/chain.rs, src/rpc/methods/gas.rs, src/tool/subcommands/api_cmd/api_compare_tests.rs, src/wallet/subcommands/wallet_cmd.rs |
Add pub struct FlattenedApiMessage { message: Message, cid: Cid } (lotus_json serialized) and change ChainGetMessage / GasEstimateMessageGas to return it; callers updated to access .message. |
Tests & snapshotssrc/tool/subcommands/api_cmd/test_snapshots.txt, src/tool/subcommands/api_cmd/api_compare_tests.rs, src/lotus_json/signed_message.rs |
Replace several snapshots and add filecoin_gasestimatemessagegas snapshot; update API compare tests to validate GasEstimateGasLimit vs GasEstimateMessageGas outputs; simplify literal paths in test data. |
Shim / TokenAmount arithmetic & traitssrc/shim/econ.rs |
Implement numeric/arithmetic traits (One, Num, Neg), add is_within_percent, implement Add/Sub/Mul/Div/Rem for owned/borrowed combinations via macros, AddAssign/SubAssign/Signed, and add unit tests. |
Minor typing/claritysrc/rpc/methods/msig.rs |
Make locked_balance explicitly TokenAmount (no behavioral change). |
Wallet: use flattened messagesrc/wallet/subcommands/wallet_cmd.rs |
In Send flow, unpack .message from GasEstimateMessageGas result before performing existing validations. |
Sequence Diagram(s)
sequenceDiagram
autonumber
participant Client
participant RPC as RPC (GasEstimateMessageGas)
participant Gas as rpc/methods/gas
participant Chain as ChainStore
participant Cfg as ChainConfig
Client->>RPC: request(msg, msg_spec, tsk)
RPC->>Gas: estimate_message_gas(msg, msg_spec, tsk)
Gas->>Chain: load_required_tipset_or_heaviest(tsk)
Chain-->>Gas: Tipset (messages, receipts)
Gas->>Gas: build Vec<GasMeta> from tipset messages
Gas->>Gas: compute_gas_premium(GasMeta list)
Gas->>Cfg: read default_max_fee
Cfg-->>Gas: TokenAmount
Gas->>Gas: cap_gas_fee(default_max_fee, msg, msg_spec)
Gas-->>RPC: FlattenedApiMessage { message, CID }
RPC-->>Client: FlattenedApiMessage
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60–90 minutes
Potential attention points:
- Correctness and implications of inverted gating in
src/chain/store/chain_store.rs. - Interaction between
cap_gas_fee,MessageSendSpec.max_fee/maximize_fee_cap, andChainConfig.default_max_fee. - RPC type changes (
FlattenedApiMessage) and all callsite updates (wallet, tests). - Boundary handling and correctness in
compute_gas_premiumand new TokenAmount arithmetic trait implementations.
Suggested reviewers
- hanabi1224
- LesnyRumcajs
- elmattic
Pre-merge checks and finishing touches
❌ Failed checks (2 warnings)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Out of Scope Changes check | ⚠️ Warning | Multiple out-of-scope changes detected: TokenAmount trait implementations (One, Num, Neg), new arithmetic operators, message chain logic inversion in messages_for_tipset, FeeConfig infrastructure, and wallet command modifications appear unrelated to the stated API fix objective. | Review and isolate changes to only gas estimation API refactoring and test coverage. Move unrelated changes (TokenAmount traits, chain logic, FeeConfig, wallet) to separate PRs with their own issue tracking. |
| Docstring Coverage | ⚠️ Warning | Docstring coverage is 65.22% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title 'fix: Fix the GasEstimateMessageGas API' directly addresses the main objective of the PR, which is to fix issues in the GasEstimateMessageGas API implementation. |
| Linked Issues check | ✅ Passed | The PR addresses issue #5940 by refactoring gas premium calculation, fixing empty price values, and adding API conformance test coverage for GasEstimateMessageGas with snapshot validation and cross-implementation testing. |
✨ Finishing touches
- [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
- [ ] Create PR with unit tests
- [ ] Post copyable unit tests in a comment
- [ ] Commit unit tests in branch
akaladarshi/fix-estimate-msg-gas
Comment @coderabbitai help to get the list of available commands and usage tips.
@akaladarshi As a wise guy once said (some Polish dude, I think),
No green checkmark, no review; it's basically the law
Codecov Report
:x: Patch coverage is 84.24821% with 66 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 40.41%. Comparing base (58352c4) to head (e2a5ad3).
:warning: Report is 1 commits behind head on main.
Additional details and impacted files
| Files with missing lines | Coverage Δ | |
|---|---|---|
| src/cli_shared/cli/config.rs | 100.00% <100.00%> (ø) |
|
| src/daemon/context.rs | 65.13% <100.00%> (+0.16%) |
:arrow_up: |
| src/lotus_json/signed_message.rs | 92.00% <100.00%> (ø) |
|
| src/networks/mod.rs | 85.28% <100.00%> (+0.13%) |
:arrow_up: |
| src/rpc/types/mod.rs | 0.00% <ø> (ø) |
|
| src/chain/store/chain_store.rs | 46.61% <0.00%> (ø) |
|
| src/rpc/methods/msig.rs | 0.00% <0.00%> (ø) |
|
| src/wallet/subcommands/wallet_cmd.rs | 0.00% <0.00%> (ø) |
|
| src/rpc/methods/eth.rs | 23.19% <0.00%> (ø) |
|
| src/rpc/methods/chain.rs | 21.05% <0.00%> (-0.05%) |
:arrow_down: |
| ... and 3 more |
... and 4 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 58352c4...e2a5ad3. Read the comment docs.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.