Reference counting fixes for Trie.

alethzero/MainWin.cpp

@@ -1053,7 +1053,7 @@ void Main::on_send_clicked()
 	u256 totalReq = value() + fee();
 	eth::ClientGuard l(&*m_client);
 	for (auto i: m_myKeys)
-		if (m_client->state().balance(i.address()) >= totalReq)
+		if (m_client->postState().balance(i.address()) >= totalReq)
 		{
 			m_client->unlock();
 			Secret s = i.secret();

libethcore/TrieDB.cpp

@@ -24,16 +24,23 @@
 using namespace std;
 using namespace eth;
 
+#define tdebug ndebug
+
 namespace eth
 {
 
 const h256 c_shaNull = sha3(rlp(""));
 
-std::string BasicMap::lookup(h256 _h, bool _enforceRefs) const
+std::string BasicMap::lookup(h256 _h) const
 {
 	auto it = m_over.find(_h);
-	if (it != m_over.end() && (!_enforceRefs || (m_refCount.count(it->first) && m_refCount.at(it->first))))
+	if (it != m_over.end())
-		return it->second;
+	{
+		if (!m_enforceRefs || (m_refCount.count(it->first) && m_refCount.at(it->first)))
+			return it->second;
+		else if (m_enforceRefs && m_refCount.count(it->first) && !m_refCount.at(it->first))
+			cwarn << "XXX Lookup required for value with no refs:" << _h.abridged();
+	}
 	return std::string();
 }
 
@@ -41,12 +48,19 @@ void BasicMap::insert(h256 _h, bytesConstRef _v)
 {
 	m_over[_h] = _v.toString();
 	m_refCount[_h]++;
+	tdebug << "INST" << _h.abridged() << "=>" << m_refCount[_h];
 }
 
 void BasicMap::kill(h256 _h)
 {
-	if (m_refCount[_h] > 0)
+	if (m_refCount.count(_h))
-		--m_refCount[_h];
+	{
+		if (m_refCount[_h] > 0)
+			--m_refCount[_h];
+		else
+			cwarn << "Decreasing DB node ref count below zero. Probably have a corrupt Trie.";
+	}
+	tdebug << "KILL" << _h.abridged() << "=>" << m_refCount[_h];
 }
 
 void BasicMap::purge()
@@ -91,9 +105,9 @@ void Overlay::rollback()
 	m_refCount.clear();
 }
 
-std::string Overlay::lookup(h256 _h, bool _enforceRefs) const
+std::string Overlay::lookup(h256 _h) const
 {
-	std::string ret = BasicMap::lookup(_h, _enforceRefs);
+	std::string ret = BasicMap::lookup(_h);
 	if (ret.empty() && m_db)
 		m_db->Get(m_readOptions, ldb::Slice((char const*)_h.data(), 32), &ret);
 	return ret;

libethcore/TrieDB.h

@@ -28,18 +28,22 @@
 #include "TrieCommon.h"
 namespace ldb = leveldb;
 
+#define tdebug ndebug
+
 namespace eth
 {
 
 class BasicMap
 {
+	friend class EnforceRefs;
+
 public:
 	BasicMap() {}
 
 	void clear() { m_over.clear(); }
 	std::map<h256, std::string> const& get() const { return m_over; }
 
-	std::string lookup(h256 _h, bool _enforceRefs = false) const;
+	std::string lookup(h256 _h) const;
 	void insert(h256 _h, bytesConstRef _v);
 	void kill(h256 _h);
 	void purge();
@@ -47,6 +51,8 @@ public:
 protected:
 	std::map<h256, std::string> m_over;
 	std::map<h256, uint> m_refCount;
+
+	bool m_enforceRefs = false;
 };
 
 inline std::ostream& operator<<(std::ostream& _out, BasicMap const& _m)
@@ -62,6 +68,7 @@ inline std::ostream& operator<<(std::ostream& _out, BasicMap const& _m)
 }
 
 class InvalidTrie: public std::exception {};
+class EnforceRefs;
 
 class Overlay: public BasicMap
 {
@@ -75,7 +82,7 @@ public:
 	void commit();
 	void rollback();
 
-	std::string lookup(h256 _h, bool _enforceRefs = false) const;
+	std::string lookup(h256 _h) const;
 
 private:
 	using BasicMap::clear;
@@ -86,6 +93,17 @@ private:
 	ldb::WriteOptions m_writeOptions;
 };
 
+class EnforceRefs
+{
+public:
+	EnforceRefs(BasicMap& _o, bool _r): m_o(_o), m_r(_o.m_enforceRefs) { _o.m_enforceRefs = _r; }
+	~EnforceRefs() { m_o.m_enforceRefs = m_r; }
+
+private:
+	BasicMap& m_o;
+	bool m_r;
+};
+
 extern const h256 c_shaNull;
 
 /**
@@ -419,6 +437,8 @@ template <class DB> void GenericTrieDB<DB>::init()
 
 template <class DB> void GenericTrieDB<DB>::insert(bytesConstRef _key, bytesConstRef _value)
 {
+	cdebug << "Insert" << toHex(_key.cropped(0, 4));
+
 	std::string rv = node(m_root);
 	assert(rv.size());
 	bytes b = mergeAt(RLP(rv), NibbleSlice(_key), _value);
@@ -469,7 +489,7 @@ template <class DB> std::string GenericTrieDB<DB>::atAux(RLP const& _here, Nibbl
 
 template <class DB> bytes GenericTrieDB<DB>::mergeAt(RLP const& _orig, NibbleSlice _k, bytesConstRef _v)
 {
-//	::operator<<(std::cout << "mergeAt ", _orig) << _k << _v.toString() << std::endl;
+	tdebug << "mergeAt " << _orig << _k << sha3(_orig.data()).abridged();
 
 	// The caller will make sure that the bytes are inserted properly.
 	// - This might mean inserting an entry into m_over
@@ -534,6 +554,7 @@ template <class DB> bytes GenericTrieDB<DB>::mergeAt(RLP const& _orig, NibbleSli
 
 template <class DB> void GenericTrieDB<DB>::mergeAtAux(RLPStream& _out, RLP const& _orig, NibbleSlice _k, bytesConstRef _v)
 {
+	tdebug << "mergeAtAux " << _orig << _k << sha3(_orig.data()).abridged() << _orig.toHash<h256>().abridged();
 	RLP r = _orig;
 	std::string s;
 	if (!r.isList() && !r.isEmpty())
@@ -541,17 +562,15 @@ template <class DB> void GenericTrieDB<DB>::mergeAtAux(RLPStream& _out, RLP cons
 		s = node(_orig.toHash<h256>());
 		r = RLP(s);
 		assert(!r.isNull());
-		killNode(_orig.toHash<h256>());
 	}
-	else
-		killNode(_orig);
 	bytes b = mergeAt(r, _k, _v);
-//	::operator<<(std::cout, RLP(b)) << std::endl;
 	streamNode(_out, b);
 }
 
 template <class DB> void GenericTrieDB<DB>::remove(bytesConstRef _key)
 {
+	tdebug << "Remove" << toHex(_key.cropped(0, 4).toBytes());
+
 	std::string rv = node(m_root);
 	bytes b = deleteAt(RLP(rv), NibbleSlice(_key));
 	if (b.size())

libethereum/State.cpp

@@ -551,9 +551,21 @@ u256 State::playbackRaw(bytesConstRef _block, BlockInfo const& _grandParent, boo
 
 	if (_fullCommit)
 	{
+		if (!isTrieGood())
+		{
+			cwarn << "INVALID TRIE prior to database commit!";
+			throw InvalidTrie();
+		}
+
 		// Commit the new trie to disk.
 		m_db.commit();
 
+		if (!isTrieGood())
+		{
+			cwarn << "INVALID TRIE immediately after database commit!";
+			throw InvalidTrie();
+		}
+
 		m_previousBlock = m_currentBlock;
 	}
 	else
@@ -850,20 +862,41 @@ bytes const& State::code(Address _contract) const
 
 bool State::isTrieGood()
 {
-	for (auto const& i: m_state)
 	{
-		RLP r(i.second);
+		EnforceRefs r(m_db, false);
-		TrieDB<h256, Overlay> storageDB(&m_db, r[2].toHash<h256>());
+		for (auto const& i: m_state)
-		try
 		{
-			for (auto const& j: storageDB) {}
+			RLP r(i.second);
+			TrieDB<h256, Overlay> storageDB(&m_db, r[2].toHash<h256>());
+			try
+			{
+				for (auto const& j: storageDB) { (void)j; }
+			}
+			catch (InvalidTrie)
+			{
+				cwarn << "BAD TRIE [unenforced refs]";
+				return false;
+			}
+			if (r[3].toHash<h256>() != EmptySHA3 &&  m_db.lookup(r[3].toHash<h256>()).empty())
+				return false;
 		}
-		catch (InvalidTrie)
+	}
+	{
+		EnforceRefs r(m_db, true);
+		for (auto const& i: m_state)
 		{
-			return false;
+			RLP r(i.second);
+			TrieDB<h256, Overlay> storageDB(&m_db, r[2].toHash<h256>());
+			try
+			{
+				for (auto const& j: storageDB) { (void)j; }
+			}
+			catch (InvalidTrie)
+			{
+				cwarn << "BAD TRIE [enforced refs]";
+				return false;
+			}
 		}
-		if (r[3].toHash<h256>() != EmptySHA3 &&  m_db.lookup(r[3].toHash<h256>()).empty())
-			return false;
 	}
 	return true;
 }
@@ -899,7 +932,10 @@ u256 State::execute(bytesConstRef _rlp)
 	commit();
 
 	if (e.t().receiveAddress)
-		assert(!storageRoot(e.t().receiveAddress) || m_db.lookup(storageRoot(e.t().receiveAddress), true).size());
+	{
+		EnforceRefs r(m_db, true);
+		assert(!storageRoot(e.t().receiveAddress) || m_db.lookup(storageRoot(e.t().receiveAddress)).size());
+	}
 
 	cnote << "Executed; now" << rootHash();
 	cnote << old.diff(*this);