-
Notifications
You must be signed in to change notification settings - Fork 358
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
Fix weight limits in evm tracing runtimes #3210
base: master
Are you sure you want to change the base?
Conversation
WASM runtime size check:Compared to target branchMoonbase runtime: 2420 KB (no changes) 🚨 Moonbeam runtime: 2400 KB (no changes) ✅ Moonriver runtime: 2412 KB (no changes) 🚨 Compared to latest release (runtime-3501)Moonbase runtime: 2420 KB (+468 KB compared to latest release) 🚨 Moonbeam runtime: 2400 KB (+460 KB compared to latest release) Moonriver runtime: 2412 KB (+472 KB compared to latest release) 🚨 |
Coverage Report@@ Coverage Diff @@
## master rq/fix-over-weight-when-tracing +/- ##
===================================================================
- Coverage 74.57% 74.56% -0.01%
Files 381 381
+ Lines 96700 96746 +46
===================================================================
+ Hits 72114 72130 +16
+ Misses 24586 24616 +30
|
This reverts commit a58f24a.
EvmTracer::new().trace(|| Executive::apply_extrinsic(ext)); | ||
EvmTracer::new().trace(|| { | ||
if let Err(err) = Executive::apply_extrinsic(ext) { | ||
log::debug!( |
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.
should it be a warning ?
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.
Is better to be just a debug log, some extinsics are expected to fail and we should not emit a warning for those cases.
I have added the debug log here, since we were ignoring the extrinsic result, and it is useful when debugging tracing problems.
What does it do?
The changes included in this pull request fix the weight limits used in evm tracing runtimes.
Runtimes with feature
evm-tracing
enabled introduce additional (ref_time) overhead, since they have special logic to trace ethereum transactions (emitting events for each component:gasometer
,runtime
,evm
) that is then used to fill all the information for (debug_traceTransaction
,trace_filter
, ...) rpc calls.Since the real ref_time in production runtimes is smaller, it means that we could reach the block weight limits when replaying a block in a evm-tracing runtime, and as result skip transactions traces. (Which was the case in moonbeam block 9770044)
What important points should reviewers know?
The fix consists in resetting the previously consumed weight before tracing each ethereum transaction, the fix only affects code under
evm-tracing
, which is not included in any production runtime.