From ed827c771950fb1bd0cea69ff09525962c6837f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Tue, 21 Oct 2014 11:54:54 +0200 Subject: [PATCH 1/8] Enhance VM tests reports, resolves ethereum/cpp-ethereum#399 --- vm.cpp | 40 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/vm.cpp b/vm.cpp index d77906731..01ecc0d01 100644 --- a/vm.cpp +++ b/vm.cpp @@ -574,8 +574,44 @@ void doTests(json_spirit::mValue& v, bool _fillin) else BOOST_CHECK(output == fromHex(o["out"].get_str())); - BOOST_CHECK(test.toInt(o["gas"]) == gas); - BOOST_CHECK(test.addresses == fev.addresses); + BOOST_CHECK_EQUAL(test.toInt(o["gas"]), gas); + + auto& expectedAddrs = test.addresses; + auto& resultAddrs = fev.addresses; + for (auto&& expectedPair : expectedAddrs) + { + auto& expectedAddr = expectedPair.first; + auto resultAddrIt = resultAddrs.find(expectedAddr); + if (resultAddrIt == resultAddrs.end()) + BOOST_ERROR("Missing expected address " << expectedAddr); + else + { + auto& expectedState = expectedPair.second; + auto& resultState = resultAddrIt->second; + BOOST_CHECK_MESSAGE(std::get<0>(expectedState) == std::get<0>(resultState), expectedAddr << ": incorrect balance " << std::get<0>(resultState) << ", expected " << std::get<0>(expectedState)); + BOOST_CHECK_MESSAGE(std::get<1>(expectedState) == std::get<1>(resultState), expectedAddr << ": incorrect txCount " << std::get<1>(resultState) << ", expected " << std::get<1>(expectedState)); + BOOST_CHECK_MESSAGE(std::get<3>(expectedState) == std::get<3>(resultState), expectedAddr << ": incorrect code"); + + auto&& expectedStore = std::get<2>(expectedState); + auto&& resultStore = std::get<2>(resultState); + + for (auto&& expectedStorePair : expectedStore) + { + auto& expectedStoreKey = expectedStorePair.first; + auto resultStoreIt = resultStore.find(expectedStoreKey); + if (resultStoreIt == resultStore.end()) + BOOST_ERROR(expectedAddr << ": missing store key " << expectedStoreKey); + else + { + auto& expectedStoreValue = expectedStorePair.second; + auto& resultStoreValue = resultStoreIt->second; + BOOST_CHECK_MESSAGE(expectedStoreValue == resultStoreValue, expectedAddr << ": store[" << expectedStoreKey << "] = " << resultStoreValue << ", expected " << expectedStoreValue); + } + } + } + } + + BOOST_CHECK(test.addresses == fev.addresses); // Just to make sure nothing missed BOOST_CHECK(test.callcreates == fev.callcreates); } } From 6c0b16bc032b89520d737f313e31c03856783f9f Mon Sep 17 00:00:00 2001 From: Christoph Jentzsch Date: Tue, 21 Oct 2014 14:20:16 +0200 Subject: [PATCH 2/8] Random test optimizations --- createRandomTest.cpp | 67 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 54 insertions(+), 13 deletions(-) diff --git a/createRandomTest.cpp b/createRandomTest.cpp index 874869a5c..2508d89f2 100644 --- a/createRandomTest.cpp +++ b/createRandomTest.cpp @@ -42,11 +42,11 @@ void doMyTests(json_spirit::mValue& v); int main(int argc, char *argv[]) { - if (argc != 2) - { - cout << "usage: createRandomTest \n"; - return 0; - } +// if (argc != 2) +// { +// cout << "usage: createRandomTest \n"; +// return 0; +// } // create random code @@ -64,14 +64,48 @@ int main(int argc, char *argv[]) string randomCode; for (int i = 0; i < lengthOfCode; ++i) - randomCode += toHex(toCompactBigEndian(randGen())); + { + uint8_t opcode = randGen(); - // read template test file + // disregard all invalid commands, except of one (0x10) + if (dev::eth::isValidInstruction(dev::eth::Instruction(opcode)) || opcode == 0x10) + randomCode += toHex(toCompactBigEndian(opcode)); + else + i--; + } + + const string s =\ +"{\n\ + \"randomVMtest\": {\n\ + \"env\" : {\n\ + \"previousHash\" : \"5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6\",\n\ + \"currentNumber\" : \"0\",\n\ + \"currentGasLimit\" : \"1000000\",\n\ + \"currentDifficulty\" : \"256\",\n\ + \"currentTimestamp\" : 1,\n\ + \"currentCoinbase\" : \"2adc25665018aa1fe0e6bc666dac8fc2697ff9ba\"\n\ + },\n\ + \"pre\" : {\n\ + \"0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6\" : {\n\ + \"balance\" : \"1000000000000000000\",\n\ + \"nonce\" : 0,\n\ + \"code\" : \"random\",\n\ + \"storage\": {}\n\ + }\n\ + },\n\ + \"exec\" : {\n\ + \"address\" : \"0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6\",\n\ + \"origin\" : \"cd1722f3947def4cf144679da39c4c32bdc35681\",\n\ + \"caller\" : \"cd1722f3947def4cf144679da39c4c32bdc35681\",\n\ + \"value\" : \"1000000000000000000\",\n\ + \"data\" : \"\",\n\ + \"gasPrice\" : \"100000000000000\",\n\ + \"gas\" : \"10000\"\n\ + }\n\ + }\n\ +}"; mValue v; - boost::filesystem::path p(__FILE__); - boost::filesystem::path dir = p.parent_path(); - string s = asString(contents(dir.string() + "/randomTestFiller.json")); read_string(s, v); // insert new random code @@ -80,9 +114,16 @@ int main(int argc, char *argv[]) // execute code in vm doMyTests(v); - // write new test - string filename = argv[1]; - writeFile(filename, asBytes(json_spirit::write_string(v, true))); +// // write new test +// string filename = argv[1]; +// writeFile(filename, asBytes(json_spirit::write_string(v, true))); + + // write resultsing test to envirnoment variable + string str = "ETHEREUM_RANDOM_TEST=" + json_spirit::write_string(v, true); + char *cstr = new char[str.length() + 1]; + strcpy(cstr, str.c_str()); + putenv(cstr); + delete [] cstr; return 0; } From 60a7ff58a1c6061a838b5d50b7f24d1ac05193b9 Mon Sep 17 00:00:00 2001 From: Christoph Jentzsch Date: Tue, 21 Oct 2014 17:02:49 +0200 Subject: [PATCH 3/8] Change output of random test to std::out instead of file --- createRandomTest.cpp | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/createRandomTest.cpp b/createRandomTest.cpp index 2508d89f2..28e4342d7 100644 --- a/createRandomTest.cpp +++ b/createRandomTest.cpp @@ -42,11 +42,7 @@ void doMyTests(json_spirit::mValue& v); int main(int argc, char *argv[]) { -// if (argc != 2) -// { -// cout << "usage: createRandomTest \n"; -// return 0; -// } + g_logVerbosity = 0; // create random code @@ -114,16 +110,8 @@ int main(int argc, char *argv[]) // execute code in vm doMyTests(v); -// // write new test -// string filename = argv[1]; -// writeFile(filename, asBytes(json_spirit::write_string(v, true))); - - // write resultsing test to envirnoment variable - string str = "ETHEREUM_RANDOM_TEST=" + json_spirit::write_string(v, true); - char *cstr = new char[str.length() + 1]; - strcpy(cstr, str.c_str()); - putenv(cstr); - delete [] cstr; + // stream to output for further handling by the bash script + cout << json_spirit::write_string(v, true); return 0; } From 05a8e41d5d5bf58de008bbd77dab702a4d5b7e0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Mon, 20 Oct 2014 17:36:26 +0200 Subject: [PATCH 4/8] Another round of fixing ExtVM interface --- vm.cpp | 19 +++++++++++++++---- vm.h | 18 +++++++++--------- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/vm.cpp b/vm.cpp index 7306ef32d..582c3c410 100644 --- a/vm.cpp +++ b/vm.cpp @@ -35,7 +35,7 @@ using namespace dev::test; FakeExtVM::FakeExtVM(eth::BlockInfo const& _previousBlock, eth::BlockInfo const& _currentBlock, unsigned _depth): /// TODO: XXX: remove the default argument & fix. ExtVMFace(Address(), Address(), Address(), 0, 1, bytesConstRef(), bytesConstRef(), _previousBlock, _currentBlock, _depth) {} -h160 FakeExtVM::create(u256 _endowment, u256* _gas, bytesConstRef _init, OnOpFunc) +h160 FakeExtVM::create(u256 _endowment, u256* _gas, bytesConstRef _init, OnOpFunc const&) { Transaction t; t.value = _endowment; @@ -45,7 +45,7 @@ h160 FakeExtVM::create(u256 _endowment, u256* _gas, bytesConstRef _init, OnOpFun m_s.noteSending(myAddress); m_ms.internal.resize(m_ms.internal.size() + 1); - auto ret = m_s.create(myAddress, _endowment, gasPrice, _gas, _init, origin, &suicides, &m_ms ? &(m_ms.internal.back()) : nullptr, OnOpFunc(), 1); + auto ret = m_s.create(myAddress, _endowment, gasPrice, _gas, _init, origin, &suicides, &m_ms ? &(m_ms.internal.back()) : nullptr, {}, 1); if (!m_ms.internal.back().from) m_ms.internal.pop_back(); @@ -61,7 +61,7 @@ h160 FakeExtVM::create(u256 _endowment, u256* _gas, bytesConstRef _init, OnOpFun return ret; } -bool FakeExtVM::call(Address _receiveAddress, u256 _value, bytesConstRef _data, u256* _gas, bytesRef _out, OnOpFunc, Address _myAddressOverride = Address(), Address _codeAddressOverride = Address()) +bool FakeExtVM::call(Address _receiveAddress, u256 _value, bytesConstRef _data, u256* _gas, bytesRef _out, OnOpFunc const&, Address _myAddressOverride = Address(), Address _codeAddressOverride = Address()) { u256 contractgas = 0xffff; @@ -91,7 +91,7 @@ bool FakeExtVM::call(Address _receiveAddress, u256 _value, bytesConstRef _data, if (!m_s.addresses().count(myAddress)) { m_ms.internal.resize(m_ms.internal.size() + 1); - auto na = m_s.createNewAddress(myAddress, myAddress, balance(myAddress), gasPrice, &contractgas, init, origin, &suicides, &m_ms ? &(m_ms.internal.back()) : nullptr, OnOpFunc(), 1); + auto na = m_s.createNewAddress(myAddress, myAddress, balance(myAddress), gasPrice, &contractgas, init, origin, &suicides, &m_ms ? &(m_ms.internal.back()) : nullptr, {}, 1); if (!m_ms.internal.back().from) m_ms.internal.pop_back(); if (na != myAddress) @@ -578,6 +578,17 @@ void doTests(json_spirit::mValue& v, bool _fillin) else BOOST_CHECK(output == fromHex(o["out"].get_str())); + //auto a = fev.addresses.at(fev.myAddress); + //auto b = test.addresses.at(fev.myAddress); + + //auto t = a == b; + //auto t0 = get<0>(a) == get<0>(b); + //auto t1 = get<1>(a) == get<1>(b); + //auto t2 = get<2>(a) == get<2>(b); + //auto t3 = get<3>(a) == get<3>(b); + + //BOOST_CHECK_EQUAL(get<0>(a), get<0>(b)); + BOOST_CHECK(test.toInt(o["gas"]) == gas); BOOST_CHECK(test.addresses == fev.addresses); BOOST_CHECK(test.callcreates == fev.callcreates); diff --git a/vm.h b/vm.h index 34e0e855a..a9897bee3 100644 --- a/vm.h +++ b/vm.h @@ -53,15 +53,15 @@ public: FakeExtVM() {} FakeExtVM(eth::BlockInfo const& _previousBlock, eth::BlockInfo const& _currentBlock, unsigned _depth = 0); - u256 store(u256 _n) { return std::get<2>(addresses[myAddress])[_n]; } - void setStore(u256 _n, u256 _v) { std::get<2>(addresses[myAddress])[_n] = _v; } - u256 balance(Address _a) { return std::get<0>(addresses[_a]); } - void subBalance(u256 _a) { std::get<0>(addresses[myAddress]) -= _a; } - u256 txCount(Address _a) { return std::get<1>(addresses[_a]); } - void suicide(Address _a) { std::get<0>(addresses[_a]) += std::get<0>(addresses[myAddress]); addresses.erase(myAddress); } - bytes const& codeAt(Address _a) { return std::get<3>(addresses[_a]); } - h160 create(u256 _endowment, u256* _gas, bytesConstRef _init, eth::OnOpFunc); - bool call(Address _receiveAddress, u256 _value, bytesConstRef _data, u256* _gas, bytesRef _out, eth::OnOpFunc, Address, Address); + u256 store(u256 _n) override { return std::get<2>(addresses[myAddress])[_n]; } + void setStore(u256 _n, u256 _v) override { std::get<2>(addresses[myAddress])[_n] = _v; } + u256 balance(Address _a) override { return std::get<0>(addresses[_a]); } + void subBalance(u256 _a) override { std::get<0>(addresses[myAddress]) -= _a; } + u256 txCount(Address _a) override { return std::get<1>(addresses[_a]); } + void suicide(Address _a) override { std::get<0>(addresses[_a]) += std::get<0>(addresses[myAddress]); addresses.erase(myAddress); } + bytes const& codeAt(Address _a) override { return std::get<3>(addresses[_a]); } + h160 create(u256 _endowment, u256* _gas, bytesConstRef _init, eth::OnOpFunc const&) override; + bool call(Address _receiveAddress, u256 _value, bytesConstRef _data, u256* _gas, bytesRef _out, eth::OnOpFunc const&, Address, Address) override; void setTransaction(Address _caller, u256 _value, u256 _gasPrice, bytes const& _data); void setContract(Address _myAddress, u256 _myBalance, u256 _myNonce, std::map const& _storage, bytes const& _code); void set(Address _a, u256 _myBalance, u256 _myNonce, std::map const& _storage, bytes const& _code); From d5c09e6450ed8182c3e6e680e6094c1e81b4af0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Tue, 21 Oct 2014 11:51:45 +0200 Subject: [PATCH 5/8] Remove dead code --- vm.cpp | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/vm.cpp b/vm.cpp index 582c3c410..8aa186473 100644 --- a/vm.cpp +++ b/vm.cpp @@ -578,17 +578,6 @@ void doTests(json_spirit::mValue& v, bool _fillin) else BOOST_CHECK(output == fromHex(o["out"].get_str())); - //auto a = fev.addresses.at(fev.myAddress); - //auto b = test.addresses.at(fev.myAddress); - - //auto t = a == b; - //auto t0 = get<0>(a) == get<0>(b); - //auto t1 = get<1>(a) == get<1>(b); - //auto t2 = get<2>(a) == get<2>(b); - //auto t3 = get<3>(a) == get<3>(b); - - //BOOST_CHECK_EQUAL(get<0>(a), get<0>(b)); - BOOST_CHECK(test.toInt(o["gas"]) == gas); BOOST_CHECK(test.addresses == fev.addresses); BOOST_CHECK(test.callcreates == fev.callcreates); From 95e7e44c0895c76f1bb8b105c6c76658e401a34b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Wed, 22 Oct 2014 12:45:26 +0200 Subject: [PATCH 6/8] Add virtual specifier to overridden methods in FakeVM --- vm.h | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/vm.h b/vm.h index a9897bee3..3a0091a06 100644 --- a/vm.h +++ b/vm.h @@ -50,18 +50,18 @@ public: class FakeExtVM: public eth::ExtVMFace { public: - FakeExtVM() {} + FakeExtVM() = default; FakeExtVM(eth::BlockInfo const& _previousBlock, eth::BlockInfo const& _currentBlock, unsigned _depth = 0); - u256 store(u256 _n) override { return std::get<2>(addresses[myAddress])[_n]; } - void setStore(u256 _n, u256 _v) override { std::get<2>(addresses[myAddress])[_n] = _v; } - u256 balance(Address _a) override { return std::get<0>(addresses[_a]); } - void subBalance(u256 _a) override { std::get<0>(addresses[myAddress]) -= _a; } - u256 txCount(Address _a) override { return std::get<1>(addresses[_a]); } - void suicide(Address _a) override { std::get<0>(addresses[_a]) += std::get<0>(addresses[myAddress]); addresses.erase(myAddress); } - bytes const& codeAt(Address _a) override { return std::get<3>(addresses[_a]); } - h160 create(u256 _endowment, u256* _gas, bytesConstRef _init, eth::OnOpFunc const&) override; - bool call(Address _receiveAddress, u256 _value, bytesConstRef _data, u256* _gas, bytesRef _out, eth::OnOpFunc const&, Address, Address) override; + virtual u256 store(u256 _n) override { return std::get<2>(addresses[myAddress])[_n]; } + virtual void setStore(u256 _n, u256 _v) override { std::get<2>(addresses[myAddress])[_n] = _v; } + virtual u256 balance(Address _a) override { return std::get<0>(addresses[_a]); } + virtual void subBalance(u256 _a) override { std::get<0>(addresses[myAddress]) -= _a; } + virtual u256 txCount(Address _a) override { return std::get<1>(addresses[_a]); } + virtual void suicide(Address _a) override { std::get<0>(addresses[_a]) += std::get<0>(addresses[myAddress]); addresses.erase(myAddress); } + virtual bytes const& codeAt(Address _a) override { return std::get<3>(addresses[_a]); } + virtual h160 create(u256 _endowment, u256* _gas, bytesConstRef _init, eth::OnOpFunc const&) override; + virtual bool call(Address _receiveAddress, u256 _value, bytesConstRef _data, u256* _gas, bytesRef _out, eth::OnOpFunc const&, Address, Address) override; void setTransaction(Address _caller, u256 _value, u256 _gasPrice, bytes const& _data); void setContract(Address _myAddress, u256 _myBalance, u256 _myNonce, std::map const& _storage, bytes const& _code); void set(Address _a, u256 _myBalance, u256 _myNonce, std::map const& _storage, bytes const& _code); From e987e1a69289fcf05abb25a3d087681b8347b021 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Wed, 22 Oct 2014 12:54:31 +0200 Subject: [PATCH 7/8] Drop universal initializer in some places --- vm.cpp | 2 +- vm.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/vm.cpp b/vm.cpp index 8aa186473..ffa99dc01 100644 --- a/vm.cpp +++ b/vm.cpp @@ -61,7 +61,7 @@ h160 FakeExtVM::create(u256 _endowment, u256* _gas, bytesConstRef _init, OnOpFun return ret; } -bool FakeExtVM::call(Address _receiveAddress, u256 _value, bytesConstRef _data, u256* _gas, bytesRef _out, OnOpFunc const&, Address _myAddressOverride = Address(), Address _codeAddressOverride = Address()) +bool FakeExtVM::call(Address _receiveAddress, u256 _value, bytesConstRef _data, u256* _gas, bytesRef _out, OnOpFunc const&, Address _myAddressOverride, Address _codeAddressOverride) { u256 contractgas = 0xffff; diff --git a/vm.h b/vm.h index 3a0091a06..d9dca1d7a 100644 --- a/vm.h +++ b/vm.h @@ -44,7 +44,7 @@ class FakeState: public eth::State { public: /// Execute a contract-creation transaction. - h160 createNewAddress(Address _newAddress, Address _txSender, u256 _endowment, u256 _gasPrice, u256* _gas, bytesConstRef _code, Address _originAddress = Address(), std::set
* o_suicides = nullptr, eth::Manifest* o_ms = nullptr, eth::OnOpFunc const& _onOp = eth::OnOpFunc(), unsigned _level = 0); + h160 createNewAddress(Address _newAddress, Address _txSender, u256 _endowment, u256 _gasPrice, u256* _gas, bytesConstRef _code, Address _originAddress = {}, std::set
* o_suicides = nullptr, eth::Manifest* o_ms = nullptr, eth::OnOpFunc const& _onOp = {}, unsigned _level = 0); }; class FakeExtVM: public eth::ExtVMFace From c7a101dec321d5bd6fb0c0875b772527cd69a8b4 Mon Sep 17 00:00:00 2001 From: Christoph Jentzsch Date: Wed, 22 Oct 2014 22:13:08 +0200 Subject: [PATCH 8/8] bug fix --- vm.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/vm.cpp b/vm.cpp index f74076399..36837eea4 100644 --- a/vm.cpp +++ b/vm.cpp @@ -512,12 +512,10 @@ void doTests(json_spirit::mValue& v, bool _fillin) } bytes output; - u256 gas; + VM vm(fev.gas); try { - VM vm(fev.gas); output = vm.go(fev).toVector(); - gas = vm.gas(); // Get the remaining gas } catch (Exception const& _e) { @@ -554,7 +552,7 @@ void doTests(json_spirit::mValue& v, bool _fillin) o["post"] = mValue(fev.exportState()); o["callcreates"] = fev.exportCallCreates(); o["out"] = "0x" + toHex(output); - fev.push(o, "gas", gas); + fev.push(o, "gas", vm.gas()); } else { @@ -578,7 +576,7 @@ void doTests(json_spirit::mValue& v, bool _fillin) else BOOST_CHECK(output == fromHex(o["out"].get_str())); - BOOST_CHECK_EQUAL(test.toInt(o["gas"]), gas); + BOOST_CHECK_EQUAL(test.toInt(o["gas"]), vm.gas()); auto& expectedAddrs = test.addresses; auto& resultAddrs = fev.addresses; @@ -657,11 +655,13 @@ void executeTests(const string& _name) if (ptestPath == NULL) { cnote << " could not find environment variable ETHEREUM_TEST_PATH \n"; - testPath = "../../../tests/vmtests"; + testPath = "../../../tests"; } else testPath = ptestPath; + testPath += "/vmtests"; + #ifdef FILL_TESTS try { @@ -690,7 +690,7 @@ void executeTests(const string& _name) cnote << "Testing VM..." << _name; json_spirit::mValue v; string s = asString(contents(testPath + "/" + _name + ".json")); - BOOST_REQUIRE_MESSAGE(s.length() > 0, "Contents of " + _name + ".json is empty. Have you cloned the 'tests' repo branch develop and set ETHEREUM_TEST_PATH to its path?"); + BOOST_REQUIRE_MESSAGE(s.length() > 0, "Contents of " + testPath + "/" + _name + ".json is empty. Have you cloned the 'tests' repo branch develop and set ETHEREUM_TEST_PATH to its path?"); json_spirit::read_string(s, v); dev::test::doTests(v, false); }