Refactor the Environment dataclass #1053
Conversation
The current spec implementaion of the point evatuation pre-compile inttoduced in cancun makes use of eth2spec (consensus-specs). This leads to potential dependency conflicts like when execution-specs wants to use a different version of py_ecc than the consensus-specs. This commit removes the need to depend on eth2spec and implements the necessary components of KZG
Implement EIP-7702
Implement EIP-7610
…ixes Minor Prague t8n fixes
This PR incorporates the following updates to the Prague EIPs EIP-7002: add logging to system contract Update EIP-7002: add logging to system contract EIPs#8890 EIP-7251: max eb, change request to flat encoding Update EIP-7251: change request to flat encoding EIPs#8857 EIP-6110: deposits, change request to flat encoding Update EIP-6110: change request to flat encoding EIPs#8856 EIP-7002: withdrawals, change request to flat encoding Update EIP-7002: change request to flat encoding EIPs#8855 EIP-7685: change requests hash to flat hash Update EIP-7685: change requests hash to flat hash EIPs#8854
Implement Pectra Devnet 4 changes for requests and 7702
Extract t8n arguments from daemon query string
Fix up black and mypy in `fix-t8n-requests`
Encode requests correctly in `t8n`
src/ethereum/cancun/fork.py
Outdated
| if block.ommers != (): | ||
| raise InvalidBlock | ||
|
|
||
| env = vm.Environment( |
There was a problem hiding this comment.
I really like this change.
gurukamath
force-pushed
the
refactor-env
branch
from
b771715 to
c0bd64c
Compare
| @@ -57,6 +70,8 @@ class Message: | |||
| Items that are used by contract creation or message call. | |||
| """ | |||
|
|
|||
| block_env: BlockEnvironment | |||
| tx_env: TransactionEnvironment | |||
There was a problem hiding this comment.
Would it make sense to introduce a third "environment" for the frame?
class BlockEnvironment:
...
class TransactionEnvironment:
...
class FrameEnvironment:
caller: Address
target: Union[Bytes0, Address]
current_target: Address
gas: Uint
value: U256
data: Bytes
code_address: Optional[Address]
code: Bytes
depth: Uint
should_transfer_value: bool
is_static: bool
accessed_addresses: Set[Address]
accessed_storage_keys: Set[Tuple[Address, Bytes32]]
parent_evm: Optional["Evm"]
class Message:
block: BlockEnvironment
transaction: TransactionEnvironment
frame: FrameEnvironmentThere was a problem hiding this comment.
This would line up the nomenclature pretty well but I have been looking at the Message dataclass serving the same purpose as the FrameEnvironment dataclass in your suggestion. I don't have any strong opinions on either choice tbh.
There was a problem hiding this comment.
It would be purely for symmetry and organizational purposes. I can imagine the docs looking like:
An EVM message contains three levels of contextual information: block level, transaction level, and call frame level.
There was a problem hiding this comment.
The symmetry argument sounds good to me but the question largely boils down to complexity since introducing FrameEnvironment could be a large change. Also, I think we can introduce FrameEnvironment in the next iteration to avoid a huge PR
There was a problem hiding this comment.
I think that makes sense.
What was wrong?
The
Environmentdata class has some transaction scope attributes likesender,originand a few others. Ideally, this data class should only have block scope attributes and the transaction scope attributes should be moved to theMessagedata class.This refactor has a few really nice benefits including
apply_bodyfunction (We can now pass just a singleenvparameter instead of a large number of individual attributes that might change with hard forks)t8nHow was it fixed?
Move transaction scope attributes to
MessageCute Animal Picture