From 865f7466363bc09fc64b5d673c69249841ae70a6 Mon Sep 17 00:00:00 2001 From: Kay Kurokawa Date: Tue, 26 Dec 2017 14:54:23 -0500 Subject: [PATCH 1/3] add unit test to see if having a support and an abandon in the same block when there are not other claim will crash lbryrcrd --- src/test/claimtriebranching_tests.cpp | 69 ++++++++++++++++++++++++++- 1 file changed, 68 insertions(+), 1 deletion(-) diff --git a/src/test/claimtriebranching_tests.cpp b/src/test/claimtriebranching_tests.cpp index f2bdd1f9e..94a0d65ea 100644 --- a/src/test/claimtriebranching_tests.cpp +++ b/src/test/claimtriebranching_tests.cpp @@ -374,6 +374,7 @@ BOOST_AUTO_TEST_CASE(claimtriebranching_claim) fixture.DecrementBlocks(10); } + /* spent claims spending winning claim will make losing active claim winner @@ -430,6 +431,7 @@ BOOST_AUTO_TEST_CASE(claimtriebranching_spend_claim) fixture.DecrementBlocks(1); } + /* supports check support with wrong name does not work @@ -470,7 +472,72 @@ BOOST_AUTO_TEST_CASE(claimtriebranching_support) BOOST_CHECK(is_best_claim("test",tx4)); BOOST_CHECK(best_claim_effective_amount_equals("test",2)); fixture.DecrementBlocks(10); -} +} + +/* + support on abandon + supporting a claim the same block it gets abandoned, + or abandoing a support the same block claim gets abandoned, + when there were no other claims would crash lbrycrd, + make sure this doesn't happen in below three tests + (https://github.com/lbryio/lbrycrd/issues/77) +*/ +BOOST_AUTO_TEST_CASE(claimtriebranching_support_on_abandon) +{ + ClaimTrieChainFixture fixture; + CMutableTransaction tx1 = fixture.MakeClaim(fixture.GetCoinbase(),"test","one",2); + fixture.IncrementBlocks(1); + + //supporting and abandoing on the same block will cause it to crash + CMutableTransaction s1 = fixture.MakeSupport(fixture.GetCoinbase(),tx1,"test",1); + fixture.Spend(tx1); + fixture.IncrementBlocks(1); + BOOST_CHECK(!is_best_claim("test",tx1)); + + fixture.DecrementBlocks(1); + BOOST_CHECK(is_best_claim("test",tx1)); + +} + +BOOST_AUTO_TEST_CASE(claimtriebranching_support_on_abandon_2) +{ + ClaimTrieChainFixture fixture; + CMutableTransaction tx1 = fixture.MakeClaim(fixture.GetCoinbase(),"test","one",2); + CMutableTransaction s1 = fixture.MakeSupport(fixture.GetCoinbase(),tx1,"test",1); + fixture.IncrementBlocks(1); + + //abandoning a support and abandoing claim on the same block will cause it to crash + fixture.Spend(tx1); + fixture.Spend(s1); + fixture.IncrementBlocks(1); + BOOST_CHECK(!is_best_claim("test",tx1)); + + fixture.DecrementBlocks(1); + BOOST_CHECK(is_best_claim("test",tx1)); +} + +BOOST_AUTO_TEST_CASE(claimtriebranching_update_on_support) +{ + // make sure that support on abandon bug does not affect + // updates happening on the same block as a support + // (it should not) + ClaimTrieChainFixture fixture; + CMutableTransaction tx1 = fixture.MakeClaim(fixture.GetCoinbase(),"test","one",3); + fixture.IncrementBlocks(1); + + CMutableTransaction s1 = fixture.MakeSupport(fixture.GetCoinbase(),tx1,"test",1); + CMutableTransaction u1 = fixture.MakeUpdate(tx1,"test","two",ClaimIdHash(tx1.GetHash(),0),1); + fixture.IncrementBlocks(1); + + BOOST_CHECK(is_best_claim("test",u1)); + BOOST_CHECK(best_claim_effective_amount_equals("test",2)); + + fixture.DecrementBlocks(1); + BOOST_CHECK(is_best_claim("test",tx1)); +} + + + /* support spend spending suport on winning claim will cause it to lose From 40e034ef1baabe4a14448b1f387f2f2646f6fd35 Mon Sep 17 00:00:00 2001 From: Kay Kurokawa Date: Tue, 26 Dec 2017 15:19:55 -0500 Subject: [PATCH 2/3] add logging messages to debug the crashing support on abandon issue --- src/claimtrie.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/claimtrie.cpp b/src/claimtrie.cpp index 59f12eaf2..6b39f47d6 100644 --- a/src/claimtrie.cpp +++ b/src/claimtrie.cpp @@ -769,17 +769,26 @@ bool CClaimTrie::update(nodeCacheType& cache, hashMapType& hashes, std::mapfirst, itcache->second)) + { + LogPrintf("%s: Failed to update name for:%s\n", __func__, itcache->first); return false; + } } for (hashMapType::iterator ithash = hashes.begin(); ithash != hashes.end(); ++ithash) { if (!updateHash(ithash->first, ithash->second)) + { + LogPrintf("%s: Failed to update hash for:%s\n", __func__, ithash->first); return false; + } } for (std::map::iterator itheight = takeoverHeights.begin(); itheight != takeoverHeights.end(); ++itheight) { if (!updateTakeoverHeight(itheight->first, itheight->second)) + { + LogPrintf("%s: Failed to update takeover height for:%s\n", __func__, itheight->first); return false; + } } for (claimQueueType::iterator itQueueCacheRow = queueCache.begin(); itQueueCacheRow != queueCache.end(); ++itQueueCacheRow) { From d29f110a11b4abf11bf619f21ce52fbf27cba11e Mon Sep 17 00:00:00 2001 From: Kay Kurokawa Date: Tue, 26 Dec 2017 15:12:52 -0500 Subject: [PATCH 3/3] fix bug where we did not check the root node in the cache when reordering the trie --- src/claimtrie.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/claimtrie.cpp b/src/claimtrie.cpp index 6b39f47d6..b44fa255b 100644 --- a/src/claimtrie.cpp +++ b/src/claimtrie.cpp @@ -1704,7 +1704,12 @@ bool CClaimTrieCache::reorderTrieNode(const std::string& name, bool fCheckTakeov cachedNode = cache.find(name); if (cachedNode == cache.end()) { - CClaimTrieNode* currentNode = &(base->root); + CClaimTrieNode* currentNode; + cachedNode = cache.find(""); + if(cachedNode == cache.end()) + currentNode = &(base->root); + else + currentNode = cachedNode->second; for (std::string::const_iterator itCur = name.begin(); itCur != name.end(); ++itCur) { std::string sCurrentSubstring(name.begin(), itCur);