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.

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

Additional changes to fully support Cancun #285

Merged
merged 5 commits into from
Apr 8, 2024
Merged

Additional changes to fully support Cancun #285

merged 5 commits into from
Apr 8, 2024

Conversation

fselmo
Copy link
Contributor

@fselmo fselmo commented Apr 3, 2024 •
edited
Loading

  • Pull out the blob data when sending a transaction to the execution layer (py-evm). The execution layer does not deal with blob data, that's the whole point of blob transactions. We pull out the data and re-send with only the versioned hashes.

  • Sum up the correct value for blob_gas_used during block proposal. This is validated in the execution layer (py-evm).

Note: this will require: ethereum/py-evm#2165 and ethereum/py-evm#2166 to be merged and released. I've tested locally and these changes make it so we can send a blob transaction via web3.py -> EthereumTesterProvider -> eth-tester -> py-evm.

How was it fixed?

  • Remove the blob data and re-serialize blob transactions in a way that the EL expects, without the bloat of the blob data.
  • Sum up blob_gas_used and send it to EL (py-evm)
  • Add validation and normalization for Cancun-related fields on block / header and transactions.

Todo:

  • Clean up commit history
  • More testing + verify these are all of the remaining changes related to full Cancun support
  • Add entry to the release notes

Cute Animal Picture

Screenshot 2024-04-03 at 17 54 20

fselmo added a commit to fselmo/eth-tester that referenced this pull request Apr 4, 2024
@fselmo
newsfragment for PR ethereum#285
bec3179
fselmo added a commit to fselmo/eth-tester that referenced this pull request Apr 4, 2024
@fselmo
newsfragment for PR ethereum#285
f588ec6
fselmo added a commit to fselmo/eth-tester that referenced this pull request Apr 4, 2024
@fselmo
newsfragment for PR ethereum#285
0f11fbe
@fselmo fselmo force-pushed the add-cancun-support branch 2 times, most recently from a1fed23 to bcbeb3c Compare April 4, 2024 21:31
This was referenced Apr 4, 2024
Merged
Merged
@fselmo fselmo force-pushed the add-cancun-support branch 2 times, most recently from 02228be to 4adc743 Compare April 5, 2024 15:12
@fselmo fselmo marked this pull request as ready for review April 5, 2024 15:13
@fselmo fselmo requested review from pacrob, kclowes and reedsa April 5, 2024 15:14
@fselmo
Some final changes to support Cancun:
8402330 
- Pull out the blob data when sending a transaction to the execution layer (py-evm).
  The execution layer does not deal with blob data, that's the whole point of blob
  transactions. We pull out the data and re-send with only the versioned hashes.

- Sum up the correct number of `blob_gas_used` since this is validated in the execution
  layer. This is basically simulating what the CL computes and sends to EL.

 - Add validation and normalization for Cancun-related fields for blocks
  / block headers and transactions

- Add `y_parity` to non-legacy receipts

- newsfragment for PR ethereum#285
@fselmo
Add some more basic tests around cancun field validation
6d64f32
@fselmo
Keep py-evm within the Cancun minor beta version.
e82c8f5
@fselmo fselmo force-pushed the add-cancun-support branch 2 times, most recently from 007556a to 4555851 Compare April 5, 2024 16:18
@fselmo fselmo force-pushed the add-cancun-support branch 2 times, most recently from 9f576a7 to ac6845e Compare April 5, 2024 19:14
reedsa
reedsa approved these changes Apr 5, 2024
View reviewed changes
Copy link
Contributor

@reedsa reedsa left a comment

Choose a reason for hiding this comment

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

Nice work, I left a couple comments you can take or leave. Not a blocker in any sense.

eth_tester/backends/pyevm/main.py Outdated Show resolved Hide resolved
eth_tester/backends/pyevm/serializers.py Outdated Show resolved Hide resolved
tests/backends/test_pyevm.py Show resolved Hide resolved
eth_tester/backends/pyevm/serializers.py Outdated Show resolved Hide resolved
kclowes
kclowes approved these changes Apr 5, 2024
View reviewed changes
Copy link
Contributor

@kclowes kclowes left a comment

Choose a reason for hiding this comment

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

Looks good to me! I put this in the comments too, but I like @reedsa's idea to have descriptive method names instead of if any(...) and if all(...)

