Page MenuHomePhabricator

D14670.id42738.diff
No OneTemporary

D14670.id42738.diff

diff --git a/src/avalanche/peermanager.h b/src/avalanche/peermanager.h
--- a/src/avalanche/peermanager.h
+++ b/src/avalanche/peermanager.h
@@ -384,7 +384,9 @@
return getProof(proofid) != nullptr;
}
- void cleanupDanglingProofs(const ProofRef &localProof);
+ void cleanupDanglingProofs(
+ const ProofRef &localProof,
+ std::unordered_set<ProofRef, SaltedProofHasher> &registeredProofs);
template <typename Callable>
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
@@ -452,7 +452,10 @@
return true;
}
-void PeerManager::cleanupDanglingProofs(const ProofRef &localProof) {
+void PeerManager::cleanupDanglingProofs(
+ const ProofRef &localProof,
+ std::unordered_set<ProofRef, SaltedProofHasher> &registeredProofs) {
+ registeredProofs.clear();
const auto now = GetTime<std::chrono::seconds>();
std::vector<ProofRef> newlyDanglingProofs;
@@ -482,7 +485,9 @@
});
for (const ProofRef &proof : previouslyDanglingProofs) {
danglingProofPool.removeProof(proof->getId());
- registerProof(proof);
+ if (registerProof(proof)) {
+ registeredProofs.insert(proof);
+ }
}
for (const ProofRef &proof : newlyDanglingProofs) {
diff --git a/src/avalanche/processor.cpp b/src/avalanche/processor.cpp
--- a/src/avalanche/processor.cpp
+++ b/src/avalanche/processor.cpp
@@ -159,8 +159,12 @@
scheduler.scheduleEvery(
[this]() -> bool {
- WITH_LOCK(cs_peerManager,
- peerManager->cleanupDanglingProofs(getLocalProof()));
+ std::unordered_set<ProofRef, SaltedProofHasher> registeredProofs;
+ WITH_LOCK(cs_peerManager, peerManager->cleanupDanglingProofs(
+ getLocalProof(), registeredProofs));
+ for (const auto &proof : registeredProofs) {
+ addToReconcile(proof);
+ }
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
@@ -67,10 +67,18 @@
return scores;
}
+ static void cleanupDanglingProofs(
+ PeerManager &pm,
+ std::unordered_set<ProofRef, SaltedProofHasher> &registeredProofs,
+ const ProofRef &localProof = ProofRef()) {
+ pm.cleanupDanglingProofs(localProof, registeredProofs);
+ }
+
static void
cleanupDanglingProofs(PeerManager &pm,
const ProofRef &localProof = ProofRef()) {
- pm.cleanupDanglingProofs(localProof);
+ std::unordered_set<ProofRef, SaltedProofHasher> dummy;
+ pm.cleanupDanglingProofs(localProof, dummy);
}
static std::optional<RemoteProof> getRemoteProof(const PeerManager &pm,
@@ -1516,14 +1524,14 @@
buildRandomProof(chainman.ActiveChainstate(), MIN_VALID_PROOF_SCORE);
BOOST_CHECK(pm.registerProof(proof2));
BOOST_CHECK(!pm.isDangling(proof2->getId()));
- pm.cleanupDanglingProofs(ProofRef());
+ TestPeerManager::cleanupDanglingProofs(pm);
BOOST_CHECK(!pm.isDangling(proof2->getId()));
BOOST_CHECK(!pm.shouldRequestMoreNodes());
// After some time the proof will be considered dangling and more nodes will
// be requested.
SetMockTime(GetTime() + 15 * 60);
- pm.cleanupDanglingProofs(ProofRef());
+ TestPeerManager::cleanupDanglingProofs(pm);
BOOST_CHECK(pm.isDangling(proof2->getId()));
BOOST_CHECK(pm.shouldRequestMoreNodes());
@@ -1551,13 +1559,13 @@
BOOST_CHECK(!pm.addNode(11, proof2->getId()));
BOOST_CHECK(pm.registerProof(proof2));
SetMockTime(GetTime() + 15 * 60);
- pm.cleanupDanglingProofs(ProofRef());
+ TestPeerManager::cleanupDanglingProofs(pm);
BOOST_CHECK(!pm.isDangling(proof2->getId()));
BOOST_CHECK(!pm.shouldRequestMoreNodes());
// Disconnect the node, the proof is dangling again
BOOST_CHECK(pm.removeNode(11));
- pm.cleanupDanglingProofs(ProofRef());
+ TestPeerManager::cleanupDanglingProofs(pm);
BOOST_CHECK(pm.isDangling(proof2->getId()));
BOOST_CHECK(pm.shouldRequestMoreNodes());
@@ -2696,11 +2704,14 @@
}
// The proofs should be added back as a peer
- TestPeerManager::cleanupDanglingProofs(pm, ProofRef());
+ std::unordered_set<ProofRef, SaltedProofHasher> registeredProofs;
+ TestPeerManager::cleanupDanglingProofs(pm, registeredProofs, ProofRef());
for (const auto &proof : proofs) {
BOOST_CHECK(pm.isBoundToPeer(proof->getId()));
BOOST_CHECK(!pm.isDangling(proof->getId()));
+ BOOST_CHECK_EQUAL(registeredProofs.count(proof), 1);
}
+ BOOST_CHECK_EQUAL(proofs.size(), registeredProofs.size());
// Remove the proofs from the remotes
for (NodeId nodeid = 0; nodeid < 10; nodeid++) {
@@ -2717,7 +2728,8 @@
}
// The proofs are not dangling yet as they have been registered recently
- TestPeerManager::cleanupDanglingProofs(pm, ProofRef());
+ TestPeerManager::cleanupDanglingProofs(pm, registeredProofs, ProofRef());
+ BOOST_CHECK(registeredProofs.empty());
for (const auto &proof : proofs) {
BOOST_CHECK(pm.isBoundToPeer(proof->getId()));
BOOST_CHECK(!pm.isDangling(proof->getId()));
@@ -2727,7 +2739,8 @@
mockTime += avalanche::Peer::DANGLING_TIMEOUT + 1s;
SetMockTime(mockTime);
- TestPeerManager::cleanupDanglingProofs(pm, ProofRef());
+ TestPeerManager::cleanupDanglingProofs(pm, registeredProofs, ProofRef());
+ BOOST_CHECK(registeredProofs.empty());
for (const auto &proof : proofs) {
BOOST_CHECK(!pm.isBoundToPeer(proof->getId()));
BOOST_CHECK(pm.isDangling(proof->getId()));
@@ -2740,11 +2753,13 @@
}
}
- TestPeerManager::cleanupDanglingProofs(pm, ProofRef());
+ TestPeerManager::cleanupDanglingProofs(pm, registeredProofs, ProofRef());
for (const auto &proof : proofs) {
BOOST_CHECK(pm.isBoundToPeer(proof->getId()));
BOOST_CHECK(!pm.isDangling(proof->getId()));
+ BOOST_CHECK_EQUAL(registeredProofs.count(proof), 1);
}
+ BOOST_CHECK_EQUAL(proofs.size(), registeredProofs.size());
}
BOOST_AUTO_TEST_SUITE_END()

File Metadata

Mime Type
text/plain
Expires
Sat, Apr 26, 10:34 (5 h, 17 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
5570309
Default Alt Text
D14670.id42738.diff (6 KB)

Event Timeline