From 1d8fe9b519dea1aa206661ca32a88fd8a4ec7a9f Mon Sep 17 00:00:00 2001 From: Ayrat Badykov Date: Fri, 12 Oct 2018 13:57:19 +0300 Subject: [PATCH 1/4] fix writing to memory --- apps/evm/lib/evm/memory.ex | 18 +++++++++++------- apps/evm/lib/evm/message_call.ex | 2 +- apps/evm/test/evm/memory_test.exs | 30 ++++++++++++++++++++++++++---- 3 files changed, 38 insertions(+), 12 deletions(-) diff --git a/apps/evm/lib/evm/memory.ex b/apps/evm/lib/evm/memory.ex index d48b9ee2a..8af67e5c2 100644 --- a/apps/evm/lib/evm/memory.ex +++ b/apps/evm/lib/evm/memory.ex @@ -79,13 +79,17 @@ defmodule EVM.Memory do def write(machine_state = %MachineState{}, offset_bytes, original_data, size) do data = - if size do - original_data - |> :binary.decode_unsigned() - |> rem(size * EVM.word_size()) - |> :binary.encode_unsigned() - else - original_data + cond do + is_nil(size) -> + original_data + + size > byte_size(original_data) -> + original_data + + true -> + <> = original_data + + data end memory_size = byte_size(machine_state.memory) diff --git a/apps/evm/lib/evm/message_call.ex b/apps/evm/lib/evm/message_call.ex index cb7d09182..3635b59fd 100644 --- a/apps/evm/lib/evm/message_call.ex +++ b/apps/evm/lib/evm/message_call.ex @@ -200,7 +200,7 @@ defmodule EVM.MessageCall do %{machine_state | last_return_data: output} true -> - machine_state = Memory.write(machine_state, out_offset, output) + machine_state = Memory.write(machine_state, out_offset, output, out_size) %{machine_state | last_return_data: output} end diff --git a/apps/evm/test/evm/memory_test.exs b/apps/evm/test/evm/memory_test.exs index 388ff6880..9851e8eda 100644 --- a/apps/evm/test/evm/memory_test.exs +++ b/apps/evm/test/evm/memory_test.exs @@ -3,9 +3,31 @@ defmodule EVM.MemoryTest do use ExUnit.Case, async: true doctest EVM.Memory - test "returns the machine_state unchanged if data size is zero" do - machine_state = %MachineState{} - updated_machine_state = Memory.write(machine_state, 256, <<>>, 0) - assert machine_state == updated_machine_state + describe "write/4" do + test "returns the machine_state unchanged if data size is zero" do + machine_state = %MachineState{} + updated_machine_state = Memory.write(machine_state, 256, <<>>, 0) + assert machine_state == updated_machine_state + end + + test "truncates input if the size parameter is smaller than input's size" do + machine_state = %MachineState{} + input = Enum.reduce(1..60, <<>>, fn i, acc -> <> <> acc end) + + updated_machine_state = Memory.write(machine_state, 0, input, 2) + + assert updated_machine_state.memory == <<60, 59>> + assert updated_machine_state.active_words == 1 + end + + test "does not truncate input if the size parameter is bigger than input's size" do + machine_state = %MachineState{} + input = Enum.reduce(1..60, <<>>, fn i, acc -> <> <> acc end) + + updated_machine_state = Memory.write(machine_state, 0, input, 61) + + assert updated_machine_state.memory == input + assert updated_machine_state.active_words == 2 + end end end From cc9d770bd3715eae82c06385caedef85808957f4 Mon Sep 17 00:00:00 2001 From: Ayrat Badykov Date: Fri, 12 Oct 2018 16:01:37 +0300 Subject: [PATCH 2/4] fix mstore8, add tests for truncated message call output --- .../blockchain/test/blockchain/state_test.exs | 17 +++------ .../stack_memory_storage_and_flow.ex | 7 +++- apps/evm/test/evm/message_call_test.exs | 38 ++++++++++++++++++- 3 files changed, 47 insertions(+), 15 deletions(-) diff --git a/apps/blockchain/test/blockchain/state_test.exs b/apps/blockchain/test/blockchain/state_test.exs index 618d02e22..c1210724b 100644 --- a/apps/blockchain/test/blockchain/state_test.exs +++ b/apps/blockchain/test/blockchain/state_test.exs @@ -21,14 +21,11 @@ defmodule Blockchain.StateTest do "stReturnDataTest/returndatacopy_following_call", "stReturnDataTest/returndatacopy_following_revert", "stReturnDataTest/returndatacopy_following_revert_in_create", - "stRevertTest/PythonRevertTestTue201814-1430", "stRevertTest/RevertInCallCode", "stRevertTest/RevertInCreateInInit", "stRevertTest/RevertInDelegateCall", "stRevertTest/RevertOpcodeInCreateReturns", - "stRevertTest/RevertOpcodeMultipleSubCalls", - "stSpecialTest/failed_tx_xcf416c53", - "stStaticCall/static_Call1024PreCalls2" + "stSpecialTest/failed_tx_xcf416c53" ], "Constantinople" => [ "stCreate2/CREATE2_ContractSuicideDuringInit_ThenStoreThenReturn", @@ -58,16 +55,12 @@ defmodule Blockchain.StateTest do "stCreate2/returndatacopy_0_0_following_successful_create", "stCreate2/returndatacopy_afterFailing_create", "stCreate2/returndatacopy_following_revert_in_create", - "stCreate2/returndatasize_following_successful_create", - "stRevertTest/RevertOpcodeMultipleSubCalls" + "stCreate2/returndatasize_following_successful_create" ], "Frontier" => [], - "Homestead" => ["stRevertTest/RevertOpcodeMultipleSubCalls"], - "SpuriousDragon" => [ - "stRevertTest/RevertOpcodeMultipleSubCalls", - "stSpecialTest/failed_tx_xcf416c53" - ], - "TangerineWhistle" => ["stRevertTest/RevertOpcodeMultipleSubCalls"] + "Homestead" => [], + "SpuriousDragon" => ["stSpecialTest/failed_tx_xcf416c53"], + "TangerineWhistle" => [] } @fifteen_minutes 1000 * 60 * 15 diff --git a/apps/evm/lib/evm/operation/stack_memory_storage_and_flow.ex b/apps/evm/lib/evm/operation/stack_memory_storage_and_flow.ex index 0e245d69d..7fb6e5f17 100644 --- a/apps/evm/lib/evm/operation/stack_memory_storage_and_flow.ex +++ b/apps/evm/lib/evm/operation/stack_memory_storage_and_flow.ex @@ -65,7 +65,12 @@ defmodule EVM.Operation.StackMemoryStorageAndFlow do """ @spec mstore8(Operation.stack_args(), Operation.vm_map()) :: Operation.op_result() def mstore8([offset, value], %{machine_state: machine_state}) do - machine_state = Memory.write(machine_state, offset, value, EVM.byte_size()) + byte_value = + value + |> rem(8 * EVM.word_size()) + |> :binary.encode_unsigned() + + machine_state = Memory.write(machine_state, offset, byte_value) %{machine_state: machine_state} end diff --git a/apps/evm/test/evm/message_call_test.exs b/apps/evm/test/evm/message_call_test.exs index 49758bd62..4a5dcb6bc 100644 --- a/apps/evm/test/evm/message_call_test.exs +++ b/apps/evm/test/evm/message_call_test.exs @@ -38,7 +38,8 @@ defmodule EVM.MessageCallTest do current_machine_state: pre_machine_state, recipient: recipient.address, code_owner: recipient.address, - execution_value: 100 + execution_value: 100, + output_params: {0, 256} ) %{machine_state: machine_state, exec_env: _exec_env, sub_state: _sub_state} = @@ -47,7 +48,7 @@ defmodule EVM.MessageCallTest do # Code was to add 1 + 3 = 4 and store in memory. assert machine_state.memory == <<0x4::256>> assert machine_state.gas == message_call.execution_value - 24 - assert machine_state.active_words == pre_machine_state.active_words + 1 + assert machine_state.active_words == 8 assert machine_state.stack == pre_machine_state.stack ++ [1] end @@ -146,4 +147,37 @@ defmodule EVM.MessageCallTest do assert active_words == 0 end + + test "truncates message call's output if output's byte size is bigger than output_params" do + code = build(:machine_code, operations: [:push1, 1, :push1, 3, :add, :push1, 0x00, :mstore]) + + recipient = %{address: <<0x9::160>>, code: code} + + account_interface = + build(:mock_account_interface) + |> MockAccountInterface.add_account(recipient.address, %{balance: 10, code: recipient.code}) + + pre_exec_env = + build(:exec_env, + account_interface: account_interface + ) + + pre_machine_state = build(:machine_state) + + message_call = + build(:message_call, + current_exec_env: pre_exec_env, + current_machine_state: pre_machine_state, + recipient: recipient.address, + code_owner: recipient.address, + execution_value: 100, + output_params: {0, 2} + ) + + %{machine_state: machine_state, exec_env: _exec_env, sub_state: _sub_state} = + MessageCall.call(message_call) + + assert machine_state.memory == <<0, 0>> + assert machine_state.active_words == 1 + end end From cc89ba6a9f5e38f611df5c57f9f00b84379b92bb Mon Sep 17 00:00:00 2001 From: Ayrat Badykov Date: Fri, 12 Oct 2018 16:10:29 +0300 Subject: [PATCH 3/4] update test numbers --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 006b69f79..7e98e9adf 100644 --- a/README.md +++ b/README.md @@ -129,13 +129,13 @@ Ethereum common tests are created for all clients to test against. We plan to pr - [GeneralStateTests](https://github.com/ethereum/tests/tree/develop/GeneralStateTests) 1026/1026 = 100% passing - [x] Homestead - [BlockchainTests](https://github.com/ethereum/tests/tree/develop/BlockchainTests) (Includes GeneralStateTests) 2231/2231 = 100% passing - - [GeneralStateTests](https://github.com/ethereum/tests/tree/develop/GeneralStateTests) 2059/2061 = 99.9% passing + - [GeneralStateTests](https://github.com/ethereum/tests/tree/develop/GeneralStateTests) 2061/2061 = 100.0% passing - [x] EIP150 - [BlockchainTests](https://github.com/ethereum/tests/tree/develop/BlockchainTests) (Includes GeneralStateTests) 1275/1275 = 100% passing - - [GeneralStateTests](https://github.com/ethereum/tests/tree/develop/GeneralStateTests) 1109/1113 = 99.6% passing + - [GeneralStateTests](https://github.com/ethereum/tests/tree/develop/GeneralStateTests) 1113/1113 = 100.0% passing - [x] EIP158 - [BlockchainTests](https://github.com/ethereum/tests/tree/develop/BlockchainTests) (Includes GeneralStateTests) 1232/1233 = 99.9% passing - - [GeneralStateTests](https://github.com/ethereum/tests/tree/develop/GeneralStateTests) 1176/1181 = 99.6% passing + - [GeneralStateTests](https://github.com/ethereum/tests/tree/develop/GeneralStateTests) 1180/1181 = 99.9% passing - [x] Byzantium - [BlockchainTests](https://github.com/ethereum/tests/tree/develop/BlockchainTests) (Includes GeneralStateTests) 4915/4959 = 99.1% passing - [GeneralStateTests](https://github.com/ethereum/tests/tree/develop/GeneralStateTests) 4758/4784 = 99.5% passing From acd49b8a7043a5fb2e12c21bfe717f555e3c9d30 Mon Sep 17 00:00:00 2001 From: Ayrat Badykov Date: Fri, 12 Oct 2018 16:25:11 +0300 Subject: [PATCH 4/4] fix rebasing --- apps/evm/test/evm/message_call_test.exs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/evm/test/evm/message_call_test.exs b/apps/evm/test/evm/message_call_test.exs index 4a5dcb6bc..0bb84014e 100644 --- a/apps/evm/test/evm/message_call_test.exs +++ b/apps/evm/test/evm/message_call_test.exs @@ -153,13 +153,13 @@ defmodule EVM.MessageCallTest do recipient = %{address: <<0x9::160>>, code: code} - account_interface = - build(:mock_account_interface) - |> MockAccountInterface.add_account(recipient.address, %{balance: 10, code: recipient.code}) + account_repo = + build(:mock_account_repo) + |> MockAccountRepo.add_account(recipient.address, %{balance: 10, code: recipient.code}) pre_exec_env = build(:exec_env, - account_interface: account_interface + account_repo: account_repo ) pre_machine_state = build(:machine_state)