Skip to content
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

Merged
merged 16 commits into from
Sep 26, 2022

Conversation

j75689
Copy link
Contributor

@j75689 j75689 commented Sep 26, 2022

Description

This PR aims to remove duktape engine dependency, so that we can support cross-compile on arm

  1. ci: add linux-arm binaries.
  2. docker: fix dependency issue in Dockerfile
  3. eth/tracers: drop duktape engine

Rationale

N/A

Example

image

Changes

Notable changes:

  • Dockerfile
  • Makefile
  • Ci scripts
  • Tracers

s1na and others added 14 commits September 24, 2022 13:09
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 <martin@swende.se>

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.
@j75689 j75689 added the r4r ready for review label Sep 26, 2022
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
Copy link
Collaborator

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 ~=

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

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
Copy link
Collaborator

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?

Copy link
Contributor Author

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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please run go tidy

Copy link
Contributor Author

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

Copy link
Collaborator

@brilliant-lx brilliant-lx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@unclezoro unclezoro merged commit 816e301 into bnb-chain:develop Sep 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
r4r ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants