Skip to content

core/vm: implement EIP-3541 as an "extra EIP"#22768

Closed
chfast wants to merge 2 commits intoethereum:masterfrom
ewasm:eip_3541
Closed

core/vm: implement EIP-3541 as an "extra EIP"#22768
chfast wants to merge 2 commits intoethereum:masterfrom
ewasm:eip_3541

Conversation

@chfast
Copy link
Copy Markdown
Member

@chfast chfast commented Apr 29, 2021

No description provided.

Comment thread core/vm/evm.go Outdated
Comment on lines 472 to 479
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd just do evm.chainRules.IsEip3541 -- and later on change it to IsLondon or whatever

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I only refactored this. We are happy that we can create tests for Berlin+3541 for this.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I only refactored this. We are happy that we can create tests for Berlin+3541 for this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, that's fine, but in the long run we should have a proper config and tie it to a forkname instead. That ^ is a bit of a hack

@fjl fjl changed the title evm: Implement EIP-3541 as an "extra EIP" core/vm: implement EIP-3541 as an "extra EIP" Apr 30, 2021
@holiman holiman mentioned this pull request May 4, 2021
@holiman
Copy link
Copy Markdown
Contributor

holiman commented May 4, 2021

This needs a rebase. I tried doing it myself, but the remote does not allow maintainer push

@chfast
Copy link
Copy Markdown
Member Author

chfast commented May 4, 2021

This needs a rebase. I tried doing it myself, but the remote does not allow maintainer push

This option is not available if a repo is not a personal one.

Comment thread core/vm/errors.go
ErrWriteProtection = errors.New("write protection")
ErrReturnDataOutOfBounds = errors.New("return data out of bounds")
ErrGasUintOverflow = errors.New("gas uint64 overflow")
ErrInvalidCode = errors.New("invalid code")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Slightly unrelated question (not blocking this PR): EVMC has a CONTRACT_VALIDATION_FAILED error code (used as part of Ewasm 1.0). Probably we could use that error code for EOF1 validation, but should we use it already for this use case, or it makes more sense separating the error codes?

@chfast
Copy link
Copy Markdown
Member Author

chfast commented May 10, 2021

Done in #22809.

@chfast chfast closed this May 10, 2021
@chfast chfast deleted the eip_3541 branch May 10, 2021 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants