-
Notifications
You must be signed in to change notification settings - Fork 310
Implement EIP-5920 #1225
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
base: devnets/osaka/0
Are you sure you want to change the base?
Implement EIP-5920 #1225
Changes from all commits
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 |
---|---|---|
|
@@ -22,6 +22,9 @@ | |
from ethereum.utils.byte import left_pad_zero_bytes | ||
|
||
from ..fork_types import Address | ||
from ..vm.exceptions import InvalidParameter | ||
|
||
MAX_ADDRESS = Address(b"\xff" * 20) | ||
|
||
|
||
def to_address(data: Union[Uint, U256]) -> Address: | ||
gurukamath marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
@@ -41,6 +44,31 @@ def to_address(data: Union[Uint, U256]) -> Address: | |
return Address(data.to_be_bytes32()[-20:]) | ||
|
||
|
||
def to_address_unmasked(data: U256) -> Address: | ||
""" | ||
Convert a U256 value to a valid address (20 bytes), | ||
raising an error if the input is too large to fit. | ||
|
||
Parameters | ||
---------- | ||
data : | ||
The string to be converted to bytes. | ||
|
||
Raises | ||
------ | ||
InvalidParameter | ||
If `data` is larger than `MAX_ADDRESS`. | ||
|
||
Returns | ||
------- | ||
address : `Address` | ||
The obtained address. | ||
""" | ||
if data > U256.from_be_bytes(MAX_ADDRESS): | ||
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. And then query the |
||
raise InvalidParameter("Address is too large") | ||
return to_address(data) | ||
|
||
|
||
def compute_contract_address(address: Address, nonce: Uint) -> Address: | ||
""" | ||
Computes address of the new account that needs to be created. | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -31,6 +31,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||
compute_contract_address, | ||||||||||||||||||||||||||||||||||||||||||||||||||
compute_create2_contract_address, | ||||||||||||||||||||||||||||||||||||||||||||||||||
to_address, | ||||||||||||||||||||||||||||||||||||||||||||||||||
to_address_unmasked, | ||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||
from ...vm.eoa_delegation import access_delegation | ||||||||||||||||||||||||||||||||||||||||||||||||||
from .. import ( | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -742,3 +743,50 @@ def revert(evm: Evm) -> None: | |||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
# PROGRAM COUNTER | ||||||||||||||||||||||||||||||||||||||||||||||||||
pass | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
def pay(evm: Evm) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||
Transfer ether to an account without executing its code. | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
Parameters | ||||||||||||||||||||||||||||||||||||||||||||||||||
---------- | ||||||||||||||||||||||||||||||||||||||||||||||||||
evm : | ||||||||||||||||||||||||||||||||||||||||||||||||||
The current EVM frame. | ||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||
# STACK | ||||||||||||||||||||||||||||||||||||||||||||||||||
to = to_address_unmasked(pop(evm.stack)) | ||||||||||||||||||||||||||||||||||||||||||||||||||
value = pop(evm.stack) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
# GAS | ||||||||||||||||||||||||||||||||||||||||||||||||||
if to in evm.accessed_addresses: | ||||||||||||||||||||||||||||||||||||||||||||||||||
access_gas_cost = GAS_WARM_ACCESS | ||||||||||||||||||||||||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||||||||||||||||||||||||
evm.accessed_addresses.add(to) | ||||||||||||||||||||||||||||||||||||||||||||||||||
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. If the method encounters an error later, is the 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. It will continue to be a part of the warm addresses in the current execution frame but will not be added to the warm addresses of the parent frame (contrast If we are in the initial frame, the warmed addresses are not explicitly removed. However, since an error is basically the end of the execution in that case, the warmed addresses play not further role here either. |
||||||||||||||||||||||||||||||||||||||||||||||||||
access_gas_cost = GAS_COLD_ACCOUNT_ACCESS | ||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+762
to
+766
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.
Suggested change
Personally I find this more readable, but I'm weird. 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. This is just the way we define |
||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
create_gas_cost = GAS_NEW_ACCOUNT | ||||||||||||||||||||||||||||||||||||||||||||||||||
if value == 0 or is_account_alive(evm.message.block_env.state, to): | ||||||||||||||||||||||||||||||||||||||||||||||||||
create_gas_cost = Uint(0) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
transfer_gas_cost = Uint(0) if value == U256(0) else GAS_CALL_VALUE | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
charge_gas(evm, access_gas_cost + create_gas_cost + transfer_gas_cost) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
# OPERATION | ||||||||||||||||||||||||||||||||||||||||||||||||||
if evm.message.is_static: | ||||||||||||||||||||||||||||||||||||||||||||||||||
raise WriteInStaticContext("Cannot PAY in static context") | ||||||||||||||||||||||||||||||||||||||||||||||||||
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. In the spec, this check is done before any other operation (so also before popping any value from stack). I don't think moving this around in this method would yield a different outcome, but it would match the spec better to do this check prior to anything else. So I would suggest moving this check up. 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. Yes. This does not make a difference to the outcome since all the gas in consumed and warmed addresses are discarded upon error. However, I implemented it in this order since that makes it consistent with the implementation of |
||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
try: | ||||||||||||||||||||||||||||||||||||||||||||||||||
move_ether( | ||||||||||||||||||||||||||||||||||||||||||||||||||
evm.message.block_env.state, | ||||||||||||||||||||||||||||||||||||||||||||||||||
evm.message.current_target, | ||||||||||||||||||||||||||||||||||||||||||||||||||
to, | ||||||||||||||||||||||||||||||||||||||||||||||||||
value, | ||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||
push(evm.stack, U256(1)) | ||||||||||||||||||||||||||||||||||||||||||||||||||
except AssertionError: | ||||||||||||||||||||||||||||||||||||||||||||||||||
push(evm.stack, U256(0)) | ||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+780
to
+789
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. We should not be using exception handling.
Suggested change
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. I would have also prefered to not catch a general |
||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
# PROGRAM COUNTER | ||||||||||||||||||||||||||||||||||||||||||||||||||
evm.pc += Uint(1) |
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.
As optimization you could add
MAX_ADDRESS_UINT256
here as a constant