Skip to content
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.

Sign up for GitHub

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 #433

Merged
merged 6 commits into from
Sep 20, 2018
Merged

EIP-1283 #433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
288a161
add simple storage cache
ayrat555 Sep 19, 2018
e6c3033
work only with storage cache in evm
ayrat555 Sep 20, 2018
fe966b5
fix cache in recursive operations
ayrat555 Sep 20, 2018
2e8ad9d
add eip-1283 sstore gas cost changes
ayrat555 Sep 20, 2018
07fe295
refactor eip1283 gas cost
ayrat555 Sep 20, 2018
514a6df
move sstore refunds logic to separate module
ayrat555 Sep 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .dialyzer.ignore-warnings
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ apps/evm/lib/evm/gas.ex:308: Function operation_cost/2 has no local return
apps/evm/lib/evm/gas.ex:308: The call 'Elixir.EVM.Gas':operation_cost(__@1::any(),__@2::any(),'nil','nil') breaks the contract (atom(),['Elixir.EVM':val()],['Elixir.EVM':val()],'Elixir.EVM.MachineState':t()) -> t() | 'nil'
apps/evm/lib/evm/gas.ex:308: Function operation_cost/3 has no local return
apps/evm/lib/evm/gas.ex:308: The call 'Elixir.EVM.Gas':operation_cost(__@1::any(),__@2::any(),__@3::any(),'nil') breaks the contract (atom(),['Elixir.EVM':val()],['Elixir.EVM':val()],'Elixir.EVM.MachineState':t()) -> t() | 'nil'
apps/evm/lib/evm/gas.ex:573: Function gas_cost_for_nested_operation/2 will never be called
apps/evm/lib/evm/gas.ex:577: Function gas_cost_for_nested_operation/2 will never be called
apps/evm/lib/evm/machine_state.ex:53: Function subtract_gas/2 has no local return
apps/evm/lib/evm/operation/environmental_information.ex:114: Function calldataload/2 has no local return
apps/evm/lib/evm/operation/environmental_information.ex:117: The call 'Elixir.EVM.Helpers':decode_signed(binary()) breaks the contract (integer() | 'nil') -> 'Elixir.EVM':val() | 'nil'
Expand Down
37 changes: 20 additions & 17 deletions apps/blockchain/lib/blockchain/contract/create_contract.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ defmodule Blockchain.Contract.CreateContract do
alias Blockchain.Account
alias EVM.{SubState, Gas}

