diff --git a/src/avalanche/peermanager.h b/src/avalanche/peermanager.h --- a/src/avalanche/peermanager.h +++ b/src/avalanche/peermanager.h @@ -30,6 +30,8 @@ #include #include +class CScheduler; + namespace avalanche { /** @@ -224,6 +226,8 @@ uint32_t connectedPeersScore = 0; public: + PeerManager(CScheduler &scheduler); + /** * Node API. */ diff --git a/src/avalanche/peermanager.cpp b/src/avalanche/peermanager.cpp --- a/src/avalanche/peermanager.cpp +++ b/src/avalanche/peermanager.cpp @@ -8,12 +8,21 @@ #include #include #include +#include #include // For ChainstateActive() #include #include namespace avalanche { +PeerManager::PeerManager(CScheduler &scheduler) { + scheduler.scheduleEvery( + [this]() -> bool { + this->cleanupDanglingProofs(); + return true; + }, + 5min); +} bool PeerManager::addNode(NodeId nodeid, const ProofId &proofid) { auto &pview = peers.get(); diff --git a/src/avalanche/processor.h b/src/avalanche/processor.h --- a/src/avalanche/processor.h +++ b/src/avalanche/processor.h @@ -174,8 +174,9 @@ std::unique_ptr chainNotificationsHandler; Processor(const ArgsManager &argsman, interfaces::Chain &chain, - CConnman *connmanIn, std::unique_ptr peerDataIn, - CKey sessionKeyIn, uint32_t minQuorumTotalScoreIn, + CConnman *connmanIn, CScheduler &scheduler, + std::unique_ptr peerDataIn, CKey sessionKeyIn, + uint32_t minQuorumTotalScoreIn, double minQuorumConnectedScoreRatioIn, int64_t minAvaproofsNodeCountIn, uint32_t staleVoteThresholdIn, uint32_t staleVoteFactorIn); @@ -186,6 +187,7 @@ static std::unique_ptr MakeProcessor(const ArgsManager &argsman, interfaces::Chain &chain, CConnman *connman, + CScheduler &scheduler, bilingual_str &error); void setQueryTimeoutDuration(std::chrono::milliseconds d) { diff --git a/src/avalanche/processor.cpp b/src/avalanche/processor.cpp --- a/src/avalanche/processor.cpp +++ b/src/avalanche/processor.cpp @@ -121,15 +121,16 @@ }; Processor::Processor(const ArgsManager &argsman, interfaces::Chain &chain, - CConnman *connmanIn, std::unique_ptr peerDataIn, - CKey sessionKeyIn, uint32_t minQuorumTotalScoreIn, + CConnman *connmanIn, CScheduler &scheduler, + std::unique_ptr peerDataIn, CKey sessionKeyIn, + uint32_t minQuorumTotalScoreIn, double minQuorumConnectedScoreRatioIn, int64_t minAvaproofsNodeCountIn, uint32_t staleVoteThresholdIn, uint32_t staleVoteFactorIn) : connman(connmanIn), queryTimeoutDuration(argsman.GetArg( "-avatimeout", AVALANCHE_DEFAULT_QUERY_TIMEOUT.count())), - round(0), peerManager(std::make_unique()), + round(0), peerManager(std::make_unique(scheduler)), peerData(std::move(peerDataIn)), sessionKey(std::move(sessionKeyIn)), minQuorumScore(minQuorumTotalScoreIn), minQuorumConnectedScoreRatio(minQuorumConnectedScoreRatioIn), @@ -149,6 +150,7 @@ std::unique_ptr Processor::MakeProcessor(const ArgsManager &argsman, interfaces::Chain &chain, CConnman *connman, + CScheduler &scheduler, bilingual_str &error) { std::unique_ptr peerData; CKey masterKey; @@ -308,9 +310,10 @@ // We can't use std::make_unique with a private constructor return std::unique_ptr(new Processor( - argsman, chain, connman, std::move(peerData), std::move(sessionKey), - Proof::amountToScore(minQuorumStake), minQuorumConnectedStakeRatio, - minAvaproofsNodeCount, staleVoteThreshold, staleVoteFactor)); + argsman, chain, connman, scheduler, std::move(peerData), + std::move(sessionKey), Proof::amountToScore(minQuorumStake), + minQuorumConnectedStakeRatio, minAvaproofsNodeCount, staleVoteThreshold, + staleVoteFactor)); } bool Processor::addBlockToReconcile(const CBlockIndex *pindex) { diff --git a/src/avalanche/test/peermanager_tests.cpp b/src/avalanche/test/peermanager_tests.cpp --- a/src/avalanche/test/peermanager_tests.cpp +++ b/src/avalanche/test/peermanager_tests.cpp @@ -290,7 +290,7 @@ BOOST_AUTO_TEST_CASE(peer_probabilities) { // No peers. - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); BOOST_CHECK_EQUAL(pm.selectNode(), NO_NODE); const NodeId node0 = 42, node1 = 69, node2 = 37; @@ -326,7 +326,7 @@ BOOST_AUTO_TEST_CASE(remove_peer) { // No peers. - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); BOOST_CHECK_EQUAL(pm.selectPeer(), NO_PEER); // Add 4 peers. @@ -401,7 +401,7 @@ } BOOST_AUTO_TEST_CASE(compact_slots) { - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); // Add 4 peers. std::array peerids; @@ -430,7 +430,7 @@ } BOOST_AUTO_TEST_CASE(node_crud) { - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); // Create one peer. auto proof = buildRandomProof(10000000 * MIN_VALID_PROOF_SCORE); @@ -490,7 +490,7 @@ } BOOST_AUTO_TEST_CASE(node_binding) { - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); auto proof = buildRandomProof(MIN_VALID_PROOF_SCORE); const ProofId &proofid = proof->getId(); @@ -589,7 +589,7 @@ } BOOST_AUTO_TEST_CASE(node_binding_reorg) { - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); auto key = CKey::MakeCompressedKey(); @@ -656,7 +656,7 @@ addCoin({txid2, i}, key); } - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); CKey masterKey = CKey::MakeCompressedKey(); const auto getPeerId = [&](const std::vector &outpoints) { return TestPeerManager::registerAndGetPeerId( @@ -710,7 +710,7 @@ BOOST_AUTO_TEST_CASE(orphan_proofs) { gArgs.ForceSetArg("-avaproofstakeutxoconfirmations", "2"); - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); auto key = CKey::MakeCompressedKey(); @@ -899,7 +899,7 @@ } BOOST_AUTO_TEST_CASE(dangling_node) { - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); auto proof = buildRandomProof(MIN_VALID_PROOF_SCORE); PeerId peerid = TestPeerManager::registerAndGetPeerId(pm, proof); @@ -944,7 +944,7 @@ } BOOST_AUTO_TEST_CASE(proof_accessors) { - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); constexpr int numProofs = 10; @@ -988,7 +988,7 @@ } BOOST_FIXTURE_TEST_CASE(conflicting_proof_rescan, NoCoolDownFixture) { - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); const CKey key = CKey::MakeCompressedKey(); @@ -1043,7 +1043,7 @@ BOOST_CHECK_EQUAL(comparator(candidate, reference), expectAccepted); BOOST_CHECK_EQUAL(comparator(reference, candidate), !expectAccepted); - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); BOOST_CHECK(pm.registerProof(reference)); BOOST_CHECK(pm.isBoundToPeer(reference->getId())); @@ -1113,7 +1113,7 @@ } BOOST_AUTO_TEST_CASE(conflicting_orphans) { - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); const CKey key = CKey::MakeCompressedKey(); @@ -1160,7 +1160,7 @@ } BOOST_FIXTURE_TEST_CASE(preferred_conflicting_proof, NoCoolDownFixture) { - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); const CKey key = CKey::MakeCompressedKey(); const COutPoint conflictingOutpoint = createUtxo(key); @@ -1191,7 +1191,7 @@ } BOOST_FIXTURE_TEST_CASE(update_next_conflict_time, NoCoolDownFixture) { - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); auto now = GetTime(); SetMockTime(now.count()); @@ -1224,7 +1224,7 @@ } BOOST_FIXTURE_TEST_CASE(register_force_accept, NoCoolDownFixture) { - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); const CKey key = CKey::MakeCompressedKey(); @@ -1288,7 +1288,7 @@ } BOOST_FIXTURE_TEST_CASE(evicted_proof, NoCoolDownFixture) { - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); const CKey key = CKey::MakeCompressedKey(); @@ -1318,7 +1318,7 @@ } BOOST_AUTO_TEST_CASE(conflicting_proof_cooldown) { - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); const CKey key = CKey::MakeCompressedKey(); @@ -1386,7 +1386,7 @@ } BOOST_FIXTURE_TEST_CASE(reject_proof, NoCoolDownFixture) { - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); const CKey key = CKey::MakeCompressedKey(); @@ -1461,7 +1461,7 @@ } BOOST_AUTO_TEST_CASE(should_request_more_nodes) { - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); auto proof = buildRandomProof(MIN_VALID_PROOF_SCORE); BOOST_CHECK(pm.registerProof(proof)); @@ -1509,7 +1509,7 @@ } BOOST_AUTO_TEST_CASE(score_ordering) { - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); std::vector expectedScores(10); // Expect the peers to be ordered by descending score @@ -1535,7 +1535,7 @@ BOOST_FIXTURE_TEST_CASE(known_score_tracking, NoCoolDownFixture) { gArgs.ForceSetArg("-avaproofstakeutxoconfirmations", "2"); - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); const CKey key = CKey::MakeCompressedKey(); @@ -1645,7 +1645,7 @@ } BOOST_AUTO_TEST_CASE(connected_score_tracking) { - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); const auto checkScores = [&pm](uint32_t known, uint32_t connected) { BOOST_CHECK_EQUAL(pm.getTotalPeersScore(), known); @@ -1733,7 +1733,7 @@ } BOOST_FIXTURE_TEST_CASE(proof_radix_tree, NoCoolDownFixture) { - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); gArgs.ForceSetArg("-enableavalancheproofreplacement", "1"); @@ -1849,7 +1849,7 @@ } BOOST_AUTO_TEST_CASE(received_avaproofs) { - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); auto addNode = [&](NodeId nodeid) { auto proof = buildRandomProof(MIN_VALID_PROOF_SCORE); @@ -1872,7 +1872,7 @@ BOOST_FIXTURE_TEST_CASE(cleanup_dangling_proof, NoCoolDownFixture) { gArgs.ForceSetArg("-enableavalancheproofreplacement", "1"); - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); const auto now = GetTime(); auto mocktime = now; 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 @@ -107,7 +107,8 @@ // Get the processor ready. bilingual_str error; m_processor = Processor::MakeProcessor(*m_node.args, *m_node.chain, - m_node.connman.get(), error); + m_node.connman.get(), + *m_node.scheduler, error); BOOST_CHECK(m_processor); gArgs.ForceSetArg("-avaproofstakeutxoconfirmations", "1"); @@ -1143,7 +1144,8 @@ bilingual_str error; std::unique_ptr processor = Processor::MakeProcessor( - *m_node.args, *m_node.chain, m_node.connman.get(), error); + *m_node.args, *m_node.chain, m_node.connman.get(), *m_node.scheduler, + error); BOOST_CHECK(processor != nullptr); BOOST_CHECK(processor->getLocalProof() != nullptr); @@ -1291,7 +1293,8 @@ bilingual_str error; std::unique_ptr processor = Processor::MakeProcessor( - *m_node.args, *m_node.chain, m_node.connman.get(), error); + *m_node.args, *m_node.chain, m_node.connman.get(), + *m_node.scheduler, error); if (std::get<3>(*it)) { BOOST_CHECK(processor != nullptr); @@ -1320,7 +1323,8 @@ bilingual_str error; auto processor = Processor::MakeProcessor(argsman, *m_node.chain, - m_node.connman.get(), error); + m_node.connman.get(), + *m_node.scheduler, error); BOOST_CHECK_EQUAL(processor->isQuorumEstablished(), minAvaproofsMessages <= 0); @@ -1375,7 +1379,8 @@ bilingual_str error; m_processor = Processor::MakeProcessor(*m_node.args, *m_node.chain, - m_node.connman.get(), error); + m_node.connman.get(), + *m_node.scheduler, error); BOOST_CHECK(m_processor != nullptr); BOOST_CHECK(error.empty()); 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 @@ -77,7 +77,7 @@ BOOST_AUTO_TEST_CASE(rescan) { gArgs.ForceSetArg("-avaproofstakeutxoconfirmations", "1"); ProofPool testPool; - avalanche::PeerManager pm; + avalanche::PeerManager pm(*m_node.scheduler); testPool.rescan(pm); BOOST_CHECK_EQUAL(testPool.size(), 0); diff --git a/src/init.cpp b/src/init.cpp --- a/src/init.cpp +++ b/src/init.cpp @@ -2569,7 +2569,7 @@ // Step 6.5 (I guess ?): Initialize Avalanche. bilingual_str avalancheError; g_avalanche = avalanche::Processor::MakeProcessor( - args, *node.chain, node.connman.get(), avalancheError); + args, *node.chain, node.connman.get(), *node.scheduler, avalancheError); if (!g_avalanche) { InitError(avalancheError); return false; diff --git a/test/functional/abc_feature_proof_cleanup.py b/test/functional/abc_feature_proof_cleanup.py new file mode 100644 --- /dev/null +++ b/test/functional/abc_feature_proof_cleanup.py @@ -0,0 +1,96 @@ +#!/usr/bin/env python3 +# Copyright (c) 2022 The Bitcoin developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +""" +Test the dangling proofs cleanup +""" + +import time + +from test_framework.avatools import ( + gen_proof, + get_ava_p2p_interface, + get_proof_ids, +) +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal + +# Interval between 2 proof cleanups +AVALANCHE_CLEANUP_INTERVAL = 5 * 60 +# Dangling proof timeout +AVALANCHE_DANGLING_PROOF_TIMEOUT = 15 * 60 + + +class ProofsCleanupTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 1 + self.extra_args = [[ + '-enableavalanche=1', + '-avaproofstakeutxoconfirmations=1', + ]] * self.num_nodes + + def run_test(self): + node = self.nodes[0] + + mocktime = int(time.time()) + node.setmocktime(mocktime) + + proofs = [] + peers = [] + # The first 5 peers have a node attached + for _ in range(5): + key, proof = gen_proof(node) + + peer = get_ava_p2p_interface(node) + node.addavalanchenode( + peer.nodeid, + key.get_pubkey().get_bytes().hex(), + proof.serialize().hex()) + + proofs.append(proof) + peers.append(peer) + + # The last 5 peers have no node attached + for _ in range(5): + _, proof = gen_proof(node) + node.sendavalancheproof(proof.serialize().hex()) + proofs.append(proof) + + peer_info = node.getavalanchepeerinfo() + assert_equal(len(peer_info), 10) + assert_equal(set(get_proof_ids(node)), + set([proof.proofid for proof in proofs])) + + self.log.info("No proof is cleaned before the timeout expires") + + mocktime += AVALANCHE_DANGLING_PROOF_TIMEOUT - 1 + node.setmocktime(mocktime) + # Run the cleanup, the proofs are still there + node.mockscheduler(AVALANCHE_CLEANUP_INTERVAL) + assert_equal(len(peer_info), 10) + + self.log.info("Check the proofs with attached nodes are not cleaned") + + # Expire the dangling proof timeout + mocktime += 1 + node.setmocktime(mocktime) + + # Run the cleanup, the proofs with no node are cleaned + node.mockscheduler(AVALANCHE_CLEANUP_INTERVAL) + self.wait_until(lambda: set(get_proof_ids(node)) == set( + [proof.proofid for proof in proofs[:5]]), timeout=5) + + self.log.info( + "Check the proofs are cleaned on next cleanup after the nodes disconnected") + + for peer in peers: + peer.peer_disconnect() + peer.wait_for_disconnect() + + node.mockscheduler(AVALANCHE_CLEANUP_INTERVAL) + self.wait_until(lambda: get_proof_ids(node) == []) + + +if __name__ == '__main__': + ProofsCleanupTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -129,6 +129,7 @@ "wallet_watchonly.py": [["--usecli"]], # Avalanche tests running with and without the legacy proof format + "abc_feature_proof_cleanup.py": [["--nolegacyavaproof"]], "abc_p2p_avalanche_peer_discovery.py": [["--nolegacyavaproof"]], "abc_p2p_avalanche_proof_voting.py": [["--nolegacyavaproof"]], "abc_p2p_avalanche_quorum.py": [["--nolegacyavaproof"]],