Skip to content
This repository has been archived by the owner on Nov 20, 2023. It is now read-only.

fix: missing EIP-2718 support checking tx type #21

Merged
merged 2 commits into from
May 15, 2023

Conversation

vimpunk
Copy link
Contributor

@vimpunk vimpunk commented May 8, 2023

This PR fixes the test suite runner in light of new failing geth tests checking for whether on pre-Berlin forks non-legacy transactions are properly rejected. This was not the case for sputnik's evm-tests, because the Berlin hardfork implementation (rust-ethereum/evm#40) opted to shift responsibility of implementing EIP-2718 (tx type filtering) to the layer hosting sputnik. In this case, the host is evm-test, which was missing that check.

IMPORTANT: This PR depends on rust-ethereum/evm#162, which needs to be merged, and then the git submodule on this branch needs to be updated to master of rust-blockchain/evm.

@vimpunk
Copy link
Contributor Author

vimpunk commented May 8, 2023

@sorpaas, here's a new one for you :)

@@ -9,7 +9,8 @@ keywords = ["no_std", "ethereum"]
edition = "2018"

[dependencies]
evm = { path = "../evm" }
# evm = { path = "../evm" }
evm = { git = "https://github.com/mandreyel/evm", branch = "feat/eip-4399" }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: Merge evm PR and then revert these changes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please just revert this now. We only have CI in evm repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, ready for merging.

@vimpunk
Copy link
Contributor Author

vimpunk commented May 12, 2023

@sorpaas ser, wen merge? :)

Cargo.lock Outdated
@@ -581,6 +584,7 @@ dependencies = [
[[package]]
name = "evm-runtime"
version = "0.37.0"
source = "git+https://github.com/mandreyel/evm?branch=feat/eip-4399#d8324b41b5baf0a6f4221c3cacc3a1828a191e60"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Those should be reverted also.

Just run cargo build and commit the Cargo.lock file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, that was my bad. I reverted that and also added another fix for a sputnik fix I'm going to push soon. Please check the diff, it's small.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, while I have your attention, can we merge rust-ethereum/evm#162? It's been sitting open for a while but it's done.

@sorpaas sorpaas merged commit c1e6512 into rust-blockchain:master May 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants