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

London support #206

Merged
merged 3 commits into from
Nov 3, 2021
Merged

London support #206

merged 3 commits into from
Nov 3, 2021

Conversation

fselmo
Copy link
Contributor

@fselmo fselmo commented Sep 20, 2021 •
edited
Loading

What was wrong?

  • Support for London is needed. This cannot be done until py-evm changes are in [they're in!].
  • Support for access list + access list transactions was needed (Berlin)

How was it fixed?

  • py-evm version upgraded to v0.5.0-alpha.1
  • London support was added
    • Dynamic fee transaction support: type='0x2', max_fee_per_gas, max_priority_fee_per_gas, access_list, y_parity
    • base_fee_per_gas support added to blocks
  • Access list support was added (Berlin)
    • Access list transaction support: type='0x1', access_list, chain_id, y_parity
  • Support for transaction params chain_id, type, y_parity for typed transactions
  • Support for receipt params type, effective_gas_price
  • Tests added

To-Do:

  • Add support for new transaction params (access list, max fees, transaction type, chain id?)
  • Add support for new block field base_fee_per_gas
  • Get tests to pass locally
  • When tests pass, make sure everything is actually implemented appropriately + clean up rough initial commit
  • Validate new fields appropriately
  • Add new tests
  • Add entry to the CHANGELOG
  • Clean up commit history after PR is approved

Cute Animal Picture

IMG_20211019_093537

@fselmo fselmo force-pushed the berlin-and-london-updates branch 2 times, most recently from 47dabd7 to 804bdee Compare September 20, 2021 22:10
@fselmo fselmo force-pushed the berlin-and-london-updates branch 7 times, most recently from 912b2f3 to 0cb25d9 Compare October 8, 2021 00:26
@fselmo fselmo force-pushed the berlin-and-london-updates branch 7 times, most recently from 85a5bf0 to 264d699 Compare October 14, 2021 16:44
@fselmo fselmo changed the title [WIP] London support London support Oct 20, 2021
@fselmo fselmo marked this pull request as ready for review October 21, 2021 15:43
fselmo
fselmo commented Oct 21, 2021
View reviewed changes
eth_tester/backends/pyevm/main.py Outdated Show resolved Hide resolved
eth_tester/utils/backend_testing.py Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
eth_tester/backends/mock/factory.py Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
eth_tester/validation/inbound.py Outdated Show resolved Hide resolved
eth_tester/validation/outbound.py Outdated Show resolved Hide resolved
eth_tester/validation/outbound.py Outdated Show resolved Hide resolved
eth_tester/validation/outbound.py Outdated Show resolved Hide resolved
tests/backends/test_pyevm.py Outdated Show resolved Hide resolved
@fselmo fselmo force-pushed the berlin-and-london-updates branch 6 times, most recently from 56bbf3b to 81062cf Compare October 21, 2021 23:58
wolovim
wolovim approved these changes Nov 3, 2021
View reviewed changes
Copy link
Member

@wolovim wolovim left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Big thanks for the whopper 🍔 of a changeset. Looks good from IRL review, and just did some more smoke tests locally with the relevant branches of py-evm and web3.py. Just wanted to document one UX bit from that testing:

If building a transaction manually and you only include one of the 1559 keys, w3.eth.account.sign_transaction(tx, acct.address) will raise from within eth-account, having decided it's a legacy transaction without a gasPrice.

(w3.eth.send_transaction provides cleaner error messages, e.g., web3.exceptions.InvalidTransaction: maxPriorityFeePerGas must be defined in a 1559 transaction.)

kclowes
kclowes approved these changes Nov 3, 2021
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.

LGTM!! 🎉 🚢 :shipit:

@fselmo
bulk update: London support + access list transaction support (Berlin):
fa99e9d 
- spelling corrections across codebase
- some refactoring for better readability
- access list support for transactions (type = 1 transaction support)
- dynamic fees support for transactions (type = 2 transaction support)
- y_parity support for typed transactions
- base_fee_per_gas support for blocks
- gas_limit calculation for MockBackend (copied from py-evm calculation) :)
- default gas_price should be >= 1000000000, the genesis base fee per gas post-London
- update README.md to reflect new values
- update tests and add more tests for new transaction types and parameters
- cleanups from initial PR review with @kclowes
@fselmo
London support final touches:
8d18f65 
- changes from last PR review with @kclowes
- changes from self PR review comments
- small README fix
- add effective_gas_price and type to transaction receipt object
- add tests for calculating effective_gas_price and checking type in txn receipts
- update README.md example displaying the new fields
- update the effective gas price calculation + validation + transaction param cleanup
- update CHANGELOG with London changes overview
@fselmo fselmo merged commit 4246265 into ethereum:master Nov 3, 2021
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

@wolovim wolovim wolovim 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.
3 participants
@fselmo @wolovim @kclowes