Skip to content

eth/tracers: extend tracers for CREATE2 #17565

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

eth/tracers: extend tracers for CREATE2 #17565

wants to merge 1 commit into from

Conversation

YaoZengzeng
Copy link
Contributor

ref: #17196

@holiman
Copy link
Contributor

holiman commented Sep 1, 2018

👍

@holiman
Copy link
Contributor

holiman commented Sep 20, 2018

If we just merge this, it will have no effect, the assets.go needs to be regenerated aswell, using go-bindata.
See https://github.com/ethereum/go-ethereum/blob/beb141aa9bdf3513e241bf3db802a0bc796ee0fa/eth/tracers/internal/tracers/assets.go

@karalabe
Copy link
Member

You just need to do go generate in the tracers folder. Do go get -u github.com/kevinburke/go-bindata/go-bindata beforehand to make sure you have the latest bindata.

@karalabe karalabe added this to the 1.8.17 milestone Sep 20, 2018
@YaoZengzeng
Copy link
Contributor Author

@holiman @karalabe Updated

karalabe
karalabe reviewed Sep 20, 2018
View reviewed changes
eth/tracers/internal/tracers/prestate_tracer.js Outdated
@@ -85,7 +85,7 @@
case "EXTCODECOPY": case "EXTCODESIZE": case "BALANCE":
this.lookupAccount(toAddress(log.stack.peek(0).toString(16)), db);
break;
case "CREATE":
case "CREATE": case "CREATE2":
Copy link
Member

Choose a reason for hiding this comment

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

This is not correct. The code below uses toContract(from, db.getNonce(from)) to generate the contract address, but CREATE2 has a different contract address generation scheme. You'll need to implement a toContract2 similar to https://github.com/ethereum/go-ethereum/blob/master/eth/tracers/tracer.go#L354

@YaoZengzeng YaoZengzeng force-pushed the extend-tracers branch 2 times, most recently from f52a41c to f10bb14 Compare September 21, 2018 15:29
@YaoZengzeng
eth/tracers: extend tracers for CREATE2
c49e754 
Signed-off-by: YaoZengzeng <yaozengzeng@zju.edu.cn>
@YaoZengzeng
Copy link
Contributor Author

@karalabe Updated, actually I know nothing about js and I implement toContract2 with the knowledge of existing functions like toContract and toAddress 😂

PTAL if it is correct :)

@karalabe karalabe modified the milestones: 1.8.17, 1.8.18 Oct 9, 2018
holiman
holiman suggested changes Nov 29, 2018
View reviewed changes
eth/tracers/tracer.go
code := common.CopyBytes(makeSlice(ctx.GetBuffer(-1)))
ctx.Pop3()

contract := crypto.CreateAddress2(from, salt, code)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is now wrong, CreateAddress2 now uses codeHash , not code.

@holiman
Copy link
Contributor

holiman commented Dec 13, 2018

@YaoZengzeng could you update it to use codehash instead?

@gballet gballet self-requested a review December 13, 2018 09:40
@YaoZengzeng
Copy link
Contributor Author

YaoZengzeng commented Dec 13, 2018 •
edited
Loading

I am almost unfamiliar with the code base now.

I will close this PR and maybe @holiman could make another PR to fix it.

Sorry for that.

@rjl493456442 rjl493456442 mentioned this pull request Dec 16, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@karalabe karalabe karalabe left review comments

@holiman holiman holiman requested changes

@gballet gballet Awaiting requested review from gballet

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.8.18
Development

Successfully merging this pull request may close these issues.
3 participants
@YaoZengzeng @holiman @karalabe