-
Notifications
You must be signed in to change notification settings - Fork 69
fix(eth): add index to callTracer log #33629 #2044
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev-upgrade
Are you sure you want to change the base?
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing touches🧪 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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR aligns the native callTracer log output with receipt log semantics by adding an index field to traced logs, and updates the internal tracer test fixtures accordingly.
Changes:
- Add
indextocallTracer’scallLogJSON output and populate it fromtypes.Log.Index. - Update tracer test harness structs to include the new
indexfield. - Update
call_tracer_withLogJSON fixtures (and one internal string expectation) to includeindex.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| eth/tracers/native/call.go | Adds callLog.Index and populates it for EVM logs in OnLog. |
| eth/tracers/internal/tracetest/calltrace_test.go | Updates callLog test struct and expected internal JSON to include index. |
| eth/tracers/internal/tracetest/testdata/call_tracer_withLog/simple.json | Adds index field to expected log output. |
| eth/tracers/internal/tracetest/testdata/call_tracer_withLog/notopic.json | Adds index field to expected log output. |
| eth/tracers/internal/tracetest/testdata/call_tracer_withLog/multilogs.json | Adds index field to expected log output for many logs. |
| eth/tracers/internal/tracetest/testdata/call_tracer_withLog/multi_contracts.json | Adds index field to expected log output across multiple contracts/calls. |
| eth/tracers/internal/tracetest/testdata/call_tracer_withLog/delegatecall.json | Adds index field to expected log output for delegatecall traces. |
| eth/tracers/internal/tracetest/testdata/call_tracer_withLog/calldata.json | Adds index field to expected log output for calldata-related trace. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| l := callLog{ | ||
| Address: log.Address, | ||
| Topics: log.Topics, | ||
| Data: log.Data, | ||
| Index: hexutil.Uint(log.Index), | ||
| Position: hexutil.Uint(len(t.callstack[lastIdx].Calls)), | ||
| } |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
callLog now includes an index field, but in OnLog it’s only populated for regular EVM transactions. For non-EVM transactions (t.isNonEVMTx branch earlier in this method), Index is left at the zero value, even though StateDB.AddLog assigns log.Index before invoking the hook. This will cause all non-EVM logs to be emitted with index: "0x0" regardless of their actual order. Populate Index for the non-EVM branch too (e.g., from log.Index, or synthesize from len(t.nonEVMCall.Logs) if needed).
Proposed changes
Ref: ethereum#33629
Types of changes
What types of changes does your code introduce to XDC network?
Put an
✅in the boxes that applyImpacted Components
Which part of the codebase this PR will touch base on,
Put an
✅in the boxes that applyChecklist
Put an
✅in the boxes once you have confirmed below actions (or provide reasons on not doing so) that