diff --git a/src/avalanche/peermanager.h b/src/avalanche/peermanager.h --- a/src/avalanche/peermanager.h +++ b/src/avalanche/peermanager.h @@ -335,7 +335,7 @@ return getProof(proofid) != nullptr; } - void cleanupDanglingProofs(); + void cleanupDanglingProofs(const ProofRef &localProof); template bool forPeer(const ProofId &proofid, Callable &&func) const { diff --git a/src/avalanche/peermanager.cpp b/src/avalanche/peermanager.cpp --- a/src/avalanche/peermanager.cpp +++ b/src/avalanche/peermanager.cpp @@ -411,14 +411,15 @@ return true; } -void PeerManager::cleanupDanglingProofs() { +void PeerManager::cleanupDanglingProofs(const ProofRef &localProof) { const auto now = GetTime(); std::vector newlyDanglingProofIds; for (const Peer &peer : peers) { - // If the peer has been registered for some time and has no node - // attached, discard it. - if (peer.node_count == 0 && + // If the peer is not our local proof, has been registered for some + // time and has no node attached, discard it. + if ((!localProof || peer.getProofId() != localProof->getId()) && + peer.node_count == 0 && (peer.registration_time + Peer::DANGLING_TIMEOUT) <= now) { newlyDanglingProofIds.push_back(peer.getProofId()); } diff --git a/src/avalanche/processor.cpp b/src/avalanche/processor.cpp --- a/src/avalanche/processor.cpp +++ b/src/avalanche/processor.cpp @@ -151,7 +151,9 @@ scheduler.scheduleEvery( [this]() -> bool { - WITH_LOCK(cs_peerManager, peerManager->cleanupDanglingProofs()); + WITH_LOCK(cs_peerManager, + peerManager->cleanupDanglingProofs( + peerData ? peerData->proof : ProofRef())); return true; }, 5min); 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 @@ -58,8 +58,10 @@ return scores; } - static void cleanupDanglingProofs(PeerManager &pm) { - pm.cleanupDanglingProofs(); + static void + cleanupDanglingProofs(PeerManager &pm, + const ProofRef &localProof = ProofRef()) { + pm.cleanupDanglingProofs(localProof); } }; @@ -1931,9 +1933,21 @@ // Elapse the timeout for the newly promoted conflicting proofs elapseTime(avalanche::Peer::DANGLING_TIMEOUT); + + // Use the first conflicting proof as our local proof + TestPeerManager::cleanupDanglingProofs(pm, conflictingProofs[1]); + + for (size_t i = 0; i < numProofs; i++) { + // All other proofs have now been discarded + BOOST_CHECK(!pm.exists(proofs[i]->getId())); + BOOST_CHECK_EQUAL(!pm.exists(conflictingProofs[i]->getId()), i != 1); + } + + // Cleanup one more time with no local proof TestPeerManager::cleanupDanglingProofs(pm); + for (size_t i = 0; i < numProofs; i++) { - // All proofs have now been discarded + // All proofs have finally been discarded BOOST_CHECK(!pm.exists(proofs[i]->getId())); BOOST_CHECK(!pm.exists(conflictingProofs[i]->getId())); } diff --git a/test/functional/abc_feature_proof_cleanup.py b/test/functional/abc_feature_proof_cleanup.py --- a/test/functional/abc_feature_proof_cleanup.py +++ b/test/functional/abc_feature_proof_cleanup.py @@ -12,6 +12,7 @@ gen_proof, get_ava_p2p_interface, get_proof_ids, + wait_for_proof, ) from test_framework.key import ECKey from test_framework.test_framework import BitcoinTestFramework @@ -38,11 +39,21 @@ def run_test(self): node = self.nodes[0] + master_key, local_proof = gen_proof(node) + + self.restart_node(0, self.extra_args[0] + [ + "-avaproof={}".format(local_proof.serialize().hex()), + "-avamasterkey={}".format(bytes_to_wif(master_key.get_bytes())), + ]) + + node.generate(1) + wait_for_proof(node, f"{local_proof.proofid:0{64}x}") + mocktime = int(time.time()) node.setmocktime(mocktime) - proofs = [] - keys = [] + proofs = [local_proof] + keys = [master_key] peers = [] # The first 5 peers have a node attached for _ in range(5): @@ -65,7 +76,7 @@ proofs.append(proof) peer_info = node.getavalanchepeerinfo() - assert_equal(len(peer_info), 10) + assert_equal(len(peer_info), 11) assert_equal(set(get_proof_ids(node)), set([proof.proofid for proof in proofs])) @@ -75,7 +86,7 @@ node.setmocktime(mocktime) # Run the cleanup, the proofs are still there node.mockscheduler(AVALANCHE_CLEANUP_INTERVAL) - assert_equal(len(peer_info), 10) + assert_equal(len(peer_info), 11) self.log.info("Check the proofs with attached nodes are not cleaned") @@ -83,10 +94,11 @@ mocktime += 1 node.setmocktime(mocktime) - # Run the cleanup, the proofs with no node are cleaned + # Run the cleanup, the proofs with no node are cleaned excepted our + # local proof node.mockscheduler(AVALANCHE_CLEANUP_INTERVAL) self.wait_until(lambda: set(get_proof_ids(node)) == set( - [proof.proofid for proof in proofs[:5]]), timeout=5) + [proof.proofid for proof in proofs[:6]]), timeout=5) self.log.info( "Check the proofs are cleaned on next cleanup after the nodes disconnected") @@ -96,16 +108,16 @@ peer.wait_for_disconnect() node.mockscheduler(AVALANCHE_CLEANUP_INTERVAL) - self.wait_until(lambda: get_proof_ids(node) == []) + self.wait_until(lambda: get_proof_ids(node) == [local_proof.proofid]) self.log.info("Check the cleaned up proofs are no longer accepted...") sender = get_ava_p2p_interface(node) - for proof in proofs: + for proof in proofs[1:]: with node.assert_debug_log(["dangling-proof"]): sender.send_avaproof(proof) - assert_equal(get_proof_ids(node), []) + assert_equal(get_proof_ids(node), [local_proof.proofid]) self.log.info("...until there is a node to attach") @@ -114,8 +126,8 @@ avanode = get_ava_p2p_interface(node) - avanode_key = keys[0] - avanode_proof = proofs[0] + avanode_key = keys[1] + avanode_proof = proofs[1] delegated_key = ECKey() delegated_key.generate()