From f9cb79349b51d8099d3e85a782b6af6c0c8cdf2a Mon Sep 17 00:00:00 2001 From: Brannon King Date: Tue, 17 Dec 2019 16:40:28 -0700 Subject: [PATCH] added additional unit tests, optimized queries --- src/chainparams.cpp | 6 ++++ src/claimtrie/trie.cpp | 40 +++++++++++------------ src/claimtrie/trie.h | 3 ++ src/consensus/params.h | 3 ++ src/init.cpp | 2 ++ src/rpc/claimtrie.cpp | 2 +- src/test/claimtriebranching_tests.cpp | 35 +++++++++++++++++++- src/test/claimtriefixture.cpp | 33 ++++++++++++++++++- src/test/claimtriefixture.h | 5 +++ src/test/claimtrienormalization_tests.cpp | 38 +++++++++++++++++++++ src/test/claimtrierpc_tests.cpp | 4 +-- src/test/setup_common.cpp | 2 ++ 12 files changed, 147 insertions(+), 26 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index d3f60d5b8..5a1af7619 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -135,6 +135,8 @@ public: consensus.nAllowMinDiffMinHeight = -1; consensus.nAllowMinDiffMaxHeight = -1; consensus.nNormalizedNameForkHeight = 539940; // targeting 21 March 2019 + consensus.nMinRemovalWorkaroundHeight = 297706; + consensus.nMaxRemovalWorkaroundHeight = 100000000; consensus.nWitnessForkHeight = 680770; // targeting 11 Dec 2019 consensus.nAllClaimsInMerkleForkHeight = 658310; // targeting 30 Oct 2019 consensus.fPowAllowMinDifficultyBlocks = false; @@ -244,6 +246,8 @@ public: consensus.nAllowMinDiffMinHeight = 277299; consensus.nAllowMinDiffMaxHeight = 1100000; consensus.nNormalizedNameForkHeight = 993380; // targeting, 21 Feb 2019 + consensus.nMinRemovalWorkaroundHeight = 99; + consensus.nMaxRemovalWorkaroundHeight = 100000000; consensus.nWitnessForkHeight = 1198600; consensus.nAllClaimsInMerkleForkHeight = 1198560; // targeting 30 Sep 2019 consensus.fPowAllowMinDifficultyBlocks = true; @@ -341,6 +345,8 @@ public: consensus.nAllowMinDiffMinHeight = -1; consensus.nAllowMinDiffMaxHeight = -1; consensus.nNormalizedNameForkHeight = 250; // SDK depends upon this number + consensus.nMinRemovalWorkaroundHeight = -1; + consensus.nMaxRemovalWorkaroundHeight = -1; consensus.nWitnessForkHeight = 150; consensus.nAllClaimsInMerkleForkHeight = 350; consensus.fPowAllowMinDifficultyBlocks = false; diff --git a/src/claimtrie/trie.cpp b/src/claimtrie/trie.cpp index 26e9d1816..49d865741 100644 --- a/src/claimtrie/trie.cpp +++ b/src/claimtrie/trie.cpp @@ -49,6 +49,8 @@ void applyPragmas(sqlite::database& db, std::size_t cache) CClaimTrie::CClaimTrie(std::size_t cacheBytes, bool fWipe, int height, const std::string& dataDir, int nNormalizedNameForkHeight, + int nMinRemovalWorkaroundHeight, + int nMaxRemovalWorkaroundHeight, int64_t nOriginalClaimExpirationTime, int64_t nExtendedClaimExpirationTime, int64_t nExtendedClaimExpirationForkHeight, @@ -59,6 +61,8 @@ CClaimTrie::CClaimTrie(std::size_t cacheBytes, bool fWipe, int height, dbFile(dataDir + "/claims.sqlite"), db(dbFile, sharedConfig), nProportionalDelayFactor(proportionalDelayFactor), nNormalizedNameForkHeight(nNormalizedNameForkHeight), + nMinRemovalWorkaroundHeight(nMinRemovalWorkaroundHeight), + nMaxRemovalWorkaroundHeight(nMaxRemovalWorkaroundHeight), nOriginalClaimExpirationTime(nOriginalClaimExpirationTime), nExtendedClaimExpirationTime(nExtendedClaimExpirationTime), nExtendedClaimExpirationForkHeight(nExtendedClaimExpirationForkHeight), @@ -639,24 +643,13 @@ bool CClaimTrieCacheBase::removeClaim(const uint160& claimId, const COutPoint& o // when node should be deleted from cache but instead it's kept // 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 (nNextHeight >= 297706) { // TODO: hard fork this out (which we already tried once but failed) - // we have to jump through some hoops here to make the claim_nodeName index be used on partial blobs - // neither LIKE nor SUBSTR will use an index on a blob (which is lame because it would be simple) - auto end = nodeName; - auto usingRange = false; - if (!end.empty() && end.back() < std::numeric_limits::max()) { - ++end.back(); // the fast path - usingRange = true; - } - // else - // end += std::string(256U, std::numeric_limits::max()); // 256 is our supposed max length claim, but that's not enforced anywhere presently - auto query = usingRange ? - (db << "SELECT nodeName FROM claim WHERE nodeName BETWEEN ?1 AND ?2 " - "AND nodeName != ?2 AND activationHeight < ?3 AND expirationHeight > ?3 ORDER BY nodeName LIMIT 1" - << nodeName << end << nNextHeight) : - (db << "SELECT nodeName FROM claim INDEXED BY claim_nodeName WHERE SUBSTR(nodeName, 1, ?3) = ?1 " - "AND activationHeight < ?2 AND expirationHeight > ?2 ORDER BY nodeName LIMIT 1" - << nodeName << nNextHeight << nodeName.size()); + if (nNextHeight >= base->nMinRemovalWorkaroundHeight + && nNextHeight < base->nMaxRemovalWorkaroundHeight) { // TODO: hard fork this out (which we already tried once but failed) + // neither LIKE nor SUBSTR will use an index on a blob, but BETWEEN is a good, fast alternative + auto end = nodeName + std::string( 256, std::numeric_limits::max()); // 256 == MAX_CLAIM_NAME_SIZE + 1 + auto query = db << "SELECT nodeName FROM claim WHERE nodeName BETWEEN ?1 AND ?2 " + "AND activationHeight < ?3 AND expirationHeight >= ?3 ORDER BY nodeName LIMIT 1" + << nodeName << end << nNextHeight; for (auto&& row: query) { std::string shortestMatch; row >> shortestMatch; @@ -875,10 +868,15 @@ bool CClaimTrieCacheBase::getProofForName(const std::string& name, const uint160 bool CClaimTrieCacheBase::findNameForClaim(std::vector claim, CClaimValue& value, std::string& name) const { - std::reverse(claim.begin(), claim.end()); + if (claim.size() > 20U) // expecting RIPEMD160 -- 20 chars + return false; + // because the data coming in here is reversed we support removing chars from the left of the claimID + auto start = std::string(claim.rbegin(), claim.rend()); + auto end = start + std::string(21U - claim.size(), std::numeric_limits::max()); auto query = db << "SELECT nodeName, claimID, txID, txN, amount, activationHeight, blockHeight " - "FROM claim WHERE SUBSTR(claimID, ?1) = ?2 AND activationHeight < ?3 AND expirationHeight >= ?3" - << -int(claim.size()) << claim << nNextHeight; + "FROM claim WHERE claimID BETWEEN ?1 AND ?2 AND activationHeight < ?3 AND expirationHeight >= ?3 " + "LIMIT 2" + << start << end << nNextHeight; auto hit = false; for (auto&& row: query) { if (hit) return false; diff --git a/src/claimtrie/trie.h b/src/claimtrie/trie.h index 5358678b4..5beea596d 100644 --- a/src/claimtrie/trie.h +++ b/src/claimtrie/trie.h @@ -30,6 +30,8 @@ public: CClaimTrie(std::size_t cacheBytes, bool fWipe, int height = 0, const std::string& dataDir = ".", int nNormalizedNameForkHeight = 1, + int nMinRemovalWorkaroundHeight = 1, + int nMaxRemovalWorkaroundHeight = -1, int64_t nOriginalClaimExpirationTime = 1, int64_t nExtendedClaimExpirationTime = 1, int64_t nExtendedClaimExpirationForkHeight = 1, @@ -50,6 +52,7 @@ protected: const int nProportionalDelayFactor; const int nNormalizedNameForkHeight; + const int nMinRemovalWorkaroundHeight, nMaxRemovalWorkaroundHeight; const int64_t nOriginalClaimExpirationTime; const int64_t nExtendedClaimExpirationTime; const int64_t nExtendedClaimExpirationForkHeight; diff --git a/src/consensus/params.h b/src/consensus/params.h index afbbfcd45..33dd801f7 100644 --- a/src/consensus/params.h +++ b/src/consensus/params.h @@ -85,6 +85,9 @@ struct Params { int nAllowMinDiffMaxHeight; int nNormalizedNameForkHeight; + int nMinRemovalWorkaroundHeight; + int nMaxRemovalWorkaroundHeight; + int nWitnessForkHeight; int64_t nPowTargetSpacing; diff --git a/src/init.cpp b/src/init.cpp index c74304cc6..d745fcc6b 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1546,6 +1546,8 @@ bool AppInitMain(InitInterfaces& interfaces) pclaimTrie = new CClaimTrie(nClaimtrieCache, fReindex || fReindexChainState, 0, GetDataDir().string(), consensus.nNormalizedNameForkHeight, + consensus.nMinRemovalWorkaroundHeight, + consensus.nMaxRemovalWorkaroundHeight, consensus.nOriginalClaimExpirationTime, consensus.nExtendedClaimExpirationTime, consensus.nExtendedClaimExpirationForkHeight, diff --git a/src/rpc/claimtrie.cpp b/src/rpc/claimtrie.cpp index 9034dbfdb..dfd12daee 100644 --- a/src/rpc/claimtrie.cpp +++ b/src/rpc/claimtrie.cpp @@ -453,7 +453,7 @@ UniValue getclaimbyid(const JSONRPCRequest& request) std::string claimId; ParseClaimtrieId(request.params[0], claimId, T_CLAIMID " (parameter 1)"); - if (claimId.length() < 3) + if (claimId.length() < 6) throw JSONRPCError(RPC_INVALID_PARAMETER, T_CLAIMID " (parameter 1) should be at least 3 chars"); std::string foundName; diff --git a/src/test/claimtriebranching_tests.cpp b/src/test/claimtriebranching_tests.cpp index 29cbb40da..09bb3a458 100644 --- a/src/test/claimtriebranching_tests.cpp +++ b/src/test/claimtriebranching_tests.cpp @@ -56,7 +56,40 @@ BOOST_AUTO_TEST_CASE(unaffected_children_get_new_parents_test) { auto n2 = fixture.getNodeChildren("longest1"); BOOST_CHECK_EQUAL(n1[0], "longest1"); BOOST_CHECK_EQUAL(n2[0], "longest123"); - // TODO: this test at present fails to cover the split node case of the same thing (which occurs on block 202577) + BOOST_CHECK_EQUAL(4, fixture.nodeCount()); + + // now test the split node case of the same thing (which occurs on block 202577) + CMutableTransaction tx5 = fixture.MakeClaim(fixture.GetCoinbase(), "longing", "5", 5); + CMutableTransaction tx6 = fixture.MakeClaim(fixture.GetCoinbase(), "longing1234", "5", 5); + fixture.IncrementBlocks(1); + BOOST_CHECK_EQUAL(7, fixture.nodeCount()); + auto n3 = fixture.getNodeChildren("long"); + auto n4 = fixture.getNodeChildren("longing"); + BOOST_CHECK_EQUAL(n4[0], "longing1234"); + BOOST_CHECK_EQUAL(2U, n3.size()); +} + +BOOST_AUTO_TEST_CASE(gen2_update_wins) { + ClaimTrieChainFixture fixture; + CMutableTransaction tx1 = fixture.MakeClaim(fixture.GetCoinbase(), "a", "1", 1); + fixture.IncrementBlocks(2); + CMutableTransaction tx2 = fixture.MakeClaim(fixture.GetCoinbase(), "a", "2", 2); + fixture.IncrementBlocks(2); + CMutableTransaction tx3 = fixture.MakeUpdate(tx1, "a", "3", ClaimIdHash(tx1.GetHash(), 0), 3); + fixture.IncrementBlocks(2); + BOOST_CHECK(fixture.is_best_claim("a",tx3)); +} + +BOOST_AUTO_TEST_CASE(gen3_update_wins) { + ClaimTrieChainFixture fixture; + CMutableTransaction tx1 = fixture.MakeClaim(fixture.GetCoinbase(), "a", "1", 1); + CMutableTransaction tx2 = fixture.MakeClaim(fixture.GetCoinbase(), "a", "2", 4); + fixture.IncrementBlocks(1); + CMutableTransaction tx3 = fixture.MakeUpdate(tx1, "a", "3", ClaimIdHash(tx1.GetHash(), 0), 3); + fixture.IncrementBlocks(1); + CMutableTransaction tx4 = fixture.MakeUpdate(tx3, "a", "3", ClaimIdHash(tx1.GetHash(), 0), 5); + fixture.IncrementBlocks(3); + BOOST_CHECK(fixture.is_best_claim("a",tx4)); } /* diff --git a/src/test/claimtriefixture.cpp b/src/test/claimtriefixture.cpp index b7aa05794..411d61658 100644 --- a/src/test/claimtriefixture.cpp +++ b/src/test/claimtriefixture.cpp @@ -74,7 +74,8 @@ BlockAssembler AssemblerForTest() } ClaimTrieChainFixture::ClaimTrieChainFixture() : CClaimTrieCache(pclaimTrie), - unique_block_counter(0), normalization_original(-1), expirationForkHeight(-1), forkhash_original(-1) + unique_block_counter(0), normalization_original(-1), expirationForkHeight(-1), forkhash_original(-1), + minRemovalWorkaroundHeight(-1), maxRemovalWorkaroundHeight(-1) { fRequireStandard = false; BOOST_CHECK_EQUAL(nNextHeight, chainActive.Height() + 1); @@ -114,6 +115,25 @@ ClaimTrieChainFixture::~ClaimTrieChainFixture() consensus.nAllClaimsInMerkleForkHeight = forkhash_original; const_cast(base->nAllClaimsInMerkleForkHeight) = forkhash_original; } + if (minRemovalWorkaroundHeight >= 0) { + consensus.nMinRemovalWorkaroundHeight = minRemovalWorkaroundHeight; + consensus.nMaxRemovalWorkaroundHeight = maxRemovalWorkaroundHeight; + const_cast(base->nMinRemovalWorkaroundHeight) = minRemovalWorkaroundHeight; + const_cast(base->nMaxRemovalWorkaroundHeight) = maxRemovalWorkaroundHeight; + } +} + +void ClaimTrieChainFixture::setRemovalWorkaroundHeight(int targetMinusCurrent, int blocks = 1000) { + int target = chainActive.Height() + targetMinusCurrent; + auto& consensus = const_cast(Params().GetConsensus()); + if (minRemovalWorkaroundHeight < 0) { + minRemovalWorkaroundHeight = consensus.nMinRemovalWorkaroundHeight; + maxRemovalWorkaroundHeight = consensus.nMaxRemovalWorkaroundHeight; + } + consensus.nMinRemovalWorkaroundHeight = target; + consensus.nMaxRemovalWorkaroundHeight = target + blocks; + const_cast(base->nMinRemovalWorkaroundHeight) = target; + const_cast(base->nMaxRemovalWorkaroundHeight) = target + blocks; } void ClaimTrieChainFixture::setExpirationForkHeight(int targetMinusCurrent, int64_t preForkExpirationTime, int64_t postForkExpirationTime) @@ -263,6 +283,11 @@ CMutableTransaction ClaimTrieChainFixture::MakeSupport(const CTransaction &prev, CMutableTransaction ClaimTrieChainFixture::MakeUpdate(const CTransaction &prev, const std::string& name, const std::string& value, const uint160& claimId, CAmount quantity) { CMutableTransaction tx = BuildTransaction(prev, 0); + if (prev.vout[0].nValue < quantity) { + auto coverIt = GetCoinbase(); + tx.vin.push_back(CTxIn(coverIt.GetHash(), 0)); + } + tx.vout[0].scriptPubKey = UpdateClaimScript(name, claimId, value); tx.vout[0].nValue = quantity; @@ -418,6 +443,12 @@ bool ClaimTrieChainFixture::getClaimById(const uint160 &claimId, std::string &na return hit; } +int64_t ClaimTrieChainFixture::nodeCount() const { + int64_t ret = 0; + db << "SELECT COUNT(*) FROM node" >> ret; + return ret; +} + std::vector ClaimTrieChainFixture::getNodeChildren(const std::string &name) { std::vector ret; diff --git a/src/test/claimtriefixture.h b/src/test/claimtriefixture.h index 61274c3cb..f60e1b88d 100644 --- a/src/test/claimtriefixture.h +++ b/src/test/claimtriefixture.h @@ -49,6 +49,7 @@ struct ClaimTrieChainFixture: public CClaimTrieCache int64_t originalExpiration; int64_t extendedExpiration; int64_t forkhash_original; + int minRemovalWorkaroundHeight, maxRemovalWorkaroundHeight; using CClaimTrieCache::getSupportsForName; @@ -62,6 +63,8 @@ struct ClaimTrieChainFixture: public CClaimTrieCache void setHashForkHeight(int targetMinusCurrent); + void setRemovalWorkaroundHeight(int targetMinusCurrent, int blocks); + bool CreateBlock(const std::unique_ptr& pblocktemplate); bool CreateCoinbases(unsigned int num_coinbases, std::vector& coinbases); @@ -105,6 +108,8 @@ struct ClaimTrieChainFixture: public CClaimTrieCache bool getClaimById(const uint160& claimId, std::string& name, CClaimValue& value); + int64_t nodeCount() const; + // is a claim in queue boost::test_tools::predicate_result is_claim_in_queue(const std::string& name, const CTransaction &tx); diff --git a/src/test/claimtrienormalization_tests.cpp b/src/test/claimtrienormalization_tests.cpp index 263a8fbf6..6f127bf16 100644 --- a/src/test/claimtrienormalization_tests.cpp +++ b/src/test/claimtrienormalization_tests.cpp @@ -375,6 +375,44 @@ BOOST_AUTO_TEST_CASE(normalization_does_not_fail_on_spend) BOOST_CHECK(fixture.is_best_claim(sName1, tx1)); } +BOOST_AUTO_TEST_CASE(unnormalized_expires_before_fork) +{ + ClaimTrieChainFixture fixture; + fixture.setNormalizationForkHeight(4); + fixture.setExpirationForkHeight(1, 3, 3); + + CMutableTransaction tx1 = fixture.MakeClaim(fixture.GetCoinbase(), "A", "1", 1); + fixture.IncrementBlocks(3); + CMutableTransaction sp1 = fixture.MakeSupport(fixture.GetCoinbase(), tx1, "A", 1); + BOOST_CHECK(fixture.is_best_claim("A", tx1)); + fixture.IncrementBlocks(1); + BOOST_CHECK(!fixture.is_best_claim("A", tx1)); + BOOST_CHECK(!fixture.is_best_claim("a", tx1)); + fixture.IncrementBlocks(1); + BOOST_CHECK(!fixture.is_best_claim("A", tx1)); + BOOST_CHECK(!fixture.is_best_claim("a", tx1)); + fixture.DecrementBlocks(2); + BOOST_CHECK(fixture.is_best_claim("A", tx1)); +} + +BOOST_AUTO_TEST_CASE(unnormalized_activates_on_fork) +{ + // this was an unfortunate bug; the normalization fork should not have activated anything + // alas, it's now part of our history; we hereby test it to keep it that way + ClaimTrieChainFixture fixture; + fixture.setNormalizationForkHeight(4); + CMutableTransaction tx1 = fixture.MakeClaim(fixture.GetCoinbase(), "A", "1", 1); + fixture.IncrementBlocks(3); + BOOST_CHECK(fixture.is_best_claim("A", tx1)); + CMutableTransaction tx2 = fixture.MakeClaim(fixture.GetCoinbase(), "A", "2", 2); + fixture.IncrementBlocks(1); + BOOST_CHECK(fixture.is_best_claim("a", tx2)); + fixture.IncrementBlocks(2); + BOOST_CHECK(fixture.is_best_claim("a", tx2)); + fixture.DecrementBlocks(3); + BOOST_CHECK(fixture.is_best_claim("A", tx1)); +} + BOOST_AUTO_TEST_CASE(normalization_does_not_kill_sort_order) { ClaimTrieChainFixture fixture; diff --git a/src/test/claimtrierpc_tests.cpp b/src/test/claimtrierpc_tests.cpp index 6fd149565..e6a039883 100644 --- a/src/test/claimtrierpc_tests.cpp +++ b/src/test/claimtrierpc_tests.cpp @@ -306,7 +306,7 @@ BOOST_AUTO_TEST_CASE(hash_bid_seq_claim_changes_test) // check by partial id (at least 3 chars) req.params = UniValue(UniValue::VARR); - req.params.push_back(UniValue(claimId3.GetHex().substr(0, 3))); + req.params.push_back(UniValue(claimId3.GetHex().substr(34))); result = getclaimbyid(req); BOOST_CHECK_EQUAL(result[T_LASTTAKEOVERHEIGHT].get_int(), height); @@ -331,7 +331,7 @@ BOOST_AUTO_TEST_CASE(hash_bid_seq_claim_changes_test) req.params = UniValue(UniValue::VARR); req.params.push_back(UniValue(name)); req.params.push_back(UniValue(blockhash.GetHex())); - req.params.push_back(UniValue(claimId2.GetHex().substr(0, 2))); + req.params.push_back(UniValue(claimId2.GetHex().substr(14))); result = getnameproof(req); claimHash = getValueHash(COutPoint(tx2.GetHash(), 0), result[T_LASTTAKEOVERHEIGHT].get_int()); diff --git a/src/test/setup_common.cpp b/src/test/setup_common.cpp index 032ea6fc5..f9a52f29b 100644 --- a/src/test/setup_common.cpp +++ b/src/test/setup_common.cpp @@ -129,6 +129,8 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha auto &consensus = chainparams.GetConsensus(); pclaimTrie = new CClaimTrie(20000000U, true, 0, GetDataDir().string(), consensus.nNormalizedNameForkHeight, + consensus.nMinRemovalWorkaroundHeight, + consensus.nMaxRemovalWorkaroundHeight, consensus.nOriginalClaimExpirationTime, consensus.nExtendedClaimExpirationTime, consensus.nExtendedClaimExpirationForkHeight,