defstruct state: %{},
defstruct account_interface: %AccountInterface{},
sender: <<>>,
originator: <<>>,
available_gas: 0,
Expand All @@ -34,7 +34,7 @@ defmodule Blockchain.Contract.CreateContract do
- e: stack_depth
"""
@type t :: %__MODULE__{
state: EVM.state(),
account_interface: AccountInterface.t(),
sender: EVM.address(),
originator: EVM.address(),
available_gas: EVM.Gas.t(),
Expand All @@ -49,23 +49,24 @@ defmodule Blockchain.Contract.CreateContract do

@spec execute(t()) :: {:ok | :error, {EVM.state(), EVM.Gas.t(), EVM.SubState.t()}}
def execute(params) do
original_state = params.account_interface.state
contract_address = new_account_address(params)
account = Account.get_account(params.state, contract_address)
account = Account.get_account(original_state, contract_address)

if Account.exists?(account) do
cond do
account_will_collide?(account) ->
error(params.state)
error(original_state)

account.nonce == 0 && Account.is_simple_account?(account) &&
not_in_contract_create_transaction?(params) ->
new_state =
increment_nonce_of_touched_account(params.state, params.config, contract_address)
increment_nonce_of_touched_account(original_state, params.config, contract_address)

{:ok, {new_state, params.available_gas, SubState.empty()}}

true ->
{:ok, {params.state, 0, SubState.empty()}}
{:ok, {original_state, 0, SubState.empty()}}
end
else
result = {rem_gas, _, _, output} = create(params, contract_address)
Expand All @@ -78,8 +79,8 @@ defmodule Blockchain.Contract.CreateContract do
# point immediately prior to balance transfer.
#
case output do
:failed -> error(params.state)
{:revert, _} -> {:error, {params.state, rem_gas, SubState.empty()}}
:failed -> error(original_state)
{:revert, _} -> {:error, {original_state, rem_gas, SubState.empty()}}
_ -> finalize(result, params, contract_address)
end
end
Expand Down Expand Up @@ -120,7 +121,8 @@ defmodule Blockchain.Contract.CreateContract do
|> init_blank_account(address)
|> increment_nonce_of_touched_account(params.config, address)

account_interface = AccountInterface.new(state_with_blank_contract)
account_interface =
AccountInterface.new(state_with_blank_contract, params.account_interface.cache)

# Create an execution environment for a create contract call.
# This is defined in Eq.(88), Eq.(89), Eq.(90), Eq.(91), Eq.(92),
Expand All @@ -144,7 +146,7 @@ defmodule Blockchain.Contract.CreateContract do

@spec init_blank_account(t, EVM.address()) :: EVM.state()
defp init_blank_account(params, address) do
params.state
params.account_interface.state
|> Account.put_account(address, %Account{nonce: 0})
|> Account.transfer!(params.sender, address, params.endowment)
end
Expand All @@ -155,19 +157,20 @@ defmodule Blockchain.Contract.CreateContract do
EVM.address()
) :: {:ok | :error, {EVM.state(), EVM.Gas.t(), EVM.SubState.t()}}
defp finalize({remaining_gas, accrued_sub_state, exec_env, output}, params, address) do
state_after_init = exec_env.account_interface.state

original_state = params.account_interface.state
contract_creation_cost = creation_cost(output)
insufficient_gas = remaining_gas < contract_creation_cost

cond do
insufficient_gas && EVM.Configuration.fail_contract_creation_lack_of_gas?(params.config) ->
{:error, {params.state, 0, SubState.empty()}}
{:error, {original_state, 0, SubState.empty()}}

EVM.Configuration.limit_contract_code_size?(params.config, byte_size(output)) ->
{:error, {params.state, 0, SubState.empty()}}
{:error, {original_state, 0, SubState.empty()}}

true ->
commited_state = AccountInterface.commit_storage(exec_env.account_interface)

resultant_gas =
if insufficient_gas do
remaining_gas
Expand All @@ -177,9 +180,9 @@ defmodule Blockchain.Contract.CreateContract do

resultant_state =
if insufficient_gas do
state_after_init
commited_state
else
Account.put_code(state_after_init, address, output)
Account.put_code(commited_state, address, output)
end

sub_state = SubState.add_touched_account(accrued_sub_state, address)
Expand All @@ -204,7 +207,7 @@ defmodule Blockchain.Contract.CreateContract do
if params.new_account_address do
params.new_account_address
else
sender_account = Account.get_account(params.state, params.sender)
sender_account = Account.get_account(params.account_interface.state, params.sender)
Account.Address.new(params.sender, sender_account.nonce)
end
end
Expand Down
20 changes: 11 additions & 9 deletions apps/blockchain/lib/blockchain/contract/message_call.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ defmodule Blockchain.Contract.MessageCall do
alias EVM.SubState
alias EVM.MessageCall

defstruct state: %{},
defstruct account_interface: %AccountInterface{},
sender: <<>>,
originator: <<>>,
recipient: <<>>,
Expand Down Expand Up @@ -39,7 +39,7 @@ defmodule Blockchain.Contract.MessageCall do
e: stack_depth
"""
@type t :: %__MODULE__{
state: EVM.state(),
account_interface: AccountInterface.t(),
sender: EVM.address(),
originator: EVM.address(),
recipient: EVM.address(),
Expand All @@ -66,17 +66,18 @@ defmodule Blockchain.Contract.MessageCall do
@spec execute(t()) ::
{:ok | :error, {EVM.state(), EVM.Gas.t(), EVM.SubState.t(), EVM.VM.output()}}
def execute(params) do
original_state = params.account_interface.state
run = MessageCall.get_run_function(params.recipient, params.config)

# Note, this could fail if machine code is not in state
{:ok, machine_code} = Account.get_machine_code(params.state, params.contract)
{:ok, machine_code} = Account.get_machine_code(original_state, params.contract)

# Initiates message call by transfering balance from sender to receiver.
# This covers Eq.(101), Eq.(102), Eq.(103) and Eq.(104) of the Yellow Paper.
# TODO: make copy of original state or use cache for making changes
state = Account.transfer!(params.state, params.sender, params.recipient, params.value)
state = Account.transfer!(original_state, params.sender, params.recipient, params.value)

account_interace = AccountInterface.new(state)
account_interface = AccountInterface.new(state, params.account_interface.cache)

# Create an execution environment for a message call.
# This is defined in Eq.(107), Eq.(108), Eq.(109), Eq.(110),
Expand All @@ -91,7 +92,7 @@ defmodule Blockchain.Contract.MessageCall do
machine_code: machine_code,
stack_depth: params.stack_depth,
block_interface: BlockInterface.new(params.block_header, state.db),
account_interface: account_interace,
account_interface: account_interface,
config: params.config
}

