Skip to content

Refactor Environment#1131

Merged
gurukamath merged 25 commits intoethereum:forks/praguefrom
gurukamath:refactor-env-1
Mar 5, 2025
Merged

Refactor Environment#1131
gurukamath merged 25 commits intoethereum:forks/praguefrom
gurukamath:refactor-env-1

Conversation

@gurukamath
Copy link
Contributor

@gurukamath gurukamath commented Feb 21, 2025

(closes #1130 )
(closes #1115 )
(closes #799)
(closes #993 )

What was wrong?

The Environment data class has some transaction scope attributes like sender, origin and a few others. Ideally, this data class should only have block scope attributes and the transaction scope attributes should be moved to a different data class.

This refactor has a few really nice benefits including

Clear and logical separation between the scopes
Simplifying the signature of a lot of functions including the apply_body function (We can now pass just a single block_env parameter instead of a large number of individual attributes that might change with hard forks)
Simplification of the code in t8n
Simplification of system transaction processing code

How was it fixed?

  • Create BlockEnvironment, TransactionEnvironment and BlockOutput dataclasses
  • Update test running to consume EEST test fixtures as well as ethereum/tests fixtures
  • Update t8n code
  • Update the tracing code

395200232-c06e10e7-093a-4782-a2fd-249e6bb9836e

There are parallels between how the regular block gas is handled and how the blob gas is handled. This commit
refactors blob gas canculation to bring them in line with block gas
Create the BlockEnvironment and TransactionEnvironment dataclasses

BlockEnvironment holds data that is chain or block scoped
TransactionEnvironment holds data that is transaction scoped
Message holds data that is specific to a message call
@gurukamath gurukamath marked this pull request as ready for review February 21, 2025 11:12
@gurukamath gurukamath requested review from SamWilsn and petertdavies and removed request for SamWilsn February 25, 2025 06:46
Copy link
Contributor

@petertdavies petertdavies left a comment

Choose a reason for hiding this comment

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

This is really good! I think you missed some cleanup in your test refactoring, but otherwise I am happy to approve.

The tests from execution_spec_tests should now be a part of EEST
Copy link
Contributor

@SamWilsn SamWilsn left a comment

Choose a reason for hiding this comment

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

Part one of two hundred and three.

Copy link
Contributor

@SamWilsn SamWilsn left a comment

Choose a reason for hiding this comment

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

Note to self, you're done arrow glacier.

Items containing contract creation or message call specific data.
"""
accessed_addresses = set()
accessed_addresses.add(caller)
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, the old code here was pretty misleading.

gas: Uint
access_list_addresses: Set[Address]
access_list_storage_keys: Set[Tuple[Address, Bytes32]]
tx_index: Uint
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this be more descriptive (and still accurate)?

Suggested change
tx_index: Uint
index_in_block: Uint

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure how index_in_block fits in with system transactions processing since there isn't an actual transaction in the block.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, interesting. So system transactions have an index, but do not appear in the list of txs in the block?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually on second thought, we should set the index to None for the system transactions and rename to index_in_block for actual transactions. This is only used in the context of tracing where the index and hash is used as an identifier for the generated trace. I will update this name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@gurukamath gurukamath merged commit 51f2d6a into ethereum:forks/prague Mar 5, 2025
5 checks passed
fselmo pushed a commit to fselmo/execution-specs that referenced this pull request Apr 9, 2025
* refactor blob gas logic

There are parallels between how the regular block gas is handled and how the blob gas is handled. This commit
refactors blob gas canculation to bring them in line with block gas

* create BlockEnvironment and TransactionEnvironment

Create the BlockEnvironment and TransactionEnvironment dataclasses

BlockEnvironment holds data that is chain or block scoped
TransactionEnvironment holds data that is transaction scoped
Message holds data that is specific to a message call

* refactor validate_header

* update t8n

* update evm tracing

* backport changes to cancun

* port to shanghai

* port changes to paris

* port to london

* port berlin

* port istanbul

* port constantinople

* port byzantium

* port spurious_dragon

* port older forks

* fix t8n receipts

* update testing

* process_withdrawals update

* fixes for doc

* remove execution_spec_tests entry

The tests from execution_spec_tests should now be a part of EEST

* use default_factory in BlockOutput

* minor fixes 1

* define current_block_number in block_hash opcode

* rename tx_index to index_in_block

* fix vm test runs
SamWilsn pushed a commit to SamWilsn/eth1.0-specs that referenced this pull request Apr 11, 2025
* refactor blob gas logic

There are parallels between how the regular block gas is handled and how the blob gas is handled. This commit
refactors blob gas canculation to bring them in line with block gas

* create BlockEnvironment and TransactionEnvironment

Create the BlockEnvironment and TransactionEnvironment dataclasses

BlockEnvironment holds data that is chain or block scoped
TransactionEnvironment holds data that is transaction scoped
Message holds data that is specific to a message call

* refactor validate_header

* update t8n

* update evm tracing

* backport changes to cancun

* port to shanghai

* port changes to paris

* port to london

* port berlin

* port istanbul

* port constantinople

* port byzantium

* port spurious_dragon

* port older forks

* fix t8n receipts

* update testing

* process_withdrawals update

* fixes for doc

* remove execution_spec_tests entry

The tests from execution_spec_tests should now be a part of EEST

* use default_factory in BlockOutput

* minor fixes 1

* define current_block_number in block_hash opcode

* rename tx_index to index_in_block

* fix vm test runs
SamWilsn pushed a commit that referenced this pull request May 15, 2025
* refactor blob gas logic

There are parallels between how the regular block gas is handled and how the blob gas is handled. This commit
refactors blob gas canculation to bring them in line with block gas

* create BlockEnvironment and TransactionEnvironment

Create the BlockEnvironment and TransactionEnvironment dataclasses

BlockEnvironment holds data that is chain or block scoped
TransactionEnvironment holds data that is transaction scoped
Message holds data that is specific to a message call

* refactor validate_header

* update t8n

* update evm tracing

* backport changes to cancun

* port to shanghai

* port changes to paris

* port to london

* port berlin

* port istanbul

* port constantinople

* port byzantium

* port spurious_dragon

* port older forks

* fix t8n receipts

* update testing

* process_withdrawals update

* fixes for doc

* remove execution_spec_tests entry

The tests from execution_spec_tests should now be a part of EEST

* use default_factory in BlockOutput

* minor fixes 1

* define current_block_number in block_hash opcode

* rename tx_index to index_in_block

* fix vm test runs
danceratopz pushed a commit to danceratopz/execution-specs that referenced this pull request Oct 22, 2025
* export chain_id in fixtureConfig

* adjust unit tests

* Update src/pytest_plugins/consume/hive_simulators/conftest.py

* fix

* fix: Make fixture readers backwards compatible

---------

Co-authored-by: Mario Vega <marioevz@gmail.com>
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

Comments