add best block hash to ncc trie, and remove possibility of a null base to ncc cache

ensure the ncc trie and any caches are always in sync with the current best block
by informing the trie when the block changes, and have the ncc trie persist that
in the database. check against that whenever modifying the trie.

get rid of the possibility of a CNCCTrieCache having a null base, and create a
CNCCTrie in test_bitcoin.cpp so one exists during testing.

src/main.cpp

@@ -1641,6 +1641,7 @@ static bool ApplyTxInUndo(const CTxInUndo& undo, CCoinsViewCache& view, CNCCTrie
 bool DisconnectBlock(CBlock& block, CValidationState& state, CBlockIndex* pindex, CCoinsViewCache& view, CNCCTrieCache& trieCache, bool* pfClean)
 {
     assert(pindex->GetBlockHash() == view.GetBestBlock());
+    assert(pindex->GetBlockHash() == trieCache.getBestBlock());
 
     if (pfClean)
         *pfClean = false;
@@ -1714,6 +1715,7 @@ bool DisconnectBlock(CBlock& block, CValidationState& state, CBlockIndex* pindex
 
     // move best block pointer to prevout block
     view.SetBestBlock(pindex->pprev->GetBlockHash());
+    trieCache.setBestBlock(pindex->pprev->GetBlockHash());
     assert(trieCache.getMerkleHash() == pindex->pprev->hashNCCTrie);
 
     if (pfClean) {
@@ -1773,11 +1775,17 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin
     uint256 hashPrevBlock = pindex->pprev == NULL ? uint256() : pindex->pprev->GetBlockHash();
     assert(hashPrevBlock == view.GetBestBlock());
 
+    // also verify that the trie cache's current state corresponds to the previous block
+    assert(hashPrevBlock == trieCache.getBestBlock());
+
     // Special case for the genesis block, skipping connection of its transactions
     // (its coinbase is unspendable)
     if (block.GetHash() == Params().HashGenesisBlock()) {
         if (!fJustCheck)
+        {
             view.SetBestBlock(pindex->GetBlockHash());
+            trieCache.setBestBlock(pindex->GetBlockHash());
+        }
         return true;
     }
 
@@ -1960,6 +1968,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin
 
     // add this block to the view's block chain
     view.SetBestBlock(pindex->GetBlockHash());
+    trieCache.setBestBlock(pindex->GetBlockHash());
 
     int64_t nTime3 = GetTimeMicros(); nTimeIndex += nTime3 - nTime2;
     LogPrint("bench", "    - Index writing: %.2fms [%.2fs]\n", 0.001 * (nTime3 - nTime2), nTimeIndex * 0.000001);

src/ncctrie.cpp

@@ -175,7 +175,7 @@ bool CNCCTrie::recursiveCheckConsistency(CNCCTrieNode* node)
     return calculatedHash == node->hash;
 }
 
-bool CNCCTrie::update(nodeCacheType& cache, hashMapType& hashes)
+bool CNCCTrie::update(nodeCacheType& cache, hashMapType& hashes, const uint256& hashBlockIn)
 {
     // General strategy: the cache is ordered by length, ensuring child
     // nodes are always inserted after their parents. Insert each node
@@ -196,10 +196,6 @@ bool CNCCTrie::update(nodeCacheType& cache, hashMapType& hashes)
     // appending extra data to the normal txundo, which will call the
     // normal insert/remove names, but obviously the opposite and in
     // reverse order (though the order shouldn't ever matter).
-    if (cache.empty())
-    {
-        return true;
-    }
     bool success = true;
     std::vector<std::string> deletedNames;
     for (nodeCacheType::iterator itcache = cache.begin(); itcache != cache.end(); ++itcache)
@@ -217,7 +213,8 @@ bool CNCCTrie::update(nodeCacheType& cache, hashMapType& hashes)
             return false;
         changedNodes[ithash->first] = pNode;
     }
-    BatchWrite(changedNodes, deletedNames);
+    BatchWrite(changedNodes, deletedNames, hashBlockIn);
+    hashBlock = hashBlockIn;
     return true;
 }
 
@@ -308,13 +305,14 @@ void BatchEraseNode(CLevelDBBatch& batch, const std::string& name)
     batch.Erase(std::make_pair('n', name));
 }
 
-bool CNCCTrie::BatchWrite(nodeCacheType& changedNodes, std::vector<std::string>& deletedNames)
+bool CNCCTrie::BatchWrite(nodeCacheType& changedNodes, std::vector<std::string>& deletedNames, const uint256& hashBlockIn)
 {
     CLevelDBBatch batch;
     for (nodeCacheType::iterator itcache = changedNodes.begin(); itcache != changedNodes.end(); ++itcache)
         BatchWriteNode(batch, itcache->first, itcache->second);
     for (std::vector<std::string>::iterator itname = deletedNames.begin(); itname != deletedNames.end(); ++itname)
         BatchEraseNode(batch, *itname);
+    batch.Write('h', hashBlockIn);
     return db.WriteBatch(batch);
 }
 
@@ -339,6 +337,9 @@ bool CNCCTrie::InsertFromDisk(const std::string& name, CNCCTrieNode* node)
 
 bool CNCCTrie::ReadFromDisk(bool check)
 {
+    if (!db.Read('h', hashBlock))
+        LogPrintf("%s: Couldn't read the best block's hash\n", __func__);
+    
     boost::scoped_ptr<leveldb::Iterator> pcursor(const_cast<CLevelDBWrapper*>(&db)->NewIterator());
     pcursor->SeekToFirst();
     
@@ -459,7 +460,7 @@ uint256 CNCCTrieCache::getMerkleHash() const
 
 bool CNCCTrieCache::empty() const
 {
-    return !base || (base->empty() && cache.empty());
+    return base->empty() && cache.empty();
 }
 
 bool CNCCTrieCache::insertName(const std::string name, uint256 txhash, int nOut, CAmount nAmount, int nHeight) const
@@ -673,6 +674,19 @@ bool CNCCTrieCache::recursivePruneName(CNCCTrieNode* tnCurrent, unsigned int nPo
     return true;
 }
 
+uint256 CNCCTrieCache::getBestBlock()
+{
+    if (hashBlock.IsNull())
+        if (base != NULL)
+            hashBlock = base->hashBlock;
+    return hashBlock;
+}
+
+void CNCCTrieCache::setBestBlock(const uint256& hashBlockIn)
+{
+    hashBlock = hashBlockIn;
+}
+
 bool CNCCTrieCache::clear() const
 {
     for (nodeCacheType::iterator itcache = cache.begin(); itcache != cache.end(); ++itcache)
@@ -689,9 +703,7 @@ bool CNCCTrieCache::flush()
 {
     if (dirty())
         getMerkleHash();
-    if (!base)
+    bool success = base->update(cache, cacheHashes, hashBlock);
-        return true;
-    bool success = base->update(cache, cacheHashes);
     if (success)
     {
         success = clear();

src/ncctrie.h

@@ -133,27 +133,30 @@ public:
     json_spirit::Object getInfoForName(const std::string& name) const;
     friend class CNCCTrieCache;
 private:
-    bool update(nodeCacheType& cache, hashMapType& hashes);
+    bool update(nodeCacheType& cache, hashMapType& hashes, const uint256& hashBlock);
     bool updateName(const std::string& name, CNCCTrieNode* updatedNode, std::vector<std::string>& deletedNames);
     bool updateHash(const std::string& name, uint256& hash, CNCCTrieNode** pNodeRet);
     bool recursiveNullify(CNCCTrieNode* node, std::string& name, std::vector<std::string>& deletedNames);
     bool recursiveCheckConsistency(CNCCTrieNode* node);
-    bool BatchWrite(nodeCacheType& changedNodes, std::vector<std::string>& deletedNames);
+    bool BatchWrite(nodeCacheType& changedNodes, std::vector<std::string>& deletedNames, const uint256& hashBlock);
     bool InsertFromDisk(const std::string& name, CNCCTrieNode* node);
     bool recursiveDumpToJSON(const std::string& name, const CNCCTrieNode* current, json_spirit::Array& ret) const;
     CNCCTrieNode root;
+    uint256 hashBlock;
 };
 
 class CNCCTrieCache
 {
 public:
-    CNCCTrieCache(CNCCTrie* base): base(base) {}
+    CNCCTrieCache(CNCCTrie* base): base(base) {assert(base);}
     uint256 getMerkleHash() const;
     bool empty() const;
     bool flush();
     bool dirty() const { return !dirtyHashes.empty(); }
     bool insertName (const std::string name, uint256 txhash, int nOut, CAmount nAmount, int nDepth) const;
     bool removeName (const std::string name, uint256 txhash, int nOut) const;
+    uint256 getBestBlock();
+    void setBestBlock(const uint256& hashBlock);
     ~CNCCTrieCache() { clear(); }
 private:
     CNCCTrie* base;
@@ -164,6 +167,7 @@ private:
     bool recursiveComputeMerkleHash(CNCCTrieNode* tnCurrent, std::string sPos) const;
     bool recursivePruneName(CNCCTrieNode* tnCurrent, unsigned int nPos, std::string sName, bool* pfNullified = NULL) const;
     bool clear() const;
+    uint256 hashBlock;
 };
 
 #endif // BITCOIN_NCCTRIE_H

src/test/ncctrie_tests.cpp

@@ -2,6 +2,7 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://opensource.org/licenses/mit-license.php
 
+#include "main.h"
 #include "primitives/transaction.h"
 #include "ncctrie.h"
 #include "coins.h"
@@ -52,15 +53,13 @@ BOOST_AUTO_TEST_CASE(ncctrie_create_insert_remove)
     uint256 hash4;
     hash4.SetHex("a79e8a5b28f7fa5e8836a4b48da9988bdf56ce749f81f413cb754f963a516200");
 
-    CNCCTrie trie;
+    BOOST_CHECK(pnccTrie->empty());
 
-    BOOST_CHECK(trie.empty());
+    CNCCTrieCache ntState(pnccTrie);
-
-    CNCCTrieCache ntState(&trie);
     ntState.insertName(std::string("test"), tx1.GetHash(), 0, 50, 100);
     ntState.insertName(std::string("test2"), tx2.GetHash(), 0, 50, 100);
 
-    BOOST_CHECK(trie.empty());
+    BOOST_CHECK(pnccTrie->empty());
     BOOST_CHECK(!ntState.empty());
     BOOST_CHECK(ntState.getMerkleHash() == hash1);
 
@@ -73,11 +72,11 @@ BOOST_AUTO_TEST_CASE(ncctrie_create_insert_remove)
     BOOST_CHECK(ntState.getMerkleHash() == hash2);
     ntState.flush();
 
-    BOOST_CHECK(!trie.empty());
+    BOOST_CHECK(!pnccTrie->empty());
-    BOOST_CHECK(trie.getMerkleHash() == hash2);
+    BOOST_CHECK(pnccTrie->getMerkleHash() == hash2);
-    BOOST_CHECK(trie.checkConsistency());
+    BOOST_CHECK(pnccTrie->checkConsistency());
 
-    CNCCTrieCache ntState1(&trie);
+    CNCCTrieCache ntState1(pnccTrie);
     ntState1.removeName(std::string("test"), tx1.GetHash(), 0);
     ntState1.removeName(std::string("test2"), tx2.GetHash(), 0);
     ntState1.removeName(std::string("test"), tx3.GetHash(), 0);
@@ -85,7 +84,7 @@ BOOST_AUTO_TEST_CASE(ncctrie_create_insert_remove)
 
     BOOST_CHECK(ntState1.getMerkleHash() == hash0);
 
-    CNCCTrieCache ntState2(&trie);
+    CNCCTrieCache ntState2(pnccTrie);
     ntState2.insertName(std::string("abab"), tx6.GetHash(), 0, 50, 100);
     ntState2.removeName(std::string("test"), tx1.GetHash(), 0);
 
@@ -93,43 +92,43 @@ BOOST_AUTO_TEST_CASE(ncctrie_create_insert_remove)
 
     ntState2.flush();
 
-    BOOST_CHECK(!trie.empty());
+    BOOST_CHECK(!pnccTrie->empty());
-    BOOST_CHECK(trie.getMerkleHash() == hash3);
+    BOOST_CHECK(pnccTrie->getMerkleHash() == hash3);
-    BOOST_CHECK(trie.checkConsistency());
+    BOOST_CHECK(pnccTrie->checkConsistency());
 
-    CNCCTrieCache ntState3(&trie);
+    CNCCTrieCache ntState3(pnccTrie);
     ntState3.insertName(std::string("test"), tx1.GetHash(), 0, 50, 100);
     BOOST_CHECK(ntState3.getMerkleHash() == hash4);
     ntState3.flush();
-    BOOST_CHECK(!trie.empty());
+    BOOST_CHECK(!pnccTrie->empty());
-    BOOST_CHECK(trie.getMerkleHash() == hash4);
+    BOOST_CHECK(pnccTrie->getMerkleHash() == hash4);
-    BOOST_CHECK(trie.checkConsistency());
+    BOOST_CHECK(pnccTrie->checkConsistency());
 
-    CNCCTrieCache ntState4(&trie);
+    CNCCTrieCache ntState4(pnccTrie);
     ntState4.removeName(std::string("abab"), tx6.GetHash(), 0);
     BOOST_CHECK(ntState4.getMerkleHash() == hash2);
     ntState4.flush();
-    BOOST_CHECK(!trie.empty());
+    BOOST_CHECK(!pnccTrie->empty());
-    BOOST_CHECK(trie.getMerkleHash() == hash2);
+    BOOST_CHECK(pnccTrie->getMerkleHash() == hash2);
-    BOOST_CHECK(trie.checkConsistency());
+    BOOST_CHECK(pnccTrie->checkConsistency());
 
-    CNCCTrieCache ntState5(&trie);
+    CNCCTrieCache ntState5(pnccTrie);
     ntState5.removeName(std::string("test"), tx3.GetHash(), 0);
 
     BOOST_CHECK(ntState5.getMerkleHash() == hash2);
     ntState5.flush();
-    BOOST_CHECK(!trie.empty());
+    BOOST_CHECK(!pnccTrie->empty());
-    BOOST_CHECK(trie.getMerkleHash() == hash2);
+    BOOST_CHECK(pnccTrie->getMerkleHash() == hash2);
-    BOOST_CHECK(trie.checkConsistency());
+    BOOST_CHECK(pnccTrie->checkConsistency());
 
-    CNCCTrieCache ntState6(&trie);
+    CNCCTrieCache ntState6(pnccTrie);
     ntState6.insertName(std::string("test"), tx3.GetHash(), 0, 50, 101);
 
     BOOST_CHECK(ntState6.getMerkleHash() == hash2);
     ntState6.flush();
-    BOOST_CHECK(!trie.empty());
+    BOOST_CHECK(!pnccTrie->empty());
-    BOOST_CHECK(trie.getMerkleHash() == hash2);
+    BOOST_CHECK(pnccTrie->getMerkleHash() == hash2);
-    BOOST_CHECK(trie.checkConsistency());
+    BOOST_CHECK(pnccTrie->checkConsistency());
 }
 
 BOOST_AUTO_TEST_CASE(ncctrienode_serialize_unserialize)

src/test/test_bitcoin.cpp

@@ -42,6 +42,7 @@ struct TestingSetup {
         pblocktree = new CBlockTreeDB(1 << 20, true);
         pcoinsdbview = new CCoinsViewDB(1 << 23, true);
         pcoinsTip = new CCoinsViewCache(pcoinsdbview);
+        pnccTrie = new CNCCTrie();
         InitBlockIndex();
 #ifdef ENABLE_WALLET
         bool fFirstRun;
@@ -63,6 +64,7 @@ struct TestingSetup {
         delete pwalletMain;
         pwalletMain = NULL;
 #endif
+        delete pnccTrie;
         delete pcoinsTip;
         delete pcoinsdbview;
         delete pblocktree;