Support withLog tracer config key for callTracer
#711
Conversation
m-Peter
requested review from
janezpodhostnik,
peterargue and
zhangchiqing
as code owners
WalkthroughThis pull request introduces enhancements to the tracing and logging capabilities across multiple components of the EVM system. The changes focus on improving log capture during transaction tracing, specifically enabling the ability to return logs when the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
m-Peter
force-pushed
the
feature/local-tx-reexecution
branch
from
2121836 to
a746c82
Compare
m-Peter
force-pushed
the
mpeter/support-call-tracer-with-logs
branch
from
ac8a8c8 to
9f7ca96
Compare
m-Peter
force-pushed
the
mpeter/support-call-tracer-with-logs
branch
from
9f7ca96 to
4846135
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
tests/web3js/debug_traces_test.js (2)
25-26: Prefer stable naming or comments explaining these new tracer settings.By setting "withLog" to true and "onlyTopCall" to false, the call tracer will capture logs and delve into nested calls. This can produce more verbose results, which may be intended. If performance concerns arise, consider documenting or gating these settings.
569-570: Logs disabled for deeper calls.Setting "withLog" to false but "onlyTopCall" to false disables log capture yet still explores subcalls. If the intent is to skip logs entirely, ensure clarity in documentation about handling deeper calls.
services/evm/executor.go (1)
109-113: Send transaction logs to tracer for external handling.You’ve added logic to pass each log in “res.Logs” to the tracer. This is crucial for capturing on-chain events. If logs can be large or numerous, monitor the potential performance impact, and consider batching or streaming if needed.
api/debug.go (1)
244-249: Push logs captured from DryCall to the tracer’s OnLog method.This block ensures that logs from a simulated (DryCall) transaction are made visible to the tracer. Validate that only relevant events are included—especially if calls are expanded or partial logs are needed for performance reasons.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
api/debug.go(2 hunks)services/evm/executor.go(1 hunks)tests/web3js/debug_traces_test.js(12 hunks)
🔇 Additional comments (14)
tests/web3js/debug_traces_test.js (12)
74-74: Confirm correctness of the new method name.
The function is now “storeWithLog(100n)” instead of the old “store”. Ensure that dependencies and references to the old method are updated throughout the codebase, especially on the contract side.
96-97: Validate updated gas usage assertions.
Gas and gasUsed have changed to "0x6f9a" and "0x6e3f" respectively. Confirm that these new expected values reflect actual execution costs and are not incidental or prone to fluctuation.
131-131: Verify the updated balance in prestate tracer.
The expected “0x4098ea” balance should align with the actual transaction logic. Confirm that this matches any discount, fee, or updated logic in the contract or the test environment.
135-135: Check consistency for newly reported nonce.
The updated state includes nonce = 2. Ensure prior transactions and the final expected state transitions all align with this incremented nonce.
149-149: Confirm 4-byte tracer outputs.
The tracer output “{ '0x6babb224-32': 1 }” indicates a traced signature. Verify that this is indeed the correct mapping for the storeWithLog or sum method signature.
165-183: Enhanced log checking for the transaction trace.
Multiple lines here assert correct logs, contract address, input data, and the transaction hash. Confirm that the log’s data and topics (lines 175-181) match the contract event signature for storeWithLog(100n).
189-189: Confirm consistency of new transaction hash.
The transaction “0x828d174e…” is expected to produce outcomes distinct from the first transaction. Ensure the test environment consistently differentiates these two traces.
196-196: Review updated “value” field in logs.
The test sets “value” to 0x4098ea. Confirm that this aligns with the contract’s logic and any newly introduced transaction costs or adjustments.
216-234: Consolidated log trace validation.
Similar to line 165, these lines confirm the event logs for the same transaction in debug_traceBlockByHash. Ensure consistency between traceBlockByNumber and traceBlockByHash results for this log set.
240-247: Verify new transaction hash and minted value.
The second transaction "0x828d174e..." is assigned a minted or transferred value of 0x4098ea. Validate that the block’s final state matches these expected changes, especially if any fees or additional logic are triggered.
606-613: Reintroducing log capture for subcalls.
Switching back to "withLog: true" while "onlyTopCall: false" allows capturing all logs from nested calls. Confirm the extra data doesn’t cause performance degradation in production.
615-657: New sum(500, 100) call trace.
This final block tests the newly introduced “sum” functionality and verifies logs. Ensure the contract properly emits the log with correct data in “data: '0x000000000000000...00000258'” and the computed total (0x258 = 600).
api/debug.go (2)
232-232: Dry-call integration point.
You’re capturing the transaction result from “DryCall” to apply exposure to tracers. Ensure any future changes to “DryCall” signature or returned data structure remain backward-compatible with existing instrumentation code.
488-490: Whitespace trimming for default tracer config.
This addresses configuration mismatch caused by spacing. If user-supplied JSON includes newlines or other whitespace variations, trimming ensures consistent comparisons to “replayer.TracerConfig”. This minor detail prevents subtle configuration bugs.
m-Peter
force-pushed
the
mpeter/support-call-tracer-with-logs
branch
from
4846135 to
d86fa3d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/web3js/debug_traces_test.js (2)
Line range hint
315-358: Consider adding negative test casesWhile the positive test case for
withLog: trueis good, consider adding test cases for:
withLog: falseto verify logs are not included- Invalid configurations to verify error handling
608-658: Enhance test coverage for the newsummethodThe test case for the
summethod looks good, but consider adding:
- Edge cases (e.g., large numbers, zero values)
- Error cases (e.g., overflow scenarios)
Example test cases to add:
// Test edge cases callData = deployed.contract.methods.sum(Number.MAX_SAFE_INTEGER, 1).encodeABI() // Test zero values callData = deployed.contract.methods.sum(0, 0).encodeABI()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
api/debug.go(2 hunks)services/evm/executor.go(1 hunks)tests/web3js/debug_traces_test.js(13 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- services/evm/executor.go
- api/debug.go
🔇 Additional comments (3)
tests/web3js/debug_traces_test.js (3)
25-26: LGTM: Proper configuration of withLog and onlyTopCall
The configuration aligns with the PR objective to support the withLog tracer config key.
642-655: Verify the log data format for the sum event
The test asserts a specific format for the sum event log:
- Data field contains the sum result
- Topics include the event signature and two indexed parameters
Please verify this matches the contract's event definition.
✅ Verification successful
The log format matches the contract's event definition
The test's log format correctly matches the Calculated event definition from the contract:
- Event signature in first topic (0x76efea...)
- Three indexed parameters in the remaining topics:
sender(address)numA(uint256)numB(uint256)
- Non-indexed
sumparameter in the data field
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find the sum event definition
ast-grep --pattern 'event $name(address indexed sender, uint256 indexed a, uint256 indexed b, uint256 sum) {
$$$
}'
Length of output: 119
Script:
#!/bin/bash
# Let's try a different pattern to find event definitions related to sum
ast-grep --pattern 'event $name($$$) $$$'
# Also search for Solidity files that might contain the event definition
fd -e sol
# Search for files containing "sum" and "event" keywords
rg -l "event.*sum|sum.*event"
Length of output: 251
Script:
#!/bin/bash
# Let's examine the storage.sol file which seems to contain the event definition
cat tests/fixtures/storage.sol
# Also check the test file for context
rg -A 5 -B 5 "event.*sum|sum.*event" tests/fixtures/storage.sol
Length of output: 3579
173-184: Verify the log event signature and topics
The test asserts the presence of logs with specific topics. Please verify:
- The event signature hash matches the contract's event definition
- The indexed parameters are correctly ordered in the topics array
✅ Verification successful
The previous command didn't yield results because the pattern wasn't matching Solidity event definitions correctly. Let's try searching for event definitions in Solidity files and examine the test file to understand the context better. This will help us verify the event signature and topics alignment.
The test file shows that we're dealing with a storage contract, and we need to verify the event signature and topics. Let's examine the contract implementation to find the event definition and verify the signature matches.
From examining the test file and contract code, I can now generate the final response. The test is verifying a log event from the storeWithLog function in the Storage contract, which emits a NewStore event.
Event signature and topics are correctly implemented
The event signature and topics in the test match the contract's event definition:
- Event signature hash
0x043cc306157a91d747b36aba0e235bbbc5771d75aba162f6e5540767d22673c6corresponds toNewStore(address indexed caller, uint256 indexed value) - Topics are correctly ordered:
- Topic[0]: Event signature hash
- Topic[1]: Indexed parameter
caller(address) - Topic[2]: Indexed parameter
value(uint256)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find the event definition in the contract
ast-grep --pattern 'event $name($$$) {
$$$
}'
Length of output: 49
Script:
#!/bin/bash
# Search for event definitions in Solidity files
rg -t sol "event.*\(" --no-filename
# Search for test file content to understand the context
cat tests/web3js/debug_traces_test.js
Length of output: 22116
Script:
#!/bin/bash
# Let's find the contract implementation to verify the event signature
fd -t f "storage" --exec cat {}
Length of output: 40519
Closes: #710
Description
For contributor use:
masterbranchFiles changedin the Github PR explorerSummary by CodeRabbit
New Features
Bug Fixes
Tests