From 363dae6a64321f5083103968bb461c5731bdd0e0 Mon Sep 17 00:00:00 2001 From: Vlad Gluhovsky Date: Tue, 7 Jul 2015 17:00:56 +0200 Subject: [PATCH 01/24] levelDB for whisper --- libwhisper/WhisperDB.cpp | 90 +++++++++++++++++++++++++++++++++++ libwhisper/WhisperDB.h | 50 +++++++++++++++++++ test/libwhisper/whisperDB.cpp | 70 +++++++++++++++++++++++++++ 3 files changed, 210 insertions(+) create mode 100644 libwhisper/WhisperDB.cpp create mode 100644 libwhisper/WhisperDB.h create mode 100644 test/libwhisper/whisperDB.cpp diff --git a/libwhisper/WhisperDB.cpp b/libwhisper/WhisperDB.cpp new file mode 100644 index 000000000..e5d40e571 --- /dev/null +++ b/libwhisper/WhisperDB.cpp @@ -0,0 +1,90 @@ +/* + This file is part of cpp-ethereum. + + cpp-ethereum is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + cpp-ethereum is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with cpp-ethereum. If not, see . +*/ +/** @file WhisperDB.cpp +* @author Vladislav Gluhovsky +* @date July 2015 + */ + +#include +#include + +#include +#include +#include +#include +#include +#include +#include "WhisperDB.h" + +using namespace std; +using namespace dev; +using namespace dev::shh; +using namespace dev::eth; + +WhisperDB::WhisperDB() +{ + string path = Defaults::dbPath(); + boost::filesystem::create_directories(path); + ldb::Options o; + o.create_if_missing = true; + ldb::DB::Open(o, path + "/whisper", &m_db); +} + +WhisperDB::~WhisperDB() +{ + delete m_db; +} + +bool WhisperDB::put(dev::h256 const& _key, string const& _value) +{ + string s = _key.hex(); + string cropped = s.substr(s.size() - 8); + leveldb::Status status = m_db->Put(m_writeOptions, s, _value); + if (status.ok()) + cdebug << "Whisper DB put:" << cropped << _value; + else + cdebug << "Whisper DB put failed:" << status.ToString() << "key:" << cropped; + + return status.ok(); +} + +string WhisperDB::get(dev::h256 const& _key) const +{ + string ret; + string s = _key.hex(); + string cropped = s.substr(s.size() - 8); + leveldb::Status status = m_db->Get(m_readOptions, s, &ret); + if (status.ok()) + cdebug << "Whisper DB get:" << cropped << ret; + else + cdebug << "Whisper DB get failed:" << status.ToString() << "key:" << cropped; + + return ret; +} + +bool WhisperDB::erase(dev::h256 const& _key) +{ + string s = _key.hex(); + string cropped = s.substr(s.size() - 8); + leveldb::Status status = m_db->Delete(m_writeOptions, s); + if (status.ok()) + cdebug << "Whisper DB erase:" << cropped; + else + cdebug << "Whisper DB erase failed:" << status.ToString() << "key:" << cropped; + + return status.ok(); +} diff --git a/libwhisper/WhisperDB.h b/libwhisper/WhisperDB.h new file mode 100644 index 000000000..70b65ef84 --- /dev/null +++ b/libwhisper/WhisperDB.h @@ -0,0 +1,50 @@ +/* + This file is part of cpp-ethereum. + + cpp-ethereum is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + cpp-ethereum is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with cpp-ethereum. If not, see . +*/ +/** @file WhisperDB.h +* @author Vladislav Gluhovsky +* @date July 2015 + */ + +#pragma once + +#include +#include +#include "Common.h" + +namespace dev +{ +namespace shh +{ + +class WhisperDB +{ + public: + WhisperDB(); + ~WhisperDB(); + + bool put(dev::h256 const& _key, std::string const& _value); + bool erase(dev::h256 const& _key); + std::string get(dev::h256 const& _key) const; + +private: + ldb::ReadOptions m_readOptions; + ldb::WriteOptions m_writeOptions; + ldb::DB* m_db = nullptr; +}; + +} +} \ No newline at end of file diff --git a/test/libwhisper/whisperDB.cpp b/test/libwhisper/whisperDB.cpp new file mode 100644 index 000000000..4b821e0c5 --- /dev/null +++ b/test/libwhisper/whisperDB.cpp @@ -0,0 +1,70 @@ +/* +This file is part of cpp-ethereum. + +cpp-ethereum is free software: you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +cpp-ethereum is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with cpp-ethereum. If not, see . +*/ +/** @file whisperMessage.cpp +* @author Vladislav Gluhovsky +* @date July 2015 +*/ + +#include +#include + +using namespace std; +using namespace dev; +using namespace dev::shh; + +BOOST_AUTO_TEST_SUITE(whisperDB) + +BOOST_AUTO_TEST_CASE(first) +{ + VerbosityHolder setTemporaryLevel(10); + cnote << "Testing Whisper DB..."; + + WhisperDB db; + + h256 h1(0x12345678); + h256 h2(0xBADD00DE); + + string s; + string const text1 = "lorem_ipsum"; + string const text2 = "dolor_sit_amet"; + + db.erase(h1); + db.erase(h2); + + db.put(h1, text2); + s = db.get(h2); + BOOST_REQUIRE(s.empty()); + s = db.get(h1); + BOOST_REQUIRE(!s.compare(text2)); + + db.put(h1, text1); + s = db.get(h2); + BOOST_REQUIRE(s.empty()); + s = db.get(h1); + BOOST_REQUIRE(!s.compare(text1)); + + db.put(h2, text2); + s = db.get(h2); + BOOST_REQUIRE(!s.compare(text2)); + s = db.get(h1); + BOOST_REQUIRE(!s.compare(text1)); + + db.erase(h1); + db.erase(h2); +} + +BOOST_AUTO_TEST_SUITE_END() From 24564e0dd32ac3715e18ea8cef1f66ac8f532d00 Mon Sep 17 00:00:00 2001 From: Vlad Gluhovsky Date: Wed, 8 Jul 2015 11:40:55 +0200 Subject: [PATCH 02/24] small fix --- libwhisper/CMakeLists.txt | 1 + libwhisper/WhisperDB.cpp | 7 ++++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/libwhisper/CMakeLists.txt b/libwhisper/CMakeLists.txt index 5af43d0b2..5cf6a865f 100644 --- a/libwhisper/CMakeLists.txt +++ b/libwhisper/CMakeLists.txt @@ -21,6 +21,7 @@ file(GLOB HEADERS "*.h") add_library(${EXECUTABLE} ${SRC_LIST} ${HEADERS}) +target_link_libraries(${EXECUTABLE} ethereum) target_link_libraries(${EXECUTABLE} ethcore) target_link_libraries(${EXECUTABLE} devcrypto) target_link_libraries(${EXECUTABLE} devcore) diff --git a/libwhisper/WhisperDB.cpp b/libwhisper/WhisperDB.cpp index e5d40e571..a880bc279 100644 --- a/libwhisper/WhisperDB.cpp +++ b/libwhisper/WhisperDB.cpp @@ -39,9 +39,10 @@ WhisperDB::WhisperDB() { string path = Defaults::dbPath(); boost::filesystem::create_directories(path); - ldb::Options o; - o.create_if_missing = true; - ldb::DB::Open(o, path + "/whisper", &m_db); + ldb::Options op; + op.create_if_missing = true; + op.max_open_files = 256; + ldb::DB::Open(op, path + "/whisper", &m_db); } WhisperDB::~WhisperDB() From bd525e0408f0488201f014b4d5ea0fdee38c9c0c Mon Sep 17 00:00:00 2001 From: Vlad Gluhovsky Date: Wed, 8 Jul 2015 13:30:42 +0200 Subject: [PATCH 03/24] removed excessive dependency --- libwhisper/CMakeLists.txt | 1 - libwhisper/WhisperDB.cpp | 5 ++--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/libwhisper/CMakeLists.txt b/libwhisper/CMakeLists.txt index 5cf6a865f..5af43d0b2 100644 --- a/libwhisper/CMakeLists.txt +++ b/libwhisper/CMakeLists.txt @@ -21,7 +21,6 @@ file(GLOB HEADERS "*.h") add_library(${EXECUTABLE} ${SRC_LIST} ${HEADERS}) -target_link_libraries(${EXECUTABLE} ethereum) target_link_libraries(${EXECUTABLE} ethcore) target_link_libraries(${EXECUTABLE} devcrypto) target_link_libraries(${EXECUTABLE} devcore) diff --git a/libwhisper/WhisperDB.cpp b/libwhisper/WhisperDB.cpp index a880bc279..b8977161c 100644 --- a/libwhisper/WhisperDB.cpp +++ b/libwhisper/WhisperDB.cpp @@ -27,17 +27,16 @@ #include #include #include -#include +#include #include "WhisperDB.h" using namespace std; using namespace dev; using namespace dev::shh; -using namespace dev::eth; WhisperDB::WhisperDB() { - string path = Defaults::dbPath(); + string path = dev::getDataDir(); boost::filesystem::create_directories(path); ldb::Options op; op.create_if_missing = true; From 5ef3677ceb508f5e231502a9f4b42652330afc9a Mon Sep 17 00:00:00 2001 From: bargst Date: Thu, 9 Jul 2015 00:06:42 +0200 Subject: [PATCH 04/24] Fix global work size logic adjustment When batch duration is too long, m_globalWorkSize must decrease. The other way if batch duration is too small, m_globalWorkSize should increase ... --- libethash-cl/ethash_cl_miner.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libethash-cl/ethash_cl_miner.cpp b/libethash-cl/ethash_cl_miner.cpp index 8b8cb0b51..e787e181f 100644 --- a/libethash-cl/ethash_cl_miner.cpp +++ b/libethash-cl/ethash_cl_miner.cpp @@ -521,13 +521,13 @@ void ethash_cl_miner::search(uint8_t const* header, uint64_t target, search_hook if (d > chrono::milliseconds(s_msPerBatch * 10 / 9)) { // cerr << "Batch of " << m_globalWorkSize << " took " << chrono::duration_cast(d).count() << " ms, >> " << _msPerBatch << " ms." << endl; - m_globalWorkSize = max(128, m_globalWorkSize + s_workgroupSize); + m_globalWorkSize = max(128, m_globalWorkSize - s_workgroupSize); // cerr << "New global work size" << m_globalWorkSize << endl; } else if (d < chrono::milliseconds(s_msPerBatch * 9 / 10)) { // cerr << "Batch of " << m_globalWorkSize << " took " << chrono::duration_cast(d).count() << " ms, << " << _msPerBatch << " ms." << endl; - m_globalWorkSize = min(pow(2, m_deviceBits) - 1, m_globalWorkSize - s_workgroupSize); + m_globalWorkSize = min(pow(2, m_deviceBits) - 1, m_globalWorkSize + s_workgroupSize); // Global work size should never be less than the workgroup size m_globalWorkSize = max(s_workgroupSize, m_globalWorkSize); // cerr << "New global work size" << m_globalWorkSize << endl; From 7450b8a0bcefd8b97878eb92412181c007e6cc2b Mon Sep 17 00:00:00 2001 From: bargst Date: Thu, 9 Jul 2015 00:13:21 +0200 Subject: [PATCH 05/24] Quick discovery of optimal global work size Use a dichotomic algo to discover optimal m_globalWorkSize: - m_wayWorkSizeAdjust is the direction steps are done (-1 or +1) - m_stepWorkSizeAdjust is the steps of adjustment (added or substracted to m_globalWorkSize) - when a change of direction is needed, step is divided by 2 --- libethash-cl/ethash_cl_miner.cpp | 22 ++++++++++++++++++---- libethash-cl/ethash_cl_miner.h | 5 +++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/libethash-cl/ethash_cl_miner.cpp b/libethash-cl/ethash_cl_miner.cpp index e787e181f..dec065f31 100644 --- a/libethash-cl/ethash_cl_miner.cpp +++ b/libethash-cl/ethash_cl_miner.cpp @@ -315,6 +315,8 @@ bool ethash_cl_miner::init( m_globalWorkSize = ((m_globalWorkSize / s_workgroupSize) + 1) * s_workgroupSize; // remember the device's address bits m_deviceBits = device.getInfo(); + // make sure first step of global work size adjustment is large enough + m_stepWorkSizeAdjust = pow(2, m_deviceBits/2+1); // patch source code // note: ETHASH_CL_MINER_KERNEL is simply ethash_cl_miner_kernel.cl compiled @@ -520,14 +522,26 @@ void ethash_cl_miner::search(uint8_t const* header, uint64_t target, search_hook { if (d > chrono::milliseconds(s_msPerBatch * 10 / 9)) { - // cerr << "Batch of " << m_globalWorkSize << " took " << chrono::duration_cast(d).count() << " ms, >> " << _msPerBatch << " ms." << endl; - m_globalWorkSize = max(128, m_globalWorkSize - s_workgroupSize); + // Divide the step by 2 when adjustment way change + if (m_wayWorkSizeAdjust > -1 ) + m_stepWorkSizeAdjust = max(1, m_stepWorkSizeAdjust / 2); + m_wayWorkSizeAdjust = -1; + // cerr << "m_stepWorkSizeAdjust: " << m_stepWorkSizeAdjust << ", m_wayWorkSizeAdjust: " << m_wayWorkSizeAdjust << endl; + + // cerr << "Batch of " << m_globalWorkSize << " took " << chrono::duration_cast(d).count() << " ms, >> " << s_msPerBatch << " ms." << endl; + m_globalWorkSize = max(128, m_globalWorkSize - m_stepWorkSizeAdjust); // cerr << "New global work size" << m_globalWorkSize << endl; } else if (d < chrono::milliseconds(s_msPerBatch * 9 / 10)) { - // cerr << "Batch of " << m_globalWorkSize << " took " << chrono::duration_cast(d).count() << " ms, << " << _msPerBatch << " ms." << endl; - m_globalWorkSize = min(pow(2, m_deviceBits) - 1, m_globalWorkSize + s_workgroupSize); + // Divide the step by 2 when adjustment way change + if (m_wayWorkSizeAdjust < 1 ) + m_stepWorkSizeAdjust = max(1, m_stepWorkSizeAdjust / 2); + m_wayWorkSizeAdjust = 1; + // cerr << "m_stepWorkSizeAdjust: " << m_stepWorkSizeAdjust << ", m_wayWorkSizeAdjust: " << m_wayWorkSizeAdjust << endl; + + // cerr << "Batch of " << m_globalWorkSize << " took " << chrono::duration_cast(d).count() << " ms, << " << s_msPerBatch << " ms." << endl; + m_globalWorkSize = min(pow(2, m_deviceBits) - 1, m_globalWorkSize + m_stepWorkSizeAdjust); // Global work size should never be less than the workgroup size m_globalWorkSize = max(s_workgroupSize, m_globalWorkSize); // cerr << "New global work size" << m_globalWorkSize << endl; diff --git a/libethash-cl/ethash_cl_miner.h b/libethash-cl/ethash_cl_miner.h index d1cb53ef9..181935cf6 100644 --- a/libethash-cl/ethash_cl_miner.h +++ b/libethash-cl/ethash_cl_miner.h @@ -82,6 +82,11 @@ private: bool m_openclOnePointOne; unsigned m_deviceBits; + /// The step used in the work size adjustment + unsigned int m_stepWorkSizeAdjust; + /// The Work Size way of adjustment, > 0 when previously increased, < 0 when previously decreased + int m_wayWorkSizeAdjust = 0; + /// The local work size for the search static unsigned s_workgroupSize; /// The initial global work size for the searches From 52b36b6b6affd6957baf64def533f0714ff55f4f Mon Sep 17 00:00:00 2001 From: Vlad Gluhovsky Date: Thu, 9 Jul 2015 19:45:13 +0200 Subject: [PATCH 06/24] naming conventions changed --- libwhisper/WhisperDB.cpp | 6 +++--- libwhisper/WhisperDB.h | 6 +++--- test/libwhisper/whisperDB.cpp | 26 +++++++++++++------------- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/libwhisper/WhisperDB.cpp b/libwhisper/WhisperDB.cpp index b8977161c..dcb3cf1e0 100644 --- a/libwhisper/WhisperDB.cpp +++ b/libwhisper/WhisperDB.cpp @@ -49,7 +49,7 @@ WhisperDB::~WhisperDB() delete m_db; } -bool WhisperDB::put(dev::h256 const& _key, string const& _value) +bool WhisperDB::insert(dev::h256 const& _key, string const& _value) { string s = _key.hex(); string cropped = s.substr(s.size() - 8); @@ -62,7 +62,7 @@ bool WhisperDB::put(dev::h256 const& _key, string const& _value) return status.ok(); } -string WhisperDB::get(dev::h256 const& _key) const +string WhisperDB::lookup(dev::h256 const& _key) const { string ret; string s = _key.hex(); @@ -76,7 +76,7 @@ string WhisperDB::get(dev::h256 const& _key) const return ret; } -bool WhisperDB::erase(dev::h256 const& _key) +bool WhisperDB::kill(dev::h256 const& _key) { string s = _key.hex(); string cropped = s.substr(s.size() - 8); diff --git a/libwhisper/WhisperDB.h b/libwhisper/WhisperDB.h index 70b65ef84..b2aa12df3 100644 --- a/libwhisper/WhisperDB.h +++ b/libwhisper/WhisperDB.h @@ -36,9 +36,9 @@ class WhisperDB WhisperDB(); ~WhisperDB(); - bool put(dev::h256 const& _key, std::string const& _value); - bool erase(dev::h256 const& _key); - std::string get(dev::h256 const& _key) const; + bool insert(dev::h256 const& _key, std::string const& _value); + bool kill(dev::h256 const& _key); + std::string lookup(dev::h256 const& _key) const; private: ldb::ReadOptions m_readOptions; diff --git a/test/libwhisper/whisperDB.cpp b/test/libwhisper/whisperDB.cpp index 4b821e0c5..9c262fb8e 100644 --- a/test/libwhisper/whisperDB.cpp +++ b/test/libwhisper/whisperDB.cpp @@ -42,29 +42,29 @@ BOOST_AUTO_TEST_CASE(first) string const text1 = "lorem_ipsum"; string const text2 = "dolor_sit_amet"; - db.erase(h1); - db.erase(h2); + db.kill(h1); + db.kill(h2); - db.put(h1, text2); - s = db.get(h2); + db.insert(h1, text2); + s = db.lookup(h2); BOOST_REQUIRE(s.empty()); - s = db.get(h1); + s = db.lookup(h1); BOOST_REQUIRE(!s.compare(text2)); - db.put(h1, text1); - s = db.get(h2); + db.insert(h1, text1); + s = db.lookup(h2); BOOST_REQUIRE(s.empty()); - s = db.get(h1); + s = db.lookup(h1); BOOST_REQUIRE(!s.compare(text1)); - db.put(h2, text2); - s = db.get(h2); + db.insert(h2, text2); + s = db.lookup(h2); BOOST_REQUIRE(!s.compare(text2)); - s = db.get(h1); + s = db.lookup(h1); BOOST_REQUIRE(!s.compare(text1)); - db.erase(h1); - db.erase(h2); + db.kill(h1); + db.kill(h2); } BOOST_AUTO_TEST_SUITE_END() From a5e8a6a76083ff58b903582a0177a4c091a59c1b Mon Sep 17 00:00:00 2001 From: bargst Date: Thu, 9 Jul 2015 23:45:37 +0200 Subject: [PATCH 07/24] Fix style issues --- libethash-cl/ethash_cl_miner.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libethash-cl/ethash_cl_miner.cpp b/libethash-cl/ethash_cl_miner.cpp index dec065f31..3b0fd9bc2 100644 --- a/libethash-cl/ethash_cl_miner.cpp +++ b/libethash-cl/ethash_cl_miner.cpp @@ -316,7 +316,7 @@ bool ethash_cl_miner::init( // remember the device's address bits m_deviceBits = device.getInfo(); // make sure first step of global work size adjustment is large enough - m_stepWorkSizeAdjust = pow(2, m_deviceBits/2+1); + m_stepWorkSizeAdjust = pow(2, m_deviceBits / 2 + 1); // patch source code // note: ETHASH_CL_MINER_KERNEL is simply ethash_cl_miner_kernel.cl compiled @@ -523,7 +523,7 @@ void ethash_cl_miner::search(uint8_t const* header, uint64_t target, search_hook if (d > chrono::milliseconds(s_msPerBatch * 10 / 9)) { // Divide the step by 2 when adjustment way change - if (m_wayWorkSizeAdjust > -1 ) + if (m_wayWorkSizeAdjust > -1) m_stepWorkSizeAdjust = max(1, m_stepWorkSizeAdjust / 2); m_wayWorkSizeAdjust = -1; // cerr << "m_stepWorkSizeAdjust: " << m_stepWorkSizeAdjust << ", m_wayWorkSizeAdjust: " << m_wayWorkSizeAdjust << endl; @@ -535,7 +535,7 @@ void ethash_cl_miner::search(uint8_t const* header, uint64_t target, search_hook else if (d < chrono::milliseconds(s_msPerBatch * 9 / 10)) { // Divide the step by 2 when adjustment way change - if (m_wayWorkSizeAdjust < 1 ) + if (m_wayWorkSizeAdjust < 1) m_stepWorkSizeAdjust = max(1, m_stepWorkSizeAdjust / 2); m_wayWorkSizeAdjust = 1; // cerr << "m_stepWorkSizeAdjust: " << m_stepWorkSizeAdjust << ", m_wayWorkSizeAdjust: " << m_wayWorkSizeAdjust << endl; From 581e4b2bd5e5042a0951785b45a9831a6155cbc8 Mon Sep 17 00:00:00 2001 From: Vlad Gluhovsky Date: Fri, 10 Jul 2015 17:03:04 +0200 Subject: [PATCH 08/24] minor changes --- libwhisper/WhisperDB.cpp | 67 ++++++++++++++-------------------------- libwhisper/WhisperDB.h | 13 +++++--- 2 files changed, 32 insertions(+), 48 deletions(-) diff --git a/libwhisper/WhisperDB.cpp b/libwhisper/WhisperDB.cpp index dcb3cf1e0..91c5f74f4 100644 --- a/libwhisper/WhisperDB.cpp +++ b/libwhisper/WhisperDB.cpp @@ -19,16 +19,9 @@ * @date July 2015 */ -#include +#include "WhisperDB.h" #include - -#include -#include -#include -#include -#include #include -#include "WhisperDB.h" using namespace std; using namespace dev; @@ -41,50 +34,36 @@ WhisperDB::WhisperDB() ldb::Options op; op.create_if_missing = true; op.max_open_files = 256; - ldb::DB::Open(op, path + "/whisper", &m_db); -} - -WhisperDB::~WhisperDB() -{ - delete m_db; -} - -bool WhisperDB::insert(dev::h256 const& _key, string const& _value) -{ - string s = _key.hex(); - string cropped = s.substr(s.size() - 8); - leveldb::Status status = m_db->Put(m_writeOptions, s, _value); - if (status.ok()) - cdebug << "Whisper DB put:" << cropped << _value; - else - cdebug << "Whisper DB put failed:" << status.ToString() << "key:" << cropped; - - return status.ok(); + ldb::DB* p = nullptr; + leveldb::Status status = ldb::DB::Open(op, path + "/whisper", &p); + m_db.reset(p); + if (!status.ok()) + BOOST_THROW_EXCEPTION(FailedToOpenLevelDB(status.ToString())); } string WhisperDB::lookup(dev::h256 const& _key) const { string ret; - string s = _key.hex(); - string cropped = s.substr(s.size() - 8); - leveldb::Status status = m_db->Get(m_readOptions, s, &ret); - if (status.ok()) - cdebug << "Whisper DB get:" << cropped << ret; - else - cdebug << "Whisper DB get failed:" << status.ToString() << "key:" << cropped; + leveldb::Slice slice((char const*)_key.data(), _key.size); + leveldb::Status status = m_db->Get(m_readOptions, slice, &ret); + if (!status.ok() && !status.IsNotFound()) + BOOST_THROW_EXCEPTION(FailedLookupInLevelDB(status.ToString())); return ret; } -bool WhisperDB::kill(dev::h256 const& _key) +void WhisperDB::insert(dev::h256 const& _key, string const& _value) +{ + leveldb::Slice slice((char const*)_key.data(), _key.size); + leveldb::Status status = m_db->Put(m_writeOptions, slice, _value); + if (!status.ok()) + BOOST_THROW_EXCEPTION(FailedInsertInLevelDB(status.ToString())); +} + +void WhisperDB::kill(dev::h256 const& _key) { - string s = _key.hex(); - string cropped = s.substr(s.size() - 8); - leveldb::Status status = m_db->Delete(m_writeOptions, s); - if (status.ok()) - cdebug << "Whisper DB erase:" << cropped; - else - cdebug << "Whisper DB erase failed:" << status.ToString() << "key:" << cropped; - - return status.ok(); + leveldb::Slice slice((char const*)_key.data(), _key.size); + leveldb::Status status = m_db->Delete(m_writeOptions, slice); + if (!status.ok()) + BOOST_THROW_EXCEPTION(FailedDeleteInLevelDB(status.ToString())); } diff --git a/libwhisper/WhisperDB.h b/libwhisper/WhisperDB.h index b2aa12df3..c20f52a75 100644 --- a/libwhisper/WhisperDB.h +++ b/libwhisper/WhisperDB.h @@ -30,20 +30,25 @@ namespace dev namespace shh { +struct FailedToOpenLevelDB: virtual Exception { FailedToOpenLevelDB(std::string _message = std::string()): Exception(_message) {} }; +struct FailedInsertInLevelDB: virtual Exception { FailedInsertInLevelDB(std::string _message = std::string()): Exception(_message) {} }; +struct FailedLookupInLevelDB: virtual Exception { FailedLookupInLevelDB(std::string _message = std::string()): Exception(_message) {} }; +struct FailedDeleteInLevelDB: virtual Exception { FailedDeleteInLevelDB(std::string _message = std::string()): Exception(_message) {} }; + class WhisperDB { public: WhisperDB(); - ~WhisperDB(); + ~WhisperDB() {} - bool insert(dev::h256 const& _key, std::string const& _value); - bool kill(dev::h256 const& _key); std::string lookup(dev::h256 const& _key) const; + void insert(dev::h256 const& _key, std::string const& _value); + void kill(dev::h256 const& _key); private: ldb::ReadOptions m_readOptions; ldb::WriteOptions m_writeOptions; - ldb::DB* m_db = nullptr; + std::unique_ptr m_db; }; } From f4eb948c1558bee8c2f7dbe5235ed6d852a47a18 Mon Sep 17 00:00:00 2001 From: Vlad Gluhovsky Date: Fri, 10 Jul 2015 18:58:20 +0200 Subject: [PATCH 09/24] new tests added --- test/libwhisper/whisperDB.cpp | 68 ++++++++++++++++++++++++++++++----- 1 file changed, 60 insertions(+), 8 deletions(-) diff --git a/test/libwhisper/whisperDB.cpp b/test/libwhisper/whisperDB.cpp index 9c262fb8e..552820621 100644 --- a/test/libwhisper/whisperDB.cpp +++ b/test/libwhisper/whisperDB.cpp @@ -19,6 +19,7 @@ along with cpp-ethereum. If not, see . * @date July 2015 */ +#include #include #include @@ -28,23 +29,24 @@ using namespace dev::shh; BOOST_AUTO_TEST_SUITE(whisperDB) -BOOST_AUTO_TEST_CASE(first) +BOOST_AUTO_TEST_CASE(basic) { VerbosityHolder setTemporaryLevel(10); cnote << "Testing Whisper DB..."; - WhisperDB db; - - h256 h1(0x12345678); - h256 h2(0xBADD00DE); - string s; - string const text1 = "lorem_ipsum"; - string const text2 = "dolor_sit_amet"; + string const text1 = "lorem"; + string const text2 = "ipsum"; + h256 h1(0xBEEF); + h256 h2(0xC0FFEE); + WhisperDB db; db.kill(h1); db.kill(h2); + s = db.lookup(h1); + BOOST_REQUIRE(s.empty()); + db.insert(h1, text2); s = db.lookup(h2); BOOST_REQUIRE(s.empty()); @@ -65,6 +67,56 @@ BOOST_AUTO_TEST_CASE(first) db.kill(h1); db.kill(h2); + + s = db.lookup(h2); + BOOST_REQUIRE(s.empty()); + s = db.lookup(h1); + BOOST_REQUIRE(s.empty()); +} + +BOOST_AUTO_TEST_CASE(persistence) +{ + VerbosityHolder setTemporaryLevel(10); + cnote << "Testing persistence of Whisper DB..."; + + string s; + string const text1 = "sator"; + string const text2 = "arepo"; + h256 const h1(0x12345678); + h256 const h2(0xBADD00DE); + + { + WhisperDB db; + db.kill(h1); + db.kill(h2); + s = db.lookup(h1); + BOOST_REQUIRE(s.empty()); + db.insert(h1, text2); + s = db.lookup(h2); + BOOST_REQUIRE(s.empty()); + s = db.lookup(h1); + BOOST_REQUIRE(!s.compare(text2)); + } + + this_thread::sleep_for(chrono::milliseconds(20)); + + { + WhisperDB db; + db.insert(h1, text1); + db.insert(h2, text2); + } + + this_thread::sleep_for(chrono::milliseconds(20)); + + { + WhisperDB db; + s = db.lookup(h2); + BOOST_REQUIRE(!s.compare(text2)); + s = db.lookup(h1); + BOOST_REQUIRE(!s.compare(text1)); + db.kill(h1); + db.kill(h2); + } } BOOST_AUTO_TEST_SUITE_END() From e7c5191b7627b50f67ea2d32a4d4ef703e6171c4 Mon Sep 17 00:00:00 2001 From: Vlad Gluhovsky Date: Fri, 10 Jul 2015 19:09:04 +0200 Subject: [PATCH 10/24] namespaces updated --- libwhisper/WhisperDB.cpp | 6 +++--- libwhisper/WhisperDB.h | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libwhisper/WhisperDB.cpp b/libwhisper/WhisperDB.cpp index 91c5f74f4..4bfa6caa7 100644 --- a/libwhisper/WhisperDB.cpp +++ b/libwhisper/WhisperDB.cpp @@ -31,11 +31,11 @@ WhisperDB::WhisperDB() { string path = dev::getDataDir(); boost::filesystem::create_directories(path); - ldb::Options op; + leveldb::Options op; op.create_if_missing = true; op.max_open_files = 256; - ldb::DB* p = nullptr; - leveldb::Status status = ldb::DB::Open(op, path + "/whisper", &p); + leveldb::DB* p = nullptr; + leveldb::Status status = leveldb::DB::Open(op, path + "/whisper", &p); m_db.reset(p); if (!status.ok()) BOOST_THROW_EXCEPTION(FailedToOpenLevelDB(status.ToString())); diff --git a/libwhisper/WhisperDB.h b/libwhisper/WhisperDB.h index c20f52a75..efea322a8 100644 --- a/libwhisper/WhisperDB.h +++ b/libwhisper/WhisperDB.h @@ -46,9 +46,9 @@ class WhisperDB void kill(dev::h256 const& _key); private: - ldb::ReadOptions m_readOptions; - ldb::WriteOptions m_writeOptions; - std::unique_ptr m_db; + leveldb::ReadOptions m_readOptions; + leveldb::WriteOptions m_writeOptions; + std::unique_ptr m_db; }; } From 9edfae2e6ae5e074ab102fe5e5e9510ab0482eec Mon Sep 17 00:00:00 2001 From: Vlad Gluhovsky Date: Sat, 11 Jul 2015 13:13:25 +0200 Subject: [PATCH 11/24] signatures and path --- libwhisper/WhisperDB.cpp | 4 ++-- libwhisper/WhisperDB.h | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libwhisper/WhisperDB.cpp b/libwhisper/WhisperDB.cpp index 4bfa6caa7..fd2eef060 100644 --- a/libwhisper/WhisperDB.cpp +++ b/libwhisper/WhisperDB.cpp @@ -29,13 +29,13 @@ using namespace dev::shh; WhisperDB::WhisperDB() { - string path = dev::getDataDir(); + string path = dev::getDataDir("shh"); boost::filesystem::create_directories(path); leveldb::Options op; op.create_if_missing = true; op.max_open_files = 256; leveldb::DB* p = nullptr; - leveldb::Status status = leveldb::DB::Open(op, path + "/whisper", &p); + leveldb::Status status = leveldb::DB::Open(op, path + "/messages", &p); m_db.reset(p); if (!status.ok()) BOOST_THROW_EXCEPTION(FailedToOpenLevelDB(status.ToString())); diff --git a/libwhisper/WhisperDB.h b/libwhisper/WhisperDB.h index efea322a8..0cb97e244 100644 --- a/libwhisper/WhisperDB.h +++ b/libwhisper/WhisperDB.h @@ -30,10 +30,10 @@ namespace dev namespace shh { -struct FailedToOpenLevelDB: virtual Exception { FailedToOpenLevelDB(std::string _message = std::string()): Exception(_message) {} }; -struct FailedInsertInLevelDB: virtual Exception { FailedInsertInLevelDB(std::string _message = std::string()): Exception(_message) {} }; -struct FailedLookupInLevelDB: virtual Exception { FailedLookupInLevelDB(std::string _message = std::string()): Exception(_message) {} }; -struct FailedDeleteInLevelDB: virtual Exception { FailedDeleteInLevelDB(std::string _message = std::string()): Exception(_message) {} }; +struct FailedToOpenLevelDB: virtual Exception { FailedToOpenLevelDB(std::string const& _message): Exception(_message) {} }; +struct FailedInsertInLevelDB: virtual Exception { FailedInsertInLevelDB(std::string const& _message): Exception(_message) {} }; +struct FailedLookupInLevelDB: virtual Exception { FailedLookupInLevelDB(std::string const& _message): Exception(_message) {} }; +struct FailedDeleteInLevelDB: virtual Exception { FailedDeleteInLevelDB(std::string const& _message): Exception(_message) {} }; class WhisperDB { From f8a3824f02d84cb1e8e99e62a18d591b05b73344 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 14 Jul 2015 11:58:16 +0200 Subject: [PATCH 12/24] Use only a single stack slot for storage references. --- libsolidity/ArrayUtils.cpp | 22 ++++-------- libsolidity/ArrayUtils.h | 4 +-- libsolidity/CompilerUtils.cpp | 16 +++------ libsolidity/ExpressionCompiler.cpp | 7 ---- libsolidity/LValue.cpp | 54 +++++++++++++++--------------- libsolidity/Types.cpp | 14 ++------ libsolidity/Types.h | 4 --- 7 files changed, 41 insertions(+), 80 deletions(-) diff --git a/libsolidity/ArrayUtils.cpp b/libsolidity/ArrayUtils.cpp index f13b28173..48ee5a052 100644 --- a/libsolidity/ArrayUtils.cpp +++ b/libsolidity/ArrayUtils.cpp @@ -37,7 +37,7 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons // this copies source to target and also clears target if it was larger // need to leave "target_ref target_byte_off" on the stack at the end - // stack layout: [source_ref] [source_byte_off] [source length] target_ref target_byte_off (top) + // stack layout: [source_ref] [source length] target_ref (top) solAssert(_targetType.location() == DataLocation::Storage, ""); IntegerType uint256(256); @@ -53,16 +53,11 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons bool haveByteOffsetTarget = !directCopy && targetBaseType->getStorageBytes() <= 16; unsigned byteOffsetSize = (haveByteOffsetSource ? 1 : 0) + (haveByteOffsetTarget ? 1 : 0); - // stack: source_ref [source_byte_off] [source_length] target_ref target_byte_off + // stack: source_ref [source_length] target_ref // store target_ref - // arrays always start at zero byte offset, pop offset - m_context << eth::Instruction::POP; for (unsigned i = _sourceType.getSizeOnStack(); i > 0; --i) m_context << eth::swapInstruction(i); - // stack: target_ref source_ref [source_byte_off] [source_length] - if (sourceIsStorage) - // arrays always start at zero byte offset, pop offset - m_context << eth::Instruction::POP; + // stack: target_ref source_ref [source_length] // stack: target_ref source_ref [source_length] // retrieve source length if (_sourceType.location() != DataLocation::CallData || !_sourceType.isDynamicallySized()) @@ -90,7 +85,6 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons m_context << eth::Instruction::POP << eth::Instruction::POP << eth::Instruction::POP << eth::Instruction::POP; - m_context << u256(0); return; } // compute hashes (data positions) @@ -136,13 +130,11 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons solAssert(byteOffsetSize == 0, "Byte offset for array as base type."); auto const& sourceBaseArrayType = dynamic_cast(*sourceBaseType); m_context << eth::Instruction::DUP3; - if (sourceIsStorage) - m_context << u256(0); - else if (sourceBaseArrayType.location() == DataLocation::Memory) + if (sourceBaseArrayType.location() == DataLocation::Memory) m_context << eth::Instruction::MLOAD; - m_context << eth::dupInstruction(sourceIsStorage ? 4 : 3) << u256(0); + m_context << eth::Instruction::DUP3; copyArrayToStorage(dynamic_cast(*targetBaseType), sourceBaseArrayType); - m_context << eth::Instruction::POP << eth::Instruction::POP; + m_context << eth::Instruction::POP; } else if (directCopy) { @@ -235,7 +227,6 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons // stack: target_ref target_data_end target_data_pos_updated clearStorageLoop(*targetBaseType); m_context << eth::Instruction::POP; - m_context << u256(0); } void ArrayUtils::copyArrayToMemory(const ArrayType& _sourceType, bool _padToWordBoundaries) const @@ -365,7 +356,6 @@ void ArrayUtils::copyArrayToMemory(const ArrayType& _sourceType, bool _padToWord u256 storageSize = _sourceType.getBaseType()->getStorageSize(); solAssert(storageSize > 1 || (storageSize == 1 && storageBytes > 0), ""); - m_context << eth::Instruction::POP; // remove offset, arrays always start new slot retrieveLength(_sourceType); // stack here: memory_offset storage_offset length // jump to end if length is zero diff --git a/libsolidity/ArrayUtils.h b/libsolidity/ArrayUtils.h index c047fdcc0..80ffc008a 100644 --- a/libsolidity/ArrayUtils.h +++ b/libsolidity/ArrayUtils.h @@ -41,8 +41,8 @@ public: /// Copies an array to an array in storage. The arrays can be of different types only if /// their storage representation is the same. - /// Stack pre: source_reference [source_byte_offset/source_length] target_reference target_byte_offset - /// Stack post: target_reference target_byte_offset + /// Stack pre: source_reference [source_length] target_reference + /// Stack post: target_reference void copyArrayToStorage(ArrayType const& _targetType, ArrayType const& _sourceType) const; /// Copies the data part of an array (which cannot be dynamically nested) from anywhere /// to a given position in memory. diff --git a/libsolidity/CompilerUtils.cpp b/libsolidity/CompilerUtils.cpp index 297e6aa09..e62f59e23 100644 --- a/libsolidity/CompilerUtils.cpp +++ b/libsolidity/CompilerUtils.cpp @@ -229,7 +229,7 @@ void CompilerUtils::encodeToMemory( if (arrayType.location() == DataLocation::CallData) m_context << eth::Instruction::DUP2; // length is on stack else if (arrayType.location() == DataLocation::Storage) - m_context << eth::Instruction::DUP3 << eth::Instruction::SLOAD; + m_context << eth::Instruction::DUP2 << eth::Instruction::SLOAD; else { solAssert(arrayType.location() == DataLocation::Memory, ""); @@ -416,13 +416,6 @@ void CompilerUtils::convertType(Type const& _typeOnStack, Type const& _targetTyp { // stack: (variably sized) unsigned stackSize = typeOnStack.getSizeOnStack(); - bool fromStorage = (typeOnStack.location() == DataLocation::Storage); - if (fromStorage) - { - stackSize--; - // remove storage offset, as requested by ArrayUtils::retrieveLength - m_context << eth::Instruction::POP; - } ArrayUtils(m_context).retrieveLength(typeOnStack); // allocate memory @@ -446,8 +439,6 @@ void CompilerUtils::convertType(Type const& _typeOnStack, Type const& _targetTyp { solAssert(typeOnStack.getBaseType()->isValueType(), ""); copyToStackTop(2 + stackSize, stackSize); - if (fromStorage) - m_context << u256(0); // add byte offset again ArrayUtils(m_context).copyArrayToMemory(typeOnStack); } else @@ -462,6 +453,8 @@ void CompilerUtils::convertType(Type const& _typeOnStack, Type const& _targetTyp copyToStackTop(3 + stackSize, stackSize); copyToStackTop(2 + stackSize, 1); ArrayUtils(m_context).accessIndex(typeOnStack, false); + if (typeOnStack.location() == DataLocation::Storage) + StorageItem(m_context, *typeOnStack.getBaseType()).retrieveValue(SourceLocation(), true); convertType(*typeOnStack.getBaseType(), *targetType.getBaseType(), _cleanupNeeded); storeInMemoryDynamic(*targetType.getBaseType(), true); m_context << eth::Instruction::SWAP1 << u256(1) << eth::Instruction::ADD; @@ -512,8 +505,7 @@ void CompilerUtils::convertType(Type const& _typeOnStack, Type const& _targetTyp if (typeOnStack.location() != DataLocation::Memory) { solAssert(typeOnStack.location() == DataLocation::Storage, ""); - // stack: - m_context << eth::Instruction::POP; + // stack: m_context << typeOnStack.memorySize(); allocateMemory(); m_context << eth::Instruction::SWAP1 << eth::Instruction::DUP2; diff --git a/libsolidity/ExpressionCompiler.cpp b/libsolidity/ExpressionCompiler.cpp index 058358187..d949265d2 100644 --- a/libsolidity/ExpressionCompiler.cpp +++ b/libsolidity/ExpressionCompiler.cpp @@ -714,7 +714,6 @@ void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess) { case DataLocation::Storage: { - m_context << eth::Instruction::POP; // structs always align to new slot pair const& offsets = type.getStorageOffsetsOfMember(member); m_context << offsets.first << eth::Instruction::ADD << u256(offsets.second); setLValueToStorageItem(_memberAccess); @@ -792,8 +791,6 @@ bool ExpressionCompiler::visit(IndexAccess const& _indexAccess) Type const& baseType = *_indexAccess.getBaseExpression().getType(); if (baseType.getCategory() == Type::Category::Mapping) { - // storage byte offset is ignored for mappings, it should be zero. - m_context << eth::Instruction::POP; // stack: storage_base_ref Type const& keyType = *dynamic_cast(baseType).getKeyType(); m_context << u256(0); // memory position @@ -812,10 +809,6 @@ bool ExpressionCompiler::visit(IndexAccess const& _indexAccess) ArrayType const& arrayType = dynamic_cast(baseType); solAssert(_indexAccess.getIndexExpression(), "Index expression expected."); - // remove storage byte offset - if (arrayType.location() == DataLocation::Storage) - m_context << eth::Instruction::POP; - _indexAccess.getIndexExpression()->accept(*this); // stack layout: [] ArrayUtils(m_context).accessIndex(arrayType); diff --git a/libsolidity/LValue.cpp b/libsolidity/LValue.cpp index 82701ea95..c754bbfca 100644 --- a/libsolidity/LValue.cpp +++ b/libsolidity/LValue.cpp @@ -152,7 +152,14 @@ void StorageItem::retrieveValue(SourceLocation const&, bool _remove) const { // stack: storage_key storage_offset if (!m_dataType.isValueType()) - return; // no distinction between value and reference for non-value types + { + solAssert(m_dataType.getSizeOnStack() == 1, "Invalid storage ref size."); + if (_remove) + m_context << eth::Instruction::POP; // remove byte offset + else + m_context << eth::Instruction::DUP2; + return; + } if (!_remove) CompilerUtils(m_context).copyToStackTop(sizeOnStack(), sizeOnStack()); if (m_dataType.getStorageBytes() == 32) @@ -236,16 +243,18 @@ void StorageItem::storeValue(Type const& _sourceType, SourceLocation const& _loc "Wrong type conversation for assignment."); if (m_dataType.getCategory() == Type::Category::Array) { + m_context << eth::Instruction::POP; // remove byte offset ArrayUtils(m_context).copyArrayToStorage( dynamic_cast(m_dataType), dynamic_cast(_sourceType)); if (_move) - utils.popStackElement(m_dataType); + m_context << eth::Instruction::POP; } else if (m_dataType.getCategory() == Type::Category::Struct) { - // stack layout: source_ref [source_offset] target_ref target_offset - // note that we have structs, so offsets should be zero and are ignored + // stack layout: source_ref target_ref target_offset + // note that we have structs, so offset should be zero and are ignored + m_context << eth::Instruction::POP; auto const& structType = dynamic_cast(m_dataType); auto const& sourceType = dynamic_cast(_sourceType); solAssert( @@ -262,44 +271,37 @@ void StorageItem::storeValue(Type const& _sourceType, SourceLocation const& _loc TypePointer sourceMemberType = sourceType.getMemberType(member.name); if (sourceType.location() == DataLocation::Storage) { - // stack layout: source_ref source_offset target_ref target_offset + // stack layout: source_ref target_ref pair const& offsets = sourceType.getStorageOffsetsOfMember(member.name); - m_context << offsets.first << eth::Instruction::DUP5 << eth::Instruction::ADD; + m_context << offsets.first << eth::Instruction::DUP3 << eth::Instruction::ADD; m_context << u256(offsets.second); - // stack: source_ref source_off target_ref target_off source_member_ref source_member_off + // stack: source_ref target_ref source_member_ref source_member_off StorageItem(m_context, *sourceMemberType).retrieveValue(_location, true); - // stack: source_ref source_off target_ref target_off source_value... + // stack: source_ref target_ref source_value... } else { solAssert(sourceType.location() == DataLocation::Memory, ""); - // stack layout: source_ref target_ref target_offset + // stack layout: source_ref target_ref TypePointer sourceMemberType = sourceType.getMemberType(member.name); m_context << sourceType.memoryOffsetOfMember(member.name); - m_context << eth::Instruction::DUP4 << eth::Instruction::ADD; + m_context << eth::Instruction::DUP3 << eth::Instruction::ADD; MemoryItem(m_context, *sourceMemberType).retrieveValue(_location, true); - // stack layout: source_ref target_ref target_offset source_value... + // stack layout: source_ref target_ref source_value... } unsigned stackSize = sourceMemberType->getSizeOnStack(); pair const& offsets = structType.getStorageOffsetsOfMember(member.name); - m_context << eth::dupInstruction(2 + stackSize) << offsets.first << eth::Instruction::ADD; + m_context << eth::dupInstruction(1 + stackSize) << offsets.first << eth::Instruction::ADD; m_context << u256(offsets.second); - // stack: source_ref [source_off] target_ref target_off source_value... target_member_ref target_member_byte_off + // stack: source_ref target_ref target_off source_value... target_member_ref target_member_byte_off StorageItem(m_context, *memberType).storeValue(*sourceMemberType, _location, true); } - // stack layout: source_ref [source_offset] target_ref target_offset - unsigned sourceStackSize = sourceType.getSizeOnStack(); + // stack layout: source_ref target_ref + solAssert(sourceType.getSizeOnStack() == 1, "Unexpected source size."); if (_move) - utils.popStackSlots(2 + sourceType.getSizeOnStack()); - else if (sourceType.getSizeOnStack() >= 1) - { - // remove the source ref - solAssert(sourceStackSize <= 2, "Invalid stack size."); - m_context << eth::swapInstruction(sourceStackSize); - if (sourceStackSize == 2) - m_context << eth::Instruction::POP; - m_context << eth::Instruction::SWAP2 << eth::Instruction::POP; - } + utils.popStackSlots(2); + else + m_context << eth::Instruction::SWAP1 << eth::Instruction::POP; } else BOOST_THROW_EXCEPTION( @@ -429,8 +431,6 @@ StorageArrayLength::StorageArrayLength(CompilerContext& _compilerContext, const m_arrayType(_arrayType) { solAssert(m_arrayType.isDynamicallySized(), ""); - // storage byte offset must be zero - m_context << eth::Instruction::POP; } void StorageArrayLength::retrieveValue(SourceLocation const&, bool _remove) const diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index 3ea9caa7d..0ecb01d26 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -823,11 +823,9 @@ unsigned ArrayType::getSizeOnStack() const if (m_location == DataLocation::CallData) // offset [length] (stack top) return 1 + (isDynamicallySized() ? 1 : 0); - else if (m_location == DataLocation::Storage) - // storage_key storage_offset - return 2; else - // offset + // storage slot or memory offset + // byte offset inside storage value is omitted return 1; } @@ -1035,14 +1033,6 @@ bool StructType::canLiveOutsideStorage() const return true; } -unsigned StructType::getSizeOnStack() const -{ - if (location() == DataLocation::Storage) - return 2; // slot and offset - else - return 1; -} - string StructType::toString(bool _short) const { string ret = "struct " + m_struct.getName(); diff --git a/libsolidity/Types.h b/libsolidity/Types.h index e17a262c6..e0f8785a1 100644 --- a/libsolidity/Types.h +++ b/libsolidity/Types.h @@ -298,7 +298,6 @@ public: virtual bool canBeStored() const override { return false; } virtual bool canLiveOutsideStorage() const override { return false; } - virtual unsigned getSizeOnStack() const override { return 1; } virtual std::string toString(bool _short) const override; virtual u256 literalValue(Literal const* _literal) const override; @@ -580,7 +579,6 @@ public: u256 memorySize() const; virtual u256 getStorageSize() const override; virtual bool canLiveOutsideStorage() const override; - virtual unsigned getSizeOnStack() const override; virtual std::string toString(bool _short) const override; virtual MemberList const& getMembers() const override; @@ -616,7 +614,6 @@ public: { return externalType()->getCalldataEncodedSize(_padded); } - virtual unsigned getSizeOnStack() const override { return 1; } virtual unsigned getStorageBytes() const override; virtual bool canLiveOutsideStorage() const override { return true; } virtual std::string toString(bool _short) const override; @@ -812,7 +809,6 @@ public: virtual bool operator==(Type const& _other) const override; virtual std::string toString(bool _short) const override; - virtual unsigned getSizeOnStack() const override { return 2; } virtual bool canLiveOutsideStorage() const override { return false; } TypePointer const& getKeyType() const { return m_keyType; } From f85ae2a1e402618dd2a83c9e2e9a66e76d5d693c Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 14 Jul 2015 12:29:13 +0200 Subject: [PATCH 13/24] Only dump small mem in extvm. Fixes #1196 --- eth/main.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/eth/main.cpp b/eth/main.cpp index 24520d8c4..67058a473 100644 --- a/eth/main.cpp +++ b/eth/main.cpp @@ -910,7 +910,12 @@ void interactiveMode(eth::Client* c, std::shared_ptr gasP f << endl << " STACK" << endl; for (auto i: vm->stack()) f << (h256)i << endl; - f << " MEMORY" << endl << dev::memDump(vm->memory()); + std::string memDump = ( + (vm->memory().size() > 1000) ? + " mem size greater than 1000 bytes " : + dev::memDump(vm->memory()) + ); + f << " MEMORY" << endl << memDump; f << " STORAGE" << endl; for (auto const& i: ext->state().storage(ext->myAddress)) f << showbase << hex << i.first << ": " << i.second << endl; From 1eca43e0b7b2dad3285879f8a510d5c3ac952be2 Mon Sep 17 00:00:00 2001 From: arkpar Date: Tue, 14 Jul 2015 12:53:57 +0200 Subject: [PATCH 14/24] save keys after deletion --- libethcore/KeyManager.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libethcore/KeyManager.cpp b/libethcore/KeyManager.cpp index 602c60b4a..26cf451d0 100644 --- a/libethcore/KeyManager.cpp +++ b/libethcore/KeyManager.cpp @@ -213,6 +213,7 @@ void KeyManager::kill(Address const& _a) m_addrLookup.erase(_a); m_keyInfo.erase(id); m_store.kill(id); + write(m_keysFile); } Addresses KeyManager::accounts() const From 11816618f011415b5d6d878961ba37357eb68ab1 Mon Sep 17 00:00:00 2001 From: Marek Kotewicz Date: Tue, 14 Jul 2015 13:12:37 +0200 Subject: [PATCH 15/24] rpc api test filler with fork simulation --- .../bcRPC_API_TestFiller.json | 68 ++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/test/libethereum/BlockchainTestsFiller/bcRPC_API_TestFiller.json b/test/libethereum/BlockchainTestsFiller/bcRPC_API_TestFiller.json index b31f1fa48..4a08a59df 100644 --- a/test/libethereum/BlockchainTestsFiller/bcRPC_API_TestFiller.json +++ b/test/libethereum/BlockchainTestsFiller/bcRPC_API_TestFiller.json @@ -27,6 +27,7 @@ }, "blocks" : [ { + "blocknumber" : "1", "transactions" : [ { "data" : "create contract: 6295ee1b4f6dd65047762f924ecd367c17eabf8f", @@ -42,7 +43,8 @@ "uncleHeaders" : [ ] }, - { + { + "blocknumber" : "2", "transactions" : [ { "data" : "getBool", @@ -59,6 +61,7 @@ ] }, { + "blocknumber" : "3", "transactions" : [ { "data" : "getInt8", @@ -75,6 +78,7 @@ ] }, { + "blocknumber" : "4", "transactions" : [ { "data" : "getUint8", @@ -127,6 +131,7 @@ ] }, { + "blocknumber" : "5", "transactions" : [ { "data" : "getInt256", @@ -143,6 +148,7 @@ ] }, { + "blocknumber" : "6", "transactions" : [ { "data" : "getUint256", @@ -159,6 +165,7 @@ ] }, { + "blocknumber" : "7", "transactions" : [ { "data" : "getAddress", @@ -175,6 +182,7 @@ ] }, { + "blocknumber" : "8", "transactions" : [ { "data" : "getBytes32", @@ -191,6 +199,7 @@ ] }, { + "blocknumber" : "9", "transactions" : [ { "data" : "setBool", @@ -207,6 +216,7 @@ ] }, { + "blocknumber" : "10", "transactions" : [ { "data" : "setBool", @@ -223,6 +233,7 @@ ] }, { + "blocknumber" : "11", "transactions" : [ { "data" : "setInt8", @@ -239,6 +250,7 @@ ] }, { + "blocknumber" : "12", "transactions" : [ { "data" : "setUint8", @@ -255,6 +267,7 @@ ] }, { + "blocknumber" : "13", "transactions" : [ { "data" : "setInt256", @@ -271,6 +284,7 @@ ] }, { + "blocknumber" : "14", "transactions" : [ { "data" : "setUint256", @@ -287,6 +301,7 @@ ] }, { + "blocknumber" : "15", "transactions" : [ { "data" : "setAddress", @@ -303,6 +318,7 @@ ] }, { + "blocknumber" : "16", "transactions" : [ { "data" : "setBytes32", @@ -319,6 +335,7 @@ ] }, { + "blocknumber" : "17", "transactions" : [ { "data" : "getInt8", @@ -335,6 +352,7 @@ ] }, { + "blocknumber" : "18", "transactions" : [ { "data" : "getUint8", @@ -351,6 +369,7 @@ ] }, { + "blocknumber" : "19", "transactions" : [ { "data" : "getInt256", @@ -367,6 +386,7 @@ ] }, { + "blocknumber" : "20", "transactions" : [ { "data" : "getUint256", @@ -383,6 +403,7 @@ ] }, { + "blocknumber" : "21", "transactions" : [ { "data" : "getAddress", @@ -399,6 +420,7 @@ ] }, { + "blocknumber" : "22", "transactions" : [ { "data" : "getBytes32", @@ -415,6 +437,7 @@ ] }, { + "blocknumber" : "23", "transactions" : [ { "data" : "log0", @@ -431,6 +454,7 @@ ] }, { + "blocknumber" : "24", "transactions" : [ { "data" : "log0a", @@ -447,6 +471,7 @@ ] }, { + "blocknumber" : "25", "transactions" : [ { "data" : "log1", @@ -463,6 +488,7 @@ ] }, { + "blocknumber" : "26", "transactions" : [ { "data" : "log1a", @@ -479,6 +505,41 @@ ] }, { + "blocknumber" : "27", + "transactions" : [ + { + "data" : "log1", + "data" : "0x4e7ad367", + "gasLimit" : "314159", + "gasPrice" : "1", + "nonce" : "26", + "secretKey" : "45a915e4d060149eb4365960e6a7a45f334393093061116b197e3240065ff2d8", + "to" : "6295ee1b4f6dd65047762f924ecd367c17eabf8f", + "value" : "10" + } + ], + "uncleHeaders" : [ + ] + }, + { + "blocknumber" : "28", + "transactions" : [ + { + "data" : "log1a", + "data" : "0x4e7ad367", + "gasLimit" : "314159", + "gasPrice" : "1", + "nonce" : "27", + "secretKey" : "45a915e4d060149eb4365960e6a7a45f334393093061116b197e3240065ff2d8", + "to" : "6295ee1b4f6dd65047762f924ecd367c17eabf8f", + "value" : "10" + } + ], + "uncleHeaders" : [ + ] + }, + { + "blocknumber" : "27", "transactions" : [ { "data" : "log2", @@ -495,6 +556,7 @@ ] }, { + "blocknumber" : "28", "transactions" : [ { "data" : "log2a", @@ -511,6 +573,7 @@ ] }, { + "blocknumber" : "29", "transactions" : [ { "data" : "log3", @@ -527,6 +590,7 @@ ] }, { + "blocknumber" : "30", "transactions" : [ { "data" : "log3a", @@ -543,6 +607,7 @@ ] }, { + "blocknumber" : "31", "transactions" : [ { "data" : "log4", @@ -559,6 +624,7 @@ ] }, { + "blocknumber" : "32", "transactions" : [ { "data" : "log4a", From 76ad4daededb483dfcb656d2a6f9bc7468d6ede6 Mon Sep 17 00:00:00 2001 From: arkpar Date: Tue, 14 Jul 2015 13:13:32 +0200 Subject: [PATCH 16/24] Allow 0x prefix for address entry in AZ --- alethzero/MainWin.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/alethzero/MainWin.cpp b/alethzero/MainWin.cpp index 60c07fd8d..6b0fc7989 100644 --- a/alethzero/MainWin.cpp +++ b/alethzero/MainWin.cpp @@ -638,18 +638,22 @@ pair Main::fromString(std::string const& _n) const if (_n == "(Create Contract)") return make_pair(Address(), bytes()); + std::string n = _n; + if (n.find_first_of("0x") == 0) + n.erase(0, 2); + auto g_newNameReg = getNameReg(); if (g_newNameReg) { - Address a = abiOut
(ethereum()->call(g_newNameReg, abiIn("addr(bytes32)", ::toString32(_n))).output); + Address a = abiOut
(ethereum()->call(g_newNameReg, abiIn("addr(bytes32)", ::toString32(n))).output); if (a) return make_pair(a, bytes()); } - if (_n.size() == 40) + if (n.size() == 40) { try { - return make_pair(Address(fromHex(_n, WhenError::Throw)), bytes()); + return make_pair(Address(fromHex(n, WhenError::Throw)), bytes()); } catch (BadHexCharacter& _e) { @@ -665,7 +669,7 @@ pair Main::fromString(std::string const& _n) const } else try { - return ICAP::decoded(_n).address([&](Address const& a, bytes const& b) -> bytes + return ICAP::decoded(n).address([&](Address const& a, bytes const& b) -> bytes { return ethereum()->call(a, b).output; }, g_newNameReg); From cc635b2d6af8490edd90d969214016dc4d2e5331 Mon Sep 17 00:00:00 2001 From: Marek Kotewicz Date: Tue, 14 Jul 2015 14:10:20 +0200 Subject: [PATCH 17/24] mark blocks in RPC_API_TestFiller as reverted --- .../libethereum/BlockchainTestsFiller/bcRPC_API_TestFiller.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/libethereum/BlockchainTestsFiller/bcRPC_API_TestFiller.json b/test/libethereum/BlockchainTestsFiller/bcRPC_API_TestFiller.json index 4a08a59df..2b867e4ae 100644 --- a/test/libethereum/BlockchainTestsFiller/bcRPC_API_TestFiller.json +++ b/test/libethereum/BlockchainTestsFiller/bcRPC_API_TestFiller.json @@ -506,6 +506,7 @@ }, { "blocknumber" : "27", + "reverted": true, "transactions" : [ { "data" : "log1", @@ -523,6 +524,7 @@ }, { "blocknumber" : "28", + "reverted": true, "transactions" : [ { "data" : "log1a", From 231b70866666dc4372baf9c88d3d05e14b555f39 Mon Sep 17 00:00:00 2001 From: arkpar Date: Tue, 14 Jul 2015 14:30:30 +0200 Subject: [PATCH 18/24] find instead of find_firt_of --- alethzero/MainWin.cpp | 2 +- test/TestHelper.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/alethzero/MainWin.cpp b/alethzero/MainWin.cpp index 6b0fc7989..dd3d5bbd3 100644 --- a/alethzero/MainWin.cpp +++ b/alethzero/MainWin.cpp @@ -639,7 +639,7 @@ pair Main::fromString(std::string const& _n) const return make_pair(Address(), bytes()); std::string n = _n; - if (n.find_first_of("0x") == 0) + if (n.find("0x") == 0) n.erase(0, 2); auto g_newNameReg = getNameReg(); diff --git a/test/TestHelper.cpp b/test/TestHelper.cpp index 9a1b16935..2ed1ed4d0 100644 --- a/test/TestHelper.cpp +++ b/test/TestHelper.cpp @@ -479,7 +479,7 @@ bytes importCode(json_spirit::mObject& _o) { bytes code; if (_o["code"].type() == json_spirit::str_type) - if (_o["code"].get_str().find_first_of("0x") != 0) + if (_o["code"].get_str().find("0x") != 0) code = compileLLL(_o["code"].get_str(), false); else code = fromHex(_o["code"].get_str().substr(2)); From 1887c600409e40d8a6100a2a74e64f55cbf4e0e3 Mon Sep 17 00:00:00 2001 From: Vlad Gluhovsky Date: Tue, 14 Jul 2015 15:22:15 +0200 Subject: [PATCH 19/24] test fixed --- test/libp2p/peer.cpp | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/test/libp2p/peer.cpp b/test/libp2p/peer.cpp index e84e86027..5417450b4 100644 --- a/test/libp2p/peer.cpp +++ b/test/libp2p/peer.cpp @@ -43,28 +43,32 @@ BOOST_AUTO_TEST_CASE(host) if (test::Options::get().nonetwork) return; - VerbosityHolder sentinel(10); - + VerbosityHolder setTemporaryLevel(10); NetworkPreferences host1prefs("127.0.0.1", 30301, false); - NetworkPreferences host2prefs("127.0.0.1", 30302, false); - + NetworkPreferences host2prefs("127.0.0.1", 30302, false); Host host1("Test", host1prefs); - host1.start(); - Host host2("Test", host2prefs); - auto node2 = host2.id(); + host1.start(); host2.start(); - - while (!host2.haveNetwork()) - this_thread::sleep_for(chrono::milliseconds(20)); + auto node2 = host2.id(); + int const step = 10; + + for (int i = 0; i < 3000 && (!host1.isStarted() || !host2.isStarted()); i += step) + this_thread::sleep_for(chrono::milliseconds(step)); + + BOOST_REQUIRE(host1.isStarted() && host2.isStarted()); host1.addNode(node2, NodeIPEndpoint(bi::address::from_string("127.0.0.1"), host2prefs.listenPort, host2prefs.listenPort)); - this_thread::sleep_for(chrono::seconds(3)); - - auto host1peerCount = host1.peerCount(); - auto host2peerCount = host2.peerCount(); - BOOST_REQUIRE_EQUAL(host1peerCount, 1); - BOOST_REQUIRE_EQUAL(host2peerCount, 1); + for (int i = 0; i < 3000 && (!host1.haveNetwork() || !host2.haveNetwork()); i += step) + this_thread::sleep_for(chrono::milliseconds(step)); + + BOOST_REQUIRE(host1.haveNetwork() && host2.haveNetwork()); + + for (int i = 0; i < 3000 && (!host1.peerCount() || !host2.peerCount()); i += step) + this_thread::sleep_for(chrono::milliseconds(step)); + + BOOST_REQUIRE_EQUAL(host1.peerCount(), 1); + BOOST_REQUIRE_EQUAL(host2.peerCount(), 1); } BOOST_AUTO_TEST_CASE(networkConfig) From 51f75cf2f3be1137813c4d78dc6840d615e9c1ec Mon Sep 17 00:00:00 2001 From: arkpar Date: Tue, 14 Jul 2015 15:23:02 +0200 Subject: [PATCH 20/24] KeyManager::kill test --- test/libethcore/keymanager.cpp | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/test/libethcore/keymanager.cpp b/test/libethcore/keymanager.cpp index 44ee9ae51..808f060fc 100644 --- a/test/libethcore/keymanager.cpp +++ b/test/libethcore/keymanager.cpp @@ -107,11 +107,10 @@ BOOST_AUTO_TEST_CASE(KeyManagerHints) BOOST_AUTO_TEST_CASE(KeyManagerAccounts) { - KeyManager km; string password = "hardPassword"; TransientDirectory tmpDir; - km.setKeysFile(tmpDir.path()+ "keysFile.json"); + KeyManager km(tmpDir.path()+ "keysFile.json", tmpDir.path()); BOOST_CHECK_NO_THROW(km.create(password)); BOOST_CHECK(km.accounts().empty()); @@ -121,4 +120,31 @@ BOOST_AUTO_TEST_CASE(KeyManagerAccounts) km.kill(a); } +BOOST_AUTO_TEST_CASE(KeyManagerKill) +{ + string password = "hardPassword"; + TransientDirectory tmpDir; + KeyPair kp = KeyPair::create(); + + { + KeyManager km(tmpDir.path() + "keysFile.json", tmpDir.path()); + BOOST_CHECK_NO_THROW(km.create(password)); + BOOST_CHECK(km.accounts().empty()); + BOOST_CHECK(km.load(password)); + BOOST_CHECK(km.import(kp.secret(), "test")); + } + { + KeyManager km(tmpDir.path() + "keysFile.json", tmpDir.path()); + BOOST_CHECK(km.load(password)); + Addresses addresses = km.accounts(); + BOOST_CHECK(addresses.size() == 1 && addresses[0] == kp.address()); + km.kill(addresses[0]); + } + { + KeyManager km(tmpDir.path() + "keysFile.json", tmpDir.path()); + BOOST_CHECK(km.load(password)); + BOOST_CHECK(km.accounts().empty()); + } +} + BOOST_AUTO_TEST_SUITE_END() From f9bcdd00d246154d1e160913aa5bb0f7e1a55c9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Tue, 14 Jul 2015 16:07:31 +0200 Subject: [PATCH 21/24] Default LLVM_DIR cmake variable for Windows. --- CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6cd9e338f..271f3ed65 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -363,6 +363,9 @@ else() endif() if (EVMJIT) + if (CMAKE_SYSTEM_NAME STREQUAL "Windows" AND NOT DEFINED LLVM_DIR) + set(LLVM_DIR "${CMAKE_SOURCE_DIR}/extdep/install/windows/x64/share/llvm/cmake") + endif() set(EVMJIT_CPP TRUE) # include CPP-JIT connector add_subdirectory(evmjit) endif() From 248eb8866737ff997b2cbda435e21ae29e1b7016 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 14 Jul 2015 16:37:11 +0200 Subject: [PATCH 22/24] Fix comparison between bytes types. Fixes #2087 --- libsolidity/ExpressionCompiler.cpp | 19 +++++++++++-------- test/libsolidity/SolidityEndToEndTest.cpp | 16 ++++++++++++++++ 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/libsolidity/ExpressionCompiler.cpp b/libsolidity/ExpressionCompiler.cpp index 058358187..2ce681118 100644 --- a/libsolidity/ExpressionCompiler.cpp +++ b/libsolidity/ExpressionCompiler.cpp @@ -942,24 +942,27 @@ void ExpressionCompiler::appendCompareOperatorCode(Token::Value _operator, Type } else { - IntegerType const& type = dynamic_cast(_type); - bool const c_isSigned = type.isSigned(); + bool isSigned = false; + if (auto type = dynamic_cast(&_type)) + isSigned = type->isSigned(); switch (_operator) { case Token::GreaterThanOrEqual: - m_context << (c_isSigned ? eth::Instruction::SLT : eth::Instruction::LT) - << eth::Instruction::ISZERO; + m_context << + (isSigned ? eth::Instruction::SLT : eth::Instruction::LT) << + eth::Instruction::ISZERO; break; case Token::LessThanOrEqual: - m_context << (c_isSigned ? eth::Instruction::SGT : eth::Instruction::GT) - << eth::Instruction::ISZERO; + m_context << + (isSigned ? eth::Instruction::SGT : eth::Instruction::GT) << + eth::Instruction::ISZERO; break; case Token::GreaterThan: - m_context << (c_isSigned ? eth::Instruction::SGT : eth::Instruction::GT); + m_context << (isSigned ? eth::Instruction::SGT : eth::Instruction::GT); break; case Token::LessThan: - m_context << (c_isSigned ? eth::Instruction::SLT : eth::Instruction::LT); + m_context << (isSigned ? eth::Instruction::SLT : eth::Instruction::LT); break; default: BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_comment("Unknown comparison operator.")); diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index ad2175461..9f806347e 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -585,6 +585,22 @@ BOOST_AUTO_TEST_CASE(inc_dec_operators) BOOST_CHECK(callContractFunction("f()") == encodeArgs(0x53866)); } +BOOST_AUTO_TEST_CASE(bytes_comparison) +{ + char const* sourceCode = R"( + contract test { + function f() returns (bool) { + bytes2 a = "a"; + bytes2 x = "aa"; + bytes2 b = "b"; + return a < x && x < b; + } + } + )"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("f()") == encodeArgs(true)); +} + BOOST_AUTO_TEST_CASE(state_smoke_test) { char const* sourceCode = "contract test {\n" From cef5646a7ade955b1b3976643a49ccc757f0957f Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 14 Jul 2015 16:51:36 +0200 Subject: [PATCH 23/24] Improved error message for wrong argument count. Fixes #2456 --- libsolidity/AST.cpp | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 161a7d35a..0cca63a80 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -426,7 +426,14 @@ void InheritanceSpecifier::checkTypeRequirements() solAssert(base, "Base contract not available."); TypePointers parameterTypes = ContractType(*base).getConstructorType()->getParameterTypes(); if (!m_arguments.empty() && parameterTypes.size() != m_arguments.size()) - BOOST_THROW_EXCEPTION(createTypeError("Wrong argument count for constructor call.")); + BOOST_THROW_EXCEPTION(createTypeError( + "Wrong argument count for constructor call: " + + toString(m_arguments.size()) + + " arguments given but expected " + + toString(parameterTypes.size()) + + "." + )); + for (size_t i = 0; i < m_arguments.size(); ++i) if (!m_arguments[i]->getType()->isImplicitlyConvertibleTo(*parameterTypes[i])) BOOST_THROW_EXCEPTION(m_arguments[i]->createTypeError( @@ -629,7 +636,13 @@ void ModifierInvocation::checkTypeRequirements(vector if (!parameters) BOOST_THROW_EXCEPTION(createTypeError("Referenced declaration is neither modifier nor base class.")); if (parameters->size() != m_arguments.size()) - BOOST_THROW_EXCEPTION(createTypeError("Wrong argument count for modifier invocation.")); + BOOST_THROW_EXCEPTION(createTypeError( + "Wrong argument count for modifier invocation: " + + toString(m_arguments.size()) + + " arguments given but expected " + + toString(parameters->size()) + + "." + )); for (size_t i = 0; i < m_arguments.size(); ++i) if (!m_arguments[i]->getType()->isImplicitlyConvertibleTo(*(*parameters)[i]->getType())) BOOST_THROW_EXCEPTION(m_arguments[i]->createTypeError( @@ -834,7 +847,13 @@ void FunctionCall::checkTypeRequirements(TypePointers const*) // function parameters TypePointers const& parameterTypes = functionType->getParameterTypes(); if (!functionType->takesArbitraryParameters() && parameterTypes.size() != m_arguments.size()) - BOOST_THROW_EXCEPTION(createTypeError("Wrong argument count for function call.")); + BOOST_THROW_EXCEPTION(createTypeError( + "Wrong argument count for function call: " + + toString(m_arguments.size()) + + " arguments given but expected " + + toString(parameterTypes.size()) + + "." + )); if (isPositionalCall) { From 6e7bed616cf55f4acd0abe27c36489120a27064d Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 14 Jul 2015 17:43:13 +0200 Subject: [PATCH 24/24] Check whether a literal is a valid literal before using it. Fixes #2078 --- libsolidity/Types.cpp | 15 ++++++++++++ libsolidity/Types.h | 3 +++ .../SolidityNameAndTypeResolution.cpp | 24 +++++++++++++++++++ 3 files changed, 42 insertions(+) diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index 3ea9caa7d..798092fcd 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -210,6 +210,8 @@ TypePointer Type::forLiteral(Literal const& _literal) case Token::FalseLiteral: return make_shared(); case Token::Number: + if (!IntegerConstantType::isValidLiteral(_literal)) + return TypePointer(); return make_shared(_literal); case Token::StringLiteral: return make_shared(_literal); @@ -321,6 +323,19 @@ const MemberList IntegerType::AddressMemberList({ {"send", make_shared(strings{"uint"}, strings{"bool"}, FunctionType::Location::Send)} }); +bool IntegerConstantType::isValidLiteral(const Literal& _literal) +{ + try + { + bigint x(_literal.getValue()); + } + catch (...) + { + return false; + } + return true; +} + IntegerConstantType::IntegerConstantType(Literal const& _literal) { m_value = bigint(_literal.getValue()); diff --git a/libsolidity/Types.h b/libsolidity/Types.h index e17a262c6..8af5b2dc5 100644 --- a/libsolidity/Types.h +++ b/libsolidity/Types.h @@ -286,6 +286,9 @@ class IntegerConstantType: public Type public: virtual Category getCategory() const override { return Category::IntegerConstant; } + /// @returns true if the literal is a valid integer. + static bool isValidLiteral(Literal const& _literal); + explicit IntegerConstantType(Literal const& _literal); explicit IntegerConstantType(bigint _value): m_value(_value) {} diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 1e40ee4f6..0f5e4800f 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -2110,6 +2110,30 @@ BOOST_AUTO_TEST_CASE(literal_strings) BOOST_CHECK_NO_THROW(parseTextAndResolveNames(text)); } +BOOST_AUTO_TEST_CASE(invalid_integer_literal_fraction) +{ + char const* text = R"( + contract Foo { + function f() { + var x = 1.20; + } + } + )"; + BOOST_CHECK_THROW(parseTextAndResolveNames(text), TypeError); +} + +BOOST_AUTO_TEST_CASE(invalid_integer_literal_exp) +{ + char const* text = R"( + contract Foo { + function f() { + var x = 1e2; + } + } + )"; + BOOST_CHECK_THROW(parseTextAndResolveNames(text), TypeError); +} + BOOST_AUTO_TEST_SUITE_END() }