From 368532c107f383c909dde8e951693c9f42f00b0d Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Wed, 13 May 2015 00:26:43 +0300 Subject: [PATCH] Extra diagnostics, assertions and fail-safes for tracking BadRoot bug. --- libdevcrypto/MemoryDB.cpp | 34 +++++++++++++++++------ libdevcrypto/MemoryDB.h | 11 ++++++-- libdevcrypto/OverlayDB.cpp | 57 +++++++++++++++++++++++++------------- libdevcrypto/OverlayDB.h | 8 +++--- libdevcrypto/TrieDB.cpp | 2 +- libdevcrypto/TrieDB.h | 36 ++++++++++++++---------- libethereum/BlockChain.cpp | 9 +++++- libethereum/Client.cpp | 2 +- libethereum/State.cpp | 12 +++++++- test/libdevcrypto/trie.cpp | 10 +++++++ 10 files changed, 128 insertions(+), 53 deletions(-) diff --git a/libdevcrypto/MemoryDB.cpp b/libdevcrypto/MemoryDB.cpp index 4b08db083..2cf56475b 100644 --- a/libdevcrypto/MemoryDB.cpp +++ b/libdevcrypto/MemoryDB.cpp @@ -32,6 +32,7 @@ const char* DBWarn::name() { return "TDB"; } std::unordered_map MemoryDB::get() const { + ReadGuard l(x_this); std::unordered_map ret; for (auto const& i: m_main) if (!m_enforceRefs || i.second.second > 0) @@ -39,21 +40,34 @@ std::unordered_map MemoryDB::get() const return ret; } +MemoryDB& MemoryDB::operator=(MemoryDB const& _c) +{ + if (this == &_c) + return *this; + ReadGuard l(_c.x_this); + WriteGuard l2(x_this); + m_main = _c.m_main; + m_aux = _c.m_aux; + return *this; +} + std::string MemoryDB::lookup(h256 const& _h) const { + ReadGuard l(x_this); auto it = m_main.find(_h); if (it != m_main.end()) { if (!m_enforceRefs || it->second.second > 0) return it->second.first; -// else if (m_enforceRefs && m_refCount.count(it->first) && !m_refCount.at(it->first)) -// cnote << "Lookup required for value with no refs. Let's hope it's in the DB." << _h; + else + cwarn << "Lookup required for value with refcount == 0. This is probably a critical trie issue" << _h; } return std::string(); } bool MemoryDB::exists(h256 const& _h) const { + ReadGuard l(x_this); auto it = m_main.find(_h); if (it != m_main.end() && (!m_enforceRefs || it->second.second > 0)) return true; @@ -62,6 +76,7 @@ bool MemoryDB::exists(h256 const& _h) const void MemoryDB::insert(h256 const& _h, bytesConstRef _v) { + WriteGuard l(x_this); auto it = m_main.find(_h); if (it != m_main.end()) { @@ -77,34 +92,34 @@ void MemoryDB::insert(h256 const& _h, bytesConstRef _v) bool MemoryDB::kill(h256 const& _h) { + ReadGuard l(x_this); if (m_main.count(_h)) { if (m_main[_h].second > 0) + { m_main[_h].second--; + return true; + } #if ETH_PARANOIA else { // If we get to this point, then there was probably a node in the level DB which we need to remove and which we have previously // used as part of the memory-based MemoryDB. Nothing to be worried about *as long as the node exists in the DB*. dbdebug << "NOKILL-WAS" << _h; - return false; } dbdebug << "KILL" << _h << "=>" << m_main[_h].second; - return true; } else { dbdebug << "NOKILL" << _h; - return false; - } -#else - } - return true; #endif + } + return false; } void MemoryDB::purge() { + WriteGuard l(x_this); for (auto it = m_main.begin(); it != m_main.end(); ) if (it->second.second) ++it; @@ -114,6 +129,7 @@ void MemoryDB::purge() h256Hash MemoryDB::keys() const { + ReadGuard l(x_this); h256Hash ret; for (auto const& i: m_main) if (i.second.second) diff --git a/libdevcrypto/MemoryDB.h b/libdevcrypto/MemoryDB.h index 3169d8fbc..169682815 100644 --- a/libdevcrypto/MemoryDB.h +++ b/libdevcrypto/MemoryDB.h @@ -23,6 +23,7 @@ #include #include +#include #include #include #include @@ -43,6 +44,9 @@ class MemoryDB public: MemoryDB() {} + MemoryDB(MemoryDB const& _c) { operator=(_c); } + + MemoryDB& operator=(MemoryDB const& _c); void clear() { m_main.clear(); } // WARNING !!!! didn't originally clear m_refCount!!! std::unordered_map get() const; @@ -53,13 +57,14 @@ public: bool kill(h256 const& _h); void purge(); - bytes lookupAux(h256 const& _h) const { try { return m_aux.at(_h).first; } catch (...) { return bytes(); } } - void removeAux(h256 const& _h) { m_aux[_h].second = false; } - void insertAux(h256 const& _h, bytesConstRef _v) { m_aux[_h] = make_pair(_v.toBytes(), true); } + bytes lookupAux(h256 const& _h) const { ReadGuard l(x_this); auto it = m_aux.find(_h); if (it != m_aux.end() && (!m_enforceRefs || it->second.second)) return it->second.first; return bytes(); } + void removeAux(h256 const& _h) { WriteGuard l(x_this); m_aux[_h].second = false; } + void insertAux(h256 const& _h, bytesConstRef _v) { WriteGuard l(x_this); m_aux[_h] = make_pair(_v.toBytes(), true); } h256Hash keys() const; protected: + mutable SharedMutex x_this; std::unordered_map> m_main; std::unordered_map> m_aux; diff --git a/libdevcrypto/OverlayDB.cpp b/libdevcrypto/OverlayDB.cpp index 87c8a0927..957c7f0e3 100644 --- a/libdevcrypto/OverlayDB.cpp +++ b/libdevcrypto/OverlayDB.cpp @@ -19,6 +19,7 @@ * @date 2014 */ +#include #include #include #include @@ -29,6 +30,8 @@ using namespace dev; namespace dev { +h256 const EmptyTrie = sha3(rlp("")); + OverlayDB::~OverlayDB() { if (m_db.use_count() == 1 && m_db.get()) @@ -41,30 +44,41 @@ void OverlayDB::commit() { ldb::WriteBatch batch; // cnote << "Committing nodes to disk DB:"; - for (auto const& i: m_main) + DEV_READ_GUARDED(x_this) { -// cnote << i.first << "#" << m_main[i.first].second; - if (i.second.second) - batch.Put(ldb::Slice((char const*)i.first.data(), i.first.size), ldb::Slice(i.second.first.data(), i.second.first.size())); - } - for (auto const& i: m_aux) - if (i.second.second) + for (auto const& i: m_main) { - bytes b = i.first.asBytes(); - b.push_back(255); // for aux - batch.Put(bytesConstRef(&b), bytesConstRef(&i.second.first)); + if (i.second.second) + batch.Put(ldb::Slice((char const*)i.first.data(), i.first.size), ldb::Slice(i.second.first.data(), i.second.first.size())); +// cnote << i.first << "#" << m_main[i.first].second; } - m_db->Write(m_writeOptions, &batch); + for (auto const& i: m_aux) + if (i.second.second) + { + bytes b = i.first.asBytes(); + b.push_back(255); // for aux + batch.Put(bytesConstRef(&b), bytesConstRef(&i.second.first)); + } + } + + while (true) + { + ldb::Status o = m_db->Write(m_writeOptions, &batch); + if (o.ok()) + break; + cwarn << "Error writing to database. Sleeping a while then retrying. If it keeps saying this, free up some space!"; + this_thread::sleep_for(chrono::milliseconds(500)); + } m_aux.clear(); m_main.clear(); } } -bytes OverlayDB::lookupAux(h256 _h) const +bytes OverlayDB::lookupAux(h256 const& _h) const { bytes ret = MemoryDB::lookupAux(_h); if (!ret.empty()) - return ret; + return move(ret); std::string v; bytes b = _h.asBytes(); b.push_back(255); // for aux @@ -76,18 +90,19 @@ bytes OverlayDB::lookupAux(h256 _h) const void OverlayDB::rollback() { + WriteGuard l(x_this); m_main.clear(); } -std::string OverlayDB::lookup(h256 _h) const +std::string OverlayDB::lookup(h256 const& _h) const { std::string ret = MemoryDB::lookup(_h); if (ret.empty() && m_db) m_db->Get(m_readOptions, ldb::Slice((char const*)_h.data(), 32), &ret); - return ret; + return move(ret); } -bool OverlayDB::exists(h256 _h) const +bool OverlayDB::exists(h256 const& _h) const { if (MemoryDB::exists(_h)) return true; @@ -97,16 +112,20 @@ bool OverlayDB::exists(h256 _h) const return !ret.empty(); } -void OverlayDB::kill(h256 _h) +void OverlayDB::kill(h256 const& _h) { -#if ETH_PARANOIA +#if ETH_PARANOIA || 1 if (!MemoryDB::kill(_h)) { std::string ret; if (m_db) m_db->Get(m_readOptions, ldb::Slice((char const*)_h.data(), 32), &ret); - if (ret.empty()) + // No point node ref decreasing for EmptyTrie since we never bother incrementing it in the first place for + // empty storage tries. + if (ret.empty() && _h != EmptyTrie) cnote << "Decreasing DB node ref count below zero with no DB node. Probably have a corrupt Trie." << _h; + + // TODO: for 1.1: ref-counted triedb. } #else MemoryDB::kill(_h); diff --git a/libdevcrypto/OverlayDB.h b/libdevcrypto/OverlayDB.h index 7f7736ac1..2e5428bdf 100644 --- a/libdevcrypto/OverlayDB.h +++ b/libdevcrypto/OverlayDB.h @@ -46,11 +46,11 @@ public: void commit(); void rollback(); - std::string lookup(h256 _h) const; - bool exists(h256 _h) const; - void kill(h256 _h); + std::string lookup(h256 const& _h) const; + bool exists(h256 const& _h) const; + void kill(h256 const& _h); - bytes lookupAux(h256 _h) const; + bytes lookupAux(h256 const& _h) const; private: using MemoryDB::clear; diff --git a/libdevcrypto/TrieDB.cpp b/libdevcrypto/TrieDB.cpp index 6f84a3e29..719bd74ad 100644 --- a/libdevcrypto/TrieDB.cpp +++ b/libdevcrypto/TrieDB.cpp @@ -25,6 +25,6 @@ using namespace std; using namespace dev; h256 const dev::c_shaNull = sha3(rlp("")); -h256 const dev::EmptyTrie = c_shaNull; +h256 const dev::EmptyTrie = sha3(rlp("")); const char* TrieDBChannel::name() { return "-T-"; } diff --git a/libdevcrypto/TrieDB.h b/libdevcrypto/TrieDB.h index ff2bcc589..29b412bab 100644 --- a/libdevcrypto/TrieDB.h +++ b/libdevcrypto/TrieDB.h @@ -79,7 +79,7 @@ public: void open(DB* _db) { m_db = _db; } void open(DB* _db, h256 const& _root, Verification _v = Verification::Normal) { m_db = _db; setRoot(_root, _v); } - void init() { setRoot(insertNode(&RLPNull)); assert(node(m_root).size()); } + void init() { setRoot(forceInsertNode(&RLPNull)); assert(node(m_root).size()); } void setRoot(h256 const& _root, Verification _v = Verification::Normal) { @@ -88,11 +88,13 @@ public: { if (m_root == c_shaNull && !m_db->exists(m_root)) init(); - - /*std::cout << "Setting root to " << _root << " (patched to " << m_root << ")" << std::endl;*/ + } + /*std::cout << "Setting root to " << _root << " (patched to " << m_root << ")" << std::endl;*/ +#if ETH_DEBUG + if (_v == Verification::Normal) +#endif if (!node(m_root).size()) BOOST_THROW_EXCEPTION(RootNotFound()); - } } /// True if the trie is uninitialised (i.e. that the DB doesn't contain the root node). @@ -282,11 +284,17 @@ private: std::string deref(RLP const& _n) const; std::string node(h256 _h) const { return m_db->lookup(_h); } - void insertNode(h256 _h, bytesConstRef _v) { m_db->insert(_h, _v); } - void killNode(h256 _h) { m_db->kill(_h); } - h256 insertNode(bytesConstRef _v) { auto h = sha3(_v); insertNode(h, _v); return h; } - void killNode(RLP const& _d) { if (_d.data().size() >= 32) killNode(sha3(_d.data())); } + // These are low-level node insertion functions that just go straight through into the DB. + h256 forceInsertNode(bytesConstRef _v) { auto h = sha3(_v); forceInsertNode(h, _v); return h; } + void forceInsertNode(h256 _h, bytesConstRef _v) { m_db->insert(_h, _v); } + void forceKillNode(h256 _h) { m_db->kill(_h); } + + // This are semantically-aware node insertion functions that only kills when the node's + // data is < 32 bytes. It can safely be used when pruning the trie but won't work correctly + // for the special case of the root (which is always looked up via a hash). In that case, + // use forceKillNode(). + void killNode(RLP const& _d) { if (_d.data().size() >= 32) forceKillNode(sha3(_d.data())); } h256 m_root; DB* m_db = nullptr; @@ -743,8 +751,8 @@ template void GenericTrieDB::insert(bytesConstRef _key, bytesCons // However, we know it's the root node and thus always hashed. // So, if it's less than 32 (and thus should have been deleted but wasn't) then we delete it here. if (rv.size() < 32) - killNode(m_root); - m_root = insertNode(&b); + forceKillNode(m_root); + m_root = forceInsertNode(&b); } template std::string GenericTrieDB::at(bytesConstRef _key) const @@ -890,8 +898,8 @@ template void GenericTrieDB::remove(bytesConstRef _key) if (b.size()) { if (rv.size() < 32) - killNode(m_root); - m_root = insertNode(&b); + forceKillNode(m_root); + m_root = forceInsertNode(&b); } } @@ -1081,7 +1089,7 @@ template RLPStream& GenericTrieDB::streamNode(RLPStream& _s, byte if (_b.size() < 32) _s.appendRaw(_b); else - _s.append(insertNode(&_b)); + _s.append(forceInsertNode(&_b)); return _s; } @@ -1122,7 +1130,7 @@ template bytes GenericTrieDB::graft(RLP const& _orig) // remove second item from the trie after derefrencing it into s & n. auto lh = _orig[1].toHash(); s = node(lh); - killNode(lh); + forceKillNode(lh); n = RLP(s); } assert(n.itemCount() == 2); diff --git a/libethereum/BlockChain.cpp b/libethereum/BlockChain.cpp index f13f83588..32a11ee53 100644 --- a/libethereum/BlockChain.cpp +++ b/libethereum/BlockChain.cpp @@ -466,7 +466,14 @@ ImportRoute BlockChain::import(bytes const& _block, OverlayDB const& _db, Import blb.blooms.push_back(s.receipt(i).bloom()); br.receipts.push_back(s.receipt(i)); } - s.cleanup(true); + try { + s.cleanup(true); + } + catch (BadRoot) + { + cwarn << "BadRoot error. Retrying import later."; + BOOST_THROW_EXCEPTION(FutureTime()); + } td = pd.totalDifficulty + tdIncrease; diff --git a/libethereum/Client.cpp b/libethereum/Client.cpp index a43c98aa2..6d934f3ad 100644 --- a/libethereum/Client.cpp +++ b/libethereum/Client.cpp @@ -164,7 +164,7 @@ const char* ClientDetail::name() { return EthTeal "⧫" EthCoal " ●"; } #endif Client::Client(p2p::Host* _extNet, std::string const& _dbPath, WithExisting _forceAction, u256 _networkId): - Worker("eth"), + Worker("eth", 0), m_vc(_dbPath), m_bc(_dbPath, max(m_vc.action(), _forceAction), [](unsigned d, unsigned t){ cerr << "REVISING BLOCKCHAIN: Processed " << d << " of " << t << "...\r"; }), m_gp(new TrivialGasPricer), diff --git a/libethereum/State.cpp b/libethereum/State.cpp index 584de461b..167d6236e 100644 --- a/libethereum/State.cpp +++ b/libethereum/State.cpp @@ -707,11 +707,21 @@ void State::cleanup(bool _fullCommit) { if (_fullCommit) { - paranoia("immediately before database commit", true); // Commit the new trie to disk. clog(StateTrace) << "Committing to disk: stateRoot" << m_currentBlock.stateRoot << "=" << rootHash() << "=" << toHex(asBytes(m_db.lookup(rootHash()))); + + try { + EnforceRefs er(m_db, true); + rootHash(); + } + catch (BadRoot const&) + { + clog(StateChat) << "Trie corrupt! :-("; + throw; + } + m_db.commit(); clog(StateTrace) << "Committed: stateRoot" << m_currentBlock.stateRoot << "=" << rootHash() << "=" << toHex(asBytes(m_db.lookup(rootHash()))); diff --git a/test/libdevcrypto/trie.cpp b/test/libdevcrypto/trie.cpp index 3b3fa1dd2..d41739a01 100644 --- a/test/libdevcrypto/trie.cpp +++ b/test/libdevcrypto/trie.cpp @@ -102,10 +102,13 @@ BOOST_AUTO_TEST_CASE(hex_encoded_securetrie_test) { next_permutation(ss.begin(), ss.end()); MemoryDB m; + EnforceRefs r(m, true); GenericTrieDB t(&m); MemoryDB hm; + EnforceRefs hr(hm, true); HashedGenericTrieDB ht(&hm); MemoryDB fm; + EnforceRefs fr(fm, true); FatGenericTrieDB ft(&fm); t.init(); ht.init(); @@ -164,10 +167,13 @@ BOOST_AUTO_TEST_CASE(trie_test_anyorder) { next_permutation(ss.begin(), ss.end()); MemoryDB m; + EnforceRefs r(m, true); GenericTrieDB t(&m); MemoryDB hm; + EnforceRefs hr(hm, true); HashedGenericTrieDB ht(&hm); MemoryDB fm; + EnforceRefs fr(fm, true); FatGenericTrieDB ft(&fm); t.init(); ht.init(); @@ -244,10 +250,13 @@ BOOST_AUTO_TEST_CASE(trie_tests_ordered) } MemoryDB m; + EnforceRefs r(m, true); GenericTrieDB t(&m); MemoryDB hm; + EnforceRefs hr(hm, true); HashedGenericTrieDB ht(&hm); MemoryDB fm; + EnforceRefs fr(fm, true); FatGenericTrieDB ft(&fm); t.init(); ht.init(); @@ -360,6 +369,7 @@ BOOST_AUTO_TEST_CASE(moreTrieTests) #endif { MemoryDB m; + EnforceRefs r(m, true); GenericTrieDB d(&m); d.init(); // initialise as empty tree. MemTrie t;