Skip to content

Commit

Permalink
charge cost for new account only if value is transferred (#356)
Browse files Browse the repository at this point in the history
* change cost for new account only if value is transferred

* check if account is empty in suicide cost calculation

* fix dialyzer

* fix suicide gas cost configuration
  • Loading branch information
ayrat555 authored Aug 16, 2018
1 parent 0559b20 commit 8bf0e7c
Show file tree
Hide file tree
Showing 14 changed files with 114 additions and 152 deletions.
2 changes: 1 addition & 1 deletion .dialyzer.ignore-warnings
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ apps/evm/lib/evm/gas.ex:297: Function operation_cost/2 has no local return
apps/evm/lib/evm/gas.ex:297: 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:297: Function operation_cost/3 has no local return
apps/evm/lib/evm/gas.ex:297: 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:526: Function gas_cost_for_nested_operation/2 will never be called
apps/evm/lib/evm/gas.ex:545: 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
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ Ethereum common tests are created for all clients to test against. We plan to pr
- [BlockchainTests](https://github.com/ethereum/tests/tree/develop/BlockchainTests) (Includes GeneralStateTests) 1274/1275 = 99.9% passing
- [GeneralStateTests](https://github.com/ethereum/tests/tree/develop/GeneralStateTests) 1088/1114 = 97.7% passing
- [x] EIP158
- [BlockchainTests](https://github.com/ethereum/tests/tree/develop/BlockchainTests) (Includes GeneralStateTests) 1108/1233 = 89.9%
- [GeneralStateTests](https://github.com/ethereum/tests/tree/develop/GeneralStateTests) 1052/1181= 89.1% passing
- [BlockchainTests](https://github.com/ethereum/tests/tree/develop/BlockchainTests) (Includes GeneralStateTests) 1167/1233 = 94.6% passing
- [GeneralStateTests](https://github.com/ethereum/tests/tree/develop/GeneralStateTests) 1107/1181= 93.7% passing
- [ ] Byzantium
- [ ] Constantinople: View the community [Constantinople Project Tracker](https://github.com/ethereum/pm/issues/53).
Expand Down
53 changes: 28 additions & 25 deletions apps/blockchain/lib/blockchain/interface/account_interface.ex
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ end
defimpl EVM.Interface.AccountInterface, for: Blockchain.Interface.AccountInterface do
alias MerklePatriciaTree.Trie
alias Blockchain.{Account, Contract}
alias EVM.Interface.AccountInterface

@doc """
Given an account interface and an address, returns the balance at that address.
Expand All @@ -58,25 +59,23 @@ defimpl EVM.Interface.AccountInterface, for: Blockchain.Interface.AccountInterfa
...> |> EVM.Interface.AccountInterface.get_account_balance(<<2::160>>)
nil
"""
@spec get_account_balance(EVM.Interface.AccountInterface.t(), EVM.address()) ::
nil | EVM.Wei.t()
@spec get_account_balance(AccountInterface.t(), EVM.address()) :: nil | EVM.Wei.t()
def get_account_balance(account_interface, address) do
case Account.get_account(account_interface.state, address) do
nil -> nil
account -> account.balance
end
end

@spec add_wei(EVM.Interface.AccountInterface.t(), EVM.address(), integer()) ::
EVM.Interface.AccountInterface.t()
@spec add_wei(AccountInterface.t(), EVM.address(), integer()) :: AccountInterface.t()
def add_wei(account_interface, address, value) do
state = Account.add_wei(account_interface.state, address, value)

Map.put(account_interface, :state, state)
end

@spec transfer(EVM.Interface.AccountInterface.t(), EVM.address(), EVM.address(), integer()) ::
EVM.Interface.AccountInterface.t()
@spec transfer(AccountInterface.t(), EVM.address(), EVM.address(), integer()) ::
AccountInterface.t()
def transfer(account_interface, from, to, value) do
{:ok, state} = Account.transfer(account_interface.state, from, to, value)

Expand Down Expand Up @@ -104,7 +103,7 @@ defimpl EVM.Interface.AccountInterface, for: Blockchain.Interface.AccountInterfa
...> |> EVM.Interface.AccountInterface.get_account_code(<<2::160>>)
<<>>
"""
@spec get_account_code(EVM.Interface.AccountInterface.t(), EVM.address()) :: nil | binary()
@spec get_account_code(AccountInterface.t(), EVM.address()) :: nil | binary()
def get_account_code(account_interface, address) do
case Account.get_machine_code(account_interface.state, address) do
{:ok, machine_code} -> machine_code
Expand All @@ -129,8 +128,8 @@ defimpl EVM.Interface.AccountInterface, for: Blockchain.Interface.AccountInterfa
iex> nonce_2
1
"""
@spec increment_account_nonce(EVM.Interface.AccountInterface.t(), EVM.address()) ::
{EVM.Interface.AccountInterface.t(), integer()}
@spec increment_account_nonce(AccountInterface.t(), EVM.address()) ::
{AccountInterface.t(), integer()}
def increment_account_nonce(account_interface, address) do
{state, before_acct, _after_acct} =
Account.increment_nonce(account_interface.state, address, true)
Expand Down Expand Up @@ -165,19 +164,26 @@ defimpl EVM.Interface.AccountInterface, for: Blockchain.Interface.AccountInterfa
...> |> EVM.Interface.AccountInterface.get_storage(<<2::160>>, 5)
:account_not_found
"""
@spec get_storage(EVM.Interface.AccountInterface.t(), EVM.address(), integer()) ::
@spec get_storage(AccountInterface.t(), EVM.address(), integer()) ::
{:ok, integer()} | :account_not_found | :key_not_found
def get_storage(account_interface, address, key) do
Account.get_storage(account_interface.state, address, key)
end

@spec account_exists?(EVM.Interface.AccountInterface.t(), EVM.address()) :: boolean()
@spec account_exists?(AccountInterface.t(), EVM.address()) :: boolean()
def account_exists?(account_interface, address) do
account = Account.get_account(account_interface.state, address)

!is_nil(account)
end

@spec empty_account?(AccountInterface.t(), EVM.address()) :: boolean()
def empty_account?(account_interface, address) do
account = Account.get_account(account_interface.state, address)

!is_nil(account) && Account.empty?(account)
end

@doc """
Given an account interface, an account address, a key and a value, puts the
value at that key location, overwriting any previous value.
Expand All @@ -191,8 +197,8 @@ defimpl EVM.Interface.AccountInterface, for: Blockchain.Interface.AccountInterfa
...> |> EVM.Interface.AccountInterface.get_storage(<<1::160>>, 5)
:account_not_found
"""
@spec put_storage(EVM.Interface.AccountInterface.t(), EVM.address(), integer(), integer()) ::
EVM.Interface.AccountInterface.t()
@spec put_storage(AccountInterface.t(), EVM.address(), integer(), integer()) ::
AccountInterface.t()
def put_storage(account_interface, address, key, value) do
if Account.get_account(account_interface.state, address) do
updated_state = Account.put_storage(account_interface.state, address, key, value)
Expand All @@ -203,8 +209,7 @@ defimpl EVM.Interface.AccountInterface, for: Blockchain.Interface.AccountInterfa
end
end

@spec remove_storage(EVM.Interface.AccountInterface.t(), EVM.address(), integer()) ::
EVM.Interface.AccountInterface.t()
@spec remove_storage(AccountInterface.t(), EVM.address(), integer()) :: AccountInterface.t()
def remove_storage(account_interface, address, key) do
if Account.get_account(account_interface.state, address) do
updated_state = Account.remove_storage(account_interface.state, address, key)
Expand Down Expand Up @@ -232,7 +237,7 @@ defimpl EVM.Interface.AccountInterface, for: Blockchain.Interface.AccountInterfa
iex> EVM.Interface.AccountInterface.get_account_nonce(account_interface, <<1::160>>)
1
"""
@spec get_account_nonce(EVM.Interface.AccountInterface.t(), EVM.address()) :: integer() | nil
@spec get_account_nonce(AccountInterface.t(), EVM.address()) :: integer() | nil
def get_account_nonce(account_interface, address) do
account = Account.get_account(account_interface.state, address)
if account, do: account.nonce, else: nil
Expand All @@ -251,7 +256,7 @@ defimpl EVM.Interface.AccountInterface, for: Blockchain.Interface.AccountInterfa
...> |> EVM.Interface.AccountInterface.dump_storage()
%{<<5>> => <<6>>}
"""
@spec dump_storage(EVM.Interface.AccountInterface.t()) :: %{EVM.address() => EVM.val()}
@spec dump_storage(AccountInterface.t()) :: %{EVM.address() => EVM.val()}
def dump_storage(account_interface) do
account_interface.state
|> Trie.Inspector.all_values()
Expand All @@ -275,7 +280,7 @@ defimpl EVM.Interface.AccountInterface, for: Blockchain.Interface.AccountInterfa
<<163, 151, 95, 0, 149, 63, 81, 220, 74, 101, 219, 175, 240, 97, 153, 167, 249, 229, 144, 75, 101, 233, 126, 177, 8, 188, 105, 165, 28, 248, 67, 156>>
"""
@spec message_call(
EVM.Interface.AccountInterface.t(),
AccountInterface.t(),
EVM.address(),
EVM.address(),
EVM.address(),
Expand All @@ -287,7 +292,7 @@ defimpl EVM.Interface.AccountInterface, for: Blockchain.Interface.AccountInterfa
binary(),
integer(),
Block.Header.t()
) :: {EVM.Interface.AccountInterface.t(), EVM.Gas.t(), EVM.SubState.t(), EVM.VM.output()}
) :: {AccountInterface.t(), EVM.Gas.t(), EVM.SubState.t(), EVM.VM.output()}
def message_call(
account_interface,
sender,
Expand Down Expand Up @@ -338,7 +343,7 @@ defimpl EVM.Interface.AccountInterface, for: Blockchain.Interface.AccountInterfa
157, 145, 31>>
"""
@spec create_contract(
EVM.Interface.AccountInterface.t(),
AccountInterface.t(),
EVM.address(),
EVM.address(),
EVM.Gas.t(),
Expand All @@ -348,7 +353,7 @@ defimpl EVM.Interface.AccountInterface, for: Blockchain.Interface.AccountInterfa
integer(),
Block.Header.t(),
EVM.Configuration.t()
) :: {:ok | :error, {EVM.Interface.AccountInterface.t(), EVM.Gas.t(), EVM.SubState.t()}}
) :: {:ok | :error, {AccountInterface.t(), EVM.Gas.t(), EVM.SubState.t()}}
def create_contract(
account_interface,
sender,
Expand Down Expand Up @@ -392,14 +397,12 @@ defimpl EVM.Interface.AccountInterface, for: Blockchain.Interface.AccountInterfa
...> |> EVM.Interface.AccountInterface.new_contract_address(<<0x01::160>>, 1)
<<82, 43, 50, 148, 230, 208, 106, 162, 90, 208, 241, 184, 137, 18, 66, 227, 53, 211, 180, 89>>
"""
@spec new_contract_address(EVM.Interface.AccountInterface.t(), EVM.address(), integer()) ::
EVM.address()
@spec new_contract_address(AccountInterface.t(), EVM.address(), integer()) :: EVM.address()
def new_contract_address(_account_interface, address, nonce) do
Contract.Address.new(address, nonce)
end

@spec clear_balance(EVM.Interface.AccountInterface.t(), EVM.address()) ::
EVM.Interface.AccountInterface.t()
@spec clear_balance(AccountInterface.t(), EVM.address()) :: AccountInterface.t()
def clear_balance(account_interface, address) do
state = Account.clear_balance(account_interface.state, address)

Expand Down
50 changes: 1 addition & 49 deletions apps/blockchain/test/blockchain/state_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ defmodule Blockchain.StateTest do
"stZeroCallsTest/ZeroValue_TransactionCALL_ToEmpty",
"stZeroCallsTest/ZeroValue_TransactionCALL",
"stZeroCallsTest/ZeroValue_SUICIDE_ToOneStorageKey",
"stZeroCallsTest/ZeroValue_SUICIDE_ToNonZeroBalance",
"stZeroCallsTest/ZeroValue_SUICIDE_ToEmpty",
"stZeroCallsTest/ZeroValue_SUICIDE",
"stZeroCallsTest/ZeroValue_CALL_ToOneStorageKey",
Expand All @@ -48,46 +47,18 @@ defmodule Blockchain.StateTest do
"stTransactionTest/UserTransactionZeroCostWithData",
"stTransactionTest/UserTransactionZeroCost",
"stTransactionTest/TransactionDataCosts652",
"stTransactionTest/SuicidesStopAfterSuicide",
"stTransactionTest/SuicidesMixingCoinbase",
"stTransactionTest/SuicidesAndSendMoneyToItselfEtherDestroyed",
"stTransactionTest/SuicidesAndInternlCallSuicidesSuccess",
"stTransactionTest/SuicidesAndInternlCallSuicidesOOG",
"stTransactionTest/SuicidesAndInternlCallSuicidesBonusGasAtCallFailed",
"stTransactionTest/SuicidesAndInternlCallSuicidesBonusGasAtCall",
"stTransactionTest/OverflowGasRequire2",
"stTransactionTest/Opcodes_TransactionInit",
"stTransactionTest/EmptyTransaction2",
"stSystemOperationsTest/suicideSendEtherToMe",
"stSystemOperationsTest/suicideSendEtherPostDeath",
"stSystemOperationsTest/suicideOrigin",
"stSystemOperationsTest/suicideNotExistingAccount",
"stSystemOperationsTest/suicideCoinbase",
"stSystemOperationsTest/suicideCallerAddresTooBigRight",
"stSystemOperationsTest/suicideCallerAddresTooBigLeft",
"stSystemOperationsTest/suicideCaller",
"stSystemOperationsTest/suicideAddress",
"stSystemOperationsTest/extcodecopy",
"stSystemOperationsTest/doubleSelfdestructTest2",
"stSystemOperationsTest/doubleSelfdestructTest",
"stSystemOperationsTest/CreateHashCollision",
"stSystemOperationsTest/ABAcallsSuicide1",
"stSystemOperationsTest/ABAcallsSuicide0",
"stSpecialTest/tx_e1c174e2",
"stSpecialTest/failed_tx_xcf416c53",
"stRevertTest/TouchToEmptyAccountRevert3",
"stRevertTest/RevertPrefoundEmptyCall",
"stRevertTest/RevertPrefound",
"stRevertTest/RevertOpcodeMultipleSubCalls",
"stRevertTest/NashatyrevSuicideRevert",
"stRefundTest/refund_singleSuicide",
"stRefundTest/refund_multimpleSuicide",
"stRefundTest/refund_TxToSuicide",
"stRefundTest/refund_CallToSuicideTwice",
"stRefundTest/refund_CallToSuicideStorage",
"stRefundTest/refund_CallToSuicideNoStorage",
"stRefundTest/refundSuicide50procentCap",
"stPreCompiledContracts2/CallSha256_5",
"stPreCompiledContracts2/CallSha256_4_gas99",
"stPreCompiledContracts2/CallSha256_4",
"stPreCompiledContracts2/CallSha256_3_prefix0",
Expand All @@ -96,18 +67,13 @@ defmodule Blockchain.StateTest do
"stPreCompiledContracts2/CallSha256_2",
"stPreCompiledContracts2/CallSha256_1",
"stPreCompiledContracts2/CallSha256_0",
"stPreCompiledContracts2/CallRipemd160_5",
"stPreCompiledContracts2/CallRipemd160_4_gas719",
"stPreCompiledContracts2/CallRipemd160_4",
"stPreCompiledContracts2/CallRipemd160_3_prefixed0",
"stPreCompiledContracts2/CallRipemd160_3_postfixed0",
"stPreCompiledContracts2/CallRipemd160_3",
"stPreCompiledContracts2/CallRipemd160_2",
"stPreCompiledContracts2/CallRipemd160_1",
"stPreCompiledContracts2/CallRipemd160_0",
"stPreCompiledContracts2/CallIdentity_5",
"stPreCompiledContracts2/CallIdentity_4_gas18",
"stPreCompiledContracts2/CallIdentity_4_gas17",
"stPreCompiledContracts2/CallIdentity_4",
"stPreCompiledContracts2/CallIdentity_3",
"stPreCompiledContracts2/CallIdentity_2",
Expand All @@ -126,30 +92,16 @@ defmodule Blockchain.StateTest do
"stPreCompiledContracts2/CallEcrecover0_overlappingInputOutput",
"stPreCompiledContracts2/CallEcrecover0_gas3000",
"stPreCompiledContracts2/CallEcrecover0_completeReturnValue",
"stPreCompiledContracts2/CallEcrecover0_Gas2999",
"stPreCompiledContracts2/CallEcrecover0_0input",
"stPreCompiledContracts2/CallEcrecover0",
"stNonZeroCallsTest/NonZeroValue_SUICIDE_ToOneStorageKey",
"stNonZeroCallsTest/NonZeroValue_SUICIDE_ToNonNonZeroBalance",
"stNonZeroCallsTest/NonZeroValue_SUICIDE_ToEmpty",
"stNonZeroCallsTest/NonZeroValue_SUICIDE",
"stNonZeroCallsTest/NonZeroValue_CALL_ToOneStorageKey",
"stNonZeroCallsTest/NonZeroValue_CALL_ToEmpty",
"stMemExpandingEIP150Calls/NewGasPriceForCodesWithMemExpandingCalls",
"stInitCodeTest/TransactionCreateAutoSuicideContract",
"stInitCodeTest/CallContractToCreateContractOOG",
"stEIP158Specific/vitalikTransactionTest",
"stEIP158Specific/CALL_ZeroVCallSuicide",
"stEIP158Specific/CALL_OneVCallSuicide",
"stCreateTest/CREATE_EContract_ThenCALLToNonExistentAcc",
"stCreateTest/CREATE_ContractSuicideDuringInit_WithValueToItself",
"stCreateTest/CREATE_ContractSuicideDuringInit_WithValue",
"stCreateTest/CREATE_ContractSuicideDuringInit_ThenStoreThenReturn",
"stCreateTest/CREATE_ContractSuicideDuringInit",
"stCreateTest/CREATE_AcreateB_BSuicide_BStore",
"stCallCodes/call_OOG_additionalGasCosts1",
"stAttackTest/CrashingTransaction",
"stAttackTest/ContractCreationSpam"
"stCallCodes/call_OOG_additionalGasCosts1"
],
"Frontier" => [
"stTransactionTest/UserTransactionGasLimitIsTooLowWhenZeroCost",
Expand Down
Loading

0 comments on commit 8bf0e7c

Please sign in to comment.