Skip to content

Call antehandlers for traceBlock #2139

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Call antehandlers for traceBlock #2139

wants to merge 1 commit into from

Conversation

codchen
Copy link
Collaborator

@codchen codchen commented Apr 22, 2025

Describe your changes and provide context

There were two issues with how antehandlers are invoked during tracing:

  1. antehandlers are not invoked for debug_traceBlock... endpoints, which would specifically cause absence of address association
  2. antehandlers should not charge gas fees as the tracer call would handle that

Testing performed to validate your change

patched on archive node

@codchen codchen requested a review from jewei1997 April 22, 2025 15:06
@codchen codchen force-pushed the tony/fix-blocktrace branch 3 times, most recently from e6ea1c7 to ce7d076 Compare April 23, 2025 03:02
Copy link

codecov bot commented Apr 23, 2025

Codecov Report

Attention: Patch coverage is 67.77251% with 68 lines in your changes missing coverage. Please review.

Project coverage is 62.27%. Comparing base (49ba7bd) to head (1157e76).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
evmrpc/simulate.go 67.12% 17 Missing and 7 partials ⚠️
precompiles/common/precompiles.go 0.00% 10 Missing and 2 partials ⚠️
app/ante.go 45.00% 11 Missing ⚠️
app/app.go 72.22% 10 Missing ⚠️
x/evm/keeper/pointer.go 80.55% 7 Missing ⚠️
evmrpc/block.go 42.85% 2 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2139      +/-   ##
==========================================
- Coverage   62.38%   62.27%   -0.11%     
==========================================
  Files         293      295       +2     
  Lines       28937    29132     +195     
==========================================
+ Hits        18052    18142      +90     
- Misses       9556     9649      +93     
- Partials     1329     1341      +12     
Files with missing lines Coverage Δ
evmrpc/send.go 43.67% <100.00%> (ø)
evmrpc/server.go 87.13% <100.00%> (ø)
evmrpc/tests/utils.go 62.66% <100.00%> (ø)
evmrpc/tracers.go 28.15% <100.00%> (-31.07%) ⬇️
evmrpc/utils.go 69.94% <100.00%> (+2.24%) ⬆️
x/evm/ante/basic.go 50.00% <ø> (ø)
x/evm/keeper/evm.go 69.48% <100.00%> (ø)
x/evm/module.go 39.35% <100.00%> (+0.28%) ⬆️
x/evm/state/statedb.go 50.43% <ø> (ø)
evmrpc/block.go 76.14% <42.85%> (-0.57%) ⬇️
... and 5 more

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@codchen codchen force-pushed the tony/fix-blocktrace branch 3 times, most recently from 09c8298 to c84d9d7 Compare April 24, 2025 07:22
Comment on lines +1901 to +1906
go func() {
<-app.httpServerStartSignal
if err := evmHTTPServer.Start(); err != nil {
panic(err)
}
}()

Check notice

Code scanning / CodeQL

Spawning a Go routine Note

Spawning a Go routine may be a possible source of non-determinism
Comment on lines +1914 to +1919
go func() {
<-app.wsServerStartSignal
if err := evmWSServer.Start(); err != nil {
panic(err)
}
}()

Check notice

Code scanning / CodeQL

Spawning a Go routine Note

Spawning a Go routine may be a possible source of non-determinism
@codchen codchen force-pushed the tony/fix-blocktrace branch 17 times, most recently from f3622cd to d763001 Compare April 30, 2025 12:20
@codchen codchen force-pushed the tony/fix-blocktrace branch from d763001 to 1157e76 Compare May 2, 2025 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant