Extra diagnostics, assertions and fail-safes for tracking BadRoot bug.

libdevcrypto/MemoryDB.cpp

@@ -32,6 +32,7 @@ const char* DBWarn::name() { return "TDB"; }
 
 std::unordered_map<h256, std::string> MemoryDB::get() const
 {
+	ReadGuard l(x_this);
 	std::unordered_map<h256, std::string> ret;
 	for (auto const& i: m_main)
 		if (!m_enforceRefs || i.second.second > 0)
@@ -39,21 +40,34 @@ std::unordered_map<h256, std::string> 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))
+		else
-//			cnote << "Lookup required for value with no refs. Let's hope it's in the DB." << _h;
+			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)

libdevcrypto/MemoryDB.h

@@ -23,6 +23,7 @@
 
 #include <unordered_map>
 #include <libdevcore/Common.h>
+#include <libdevcore/Guards.h>
 #include <libdevcore/FixedHash.h>
 #include <libdevcore/Log.h>
 #include <libdevcore/RLP.h>
@@ -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<h256, std::string> 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(); } }
+	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) { m_aux[_h].second = false; }
+	void removeAux(h256 const& _h) { WriteGuard l(x_this); m_aux[_h].second = false; }
-	void insertAux(h256 const& _h, bytesConstRef _v) { m_aux[_h] = make_pair(_v.toBytes(), true); }
+	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<h256, std::pair<std::string, unsigned>> m_main;
 	std::unordered_map<h256, std::pair<bytes, bool>> m_aux;
 

libdevcrypto/OverlayDB.cpp

@@ -19,6 +19,7 @@
  * @date 2014
  */
 
+#include <thread>
 #include <leveldb/db.h>
 #include <leveldb/write_batch.h>
 #include <libdevcore/Common.h>
@@ -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;
+			for (auto const& i: m_main)
-			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)
 			{
-				bytes b = i.first.asBytes();
+				if (i.second.second)
-				b.push_back(255);	// for aux
+					batch.Put(ldb::Slice((char const*)i.first.data(), i.first.size), ldb::Slice(i.second.first.data(), i.second.first.size()));
-				batch.Put(bytesConstRef(&b), bytesConstRef(&i.second.first));
+//				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);

libdevcrypto/OverlayDB.h

@@ -46,11 +46,11 @@ public:
 	void commit();
 	void rollback();
 
-	std::string lookup(h256 _h) const;
+	std::string lookup(h256 const& _h) const;
-	bool exists(h256 _h) const;
+	bool exists(h256 const& _h) const;
-	void kill(h256 _h);
+	void kill(h256 const& _h);
 
-	bytes lookupAux(h256 _h) const;
+	bytes lookupAux(h256 const& _h) const;
 
 private:
 	using MemoryDB::clear;

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-"; }

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; }
+	// These are low-level node insertion functions that just go straight through into the DB.
-	void killNode(RLP const& _d) { if (_d.data().size() >= 32) killNode(sha3(_d.data())); }
+	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 <class DB> void GenericTrieDB<DB>::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);
+		forceKillNode(m_root);
-	m_root = insertNode(&b);
+	m_root = forceInsertNode(&b);
 }
 
 template <class DB> std::string GenericTrieDB<DB>::at(bytesConstRef _key) const
@@ -890,8 +898,8 @@ template <class DB> void GenericTrieDB<DB>::remove(bytesConstRef _key)
 	if (b.size())
 	{
 		if (rv.size() < 32)
-			killNode(m_root);
+			forceKillNode(m_root);
-		m_root = insertNode(&b);
+		m_root = forceInsertNode(&b);
 	}
 }
 
@@ -1081,7 +1089,7 @@ template <class DB> RLPStream& GenericTrieDB<DB>::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 <class DB> bytes GenericTrieDB<DB>::graft(RLP const& _orig)
 		// remove second item from the trie after derefrencing it into s & n.
 		auto lh = _orig[1].toHash<h256>();
 		s = node(lh);
-		killNode(lh);
+		forceKillNode(lh);
 		n = RLP(s);
 	}
 	assert(n.itemCount() == 2);

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;
 

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),

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())));
 

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<MemoryDB> t(&m);
 			MemoryDB hm;
+			EnforceRefs hr(hm, true);
 			HashedGenericTrieDB<MemoryDB> ht(&hm);
 			MemoryDB fm;
+			EnforceRefs fr(fm, true);
 			FatGenericTrieDB<MemoryDB> 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<MemoryDB> t(&m);
 			MemoryDB hm;
+			EnforceRefs hr(hm, true);
 			HashedGenericTrieDB<MemoryDB> ht(&hm);
 			MemoryDB fm;
+			EnforceRefs fr(fm, true);
 			FatGenericTrieDB<MemoryDB> ft(&fm);
 			t.init();
 			ht.init();
@@ -244,10 +250,13 @@ BOOST_AUTO_TEST_CASE(trie_tests_ordered)
 		}
 
 		MemoryDB m;
+		EnforceRefs r(m, true);
 		GenericTrieDB<MemoryDB> t(&m);
 		MemoryDB hm;
+		EnforceRefs hr(hm, true);
 		HashedGenericTrieDB<MemoryDB> ht(&hm);
 		MemoryDB fm;
+		EnforceRefs fr(fm, true);
 		FatGenericTrieDB<MemoryDB> ft(&fm);
 		t.init();
 		ht.init();
@@ -360,6 +369,7 @@ BOOST_AUTO_TEST_CASE(moreTrieTests)
 #endif
 	{
 		MemoryDB m;
+		EnforceRefs r(m, true);
 		GenericTrieDB<MemoryDB> d(&m);
 		d.init();	// initialise as empty tree.
 		MemTrie t;