Expand All @@ -106,13 +107,14 @@ defmodule Blockchain.Contract.MessageCall do
# point immediately prior to balance transfer.
case output do
:failed ->
{:error, {params.state, 0, SubState.empty(), :failed}}
{:error, {original_state, 0, SubState.empty(), :failed}}

{:revert, _output} ->
{:error, {params.state, gas, SubState.empty(), :failed}}
{:error, {original_state, gas, SubState.empty(), :failed}}

_ ->
{:ok, {exec_env.account_interface.state, gas, sub_state, output}}
commited_state = AccountInterface.commit_storage(exec_env.account_interface)
{:ok, {commited_state, gas, sub_state, output}}
end
end
end
68 changes: 55 additions & 13 deletions apps/blockchain/lib/blockchain/interface/account_interface.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@ defmodule Blockchain.Interface.AccountInterface do
@moduledoc """
Defines an interface for methods to interact with contracts and accounts.
"""
alias Blockchain.Interface.AccountInterface.Cache
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 I like that this is in its own module

alias Blockchain.Account

@type t :: %__MODULE__{
state: EVM.state()
state: EVM.state(),
cache: Cache.t()
}

defstruct [
:state
:state,
:cache
]

@doc """
Expand All @@ -22,21 +26,40 @@ defmodule Blockchain.Interface.AccountInterface do
state: %MerklePatriciaTree.Trie{
db: { MerklePatriciaTree.DB.ETS, :account_interface_new },
root_hash: <<86, 232, 31, 23, 27, 204, 85, 166, 255, 131, 69, 230, 146, 192, 248, 110, 91, 72, 224, 27, 153, 108, 173, 192, 1, 98, 47, 181, 227, 99, 180, 33>>
}
},
cache: %Blockchain.Interface.AccountInterface.Cache{cache: %{}}
}
"""
@spec new(EVM.state()) :: t
def new(state) do
@spec new(EVM.state(), Cache.t()) :: t
def new(state, cache \\ %Cache{}) do
%__MODULE__{
state: state
state: state,
cache: cache
}
end