eth_tester/normalization/common.py Show resolved Hide resolved
eth_tester/validation/outbound.py Outdated Show resolved Hide resolved
pacrob
pacrob approved these changes Apr 5, 2024
View reviewed changes
Copy link
Contributor

@pacrob pacrob left a comment

Choose a reason for hiding this comment

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

Looks good! I like the Stu idea of using constants for transaction types for readability. Whaddya think?

fselmo added a commit to fselmo/eth-tester that referenced this pull request Apr 5, 2024
@fselmo
Changes from comments on PR ethereum#285:
f9057e6 
- Refactor block evm guessing logic into utility methods
- Use constants for transaction types for clarity
- Keep integer for transaction type internally, normalize to hex
  (as clients do) on the way out.
fselmo added a commit to fselmo/eth-tester that referenced this pull request Apr 5, 2024
@fselmo
Changes from comments on PR ethereum#285:
61739e8 
- Refactor block evm guessing logic into utility methods
- Use constants for transaction types for clarity
- Keep integer for transaction type internally, normalize to hex
  (as clients do) on the way out.
fselmo added a commit to fselmo/eth-tester that referenced this pull request Apr 5, 2024
@fselmo
Changes from comments on PR ethereum#285:
e88daae 
- Refactor block evm guessing logic into utility methods
- Use constants for transaction types for clarity
- Keep integer for transaction type internally, normalize to hex
  (as clients do) on the way out.
fselmo added a commit to fselmo/eth-tester that referenced this pull request Apr 5, 2024
@fselmo
Changes from comments on PR ethereum#285:
6f19aa8 
- Refactor block evm guessing logic into utility methods
- Use constants for transaction types for clarity
- Keep integer for transaction type internally, normalize to hex
  (as clients do) on the way out.
fselmo added a commit to fselmo/eth-tester that referenced this pull request Apr 5, 2024
@fselmo
Changes from comments on PR ethereum#285:
087739b 
- Refactor block evm guessing logic into utility methods
- Use constants for transaction types for clarity
- Keep integer for transaction type internally, normalize to hex
  (as clients do) on the way out.
fselmo added a commit to fselmo/eth-tester that referenced this pull request Apr 5, 2024
@fselmo
Changes from comments on PR ethereum#285:
7366878 
- Refactor block evm guessing logic into utility methods
- Use constants for transaction types for clarity
- Keep integer for transaction type internally, normalize to hex
  (as clients do) on the way out.
@fselmo
Changes from comments on PR ethereum#285:
448c9da 
- Refactor block evm guessing logic into utility methods
- Use constants for transaction types for clarity
- Keep integer for transaction type internally, normalize to hex
  (as clients do) on the way out.
@fselmo
Copy link
Contributor Author

fselmo commented Apr 5, 2024

@reedsa @pacrob @kclowes the last commit should have some refactors that I think are along the lines of what was asked for. Will wait for extra eyes there before merging.

reedsa
reedsa approved these changes Apr 8, 2024
View reviewed changes
Copy link
Contributor

@reedsa reedsa left a comment

Choose a reason for hiding this comment

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

Looks awesome, great refactor!

@fselmo fselmo merged commit 308226f into ethereum:main Apr 8, 2024
19 checks passed
fselmo added a commit that referenced this pull request Apr 8, 2024
@fselmo
Some final changes to support Cancun:
2587f60 
- Pull out the blob data when sending a transaction to the execution layer (py-evm).
  The execution layer does not deal with blob data, that's the whole point of blob
  transactions. We pull out the data and re-send with only the versioned hashes.

- Sum up the correct number of `blob_gas_used` since this is validated in the execution
  layer. This is basically simulating what the CL computes and sends to EL.

 - Add validation and normalization for Cancun-related fields for blocks
  / block headers and transactions

- Add `y_parity` to non-legacy receipts

- newsfragment for PR #285
@fselmo fselmo deleted the add-cancun-support branch April 8, 2024 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kclowes kclowes kclowes approved these changes

@pacrob pacrob pacrob approved these changes

@reedsa reedsa reedsa approved these changes

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

Successfully merging this pull request may close these issues.
4 participants
@fselmo @reedsa @pacrob @kclowes