Proper fix for ref counting in trie.

libethcore/TrieDB.cpp

@@ -29,6 +29,33 @@ namespace eth
 
 const h256 c_shaNull = sha3(rlp(""));
 
+std::string BasicMap::lookup(h256 _h, bool _enforceRefs) const
+{
+	auto it = m_over.find(_h);
+	if (it != m_over.end() && (!_enforceRefs || (m_refCount.count(it->first) && m_refCount.at(it->first))))
+		return it->second;
+	return std::string();
+}
+
+void BasicMap::insert(h256 _h, bytesConstRef _v)
+{
+	m_over[_h] = _v.toString();
+	m_refCount[_h]++;
+}
+
+void BasicMap::kill(h256 _h)
+{
+	if (m_refCount[_h] > 0)
+		--m_refCount[_h];
+}
+
+void BasicMap::purge()
+{
+	for (auto const& i: m_refCount)
+		if (!i.second)
+			m_over.erase(i.first);
+}
+
 Overlay::~Overlay()
 {
 	if (m_db.use_count() == 1 && m_db.get())
@@ -46,9 +73,13 @@ void Overlay::commit()
 {
 	if (m_db)
 	{
+//		cnote << "Committing nodes to disk DB:";
 		for (auto const& i: m_over)
-//			if (m_refCount[i.first])
+		{
+//			cnote << i.first << "#" << m_refCount[i.first];
+			if (m_refCount[i.first])
 				m_db->Put(m_writeOptions, ldb::Slice((char const*)i.first.data(), i.first.size), ldb::Slice(i.second.data(), i.second.size()));
+		}
 		m_over.clear();
 		m_refCount.clear();
 	}
@@ -60,9 +91,9 @@ void Overlay::rollback()
 	m_refCount.clear();
 }
 
-std::string Overlay::lookup(h256 _h) const
+std::string Overlay::lookup(h256 _h, bool _enforceRefs) const
 {
-	std::string ret = BasicMap::lookup(_h);
+	std::string ret = BasicMap::lookup(_h, _enforceRefs);
 	if (ret.empty() && m_db)
 		m_db->Get(m_readOptions, ldb::Slice((char const*)_h.data(), 32), &ret);
 	return ret;

libethcore/TrieDB.h

@@ -39,10 +39,10 @@ public:
 	void clear() { m_over.clear(); }
 	std::map<h256, std::string> const& get() const { return m_over; }
 
-	std::string lookup(h256 _h) const { auto it = m_over.find(_h); if (it != m_over.end()) return it->second; return std::string(); }
-	void insert(h256 _h, bytesConstRef _v) { m_over[_h] = _v.toString(); m_refCount[_h]++; }
-	void kill(h256 _h) { --m_refCount[_h]; }
-	void purge() { for (auto const& i: m_refCount) if (!i.second) m_over.erase(i.first); }
+	std::string lookup(h256 _h, bool _enforceRefs = false) const;
+	void insert(h256 _h, bytesConstRef _v);
+	void kill(h256 _h);
+	void purge();
 
 protected:
 	std::map<h256, std::string> m_over;
@@ -73,7 +73,7 @@ public:
 	void commit();
 	void rollback();
 
-	std::string lookup(h256 _h) const;
+	std::string lookup(h256 _h, bool _enforceRefs = false) const;
 
 private:
 	using BasicMap::clear;
@@ -113,6 +113,7 @@ public:
 
 	void init();
 	void setRoot(h256 _root) { m_root = _root == h256() ? c_shaNull : _root; /*std::cout << "Setting root to " << _root << " (patched to " << m_root << ")" << std::endl;*/ if (!node(m_root).size()) throw RootNotFound(); }
+	bool haveRoot(h256 _root, bool _enforceRefs = true) { return _root == h256() ? true : m_db->lookup(_root, _enforceRefs).size(); }
 
 	/// True if the trie is uninitialised (i.e. that the DB doesn't contain the root node).
 	bool isNull() const { return !node(m_root).size(); }

libethereum/State.cpp

@@ -824,6 +824,17 @@ map<u256, u256> State::storage(Address _id) const
 	return ret;
 }
 
+h256 State::storageRoot(Address _id) const
+{
+	string s = m_state.at(_id);
+	if (s.size())
+	{
+		RLP r(s);
+		return r[2].toHash<h256>();
+	}
+	return h256();
+}
+
 bytes const& State::code(Address _contract) const
 {
 	if (!addressHasCode(_contract))
@@ -846,8 +857,8 @@ u256 State::execute(bytesConstRef _rlp)
 
 	cnote << "Executing " << e.t() << "on" << h;
 
-	u256 startGasUSed = gasUsed();
-	if (startGasUSed + e.t().gas > m_currentBlock.gasLimit)
+	u256 startGasUsed = gasUsed();
+	if (startGasUsed + e.t().gas > m_currentBlock.gasLimit)
 		throw BlockGasLimitReached();	// TODO: make sure this is handled.
 
 	e.go();
@@ -855,11 +866,14 @@ u256 State::execute(bytesConstRef _rlp)
 
 	commit();
 
+	if (e.t().receiveAddress)
+		assert(m_db.lookup(storageRoot(e.t().receiveAddress), true).size());
+
 	cnote << "Executed; now" << rootHash();
 	cnote << old.diff(*this);
 
 	// Add to the user-originated transactions that we've executed.
-	m_transactions.push_back(TransactionReceipt(e.t(), m_state.root(), startGasUSed + e.gasUsed()));
+	m_transactions.push_back(TransactionReceipt(e.t(), rootHash(), startGasUsed + e.gasUsed()));
 	m_transactionSet.insert(e.t().sha3());
 	return e.gasUsed();
 }
@@ -984,9 +998,6 @@ State State::fromPending(unsigned _i) const
 
 void State::applyRewards(Addresses const& _uncleAddresses)
 {
-	// Commit here so we can definitely unapply later.
-	commit();
-
 	u256 r = m_blockReward;
 	for (auto const& i: _uncleAddresses)
 	{

libethereum/State.h

@@ -198,8 +198,11 @@ public:
 	 */
 	void subBalance(Address _id, bigint _value);
 
+	/// Get the root of the storage of an account.
+	h256 storageRoot(Address _contract) const;
+
 	/// Get the value of a storage position of an account.
-	/// @returns 0 if no contract exists at that address.
+	/// @returns 0 if no account exists at that address.
 	u256 storage(Address _contract, u256 _memory) const;
 
 	/// Set the value of a storage position of an account.
@@ -207,11 +210,11 @@ public:
 
 	/// Get the storage of an account.
 	/// @note This is expensive. Don't use it unless you need to.
-	/// @returns std::map<u256, u256> if no contract exists at that address.
+	/// @returns std::map<u256, u256> if no account exists at that address.
 	std::map<u256, u256> storage(Address _contract) const;
 
 	/// Get the code of an account.
-	/// @returns bytes() if no contract exists at that address.
+	/// @returns bytes() if no account exists at that address.
 	bytes const& code(Address _contract) const;
 
 	/// Note that the given address is sending a transaction and thus increment the associated ticker.