Fix incorrect usage of the claim index so that we can properly

maintain state through re-orgs and delayed activation states.

Includes get_claim_by_id_test_2 and get_claim_by_id_test_3 written by
Kay.

src/claimtrie.cpp

@@ -606,12 +606,12 @@ void CClaimTrie::addToClaimIndex(const std::string& name, const CClaimValue& cla
 {
     CClaimIndexElement element = { name, claim };
     LogPrintf("%s: ClaimIndex[%s] updated %s\n", __func__, claim.claimId.GetHex(), name);
-
     db.Write(std::make_pair(CLAIM_BY_ID, claim.claimId), element);
 }
 
 void CClaimTrie::removeFromClaimIndex(const CClaimValue& claim)
 {
+    LogPrintf("%s: ClaimIndex[%s] removed\n", __func__, claim.claimId.GetHex());
     db.Erase(std::make_pair(CLAIM_BY_ID, claim.claimId));
 }
 
@@ -620,9 +620,14 @@ bool CClaimTrie::getClaimById(const uint160 claimId, std::string& name, CClaimVa
     CClaimIndexElement element;
     if (db.Read(std::make_pair(CLAIM_BY_ID, claimId), element))
     {
-        name = element.name;
+        if (element.claim.claimId == claimId) {
-        claim = element.claim;
+            name = element.name;
-        return true;
+            claim = element.claim;
+            return true;
+        } else {
+            LogPrintf("%s: ClaimIndex[%s] returned unmatched claimId %s when looking for %s\n",
+                __func__, claimId.GetHex(), element.claim.claimId.GetHex(), name);
+        }
     }
     return false;
 }
@@ -1357,7 +1362,6 @@ bool CClaimTrieCache::insertClaimIntoTrie(const std::string& name, CClaimValue c
     {
         fChanged = true;
         currentNode->insertClaim(claim);
-        base->addToClaimIndex(name, claim);
     }
     else
     {
@@ -1369,6 +1373,7 @@ bool CClaimTrieCache::insertClaimIntoTrie(const std::string& name, CClaimValue c
         if (currentTop != currentNode->claims.front())
             fChanged = true;
     }
+
     if (fChanged)
     {
         for (std::string::const_iterator itCur = name.begin(); itCur != name.end(); ++itCur)
@@ -1432,8 +1437,6 @@ bool CClaimTrieCache::removeClaimFromTrie(const std::string& name, const COutPoi
     CClaimValue currentTop = currentNode->claims.front();
 
     success = currentNode->removeClaim(outPoint, claim);
-    base->removeFromClaimIndex(claim);
-
     if (!currentNode->claims.empty())
     {
         supportMapEntryType node;
@@ -1612,6 +1615,8 @@ bool CClaimTrieCache::undoSpendClaim(const std::string& name, const COutPoint& o
     {
         nameOutPointType entry(name, claim.outPoint);
         addToExpirationQueue(claim.nHeight + base->nExpirationTime, entry);
+        CClaimIndexElement element = {name, claim};
+        claimsToAdd.push_back(element);
         return insertClaimIntoTrie(name, claim, false);
     }
     else
@@ -1630,7 +1635,8 @@ bool CClaimTrieCache::addClaimToQueues(const std::string& name, CClaimValue& cla
     itQueueNameRow->second.push_back(outPointHeightType(claim.outPoint, claim.nValidAtHeight));
     nameOutPointType expireEntry(name, claim.outPoint);
     addToExpirationQueue(claim.nHeight + base->nExpirationTime, expireEntry);
-    base->addToClaimIndex(name, claim);
+    CClaimIndexElement element = {name, claim};
+    claimsToAdd.push_back(element);
     return true;
 }
 
@@ -1669,8 +1675,6 @@ bool CClaimTrieCache::removeClaimFromQueue(const std::string& name, const COutPo
             std::swap(claim, itQueue->second);
             itQueueNameRow->second.erase(itQueueName);
             itQueueRow->second.erase(itQueue);
-
-            base->removeFromClaimIndex(claim);
             return true;
         }
     }
@@ -1707,6 +1711,7 @@ bool CClaimTrieCache::removeClaim(const std::string& name, const COutPoint& outP
         nValidAtHeight = claim.nValidAtHeight;
         int expirationHeight = nHeight + base->nExpirationTime;
         removeFromExpirationQueue(name, outPoint, expirationHeight);
+        claimsToDelete.insert(claim);
     }
     return removed;
 }
@@ -2154,6 +2159,7 @@ bool CClaimTrieCache::incrementBlock(insertUndoType& insertUndo, claimQueueRowTy
         {
             CClaimValue claim;
             assert(removeClaimFromTrie(itEntry->name, itEntry->outPoint, claim, true));
+            claimsToDelete.insert(claim);
             expireUndo.push_back(std::make_pair(itEntry->name, claim));
             LogPrintf("Expiring claim %s: %s, nHeight: %d, nValidAtHeight: %d\n", claim.claimId.GetHex(), itEntry->name, claim.nHeight, claim.nValidAtHeight);
         }
@@ -2405,6 +2411,8 @@ bool CClaimTrieCache::decrementBlock(insertUndoType& insertUndo, claimQueueRowTy
         for (claimQueueRowType::iterator itExpireUndo = expireUndo.begin(); itExpireUndo != expireUndo.end(); ++itExpireUndo)
         {
             insertClaimIntoTrie(itExpireUndo->first, itExpireUndo->second, false);
+            CClaimIndexElement element = {itExpireUndo->first, itExpireUndo->second};
+            claimsToAdd.push_back(element);
             itExpireRow->second.push_back(nameOutPointType(itExpireUndo->first, itExpireUndo->second.outPoint));
         }
     }
@@ -2531,6 +2539,18 @@ bool CClaimTrieCache::flush()
 {
     if (dirty())
         getMerkleHash();
+
+    if (!claimsToDelete.empty()) {
+        for (claimIndexClaimListType::iterator it = claimsToDelete.begin(); it != claimsToDelete.end(); ++it)
+            base->removeFromClaimIndex(*it);
+        claimsToDelete.clear();
+    }
+    if (!claimsToAdd.empty()) {
+        for (claimIndexElementListType::iterator it = claimsToAdd.begin(); it != claimsToAdd.end(); ++it)
+            base->addToClaimIndex(it->name, it->claim);
+        claimsToAdd.clear();
+    }
+
     bool success = base->update(cache, cacheHashes, cacheTakeoverHeights, getBestBlock(), claimQueueCache, claimQueueNameCache, expirationQueueCache, nCurrentHeight, supportCache, supportQueueCache, supportQueueNameCache, supportExpirationQueueCache);
     if (success)
     {

src/claimtrie.h

@@ -283,6 +283,9 @@ typedef std::map<std::string, CClaimTrieNode*, nodenamecompare> nodeCacheType;
 
 typedef std::map<std::string, uint256> hashMapType;
 
+typedef std::set<CClaimValue> claimIndexClaimListType;
+typedef std::vector<CClaimIndexElement> claimIndexElementListType;
+
 struct claimsForNameType
 {
     std::vector<CClaimValue> claims;
@@ -546,6 +549,8 @@ protected:
     mutable std::map<std::string, int> cacheTakeoverHeights; 
     mutable int nCurrentHeight; // Height of the block that is being worked on, which is
                                 // one greater than the height of the chain's tip
+    mutable claimIndexElementListType claimsToAdd;
+    mutable claimIndexClaimListType claimsToDelete;
 
     uint256 hashBlock;
 

src/test/claimtriebranching_tests.cpp

@@ -3032,5 +3032,92 @@ BOOST_AUTO_TEST_CASE(bogus_claimtrie_hash_test)
     delete pblockTemp;
 }
 
+/*
+ * tests for CClaimTrie::getClaimById basic consistency checks
+ */
+BOOST_AUTO_TEST_CASE(get_claim_by_id_test_2)
+{
+    ClaimTrieChainFixture fixture;
+    const std::string name = "test";
+    CMutableTransaction tx1 = fixture.MakeClaim(fixture.GetCoinbase(), name, "one", 1);
+    uint160 claimId = ClaimIdHash(tx1.GetHash(), 0);
+    CMutableTransaction txx = fixture.MakeClaim(fixture.GetCoinbase(), name, "one", 1);
+    uint160 claimIdx = ClaimIdHash(txx.GetHash(), 0);
+
+    fixture.IncrementBlocks(1);
+
+    CClaimValue claimValue;
+    std::string claimName;
+    pclaimTrie->getClaimById(claimId, claimName, claimValue);
+    BOOST_CHECK(claimName == name);
+    BOOST_CHECK(claimValue.claimId == claimId);
+
+    CMutableTransaction txa = fixture.Spend(tx1);
+    CMutableTransaction txb = fixture.Spend(txx);
+
+    fixture.IncrementBlocks(1);
+    BOOST_CHECK(!pclaimTrie->getClaimById(claimId, claimName, claimValue));
+    BOOST_CHECK(!pclaimTrie->getClaimById(claimIdx, claimName, claimValue));
+
+    fixture.DecrementBlocks(1);
+    pclaimTrie->getClaimById(claimId, claimName, claimValue);
+    BOOST_CHECK(claimName == name);
+    BOOST_CHECK(claimValue.claimId == claimId);
+
+    // this test fails
+    pclaimTrie->getClaimById(claimIdx, claimName, claimValue);
+    BOOST_CHECK(claimName == name);
+    BOOST_CHECK(claimValue.claimId == claimIdx);
+}
+
+BOOST_AUTO_TEST_CASE(get_claim_by_id_test_3)
+{
+    ClaimTrieChainFixture fixture;
+    pclaimTrie->setExpirationTime(5);
+    const std::string name = "test";
+    CMutableTransaction tx1 = fixture.MakeClaim(fixture.GetCoinbase(), name, "one", 1);
+    uint160 claimId = ClaimIdHash(tx1.GetHash(), 0);
+
+    fixture.IncrementBlocks(1);
+
+    CClaimValue claimValue;
+    std::string claimName;
+    pclaimTrie->getClaimById(claimId, claimName, claimValue);
+    BOOST_CHECK(claimName == name);
+    BOOST_CHECK(claimValue.claimId == claimId);
+    // make second claim with activation delay 1
+    CMutableTransaction tx2 = fixture.MakeClaim(fixture.GetCoinbase(), name, "one", 2);
+    uint160 claimId2 = ClaimIdHash(tx2.GetHash(), 0);
+
+    fixture.IncrementBlocks(1);
+    // second claim is not activated yet, but can still access by claim id
+    BOOST_CHECK(is_best_claim(name, tx1));
+    BOOST_CHECK(pclaimTrie->getClaimById(claimId2, claimName, claimValue));
+    BOOST_CHECK(claimName == name);
+    BOOST_CHECK(claimValue.claimId == claimId2);
+
+    fixture.IncrementBlocks(1);
+    // second claim has activated
+    BOOST_CHECK(is_best_claim(name, tx2));
+    BOOST_CHECK(pclaimTrie->getClaimById(claimId2, claimName, claimValue));
+    BOOST_CHECK(claimName == name);
+    BOOST_CHECK(claimValue.claimId == claimId2);
+
+
+    fixture.DecrementBlocks(1);
+    // second claim has been deactivated via decrement
+    // should still be accesible via claim id
+    BOOST_CHECK(is_best_claim(name, tx1));
+    BOOST_CHECK(pclaimTrie->getClaimById(claimId2, claimName, claimValue));
+    BOOST_CHECK(claimName == name);
+    BOOST_CHECK(claimValue.claimId == claimId2);
+
+    fixture.IncrementBlocks(1);
+    // second claim should have been re activated via increment
+    BOOST_CHECK(is_best_claim(name, tx2));
+    BOOST_CHECK(pclaimTrie->getClaimById(claimId2, claimName, claimValue));
+    BOOST_CHECK(claimName == name);
+    BOOST_CHECK(claimValue.claimId == claimId2);
+}
 
 BOOST_AUTO_TEST_SUITE_END()