@spec commit_storage(t()) :: EVM.state()
def commit_storage(account_interface) do
account_interface.cache
|> Cache.to_list()
|> Enum.reduce(account_interface.state, fn {address, account_cache}, acc_state ->
account_cache
|> Map.to_list()
|> Enum.reduce(acc_state, fn {key, key_cache}, key_acc_state ->
Copy link
Contributor

Choose a reason for hiding this comment

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

This Enum.reduce within another Enum.reduce makes for complicated logic. What do you think about extracting this into a function? I think once it is extracted, we may even realize we can put part of this logic into the Cache module (though that's just a hunch, I may be wrong).

case Map.get(key_cache, :current_value) do
:deleted -> Account.remove_storage(key_acc_state, address, key)
value -> Account.put_storage(key_acc_state, address, key, value)
end
end)
end)
end
end

defimpl EVM.Interface.AccountInterface, for: Blockchain.Interface.AccountInterface do
alias MerklePatriciaTree.Trie
alias Blockchain.{Account, Contract}
alias EVM.Interface.AccountInterface
alias Blockchain.Interface.AccountInterface.Cache

@doc """
Given an account interface and an address, returns the balance at that address.
Expand Down Expand Up @@ -185,7 +208,26 @@ defimpl EVM.Interface.AccountInterface, for: Blockchain.Interface.AccountInterfa
{:ok, integer()} | :account_not_found | :key_not_found
def get_storage(account_interface, evm_address, key) do
address = Account.Address.from(evm_address)
Account.get_storage(account_interface.state, address, key)
cached_value = Cache.get_current_value(account_interface.cache, address, key)

case cached_value do
nil -> Account.get_storage(account_interface.state, address, key)
:deleted -> :key_not_found
Copy link
Contributor

Choose a reason for hiding this comment

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

We're returning :key_not_found to match the behavior of the Account.get_storage when a key isn't found, right? If so, I wonder if there's a better way to consolidate the knowledge of :key_not_found somewhere else? For example, maybe the Account module itself makes use of the cache if it finds one?

_ -> {:ok, cached_value}
end
end

@spec get_initial_storage(AccountInterface.t(), EVM.address(), integer()) ::
{:ok, integer()} | :account_not_found | :key_not_found
def get_initial_storage(account_interface, evm_address, key) do
address = Account.Address.from(evm_address)
cached_value = Cache.get_initial_value(account_interface.cache, address, key)

case cached_value do
nil -> Account.get_storage(account_interface.state, address, key)
:deleted -> :key_not_found
_ -> {:ok, cached_value}
end
end

@spec account_exists?(AccountInterface.t(), EVM.address()) :: boolean()
Expand Down Expand Up @@ -223,9 +265,9 @@ defimpl EVM.Interface.AccountInterface, for: Blockchain.Interface.AccountInterfa
address = Account.Address.from(evm_address)

if Account.get_account(account_interface.state, address) do
updated_state = Account.put_storage(account_interface.state, address, key, value)
updated_cache = Cache.update_current_value(account_interface.cache, address, key, value)

%{account_interface | state: updated_state}
%{account_interface | cache: updated_cache}
else
account_interface
end
Expand All @@ -236,9 +278,9 @@ defimpl EVM.Interface.AccountInterface, for: Blockchain.Interface.AccountInterfa
address = Account.Address.from(evm_address)

if Account.get_account(account_interface.state, address) do
updated_state = Account.remove_storage(account_interface.state, address, key)
updated_cache = Cache.remove_current_value(account_interface.cache, address, key)

%{account_interface | state: updated_state}
%{account_interface | cache: updated_cache}
else
account_interface
end
Expand Down Expand Up @@ -338,7 +380,7 @@ defimpl EVM.Interface.AccountInterface, for: Blockchain.Interface.AccountInterfa
contract = Account.Address.from(evm_contract)

params = %Contract.MessageCall{
state: account_interface.state,
account_interface: account_interface,
sender: sender,
originator: originator,
recipient: recipient,
Expand Down Expand Up @@ -402,7 +444,7 @@ defimpl EVM.Interface.AccountInterface, for: Blockchain.Interface.AccountInterfa
originator = Account.Address.from(evm_originator)

params = %Contract.CreateContract{
state: account_interface.state,
account_interface: account_interface,
sender: sender,
originator: originator,
available_gas: available_gas,
Expand Down
Loading