-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
tracers ci: drop duktape engine (#24934) and add linux-arm binaries to releases page #1100
Conversation
This PR also comes with 2 fixes in the Goja tracer and one small behavioural change: I had handled errors in the native Go functions by panicing. My oversight was that Goja only handles panics with a Goja.Value as argument. The difference is panic(goja.Value) allows JS to catch the exception whereas Interrupt(error) doesn't. There was a race in how I handled Stop. Because of 1. some of the methods that simply return nil on error (like memory.slice) now throw an exception.
* core,eth: add empty tx logger hooks * core,eth: add initial and remaining gas to tx hooks * store tx gasLimit in js tracer * use gasLimit to compute intrinsic cost for js tracer * re-use rules in transitiondb * rm logs * rm logs * Mv some fields from Start to TxStart * simplify sender lookup in prestate tracer * mv env to TxStart * Revert "mv env to TxStart" This reverts commit 656939634b9aff19f55a1cd167345faf8b1ec310. * Revert "simplify sender lookup in prestate tracer" This reverts commit ab65bce48007cab99e68232e7aac2fe008338d50. * Revert "Mv some fields from Start to TxStart" This reverts commit aa50d3d9b2559addc80df966111ef5fb5d0c1b6b. * fix intrinsic gas for prestate tracer * add comments * refactor * fix test case * simplify consumedGas calc in prestate tracer
This adds a JS tracer runtime environment based on the Goja VM. The new runtime replaces the duktape runtime, which will be removed soon. Goja is implemented in Go and is faster for cases where the Go <-> JS transition overhead dominates overall performance. It is faster because duktape is written in C, and the transition cost includes the cost of using cgo. Another reason for using Goja is that go-duktape is not maintained anymore. We expect the performace of JS tracing to be at least as good or better with this change.
This PR also comes with 2 fixes in the Goja tracer and one small behavioural change: I had handled errors in the native Go functions by panicing. My oversight was that Goja only handles panics with a Goja.Value as argument. The difference is panic(goja.Value) allows JS to catch the exception whereas Interrupt(error) doesn't. There was a race in how I handled Stop. Because of 1. some of the methods that simply return nil on error (like memory.slice) now throw an exception.
* eth/tracers: refactor traceTx to separate out struct logging review fix Update eth/tracers/api.go Co-authored-by: Martin Holst Swende <[email protected]> Mv ExecutionResult type to logger package review fix impl GetResult for StructLogger make formatLogs private confused exit and end.. account for intrinsicGas in structlogger, fix TraceCall test Add Stop method to logger Simplify traceTx Fix test rm logger from blockchain test account for refund in structLogger * use tx hooks in struct logger * minor * avoid executionResult in struct logger * revert blockchain test changes
This PR allows users to pass in a config object directly to the tracers. Previously only the struct logger was configurable. It also adds an option to the call tracer which if enabled makes it ignore any subcall and collect only information about the top-level call. See #25419 for discussion. The tracers will silently ignore if they are passed a config they don't care about.
This PR allows users to pass in a config object directly to the tracers. Previously only the struct logger was configurable. It also adds an option to the call tracer which if enabled makes it ignore any subcall and collect only information about the top-level call. See #25419 for discussion. The tracers will silently ignore if they are passed a config they don't care about.
Dockerfile
Outdated
grep~=3.7 curl~=7.83.1-r2 sed~=4.8-r0 curl~=7.83 | ||
ENV PACKAGES ca-certificates~=20220614-r0 jq~=1.6 \ | ||
bash~=5.1.16-r2 bind-tools~=9.16.33-r0 tini~=0.19.0 \ | ||
grep~=3.7 curl==7.83.1-r3 sed~=4.8-r0 |
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.
for the curl
, please do not point a must version, please use ~=
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.
fixed
.github/workflows/pre-release.yml
Outdated
run: | | ||
mkdir -p $GOPATH/src/github.com/binance-chain/bsc/ | ||
cp -r ./* $GOPATH/src/github.com/binance-chain/bsc/ | ||
cd $GOPATH/src/github.com/binance-chain/bsc/ && make geth-linux-arm |
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.
no binance-chain any more?
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.
fixed
@@ -1,14 +1,10 @@ | |||
module github.com/ethereum/go-ethereum | |||
|
|||
go 1.15 | |||
go 1.17 |
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.
Please run go tidy
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.
already updated the go.mod
and go.sum
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.
LGTM
Description
This PR aims to remove duktape engine dependency, so that we can support cross-compile on arm
Rationale
N/A
Example
Changes
Notable changes: