-
Notifications
You must be signed in to change notification settings - Fork 471
feat(spec-specs, spec-tests): add EIP-7708 ETH transfers emit a log #2844
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.
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
Changes from 22 commits
e4ee738
580f790
a08c64b
4ccec9a
d5f902c
cb61106
c962c0e
2e94c41
fb5df33
513aaa7
7ae0d53
1f1d757
34f9953
5eaa1ee
0fbab5b
4c40535
a1861be
782b78d
ee16914
70cf0a3
155c3e1
7c84033
8e7231c
d5857ef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| """ | ||
| EIP-7708: ETH transfers and burns emit a log. | ||
|
|
||
| All ETH transfers and burns emit a log. | ||
|
|
||
| https://eips.ethereum.org/EIPS/eip-7708 | ||
| """ | ||
|
|
||
| from ....base_fork import BaseFork | ||
|
|
||
|
|
||
| class EIP7708(BaseFork): | ||
| """EIP-7708 class.""" | ||
|
|
||
| pass |
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -36,8 +36,11 @@ | |||
| calculate_delegation_cost, | ||||
| ) | ||||
| from .. import ( | ||||
| CALL_SUCCESS, | ||||
| Evm, | ||||
| Message, | ||||
| emit_burn_log, | ||||
| emit_transfer_log, | ||||
| incorporate_child_on_error, | ||||
| incorporate_child_on_success, | ||||
| ) | ||||
|
|
@@ -338,7 +341,7 @@ def generic_call( | |||
| else: | ||||
| incorporate_child_on_success(evm, child_evm) | ||||
| evm.return_data = child_evm.output | ||||
| push(evm.stack, U256(1)) | ||||
| push(evm.stack, CALL_SUCCESS) | ||||
|
|
||||
| actual_output_size = min(memory_output_size, U256(len(child_evm.output))) | ||||
| memory_write( | ||||
|
|
@@ -428,6 +431,7 @@ def call(evm: Evm) -> None: | |||
| ) | ||||
| charge_gas(evm, message_call_gas.cost + extend_memory.cost) | ||||
|
|
||||
| # OPERATION | ||||
| evm.memory += b"\x00" * extend_memory.expand_by | ||||
| sender_balance = get_account(tx_state, evm.message.current_target).balance | ||||
| if sender_balance < value: | ||||
|
|
@@ -606,6 +610,15 @@ def selfdestruct(evm: Evm) -> None: | |||
| # Transfer balance | ||||
| move_ether(tx_state, originator, beneficiary, originator_balance) | ||||
|
|
||||
| # EIP-7708: Emit appropriate log based on whether ETH is burned | ||||
| # or transferred to a different account | ||||
| if originator in tx_state.created_accounts and beneficiary == originator: | ||||
| # Self-destruct to self in same tx burns the balance | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar comment to first one, it might be better to just delete the comment since the code itself is readable enough for the reader to infer why is this operation happening. |
||||
| emit_burn_log(evm, originator, originator_balance) | ||||
| elif beneficiary != originator: | ||||
| # Transfer to different beneficiary | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same idea:
Suggested change
|
||||
| emit_transfer_log(evm, originator, beneficiary, originator_balance) | ||||
|
|
||||
| # register account for deletion only if it was created | ||||
| # in the same transaction | ||||
| if originator in tx_state.created_accounts: | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -48,7 +48,7 @@ | |||
| from ..vm.eoa_delegation import get_delegated_code_address, set_delegation | ||||
| from ..vm.gas import GasCosts, charge_gas | ||||
| from ..vm.precompiled_contracts.mapping import PRE_COMPILED_CONTRACTS | ||||
| from . import Evm | ||||
| from . import Evm, emit_transfer_log | ||||
| from .exceptions import ( | ||||
| AddressCollision, | ||||
| ExceptionalHalt, | ||||
|
|
@@ -272,7 +272,13 @@ def process_message(message: Message) -> Evm: | |||
| message.current_target, | ||||
| message.value, | ||||
| ) | ||||
| # EIP-7708: Only emit transfer log to a different account | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same idea, the code is very self-explanatory.
Suggested change
|
||||
| if message.caller != message.current_target: | ||||
| emit_transfer_log( | ||||
| evm, message.caller, message.current_target, message.value | ||||
| ) | ||||
|
|
||||
| # Execute message code and handle errors | ||||
| try: | ||||
| if evm.message.code_address in PRE_COMPILED_CONTRACTS: | ||||
| if not message.disable_precompiles: | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| """Cross-client EIP-7708 Tests.""" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| general/code_coverage/eels = EIP-7708 specific code (emit_transfer_log in vm/__init__.py) has full coverage; check codecov for src/ethereum/forks/amsterdam/vm/__init__.py and src/ethereum/forks/amsterdam/vm/interpreter.py | ||
| general/code_coverage/test_coverage = Run with `--cov` flag; 99% coverage on vm/__init__.py, 92% on interpreter.py | ||
| general/code_coverage/missed_lines = Missed lines are general VM infrastructure (static context errors, gas accounting, error handling) not related to EIP-7708 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| opcode = EIP does not introduce a new opcode | ||
| precompile = EIP does not introduce a new precompile | ||
| removed_precompile = EIP does not remove a precompile | ||
| system_contract = EIP does not introduce a new system contract | ||
| transaction_type = EIP does not introduce a new transaction type | ||
| block_header_field = EIP does not add any new block header fields | ||
| block_body_field = EIP does not add any new block body fields | ||
| gas_cost_changes = EIP does not introduce any gas cost changes | ||
| gas_refunds_changes = EIP does not introduce any gas refund changes | ||
| blob_count_changes = EIP does not introduce any blob count changes | ||
| execution_layer_request = EIP does not introduce an execution layer request | ||
| new_transaction_validity_constraint = EIP does not introduce a new transaction validity constraint | ||
| modified_transaction_validity_constraint = EIP does not introduce a modified transaction validity constraint | ||
| block_level_constraint = EIP does not introduce a block-level validation constraint |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| """Defines EIP-7708 specification constants and functions.""" | ||
|
|
||
| from dataclasses import dataclass | ||
|
|
||
| from execution_testing import Address, Bytes, Hash, TransactionLog, keccak256 | ||
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class ReferenceSpec: | ||
| """Defines the reference spec version and git path.""" | ||
|
|
||
| git_path: str | ||
| version: str | ||
|
|
||
|
|
||
| ref_spec_7708 = ReferenceSpec( | ||
| "EIPS/eip-7708.md", "172188d7b090ed1afb876140f45e19ac00cba4bb" | ||
|
danceratopz marked this conversation as resolved.
Outdated
|
||
| ) | ||
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class Spec: | ||
| """ | ||
| Parameters from the EIP-7708 specifications as defined at | ||
| https://eips.ethereum.org/EIPS/eip-7708. | ||
| """ | ||
|
|
||
| SYSTEM_ADDRESS: Address = Address( | ||
| 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFE | ||
| ) | ||
| TRANSFER_TOPIC: Hash = Hash( | ||
| keccak256(b"Transfer(address,address,uint256)") | ||
| ) | ||
| BURN_TOPIC: Hash = Hash(keccak256(b"Burn(address,uint256)")) | ||
|
|
||
|
|
||
| def transfer_log( | ||
| sender: Address, recipient: Address, amount: int | ||
| ) -> TransactionLog: | ||
| """Create an expected Transfer log for EIP-7708.""" | ||
| return TransactionLog( | ||
| address=Spec.SYSTEM_ADDRESS, | ||
| topics=[ | ||
| Spec.TRANSFER_TOPIC, | ||
| Hash(bytes(sender).rjust(32, b"\x00")), | ||
| Hash(bytes(recipient).rjust(32, b"\x00")), | ||
| ], | ||
| data=Bytes(amount.to_bytes(32, "big")), | ||
| ) | ||
|
|
||
|
|
||
| def burn_log(contract_address: Address, amount: int | None) -> TransactionLog: | ||
| """Create an expected Burn log for EIP-7708.""" | ||
| return TransactionLog( | ||
| address=Spec.SYSTEM_ADDRESS, | ||
| topics=[ | ||
| Spec.BURN_TOPIC, | ||
| Hash(bytes(contract_address).rjust(32, b"\x00")), | ||
| ], | ||
| data=Bytes(amount.to_bytes(32, "big")) if amount is not None else None, | ||
| ) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't seem like we mention EIP numbers in comments anywhere else in the spec, and the comment itself seems too verbose when compared to the rest of the code in this file.
Perhaps something simpler like: