feat: Implement Forest.EthTraceCall#6412
Merged
akaladarshi merged 24 commits intomainfrom Feb 23, 2026
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR implements the Changes
Sequence DiagramsequenceDiagram
participant Client
participant RPC as RPC Handler<br/>(trace_call)
participant StateManager
participant VM
participant Store as Blockstore
Client->>RPC: call trace_call(message, traceTypes, blockTag)
RPC->>StateManager: Get state for blockTag
StateManager->>Store: Load pre-execution state
Store-->>StateManager: pre_state
RPC->>VM: Invoke transaction with gas
VM->>VM: Execute message
VM-->>RPC: execution result, gas usage
RPC->>StateManager: Get post-execution state
StateManager->>Store: Load post-execution state
Store-->>StateManager: post_state
alt if StateDiff requested
RPC->>RPC: build_state_diff(pre_state, post_state, touched_addresses)
RPC->>Store: Extract EVM storage entries
Store-->>RPC: storage map
RPC->>RPC: Compute Delta for each slot
RPC->>RPC: Compare balance, nonce, code
end
RPC->>RPC: Format traces and state diffs
RPC-->>Client: EthTraceResults (traces + optional stateDiff)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
akaladarshi
changed the title
Forest.EthTraceCallForest.EthTraceCall [Skip CI]
akaladarshi
force-pushed
the
akaladarshi/eth-trace-call
branch
2 times, most recently
from
701d095 to
a69d125
Compare
akaladarshi
force-pushed
the
akaladarshi/eth-trace-call
branch
from
a69d125 to
47b7aa3
Compare
Member
|
As discussed, let's have a short guide in docs outlining its usage. This is important in light of various formats used across the Ethereum ecosystem for the output of this method. |
LesnyRumcajs
reviewed
Feb 5, 2026
LesnyRumcajs
reviewed
Feb 5, 2026
LesnyRumcajs
reviewed
Feb 6, 2026
LesnyRumcajs
reviewed
Feb 6, 2026
LesnyRumcajs
reviewed
Feb 6, 2026
LesnyRumcajs
reviewed
Feb 6, 2026
LesnyRumcajs
reviewed
Feb 6, 2026
LesnyRumcajs
reviewed
Feb 6, 2026
LesnyRumcajs
reviewed
Feb 6, 2026
LesnyRumcajs
reviewed
Feb 6, 2026
LesnyRumcajs
reviewed
Feb 6, 2026
LesnyRumcajs
reviewed
Feb 6, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/rpc/methods/eth/types.rs`:
- Around line 785-844: Add doc comments to all public helpers related to diffs:
document Delta::from_comparison (describe that it compares old/new Option<T> and
returns Added/Removed/Changed/Unchanged), Delta::is_unchanged (explain it
returns true only for Delta::Unchanged), AccountDiff::is_unchanged (explain it
tests all fields and empty storage), StateDiff::new (note it constructs an empty
StateDiff) and StateDiff::insert_if_changed (state that it inserts the
AccountDiff only when not unchanged). Use /// style comments placed immediately
above each function/struct definition, briefly describing inputs, behavior, and
any important side-effects (e.g., that insert_if_changed skips unchanged diffs).
---
Duplicate comments:
In `@src/rpc/methods/eth.rs`:
- Around line 474-501: Add top-level doc comments for the public trace types:
place concise /// doc comments above the EthTraceType enum describing it
represents the two supported trace output modes (structured call graph "Trace"
and state changes "StateDiff") and above the EthTraceResults struct describing
it represents the combined trace result (output bytes, optional state_diff, and
call trace Vec<EthTrace>) so consumers can discover purpose and fields; keep
existing variant/field docs and serde/lifecycle attributes
(lotus_json_with_self!, #[serde(...)] and derives) intact and ensure comments
follow crate style (brief summary sentence and any important usage notes).
- Around line 4078-4091: In get_trace_output, stop swallowing decode errors by
removing the final .ok(); change the function to propagate decode failures
(e.g., change the signature from fn get_trace_output(...) -> Option<EthBytes> to
return Result<Option<EthBytes>, E>), replace decode_payload(&return_data,
CBOR).ok() with a fallible propagation (e.g., decode_payload(&return_data,
CBOR)? wrapped to produce Ok(Some(...))), and convert the early default-return
branches to Ok(Some(EthBytes::default())) or Ok(None) as appropriate so callers
of get_trace_output (and types like ApiInvocResult and EthBytes) must handle
decoding errors instead of silently receiving None.
8b47987 to
ee300c5
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/rpc/methods/eth.rs (1)
4015-4031: Prefer.context()to wrap state/apply errors.
Keeps error chains consistent with project style.As per coding guidelines: "Use `anyhow::Result` for most operations, adding context with `.context()`".♻️ Suggested tweak
- let (pre_state_root, _) = ctx - .state_manager - .tipset_state(&ts, StateLookupPolicy::Enabled) - .await - .map_err(|e| anyhow::anyhow!("failed to get tipset state: {e}"))?; + let (pre_state_root, _) = ctx + .state_manager + .tipset_state(&ts, StateLookupPolicy::Enabled) + .await + .context("failed to get tipset state")?; @@ - .await - .map_err(|e| anyhow::anyhow!("failed to apply message: {e}"))?; + .await + .context("failed to apply message")?;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rpc/methods/eth.rs` around lines 4015 - 4031, Wrap the error returns with anyhow::Context instead of map_err(anyhow::anyhow!(...)): add use anyhow::Context if missing, change the tipset_state call to use .await.context("failed to get tipset state"), change StateTree::new_from_root(...) to append .context("failed to create StateTree from root"), and change the apply_on_state_with_gas call to .await.context("failed to apply message"); keep the same messages but use .context() for consistent error chaining around tipset_state, StateTree::new_from_root, and apply_on_state_with_gas.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/docs/developers/guides/trace_call_guide.md`:
- Around line 72-75: Replace the unhyphenated compound adjectives "Geth style
tracing" and "Parity style tracing" with hyphenated forms "Geth-style tracing"
and "Parity-style tracing" in the document so the compounds are grammatically
correct; search for the exact phrases "Geth style" and "Parity style" in the
text and update them accordingly.
In `@src/rpc/methods/eth.rs`:
- Around line 474-501: Public API types EthTraceType and EthTraceResults lack
doc comments; add concise top-level Rust doc comments (/// ...) describing each
type's purpose and behavior, plus brief docs for each EthTraceType variant
(Trace, StateDiff) and for EthTraceResults fields (output, state_diff, trace) to
explain when they are populated and their semantics; ensure comments follow
camelCase/serde intent and appear above the enum, its variants, and the
struct/fields (retain existing serde attributes and lotus_json_with_self!
macros).
- Around line 4036-4038: The code currently swallows decoding failures by
calling decode_payload(...).ok() when building EthTraceResults (around the
get_trace_output/invoke_result usage); change that to propagate the decode error
instead of turning it into None — i.e., remove .ok() and use the fallible call
(e.g., decode_payload(...) ? or decode_payload(...).map_err(|e|
convert_to_rpc_error(e))?) so the enclosing function returns an Err on decode
failure; apply the same fix to the other occurrence that mirrors this behavior
later in the file (the second decode_payload(...) call used when constructing
trace output).
In `@src/rpc/methods/eth/types.rs`:
- Around line 785-845: Public API items lack doc comments; add concise doc
comments for Delta::from_comparison, Delta::is_unchanged, the AccountDiff struct
and AccountDiff::is_unchanged, and the StateDiff struct plus StateDiff::new and
StateDiff::insert_if_changed that explain purpose, parameters and return
behavior (e.g., what from_comparison does with old/new Options, what
is_unchanged checks, that StateDiff is a transparent BTreeMap of account ->
AccountDiff, new creates an empty StateDiff, and insert_if_changed only inserts
non-unchanged AccountDiffs). Keep comments short, use /// style and mention any
invariants or important behavior (like storage emptiness meaning unchanged).
---
Nitpick comments:
In `@src/rpc/methods/eth.rs`:
- Around line 4015-4031: Wrap the error returns with anyhow::Context instead of
map_err(anyhow::anyhow!(...)): add use anyhow::Context if missing, change the
tipset_state call to use .await.context("failed to get tipset state"), change
StateTree::new_from_root(...) to append .context("failed to create StateTree
from root"), and change the apply_on_state_with_gas call to
.await.context("failed to apply message"); keep the same messages but use
.context() for consistent error chaining around tipset_state,
StateTree::new_from_root, and apply_on_state_with_gas.
ee300c5 to
49d9a94
Compare
akaladarshi
force-pushed
the
akaladarshi/eth-trace-call
branch
from
7474ad9 to
342b02f
Compare
LesnyRumcajs
approved these changes
Feb 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of changes
Changes introduced in this pull request:
Forest.EthTraceCall#6195Reference issue to close (if applicable)
Closes #6185
Other information and links
Change checklist
Summary by CodeRabbit
New Features
Documentation
Tests