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.

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 684: Prevent overwriting contracts #358

Merged
merged 2 commits into from
Aug 16, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,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) 2231/2231 = 100% passing
- [GeneralStateTests](https://github.com/ethereum/tests/tree/develop/GeneralStateTests) 2024/2062 = 98.2% passing
- [x] EIP150
- [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
- [BlockchainTests](https://github.com/ethereum/tests/tree/develop/BlockchainTests) (Includes GeneralStateTests) 1275/1275 = 100% passing
- [GeneralStateTests](https://github.com/ethereum/tests/tree/develop/GeneralStateTests) 1089/1114 = 97.8% passing
- [x] EIP158
- [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
Expand Down
8 changes: 8 additions & 0 deletions apps/blockchain/lib/blockchain/account.ex
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,14 @@ defmodule Blockchain.Account do
end
end

@doc """
Checks that an account is neither nil nor empty
"""
@spec exists?(t() | nil) :: boolean()
def exists?(account) do
!(is_nil(account) || empty?(account))
end

@doc """
Checks if an account is empty.
"""
Expand Down
43 changes: 28 additions & 15 deletions apps/blockchain/lib/blockchain/contract/create_contract.ex
Original file line number Diff line number Diff line change
Expand Up @@ -42,22 +42,23 @@ defmodule Blockchain.Contract.CreateContract do
config: EVM.Configuration.t()
}

# TODO: Block header? "I_H has no special treatment and is determined from the blockchain"
@spec execute(t()) :: {EVM.state(), EVM.Gas.t(), EVM.SubState.t()}
def execute(params) do
sender_account = Account.get_account(params.state, params.sender)
contract_address = Address.new(params.sender, sender_account.nonce)
account = Account.get_account(params.state, contract_address)

if account_exists?(params, contract_address) do
account = Account.get_account(params.state, contract_address)
if Account.exists?(account) do
cond do
account_will_collide?(account) ->
error(params)

# params.stack_depth != 0 means that we're not in contract creation transaction
# because `create` evm instruction should have parameters on the stack that are pushed to it so
# it never is zero
if account.nonce == 0 && Account.is_simple_account?(account) && params.stack_depth != 0 do
{:ok, {params.state, params.available_gas, SubState.empty()}}
else
{:ok, {params.state, 0, SubState.empty()}}
account.nonce == 0 && Account.is_simple_account?(account) &&
not_in_contract_create_transaction?(params) ->
{:ok, {params.state, params.available_gas, SubState.empty()}}

true ->
{:ok, {params.state, 0, SubState.empty()}}
end
else
result = {_, _, _, output} = create(params, contract_address)
Expand All @@ -68,19 +69,31 @@ defmodule Blockchain.Contract.CreateContract do
# valid jump destination or invalid instruction), then no gas
# is refunded to the caller and the state is reverted to the
# point immediately prior to balance transfer.
#
if output == :failed do
{:error, {params.state, 0, SubState.empty()}}
error(params)
else
finalize(result, params, contract_address)
end
end
end

@spec account_exists?(t(), EVM.address()) :: boolean()
defp account_exists?(params, address) do
account = Account.get_account(params.state, address)
@spec not_in_contract_create_transaction?(t) :: boolean()
defp not_in_contract_create_transaction?(params) do
# params.stack_depth != 0 means that we're not in contract creation transaction
# because `create` evm instruction should have parameters on the stack that are pushed to it so
# it never is zero
params.stack_depth != 0
end

@spec account_will_collide?(Account.t()) :: boolean()
defp account_will_collide?(account) do
account.nonce > 0 || !Account.is_simple_account?(account)
end

!(is_nil(account) || Account.empty?(account))
@spec error(t) :: {:error, EVM.state(), 0, SubState.t()}
defp error(params) do
{:error, {params.state, 0, SubState.empty()}}
end

@spec create(t(), EVM.address()) :: {EVM.state(), EVM.Gas.t(), EVM.SubState.t()}
Expand Down
98 changes: 77 additions & 21 deletions apps/blockchain/test/blockchain/contract/create_contract_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ defmodule Blockchain.Contract.CreateContractTest do
alias EVM.{SubState, MachineCode}
alias MerklePatriciaTree.{Trie, DB}

# TODO: Add rich tests for contract creation

setup do
db = MerklePatriciaTree.Test.random_ets_db(:contract_test)
{:ok, %{db: db}}
Expand All @@ -17,24 +15,6 @@ defmodule Blockchain.Contract.CreateContractTest do
test "creates a new contract", %{db: db} do
account = %Account{balance: 11, nonce: 5}

assembly = [
:push1,
3,
:push1,
5,
:add,
:push1,
0x00,
:mstore,
:push1,
32,
:push1,
0,
:return
]

init_code = MachineCode.compile(assembly)

state =
db
|> Trie.new()
Expand All @@ -47,7 +27,7 @@ defmodule Blockchain.Contract.CreateContractTest do
available_gas: 100_000_000,
gas_price: 1,
endowment: 5,
init_code: init_code,
init_code: build_sample_code(),
stack_depth: 5,
block_header: %Block.Header{nonce: 1}
}
Expand Down Expand Up @@ -82,5 +62,81 @@ defmodule Blockchain.Contract.CreateContractTest do
assert Account.get_machine_code(state, contract_address) == {:ok, <<0x08::256>>}
assert state |> Trie.Inspector.all_keys() |> Enum.count() == 2
end

test "does not create contract if address already exists with nonce", %{db: db} do
account = %Account{balance: 11, nonce: 5}
account_address = <<0x10::160>>
collision_account = %Account{nonce: 1}
collision_account_address = Contract.Address.new(account_address, account.nonce)

state =
db
|> Trie.new()
|> Account.put_account(account_address, account)
|> Account.put_account(collision_account_address, collision_account)

params = %Contract.CreateContract{
state: state,
sender: account_address,
originator: account_address,
available_gas: 100_000_000,
gas_price: 1,
endowment: 5,
init_code: build_sample_code(),
stack_depth: 5,
block_header: %Block.Header{nonce: 1}
}

assert {:error, {^state, 0, sub_state}} = Contract.create(params)
assert SubState.empty?(sub_state)
end

test "does not create contract if address has code", %{db: db} do
account = %Account{balance: 11, nonce: 5}
account_address = <<0x10::160>>
collision_account = %Account{code_hash: build_sample_code(), nonce: 0}
collision_account_address = Contract.Address.new(account_address, account.nonce)

state =
db
|> Trie.new()
|> Account.put_account(account_address, account)
|> Account.put_account(collision_account_address, collision_account)

params = %Contract.CreateContract{
state: state,
sender: account_address,
originator: account_address,
available_gas: 100_000_000,
gas_price: 1,
endowment: 5,
init_code: build_sample_code(),
stack_depth: 5,
block_header: %Block.Header{nonce: 1}
}

assert {:error, {^state, 0, sub_state}} = Contract.create(params)
assert SubState.empty?(sub_state)
end
end

defp build_sample_code do
assembly = [
:push1,
3,
:push1,
5,
:add,
:push1,
0x00,
:mstore,
:push1,
32,
:push1,
0,
:return
]

MachineCode.compile(assembly)
end
end
2 changes: 0 additions & 2 deletions apps/blockchain/test/blockchain/state_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ defmodule Blockchain.StateTest do
"stTransactionTest/OverflowGasRequire",
"stTransactionTest/EmptyTransaction",
"stTransactionTest/CreateTransactionReverted",
"stSystemOperationsTest/CreateHashCollision",
"stRevertTest/RevertOpcodeWithBigOutputInInit",
"stRevertTest/RevertOpcodeMultipleSubCalls",
"stRevertTest/RevertOpcodeInInit",
Expand Down Expand Up @@ -53,7 +52,6 @@ defmodule Blockchain.StateTest do
"stSystemOperationsTest/suicideCoinbase",
"stSystemOperationsTest/doubleSelfdestructTest2",
"stSystemOperationsTest/doubleSelfdestructTest",
"stSystemOperationsTest/CreateHashCollision",
"stSpecialTest/tx_e1c174e2",
"stSpecialTest/failed_tx_xcf416c53",
"stRevertTest/RevertPrefoundEmptyCall",
Expand Down
5 changes: 1 addition & 4 deletions apps/blockchain/test/blockchain_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ defmodule BlockchainTest do
@failing_tests %{
"Frontier" => [],
"Homestead" => [],
"EIP150" => [
"GeneralStateTests/stSystemOperationsTest/CreateHashCollision_d0g0v0.json"
],
"EIP150" => [],
"EIP158" => [
"GeneralStateTests/stCallCodes/call_OOG_additionalGasCosts1_d0g0v0.json",
"GeneralStateTests/stCreateTest/CREATE_AcreateB_BSuicide_BStore_d0g0v0.json",
Expand Down Expand Up @@ -65,7 +63,6 @@ defmodule BlockchainTest do
"GeneralStateTests/stRevertTest/RevertPrefound_d0g0v0.json",
"GeneralStateTests/stSpecialTest/failed_tx_xcf416c53_d0g0v0.json",
"GeneralStateTests/stSpecialTest/tx_e1c174e2_d0g0v0.json",
"GeneralStateTests/stSystemOperationsTest/CreateHashCollision_d0g0v0.json",
"GeneralStateTests/stSystemOperationsTest/doubleSelfdestructTest2_d0g0v0.json",
"GeneralStateTests/stSystemOperationsTest/doubleSelfdestructTest_d0g0v0.json",
"GeneralStateTests/stTransactionTest/OverflowGasRequire2_d0g0v0.json",
Expand Down