From f47826232fee82a2ada129d881dcf16577f1040c Mon Sep 17 00:00:00 2001 From: Anthony Fieroni Date: Thu, 28 Nov 2019 18:29:24 +0200 Subject: [PATCH] Fix removing expired claim Signed-off-by: Anthony Fieroni --- src/claimtrie/forks.cpp | 48 ++--- src/claimtrie/trie.cpp | 223 ++++++++++----------- src/claimtrie/trie.h | 6 +- src/test/claimtrieexpirationfork_tests.cpp | 8 +- src/test/claimtriefixture.cpp | 6 +- src/test/claimtriefixture.h | 2 +- 6 files changed, 139 insertions(+), 154 deletions(-) diff --git a/src/claimtrie/forks.cpp b/src/claimtrie/forks.cpp index 1a6909442..d8aa15cf1 100644 --- a/src/claimtrie/forks.cpp +++ b/src/claimtrie/forks.cpp @@ -63,7 +63,7 @@ bool CClaimTrieCacheExpirationFork::finalizeDecrement(takeoverUndoType& takeover bool CClaimTrieCacheExpirationFork::forkForExpirationChange(bool increment) { - if (!transacting) { transacting = true; db << "begin"; } + ensureTransacting(); /* If increment is True, we have forked to extend the expiration time, thus items in the expiration queue @@ -73,24 +73,22 @@ bool CClaimTrieCacheExpirationFork::forkForExpirationChange(bool increment) will have their expiration extension removed. */ - auto extension = base->nExtendedClaimExpirationTime - base->nOriginalClaimExpirationTime; - if (increment) { - db << "UPDATE claims SET expirationHeight = expirationHeight + ? WHERE expirationHeight >= ?" - << extension << nNextHeight; - db << "UPDATE supports SET expirationHeight = expirationHeight + ? WHERE expirationHeight >= ?" - << extension << nNextHeight; - } - else { - db << "UPDATE claims SET expirationHeight = expirationHeight - ? WHERE expirationHeight >= ?" - << extension << nNextHeight + extension; - db << "UPDATE supports SET expirationHeight = expirationHeight - ? WHERE expirationHeight >= ?" - << extension << nNextHeight + extension; + auto height = nNextHeight; + int extension = base->nExtendedClaimExpirationTime - base->nOriginalClaimExpirationTime; + if (!increment) { + height += extension; + extension *= -1; } + + db << "UPDATE claims SET expirationHeight = expirationHeight + ? WHERE expirationHeight >= ?" << extension << height; + db << "UPDATE supports SET expirationHeight = expirationHeight + ? WHERE expirationHeight >= ?" << extension << height; + return true; } CClaimTrieCacheNormalizationFork::CClaimTrieCacheNormalizationFork(CClaimTrie* base) : CClaimTrieCacheExpirationFork(base) { + db.define("NORMALIZED", [this](const std::string& str) { return normalizeClaimName(str, true); }); } bool CClaimTrieCacheNormalizationFork::shouldNormalize() const @@ -141,10 +139,7 @@ std::string CClaimTrieCacheNormalizationFork::normalizeClaimName(const std::stri bool CClaimTrieCacheNormalizationFork::normalizeAllNamesInTrieIfNecessary(takeoverUndoType& takeoverUndo) { - if (!transacting) { transacting = true; db << "begin"; } - - // run the one-time upgrade of all names that need to change - db.define("NORMALIZED", [this](const std::string& str) { return normalizeClaimName(str, true); }); + ensureTransacting(); // make the new nodes db << "INSERT INTO nodes(name) SELECT NORMALIZED(name) AS nn FROM claims WHERE nn != nodeName " @@ -182,10 +177,7 @@ bool CClaimTrieCacheNormalizationFork::normalizeAllNamesInTrieIfNecessary(takeov bool CClaimTrieCacheNormalizationFork::unnormalizeAllNamesInTrieIfNecessary() { - if (!transacting) { transacting = true; db << "begin"; } - - // run the one-time upgrade of all names that need to change - db.define("NORMALIZED", [this](const std::string& str) { return normalizeClaimName(str, true); }); + ensureTransacting(); db << "INSERT INTO nodes(name) SELECT name FROM claims WHERE name != nodeName " "AND validHeight < ?1 AND expirationHeight > ?1 ON CONFLICT(name) DO UPDATE SET hash = NULL" << nNextHeight; @@ -376,11 +368,7 @@ bool CClaimTrieCacheHashFork::getProofForName(const std::string& name, const CUi // cache the parent nodes getMerkleHash(); proof = CClaimTrieProof(); - auto nodeQuery = db << "SELECT name, IFNULL(takeoverHeight, 0) FROM nodes WHERE " - "name IN (WITH RECURSIVE prefix(p) AS (VALUES(?) UNION ALL " - "SELECT POPS(p) FROM prefix WHERE p != '') SELECT p FROM prefix) " - "ORDER BY name" << name; - for (auto&& row: nodeQuery) { + for (auto&& row: db << proofClaimQuery << name) { std::string key; int takeoverHeight; row >> key >> takeoverHeight; @@ -416,12 +404,12 @@ bool CClaimTrieCacheHashFork::getProofForName(const std::string& name, const CUi // else it will be hash(x, claims) if (key == name) { proof.nHeightOfLastTakeover = takeoverHeight; - auto hash = childHashes.empty() ? leafHash : ComputeMerkleRoot(childHashes); + auto hash = childHashes.empty() ? leafHash : ComputeMerkleRoot(std::move(childHashes)); proof.pairs.emplace_back(true, hash); if (!claimHashes.empty()) fillPairs(claimHashes, claimIdx); } else { - auto hash = claimHashes.empty() ? emptyHash : ComputeMerkleRoot(claimHashes); + auto hash = claimHashes.empty() ? emptyHash : ComputeMerkleRoot(std::move(claimHashes)); proof.pairs.emplace_back(false, hash); if (!childHashes.empty()) fillPairs(childHashes, nextCurrentIdx); @@ -436,7 +424,7 @@ void CClaimTrieCacheHashFork::initializeIncrement() CClaimTrieCacheNormalizationFork::initializeIncrement(); // we could do this in the constructor, but that would not allow for multiple increments in a row (as done in unit tests) if (nNextHeight == base->nAllClaimsInMerkleForkHeight - 1) { - if (!transacting) { transacting = true; db << "begin"; } + ensureTransacting(); db << "UPDATE nodes SET hash = NULL"; } } @@ -445,7 +433,7 @@ bool CClaimTrieCacheHashFork::finalizeDecrement(takeoverUndoType& takeoverUndo) { auto ret = CClaimTrieCacheNormalizationFork::finalizeDecrement(takeoverUndo); if (ret && nNextHeight == base->nAllClaimsInMerkleForkHeight - 1) { - if (!transacting) { transacting = true; db << "begin"; } + ensureTransacting(); db << "UPDATE nodes SET hash = NULL"; } return ret; diff --git a/src/claimtrie/trie.cpp b/src/claimtrie/trie.cpp index 483d381d1..43d6dc899 100644 --- a/src/claimtrie/trie.cpp +++ b/src/claimtrie/trie.cpp @@ -117,14 +117,14 @@ bool CClaimTrie::SyncToDisk() bool CClaimTrie::empty() { int64_t count; - db << "SELECT COUNT(*) FROM claims WHERE validHeight < ?1 AND expirationHeight >= ?1" << nNextHeight >> count; + db << "SELECT COUNT(*) FROM (SELECT 1 FROM claims WHERE validHeight < ?1 AND expirationHeight >= ?1 LIMIT 1)" << nNextHeight >> count; return count == 0; } bool CClaimTrieCacheBase::haveClaim(const std::string& name, const CTxOutPoint& outPoint) const { auto query = db << "SELECT 1 FROM claims WHERE nodeName = ?1 AND txID = ?2 AND txN = ?3 " - "AND validHeight < ?4 AND expirationHeight >= ?4 LIMIT 1" + "AND validHeight < ?4 AND expirationHeight >= ?4 LIMIT 1" << name << outPoint.hash << outPoint.n << nNextHeight; return query.begin() != query.end(); } @@ -132,7 +132,7 @@ bool CClaimTrieCacheBase::haveClaim(const std::string& name, const CTxOutPoint& bool CClaimTrieCacheBase::haveSupport(const std::string& name, const CTxOutPoint& outPoint) const { auto query = db << "SELECT 1 FROM supports WHERE nodeName = ?1 AND txID = ?2 AND txN = ?3 " - "AND validHeight < ?4 AND expirationHeight >= ?4 LIMIT 1" + "AND validHeight < ?4 AND expirationHeight >= ?4 LIMIT 1" << name << outPoint.hash << outPoint.n << nNextHeight; return query.begin() != query.end(); } @@ -141,13 +141,13 @@ supportEntryType CClaimTrieCacheBase::getSupportsForName(const std::string& name { // includes values that are not yet valid auto query = db << "SELECT supportedClaimID, txID, txN, blockHeight, validHeight, amount " - "FROM supports WHERE nodeName = ? AND expirationHeight >= ?" << name << nNextHeight; + "FROM supports WHERE nodeName = ? AND expirationHeight >= ?" << name << nNextHeight; supportEntryType ret; for (auto&& row: query) { CSupportValue value; row >> value.supportedClaimId >> value.outPoint.hash >> value.outPoint.n >> value.nHeight >> value.nValidAtHeight >> value.nAmount; - ret.push_back(value); + ret.push_back(std::move(value)); } return ret; } @@ -155,7 +155,7 @@ supportEntryType CClaimTrieCacheBase::getSupportsForName(const std::string& name bool CClaimTrieCacheBase::haveClaimInQueue(const std::string& name, const CTxOutPoint& outPoint, int& nValidAtHeight) const { auto query = db << "SELECT validHeight FROM claims WHERE nodeName = ? AND txID = ? AND txN = ? " - "AND validHeight >= ? AND expirationHeight > validHeight LIMIT 1" + "AND validHeight >= ? AND expirationHeight >= validHeight LIMIT 1" << name << outPoint.hash << outPoint.n << nNextHeight; for (auto&& row: query) { row >> nValidAtHeight; @@ -167,7 +167,7 @@ bool CClaimTrieCacheBase::haveClaimInQueue(const std::string& name, const CTxOut bool CClaimTrieCacheBase::haveSupportInQueue(const std::string& name, const CTxOutPoint& outPoint, int& nValidAtHeight) const { auto query = db << "SELECT validHeight FROM supports WHERE nodeName = ? AND txID = ? AND txN = ? " - "AND validHeight >= ? AND expirationHeight > validHeight LIMIT 1" + "AND validHeight >= ? AND expirationHeight >= validHeight LIMIT 1" << name << outPoint.hash << outPoint.n << nNextHeight; for (auto&& row: query) { row >> nValidAtHeight; @@ -180,7 +180,7 @@ bool CClaimTrieCacheBase::deleteNodeIfPossible(const std::string& name, std::str { if (name.empty()) return false; // to remove a node it must have one or less children and no claims - db << "SELECT COUNT(*) FROM claims WHERE nodeName = ?1 AND validHeight < ?2 AND expirationHeight >= ?2 " + db << "SELECT COUNT(*) FROM (SELECT 1 FROM claims WHERE nodeName = ?1 AND validHeight < ?2 AND expirationHeight >= ?2 LIMIT 1)" << name << nNextHeight >> claims; if (claims > 0) return false; // still has claims // we now know it has no claims, but we need to check its children @@ -188,8 +188,7 @@ bool CClaimTrieCacheBase::deleteNodeIfPossible(const std::string& name, std::str std::string childName; // this line assumes that we've set the parent on child nodes already, // which means we are len(name) desc in our parent method - // alternately: SELECT COUNT(DISTINCT nodeName) FROM claims WHERE SUBSTR(nodeName, 1, len(?)) == ? AND LENGTH(nodeName) > len(?) - db << "SELECT COUNT(*),MAX(name) FROM nodes WHERE parent = ?" << name >> std::tie(count, childName); + db << "SELECT COUNT(*), MAX(name) FROM nodes WHERE parent = ?" << name >> std::tie(count, childName); if (count > 1) return false; // still has multiple children logPrint << "Removing node " << name << " with " << count << " children" << Clog::endl; // okay. it's going away @@ -307,7 +306,7 @@ std::size_t CClaimTrieCacheBase::getTotalNamesInTrie() const // you could do this select from the nodes table, but you would have to ensure it is not dirty first std::size_t ret; db << "SELECT COUNT(DISTINCT nodeName) FROM claims WHERE validHeight < ?1 AND expirationHeight >= ?1" - << nNextHeight >> ret; + << nNextHeight >> ret; return ret; } @@ -315,7 +314,7 @@ std::size_t CClaimTrieCacheBase::getTotalClaimsInTrie() const { std::size_t ret; db << "SELECT COUNT(*) FROM claims WHERE validHeight < ?1 AND expirationHeight >= ?1" - << nNextHeight >> ret; + << nNextHeight >> ret; return ret; } @@ -348,15 +347,12 @@ bool CClaimTrieCacheBase::getInfoForName(const std::string& name, CClaimValue& c CClaimSupportToName CClaimTrieCacheBase::getClaimsForName(const std::string& name) const { int nLastTakeoverHeight = 0; - { - auto query = db << "SELECT takeoverHeight FROM nodes WHERE name = ?" << name; - auto it = query.begin(); - if (it != query.end()) - *it >> nLastTakeoverHeight; - } + db << "SELECT takeoverHeight FROM nodes WHERE name = ?" << name + >> [&nLastTakeoverHeight](int takeoverHeight) { + nLastTakeoverHeight = takeoverHeight; + }; claimEntryType claims; - auto supports = getSupportsForName(name); { auto query = db << "SELECT claimID, txID, txN, blockHeight, validHeight, amount " "FROM claims WHERE nodeName = ? AND expirationHeight >= ?" @@ -365,10 +361,11 @@ CClaimSupportToName CClaimTrieCacheBase::getClaimsForName(const std::string& nam CClaimValue claim; row >> claim.claimId >> claim.outPoint.hash >> claim.outPoint.n >> claim.nHeight >> claim.nValidAtHeight >> claim.nAmount; - claims.push_back(claim); + claims.push_back(std::move(claim)); } } + auto supports = getSupportsForName(name); auto find = [&supports](decltype(supports)::iterator& it, const CClaimValue& claim) { it = std::find_if(it, supports.end(), [&claim](const CSupportValue& support) { return claim.claimId == support.supportedClaimId; @@ -496,10 +493,14 @@ CClaimTrieCacheBase::CClaimTrieCacheBase(CClaimTrie* base) : base(base), db(base->dbFile, sharedConfig), transacting(false), childHashQuery(db << "SELECT name, hash, IFNULL(takeoverHeight, 0) FROM nodes WHERE parent = ? ORDER BY name"), claimHashQuery(db << "SELECT c.txID, c.txN, c.claimID, c.blockHeight, c.validHeight, c.amount, " - "(SELECT IFNULL(SUM(s.amount),0)+c.amount FROM supports s WHERE s.supportedClaimID = c.claimID " - "AND s.nodeName = c.nodeName AND s.validHeight < ?1 AND s.expirationHeight >= ?1) as effectiveAmount " - "FROM claims c WHERE c.nodeName = ?2 AND c.validHeight < ?1 AND c.expirationHeight >= ?1 " - "ORDER BY effectiveAmount DESC, c.blockHeight, c.txID, c.txN") + "(SELECT IFNULL(SUM(s.amount),0)+c.amount FROM supports s WHERE s.supportedClaimID = c.claimID " + "AND s.nodeName = c.nodeName AND s.validHeight < ?1 AND s.expirationHeight >= ?1) as effectiveAmount " + "FROM claims c WHERE c.nodeName = ?2 AND c.validHeight < ?1 AND c.expirationHeight >= ?1 " + "ORDER BY effectiveAmount DESC, c.blockHeight, c.txID, c.txN"), + proofClaimQuery("SELECT name, IFNULL(takeoverHeight, 0) FROM nodes WHERE " + "name IN (WITH RECURSIVE prefix(p) AS (VALUES(?) UNION ALL " + "SELECT POPS(p) FROM prefix WHERE p != '') SELECT p FROM prefix) " + "ORDER BY name") { assert(base); nNextHeight = base->nNextHeight; @@ -513,6 +514,14 @@ CClaimTrieCacheBase::CClaimTrieCacheBase(CClaimTrie* base) db.define("POPS", [](std::string s) -> std::string { if (!s.empty()) s.pop_back(); return s; }); } +void CClaimTrieCacheBase::ensureTransacting() +{ + if (!transacting) { + transacting = true; + db << "begin"; + } +} + int CClaimTrieCacheBase::expirationTime() const { return base->nOriginalClaimExpirationTime; @@ -545,7 +554,7 @@ bool CClaimTrieCacheBase::getLastTakeoverForName(const std::string& name, CUint1 bool CClaimTrieCacheBase::addClaim(const std::string& name, const CTxOutPoint& outPoint, const CUint160& claimId, int64_t nAmount, int nHeight, int nValidHeight, const std::vector& metadata) { - if (!transacting) { transacting = true; db << "begin"; } + ensureTransacting(); // in the update scenario the previous one should be removed already // in the downgrade scenario, the one ahead will be removed already and the old one's valid height is input @@ -564,8 +573,8 @@ bool CClaimTrieCacheBase::addClaim(const std::string& name, const CTxOutPoint& o auto expires = expirationTime() + nHeight; db << "INSERT INTO claims(claimID, name, nodeName, txID, txN, amount, blockHeight, validHeight, expirationHeight, metadata) " - "VALUES(?, ?, ?, ?, ?, ?, ?, ?, ?, ?)" << claimId << name << nodeName - << outPoint.hash << outPoint.n << nAmount << nHeight << nValidHeight << expires << metadata; + "VALUES(?, ?, ?, ?, ?, ?, ?, ?, ?, ?)" << claimId << name << nodeName + << outPoint.hash << outPoint.n << nAmount << nHeight << nValidHeight << expires << metadata; if (nValidHeight < nNextHeight) db << "INSERT INTO nodes(name) VALUES(?) ON CONFLICT(name) DO UPDATE SET hash = NULL" << nodeName; @@ -576,15 +585,16 @@ bool CClaimTrieCacheBase::addClaim(const std::string& name, const CTxOutPoint& o bool CClaimTrieCacheBase::addSupport(const std::string& name, const CTxOutPoint& outPoint, const CUint160& supportedClaimId, int64_t nAmount, int nHeight, int nValidHeight, const std::vector& metadata) { - if (!transacting) { transacting = true; db << "begin"; } + ensureTransacting(); if (nValidHeight < 0) nValidHeight = nHeight + getDelayForName(name, supportedClaimId); auto nodeName = adjustNameForValidHeight(name, nValidHeight); auto expires = expirationTime() + nHeight; + db << "INSERT INTO supports(supportedClaimID, name, nodeName, txID, txN, amount, blockHeight, validHeight, expirationHeight, metadata) " - "VALUES(?, ?, ?, ?, ?, ?, ?, ?, ?, ?)" << supportedClaimId << name << nodeName - << outPoint.hash << outPoint.n << nAmount << nHeight << nValidHeight << expires << metadata; + "VALUES(?, ?, ?, ?, ?, ?, ?, ?, ?, ?)" << supportedClaimId << name << nodeName + << outPoint.hash << outPoint.n << nAmount << nHeight << nValidHeight << expires << metadata; if (nValidHeight < nNextHeight) db << "UPDATE nodes SET hash = NULL WHERE name = ?" << nodeName; @@ -594,7 +604,7 @@ bool CClaimTrieCacheBase::addSupport(const std::string& name, const CTxOutPoint& bool CClaimTrieCacheBase::removeClaim(const CUint160& claimId, const CTxOutPoint& outPoint, std::string& nodeName, int& validHeight) { - if (!transacting) { transacting = true; db << "begin"; } + ensureTransacting(); // this gets tricky in that we may be removing an update // when going forward we spend a claim (aka, call removeClaim) before updating it (aka, call addClaim) @@ -602,12 +612,12 @@ bool CClaimTrieCacheBase::removeClaim(const CUint160& claimId, const CTxOutPoint // we then undo the spend of the previous one by calling addClaim with the original data // in order to maintain the proper takeover height the udpater will need to use our height returned here - auto query = db << "SELECT nodeName, validHeight FROM claims WHERE claimID = ? and txID = ? and txN = ?" - << claimId << outPoint.hash << outPoint.n; + auto query = db << "SELECT nodeName, validHeight FROM claims WHERE claimID = ? AND txID = ? AND txN = ? AND expirationHeight >= ?" + << claimId << outPoint.hash << outPoint.n << nNextHeight; auto it = query.begin(); if (it == query.end()) return false; *it >> nodeName >> validHeight; - db << "DELETE FROM claims WHERE claimID = ? AND txID = ? and txN = ?" << claimId << outPoint.hash << outPoint.n; + db << "DELETE FROM claims WHERE claimID = ? AND txID = ? and txN = ?" << claimId << outPoint.hash << outPoint.n; if (!db.rows_modified()) return false; db << "UPDATE nodes SET hash = NULL WHERE name = ?" << nodeName; @@ -616,32 +626,30 @@ bool CClaimTrieCacheBase::removeClaim(const CUint160& claimId, const CTxOutPoint // because it's a parent one and should not be effectively erased // we had a bug in the old code where that situation would force a zero delay on re-add if (true) { // TODO: hard fork this out (which we already tried once but failed) - auto workaroundQuery = db << "SELECT nodeName FROM claims WHERE nodeName LIKE ?1 " - "AND validHeight < ?2 AND expirationHeight >= ?2 ORDER BY nodeName LIMIT 1" - << nodeName + "%" << nNextHeight; - auto wit = workaroundQuery.begin(); - if (wit != workaroundQuery.end()) { - std::string shortestMatch; - *wit >> shortestMatch; - if (shortestMatch != nodeName) { - // set this when there are no more claims on that name and that node still has children - removalWorkaround.insert(nodeName); - } - } + db << "SELECT nodeName FROM claims WHERE nodeName LIKE ?1 " + "AND validHeight < ?2 AND expirationHeight > ?2 ORDER BY nodeName LIMIT 1" + << nodeName + '%' << nNextHeight + >> [this, &nodeName](const std::string& shortestMatch) { + if (shortestMatch != nodeName) + // set this when there are no more claims on that name and that node still has children + removalWorkaround.insert(nodeName); + }; } return true; } bool CClaimTrieCacheBase::removeSupport(const CTxOutPoint& outPoint, std::string& nodeName, int& validHeight) { - if (!transacting) { transacting = true; db << "begin"; } + ensureTransacting(); - auto query = db << "SELECT nodeName, validHeight FROM supports WHERE txID = ? AND txN = ?" - << outPoint.hash << outPoint.n; + auto query = db << "SELECT nodeName, validHeight FROM supports WHERE txID = ? AND txN = ? AND expirationHeight >= ?" + << outPoint.hash << outPoint.n << nNextHeight; auto it = query.begin(); if (it == query.end()) return false; *it >> nodeName >> validHeight; db << "DELETE FROM supports WHERE txID = ? AND txN = ?" << outPoint.hash << outPoint.n; + if (!db.rows_modified()) + return false; db << "UPDATE nodes SET hash = NULL WHERE name = ?" << nodeName; return true; } @@ -1100,7 +1108,7 @@ bool CClaimTrieCacheBase::incrementBlock(insertUndoType& insertUndo, claimUndoTy // for every claim and support that becomes active this block set its node hash to null (aka, dirty) // for every claim and support that expires this block set its node hash to null and add it to the expire(Support)Undo // for all dirty nodes look for new takeovers - if (!transacting) { transacting = true; db << "begin"; } + ensureTransacting(); db << "INSERT INTO nodes(name) SELECT nodeName FROM claims " "WHERE validHeight = ?1 AND expirationHeight > ?1 " @@ -1117,11 +1125,11 @@ bool CClaimTrieCacheBase::incrementBlock(insertUndoType& insertUndo, claimUndoTy std::string name; row >> value.outPoint.hash >> value.outPoint.n >> name >> value.claimId >> value.nValidAtHeight >> value.nHeight >> value.nAmount; - expireUndo.emplace_back(name, value); + expireUndo.emplace_back(std::move(name), std::move(value)); } } - db << "UPDATE nodes SET hash = NULL WHERE name IN (SELECT nodeName FROM claims WHERE expirationHeight = ?)" - << nNextHeight; + + db << "UPDATE nodes SET hash = NULL WHERE name IN (SELECT nodeName FROM claims WHERE expirationHeight = ?)" << nNextHeight; assert(expireSupportUndo.empty()); { @@ -1132,12 +1140,11 @@ bool CClaimTrieCacheBase::incrementBlock(insertUndoType& insertUndo, claimUndoTy std::string name; row >> value.outPoint.hash >> value.outPoint.n >> name >> value.supportedClaimId >> value.nValidAtHeight >> value.nHeight >> value.nAmount; - expireSupportUndo.emplace_back(name, value); + expireSupportUndo.emplace_back(std::move(name), std::move(value)); } } - db << "UPDATE nodes SET hash = NULL WHERE name IN (SELECT nodeName FROM supports WHERE expirationHeight = ?)" - << nNextHeight; + db << "UPDATE nodes SET hash = NULL WHERE name IN (SELECT nodeName FROM supports WHERE expirationHeight = ?)" << nNextHeight; // takeover handling: std::vector takeovers; @@ -1147,8 +1154,7 @@ bool CClaimTrieCacheBase::incrementBlock(insertUndoType& insertUndo, claimUndoTy }; auto getTakeoverQuery = db << "SELECT IFNULL(takeoverHeight, 0), takeoverID FROM nodes WHERE name = ?"; - auto hasCandidateQuery = db << "UPDATE nodes SET takeoverHeight = ?, takeoverID = ? WHERE name = ?"; - auto noCandidateQuery = db << "UPDATE nodes SET takeoverHeight = NULL, takeoverID = NULL WHERE name = ?"; + auto candidateQuery = db << "UPDATE nodes SET takeoverHeight = ?, takeoverID = ? WHERE name = ?"; for (const auto& nameWithTakeover : takeovers) { // if somebody activates on this block and they are the new best, then everybody activates on this block @@ -1177,21 +1183,17 @@ bool CClaimTrieCacheBase::incrementBlock(insertUndoType& insertUndo, claimUndoTy if (takeoverHappening || !hasBeenSetBefore) { takeoverUndo.emplace_back(nameWithTakeover, std::make_pair(existingHeight, hasBeenSetBefore ? *existingID : CUint160())); - if (hasCandidate) { - hasCandidateQuery << nNextHeight << candidateValue.claimId << nameWithTakeover; - hasCandidateQuery++; - } - else { - noCandidateQuery << nameWithTakeover; - noCandidateQuery++; - } + if (hasCandidate) + candidateQuery << nNextHeight << candidateValue.claimId << nameWithTakeover; + else + candidateQuery << nullptr << nullptr << nameWithTakeover; + candidateQuery++; assert(db.rows_modified()); } } getTakeoverQuery.used(true); - hasCandidateQuery.used(true); - noCandidateQuery.used(true); + candidateQuery.used(true); nNextHeight++; return true; } @@ -1201,38 +1203,30 @@ bool CClaimTrieCacheBase::activateAllFor(insertUndoType& insertUndo, insertUndoT // now that we know a takeover is happening, we bring everybody in: auto ret = false; { - auto query = db << "SELECT txID, txN, validHeight FROM claims WHERE nodeName = ?1 AND validHeight > ?2 AND expirationHeight > ?2" - << name << nNextHeight; - for (auto &&row: query) { - CUint256 hash; - uint32_t n; - int oldValidHeight; - row >> hash >> n >> oldValidHeight; - insertUndo.emplace_back(name, CTxOutPoint(hash, n), oldValidHeight); - logPrint << "Early activation of claim " << name << ", " << CTxOutPoint(hash, n).ToString() << " at " << nNextHeight << Clog::endl; - } + db << "SELECT txID, txN, validHeight FROM claims WHERE nodeName = ?1 AND validHeight > ?2 AND expirationHeight > ?2" + << name << nNextHeight + >> [this, &insertUndo, &name](CUint256 txId, uint32_t n, int oldValidHeight) { + CTxOutPoint outPoint(std::move(txId), n); + logPrint << "Early activation of claim " << name << ", " << outPoint.ToString() << " at " << nNextHeight << Clog::endl; + insertUndo.emplace_back(name, std::move(outPoint), oldValidHeight); + }; } // and then update them all to activate now: - db << "UPDATE claims SET validHeight = ?1 WHERE nodeName = ?2 AND validHeight > ?1 AND expirationHeight > ?1" - << nNextHeight << name; + db << "UPDATE claims SET validHeight = ?1 WHERE nodeName = ?2 AND validHeight > ?1 AND expirationHeight > ?1" << nNextHeight << name; ret |= db.rows_modified() > 0; // then do the same for supports: { - auto query = db << "SELECT txID, txN, validHeight FROM supports WHERE nodeName = ?1 AND validHeight > ?2 AND expirationHeight > ?2" - << name << nNextHeight; - for (auto &&row: query) { - CUint256 hash; - uint32_t n; - int oldValidHeight; - row >> hash >> n >> oldValidHeight; - insertSupportUndo.emplace_back(name, CTxOutPoint(hash, n), oldValidHeight); - logPrint << "Early activation of support " << name << ", " << CTxOutPoint(hash, n).ToString() << " at " << nNextHeight << Clog::endl; - } + db << "SELECT txID, txN, validHeight FROM supports WHERE nodeName = ?1 AND validHeight > ?2 AND expirationHeight > ?2" + << name << nNextHeight + >> [this, &insertSupportUndo, &name](CUint256 txId, uint32_t n, int oldValidHeight) { + CTxOutPoint outPoint(std::move(txId), n); + logPrint << "Early activation of support " << name << ", " << outPoint.ToString() << " at " << nNextHeight << Clog::endl; + insertSupportUndo.emplace_back(name, std::move(outPoint), oldValidHeight); + }; } // and then update them all to activate now: - db << "UPDATE supports SET validHeight = ?1 WHERE nodeName = ?2 AND validHeight > ?1 AND expirationHeight > ?1" - << nNextHeight << name; + db << "UPDATE supports SET validHeight = ?1 WHERE nodeName = ?2 AND validHeight > ?1 AND expirationHeight > ?1" << nNextHeight << name; ret |= db.rows_modified() > 0; return ret; } @@ -1240,32 +1234,32 @@ bool CClaimTrieCacheBase::activateAllFor(insertUndoType& insertUndo, insertUndoT bool CClaimTrieCacheBase::decrementBlock(insertUndoType& insertUndo, claimUndoType& expireUndo, insertUndoType& insertSupportUndo, supportUndoType& expireSupportUndo) { - if (!transacting) { transacting = true; db << "begin"; } + ensureTransacting(); nNextHeight--; + auto updateNodes = "INSERT INTO nodes(name) VALUES(?) ON CONFLICT(name) DO UPDATE SET hash = NULL"; + // to actually delete the expired items and then restore them here I would have to look up the metadata in the block // that doesn't sound very fun so we modified the other queries to exclude expired items - for (auto it = expireSupportUndo.crbegin(); it != expireSupportUndo.crend(); ++it) { - db << "INSERT INTO nodes(name) VALUES(?) ON CONFLICT(name) DO UPDATE SET hash = NULL" << it->first; - } + for (auto it = expireSupportUndo.crbegin(); it != expireSupportUndo.crend(); ++it) + db << updateNodes << it->first; - for (auto it = expireUndo.crbegin(); it != expireUndo.crend(); ++it) { - db << "INSERT INTO nodes(name) VALUES(?) ON CONFLICT(name) DO UPDATE SET hash = NULL" << it->first; - } + for (auto it = expireUndo.crbegin(); it != expireUndo.crend(); ++it) + db << updateNodes << it->first; for (auto it = insertSupportUndo.crbegin(); it != insertSupportUndo.crend(); ++it) { logPrint << "Resetting support valid height to " << it->nValidHeight << " for " << it->name << Clog::endl; db << "UPDATE supports SET validHeight = ? WHERE txID = ? AND txN = ?" << it->nValidHeight << it->outPoint.hash << it->outPoint.n; - db << "INSERT INTO nodes(name) VALUES(?) ON CONFLICT(name) DO UPDATE SET hash = NULL" << it->name; + db << updateNodes << it->name; } for (auto it = insertUndo.crbegin(); it != insertUndo.crend(); ++it) { logPrint << "Resetting valid height to " << it->nValidHeight << " for " << it->name << Clog::endl; db << "UPDATE claims SET validHeight = ? WHERE nodeName = ? AND txID = ? AND txN = ?" << it->nValidHeight << it->name << it->outPoint.hash << it->outPoint.n; - db << "INSERT INTO nodes(name) VALUES(?) ON CONFLICT(name) DO UPDATE SET hash = NULL" << it->name; + db << updateNodes << it->name; } return true; @@ -1278,12 +1272,13 @@ bool CClaimTrieCacheBase::finalizeDecrement(takeoverUndoType& takeoverUndo) db << "UPDATE nodes SET hash = NULL WHERE name IN " "(SELECT nodeName FROM supports WHERE validHeight = ?1 AND expirationHeight > ?1)" << nNextHeight; + auto updateNodes = "UPDATE nodes SET takeoverHeight = ?, takeoverID = ?, hash = NULL WHERE name = ?"; + for (auto it = takeoverUndo.crbegin(); it != takeoverUndo.crend(); ++it) { if (it->second.second.IsNull()) - db << "UPDATE nodes SET takeoverHeight = NULL, takeoverID = NULL, hash = NULL WHERE name = ?" << it->first; + db << updateNodes << nullptr << nullptr << it->first; else - db << "UPDATE nodes SET takeoverHeight = ?, takeoverID = ?, hash = NULL WHERE name = ?" - << it->second.first << it->second.second << it->first; + db << updateNodes << it->second.first << it->second.second << it->first; } return true; } @@ -1319,11 +1314,7 @@ bool CClaimTrieCacheBase::getProofForName(const std::string& name, const CUint16 // cache the parent nodes getMerkleHash(); proof = CClaimTrieProof(); - auto nodeQuery = db << "SELECT name, IFNULL(takeoverHeight, 0) FROM nodes WHERE " - "name IN (WITH RECURSIVE prefix(p) AS (VALUES(?) UNION ALL " - "SELECT POPS(p) FROM prefix WHERE p != '') SELECT p FROM prefix) " - "ORDER BY name" << name; - for (auto&& row: nodeQuery) { + for (auto&& row: db << proofClaimQuery << name) { CClaimValue claim; std::string key; int takeoverHeight; @@ -1371,7 +1362,7 @@ bool CClaimTrieCacheBase::findNameForClaim(std::vector claim, CCl { std::reverse(claim.begin(), claim.end()); auto query = db << "SELECT nodeName, claimId, txID, txN, amount, validHeight, blockHeight " - "FROM claims WHERE SUBSTR(claimID, ?) = ? AND validHeight < ? AND expirationHeight >= ?" + "FROM claims WHERE SUBSTR(claimID, ?) = ? AND validHeight < ? AND expirationHeight >= ?" << -int(claim.size()) << claim << nNextHeight << nNextHeight; auto hit = false; for (auto&& row: query) { @@ -1385,11 +1376,9 @@ bool CClaimTrieCacheBase::findNameForClaim(std::vector claim, CCl void CClaimTrieCacheBase::getNamesInTrie(std::function callback) const { - auto query = db << "SELECT DISTINCT nodeName FROM claims WHERE validHeight < ? AND expirationHeight >= ?" - << nNextHeight << nNextHeight; - for (auto&& row: query) { - std::string name; - row >> name; - callback(name); - } + db << "SELECT DISTINCT nodeName FROM claims WHERE validHeight < ? AND expirationHeight >= ?" + << nNextHeight << nNextHeight + >> [&callback](const std::string& name) { + callback(name); + }; } diff --git a/src/claimtrie/trie.h b/src/claimtrie/trie.h index a41d9574f..1592885f7 100644 --- a/src/claimtrie/trie.h +++ b/src/claimtrie/trie.h @@ -130,10 +130,10 @@ public: bool findNameForClaim(std::vector claim, CClaimValue& value, std::string& name) const; protected: + int nNextHeight; // Height of the block that is being worked on, which is CClaimTrie* base; mutable sqlite::database db; - int nNextHeight; // Height of the block that is being worked on, which is - bool transacting; + const std::string proofClaimQuery; mutable std::unordered_set removalWorkaround; mutable sqlite::database_binder claimHashQuery, childHashQuery; @@ -145,8 +145,10 @@ protected: bool deleteNodeIfPossible(const std::string& name, std::string& parent, int64_t& claims); void ensureTreeStructureIsUpToDate(); + void ensureTransacting(); private: + bool transacting; // for unit test friend struct ClaimTrieChainFixture; friend class CClaimTrieCacheTest; diff --git a/src/test/claimtrieexpirationfork_tests.cpp b/src/test/claimtrieexpirationfork_tests.cpp index 2ad8e5eeb..eeea2ec46 100644 --- a/src/test/claimtrieexpirationfork_tests.cpp +++ b/src/test/claimtrieexpirationfork_tests.cpp @@ -140,11 +140,14 @@ BOOST_AUTO_TEST_CASE(hardfork_claim_test) // Ensure that we cannot update the original pre-fork expired claim CMutableTransaction u1 = fixture.MakeUpdate(tx1,"test","two",ClaimIdHash(tx1.GetHash(),0), 3); fixture.IncrementBlocks(1); - BOOST_CHECK(!fixture.is_best_claim("test",u1)); + BOOST_CHECK(!fixture.haveClaim("test", COutPoint(u1.GetHash(), 0))); + BOOST_CHECK(fixture.is_best_claim("test",tx3)); // Ensure that supports for the expired claim don't support it CMutableTransaction s1 = fixture.MakeSupport(fixture.GetCoinbase(),u1,"test",10); - BOOST_CHECK(!fixture.is_best_claim("test",u1)); + fixture.IncrementBlocks(1); + BOOST_CHECK(!fixture.haveClaim("test", COutPoint(u1.GetHash(), 0))); + BOOST_CHECK(fixture.is_best_claim("test",tx3)); // Ensure that we can update the new post-fork claim CMutableTransaction u2 = fixture.MakeUpdate(tx3,"test","two",ClaimIdHash(tx3.GetHash(),0), 1); @@ -153,6 +156,7 @@ BOOST_AUTO_TEST_CASE(hardfork_claim_test) // Ensure that supports for the new post-fork claim CMutableTransaction s2 = fixture.MakeSupport(fixture.GetCoinbase(),u2,"test",3); + fixture.IncrementBlocks(1); BOOST_CHECK(fixture.is_best_claim("test",u2)); } diff --git a/src/test/claimtriefixture.cpp b/src/test/claimtriefixture.cpp index c64c9402c..633a1bf8e 100644 --- a/src/test/claimtriefixture.cpp +++ b/src/test/claimtriefixture.cpp @@ -401,7 +401,8 @@ boost::test_tools::predicate_result ClaimTrieChainFixture::best_claim_effective_ return true; } -bool ClaimTrieChainFixture::getClaimById(const uint160 &claimId, std::string &name, CClaimValue &value) { +bool ClaimTrieChainFixture::getClaimById(const CUint160 &claimId, std::string &name, CClaimValue &value) +{ auto query = db << "SELECT nodeName, claimID, txID, txN, amount, validHeight, blockHeight " "FROM claims WHERE claimID = ?" << claimId; auto hit = false; @@ -414,7 +415,8 @@ bool ClaimTrieChainFixture::getClaimById(const uint160 &claimId, std::string &na return hit; } -std::vector ClaimTrieChainFixture::getNodeChildren(const std::string &name) { +std::vector ClaimTrieChainFixture::getNodeChildren(const std::string &name) +{ std::vector ret; for (auto&& row: db << "SELECT name FROM nodes WHERE parent = ?" << name) { ret.emplace_back(); diff --git a/src/test/claimtriefixture.h b/src/test/claimtriefixture.h index 44d82e334..8bd0900c8 100644 --- a/src/test/claimtriefixture.h +++ b/src/test/claimtriefixture.h @@ -105,7 +105,7 @@ struct ClaimTrieChainFixture: public CClaimTrieCache int proportionalDelayFactor() const; - bool getClaimById(const uint160& claimId, std::string& name, CClaimValue& value); + bool getClaimById(const CUint160& claimId, std::string& name, CClaimValue& value); // is a claim in queue boost::test_tools::predicate_result is_claim_in_queue(const std::string& name, const CTransaction &tx);