-
Notifications
You must be signed in to change notification settings - Fork 650
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
EIP-1283 #1410
EIP-1283 #1410
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 |
---|---|---|
@@ -0,0 +1,73 @@ | ||
from eth.constants import ( | ||
UINT256 | ||
) | ||
from eth.vm.forks.constantinople import ( | ||
constants | ||
) | ||
|
||
from eth.utils.hexadecimal import ( | ||
encode_hex, | ||
) | ||
|
||
|
||
def sstore_eip1283(computation): | ||
slot, value = computation.stack_pop(num_items=2, type_hint=UINT256) | ||
|
||
current_value = computation.state.account_db.get_storage( | ||
address=computation.msg.storage_address, | ||
slot=slot, | ||
) | ||
|
||
original_value = computation.state.account_db.get_storage( | ||
address=computation.msg.storage_address, | ||
slot=slot, | ||
from_journal=False | ||
) | ||
|
||
gas_refund = 0 | ||
|
||
if current_value == value: | ||
gas_cost = constants.GAS_SSTORE_EIP1283_NOOP | ||
else: | ||
if original_value == current_value: | ||
if original_value == 0: | ||
gas_cost = constants.GAS_SSTORE_EIP1283_INIT | ||
else: | ||
gas_cost = constants.GAS_SSTORE_EIP1283_CLEAN | ||
|
||
if value == 0: | ||
gas_refund += constants.GAS_SSTORE_EIP1283_CLEAR_REFUND | ||
else: | ||
gas_cost = constants.GAS_SSTORE_EIP1283_NOOP | ||
|
||
if original_value != 0: | ||
if current_value == 0: | ||
gas_refund -= constants.GAS_SSTORE_EIP1283_CLEAR_REFUND | ||
if value == 0: | ||
gas_refund += constants.GAS_SSTORE_EIP1283_CLEAR_REFUND | ||
|
||
if original_value == value: | ||
if original_value == 0: | ||
gas_refund += constants.GAS_SSTORE_EIP1283_RESET_CLEAR_REFUND | ||
else: | ||
gas_refund += constants.GAS_SSTORE_EIP1283_RESET_REFUND | ||
|
||
computation.consume_gas( | ||
gas_cost, | ||
reason="SSTORE: {0}[{1}] -> {2} (current: {3} / original: {4})".format( | ||
encode_hex(computation.msg.storage_address), | ||
slot, | ||
value, | ||
current_value, | ||
original_value, | ||
) | ||
) | ||
|
||
if gas_refund: | ||
computation.refund_gas(gas_refund) | ||
|
||
computation.state.account_db.set_storage( | ||
address=computation.msg.storage_address, | ||
slot=slot, | ||
value=value, | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
import logging | ||
from typing import ( | ||
cast | ||
Callable, | ||
cast, | ||
) | ||
from eth_utils import ( | ||
ValidationError, | ||
|
@@ -16,17 +17,35 @@ | |
) | ||
|
||
|
||
def default_refund_strategy(gas_refunded_total: int, amount: int) -> int: | ||
if amount < 0: | ||
raise ValidationError("Gas refund amount must be positive") | ||
|
||
return gas_refunded_total + amount | ||
|
||
|
||
def allow_negative_refund_strategy(gas_refunded_total: int, amount: int) -> int: | ||
return gas_refunded_total + amount | ||
|
||
|
||
RefundStrategy = Callable[[int, int], int] | ||
|
||
|
||
class GasMeter(object): | ||
|
||
start_gas = None # type: int | ||
|
||
gas_refunded = None # type: int | ||
gas_remaining = None # type: int | ||
|
||
logger = cast(TraceLogger, logging.getLogger('eth.gas.GasMeter')) | ||
|
||
def __init__(self, start_gas: int) -> None: | ||
def __init__(self, | ||
start_gas: int, | ||
refund_strategy: RefundStrategy = default_refund_strategy) -> None: | ||
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. @pipermerriam @carver I wanted to avoid 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. Yeah, seems promising, and a reasonable place to experiment with the approach because it's so simple. 👍 |
||
validate_uint256(start_gas, title="Start Gas") | ||
|
||
self.refund_strategy = refund_strategy | ||
self.start_gas = start_gas | ||
|
||
self.gas_remaining = self.start_gas | ||
|
@@ -70,10 +89,7 @@ def return_gas(self, amount: int) -> None: | |
) | ||
|
||
def refund_gas(self, amount: int) -> None: | ||
if amount < 0: | ||
raise ValidationError("Gas refund amount must be positive") | ||
|
||
self.gas_refunded += amount | ||
self.gas_refunded = self.refund_strategy(self.gas_refunded, amount) | ||
|
||
self.logger.trace( | ||
'GAS REFUND: %s + %s -> %s', | ||
|
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.
I moved this out into
constantinople/storage.py
because I strongly feel cluttering our main code base with fork specific logic is wrong. We currently have a bit of a mix (with the majority of fork specific logic being nicely isolated invm/forks/*
) Kinda related to our discussion we had in #1146There 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.
I'm completely onboard with this. I've got a code reorganization building up in my head to move all of the EVM stuff under
eth.evm
the same way the beacon chain stuff is undereth.beacon
, which would also meaning moving a lot of stuff thats ineth.validation
,eth.constants
,eth.rlp
etc around a bit to match.