diff --git a/src/avalanche/peermanager.h b/src/avalanche/peermanager.h --- a/src/avalanche/peermanager.h +++ b/src/avalanche/peermanager.h @@ -381,6 +381,8 @@ std::optional getProofRegistrationTime(const ProofId &proofid) const; + void removeObsoleteProofs(); + private: template void moveToConflictingPool(const ProofContainer &proofs); diff --git a/src/avalanche/peermanager.cpp b/src/avalanche/peermanager.cpp --- a/src/avalanche/peermanager.cpp +++ b/src/avalanche/peermanager.cpp @@ -511,6 +511,14 @@ return {}; } +void PeerManager::removeObsoleteProofs() { + auto now = GetTime(); + + // Cleanup the orphan proofs. Consider obsolete any proof that has been + // registered more than 30 minutes ago. + orphanProofPool.removeObsoleteProofs(std::max(0s, now - 30min)); +} + bool PeerManager::removePeer(const PeerId peerid) { auto it = peers.find(peerid); if (it == peers.end()) { diff --git a/src/avalanche/processor.h b/src/avalanche/processor.h --- a/src/avalanche/processor.h +++ b/src/avalanche/processor.h @@ -155,6 +155,7 @@ /** Event loop machinery. */ EventLoop eventLoop; + EventLoop cleanupTask; /** * Quorum management. @@ -224,6 +225,9 @@ bool startEventLoop(CScheduler &scheduler); bool stopEventLoop(); + bool startCleanupTask(CScheduler &scheduler); + bool stopCleanupTask(); + void avaproofsSent(NodeId nodeid); bool isQuorumEstablished(); @@ -243,6 +247,7 @@ private: void runEventLoop(); + void cleanup(); void clearTimedoutRequests(); std::vector getInvsForNextPoll(bool forPoll = true); NodeId getSuitableNodeToQuery(); diff --git a/src/avalanche/processor.cpp b/src/avalanche/processor.cpp --- a/src/avalanche/processor.cpp +++ b/src/avalanche/processor.cpp @@ -30,6 +30,11 @@ */ static constexpr std::chrono::milliseconds AVALANCHE_TIME_STEP{10}; +/** + * Run the avalanche cleanup task every 5min. + */ +static constexpr std::chrono::seconds AVALANCHE_CLEANUP_INTERVAL{5min}; + // Unfortunately, the bitcoind codebase is full of global and we are kinda // forced into it here. std::unique_ptr g_avalanche; @@ -169,6 +174,7 @@ Processor::~Processor() { chainNotificationsHandler.reset(); + stopCleanupTask(); stopEventLoop(); } @@ -671,6 +677,15 @@ return eventLoop.stopEventLoop(); } +bool Processor::startCleanupTask(CScheduler &scheduler) { + return cleanupTask.startEventLoop( + scheduler, [this]() { this->cleanup(); }, AVALANCHE_CLEANUP_INTERVAL); +} + +bool Processor::stopCleanupTask() { + return cleanupTask.stopEventLoop(); +} + std::vector Processor::getInvsForNextPoll(bool forPoll) { std::vector invs; @@ -868,6 +883,11 @@ } while (nodeid != NO_NODE); } +void Processor::cleanup() { + LOCK(cs_peerManager); + peerManager->removeObsoleteProofs(); +} + void Processor::avaproofsSent(NodeId nodeid) { if (::ChainstateActive().IsInitialBlockDownload()) { // Before IBD is complete there is no way to make sure a proof is valid diff --git a/src/avalanche/proofpool.h b/src/avalanche/proofpool.h --- a/src/avalanche/proofpool.h +++ b/src/avalanche/proofpool.h @@ -141,6 +141,9 @@ std::optional getRegistrationTime(const ProofId &proofid) const; + /** Remove all proofs that have been registered before the cutoff time */ + void removeObsoleteProofs(std::chrono::seconds cutoffTime); + size_t size() const { return pool.size(); } size_t countProofs() { return registrationTimes.size(); }; }; diff --git a/src/avalanche/proofpool.cpp b/src/avalanche/proofpool.cpp --- a/src/avalanche/proofpool.cpp +++ b/src/avalanche/proofpool.cpp @@ -141,4 +141,16 @@ return std::make_optional(it->registrationTime); } +void ProofPool::removeObsoleteProofs(std::chrono::seconds cutoffTime) { + auto &timeView = registrationTimes.get(); + auto &poolView = pool.get(); + + auto lowerBound = timeView.lower_bound(cutoffTime.count()); + for (auto it = timeView.begin(); it != lowerBound; it++) { + poolView.erase(it->proofid); + } + + timeView.erase(timeView.begin(), timeView.lower_bound(cutoffTime.count())); +} + } // namespace avalanche diff --git a/src/avalanche/test/processor_tests.cpp b/src/avalanche/test/processor_tests.cpp --- a/src/avalanche/test/processor_tests.cpp +++ b/src/avalanche/test/processor_tests.cpp @@ -62,6 +62,8 @@ static void clearavaproofsNodeCounter(Processor &p) { p.avaproofsNodeCounter = 0; } + + static void cleanup(Processor &p) { p.cleanup(); } }; } // namespace } // namespace avalanche @@ -1507,4 +1509,70 @@ gArgs.ClearForcedArg("-avastalevotefactor"); } +BOOST_AUTO_TEST_CASE(cleanup) { + auto buildOrphan = [&]() { + const CKey key = CKey::MakeCompressedKey(); + ProofBuilder pb(0, 0, key); + BOOST_CHECK( + pb.addUTXO({TxId(GetRandHash()), 0}, 10 * COIN, 10, false, key)); + return pb.build(); + }; + + auto setMockTime = [&](const std::chrono::seconds mocktime) { + SetMockTime(mocktime.count()); + }; + auto now = GetTime(); + + auto checkOrphan = [&](const ProofRef &proof, bool expectOrphan) { + m_processor->withPeerManager([&](avalanche::PeerManager &pm) { + BOOST_CHECK_EQUAL(pm.isOrphan(proof->getId()), expectOrphan); + BOOST_CHECK_EQUAL(pm.exists(proof->getId()), expectOrphan); + BOOST_CHECK_EQUAL(!!pm.getProofRegistrationTime(proof->getId()), + expectOrphan); + }); + }; + + std::vector proofs; + for (size_t i = 0; i < 10; i++) { + setMockTime(now + i * 1s); + + auto proof = buildOrphan(); + BOOST_CHECK(proof != nullptr); + + m_processor->withPeerManager([&](avalanche::PeerManager &pm) { + BOOST_CHECK(!pm.registerProof(proof)); + }); + checkOrphan(proof, true); + + proofs.push_back(std::move(proof)); + } + + // The orphans are not expired yet + AvalancheTest::cleanup(*m_processor); + for (const auto &proof : proofs) { + checkOrphan(proof, true); + } + + setMockTime(now + 30min); + // The orphans are not expired yet + AvalancheTest::cleanup(*m_processor); + for (const auto &proof : proofs) { + checkOrphan(proof, true); + } + + setMockTime(now + 30min + 5s); + // The first half of the orphans are now expired + AvalancheTest::cleanup(*m_processor); + for (size_t i = 0; i < 10; i++) { + checkOrphan(proofs[i], i > 4); + } + + setMockTime(now + 30min + 10s); + // Now they're all expired + AvalancheTest::cleanup(*m_processor); + for (const auto &proof : proofs) { + checkOrphan(proof, false); + } +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/avalanche/test/proofpool_tests.cpp b/src/avalanche/test/proofpool_tests.cpp --- a/src/avalanche/test/proofpool_tests.cpp +++ b/src/avalanche/test/proofpool_tests.cpp @@ -466,4 +466,52 @@ } } +BOOST_AUTO_TEST_CASE(remove_obsolete_proofs) { + ProofPool testPool; + + // Calling when there is no proof has no effect + testPool.removeObsoleteProofs(0s); + testPool.removeObsoleteProofs(GetTime()); + testPool.removeObsoleteProofs(-1s); + + auto now = GetTime(); + auto mocktime = now; + + auto elapseTime = [&](std::chrono::seconds seconds) { + mocktime += seconds; + SetMockTime(mocktime.count()); + }; + elapseTime(0s); + + std::vector proofs; + for (size_t i = 0; i < 10; i++) { + auto proof = buildRandomProof(MIN_VALID_PROOF_SCORE); + BOOST_CHECK(testPool.addProofIfNoConflict(proof)); + + proofs.push_back(std::move(proof)); + + elapseTime(1s); + } + + BOOST_CHECK_EQUAL(testPool.countProofs(), 10); + + testPool.removeObsoleteProofs(0s); + BOOST_CHECK_EQUAL(testPool.countProofs(), 10); + + testPool.removeObsoleteProofs(now); + BOOST_CHECK_EQUAL(testPool.countProofs(), 10); + + for (size_t i = 0; i < 10; i++) { + testPool.removeObsoleteProofs(now + 5s); + BOOST_CHECK_EQUAL(testPool.countProofs(), 5); + for (size_t j = 0; j < 10; j++) { + BOOST_CHECK_EQUAL(!testPool.getRegistrationTime(proofs[j]->getId()), + j < 5); + } + } + + testPool.removeObsoleteProofs(now + 10s); + BOOST_CHECK_EQUAL(testPool.countProofs(), 0); +} + BOOST_AUTO_TEST_SUITE_END()