Fix incorrect usage of the claim index #197

Merged
lbrynaut merged 1 commit from claimindex-fix into master 2018-09-03 18:25:16 +02:00
3 changed files with 132 additions and 20 deletions

View file

@ -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;
claim = element.claim;
return true;
if (element.claim.claimId == claimId) {
name = element.name;
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);
BrannonKing commented 2018-08-24 01:00:28 +02:00 (Migrated from github.com)
Review

Is this working around a problem in the base class? Why not fix it there?

Is this working around a problem in the base class? Why not fix it there?
lbrynaut commented 2018-08-24 02:33:23 +02:00 (Migrated from github.com)
Review

No.

No.
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)
{

View file

@ -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;

View file

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