diff --git a/src/avalanche/peermanager.h b/src/avalanche/peermanager.h --- a/src/avalanche/peermanager.h +++ b/src/avalanche/peermanager.h @@ -148,8 +148,7 @@ /** * Node API. */ - bool addNode(NodeId nodeid, const std::shared_ptr &proof, - const Delegation &delegation); + bool addNode(NodeId nodeid, const Delegation &delegation); bool removeNode(NodeId nodeid); bool forNode(NodeId nodeid, std::function func) const; diff --git a/src/avalanche/peermanager.cpp b/src/avalanche/peermanager.cpp --- a/src/avalanche/peermanager.cpp +++ b/src/avalanche/peermanager.cpp @@ -19,10 +19,10 @@ state.GetResult() == ProofValidationResult::HEIGHT_MISMATCH; } -bool PeerManager::addNode(NodeId nodeid, const std::shared_ptr &proof, - const Delegation &delegation) { - auto it = fetchOrCreatePeer(proof); - if (it == peers.end()) { +bool PeerManager::addNode(NodeId nodeid, const Delegation &delegation) { + auto &pview = peers.get(); + auto it = pview.find(delegation.getProofId()); + if (it == pview.end()) { return false; } @@ -32,7 +32,7 @@ return false; } - return addOrUpdateNode(it, nodeid, std::move(pubkey)); + return addOrUpdateNode(peers.project<0>(it), nodeid, std::move(pubkey)); } bool PeerManager::addOrUpdateNode(const PeerSet::iterator &it, NodeId nodeid, diff --git a/src/avalanche/processor.h b/src/avalanche/processor.h --- a/src/avalanche/processor.h +++ b/src/avalanche/processor.h @@ -281,8 +281,7 @@ bool registerVotes(NodeId nodeid, const Response &response, std::vector &updates); - bool addNode(NodeId nodeid, const std::shared_ptr &proof, - const Delegation &delegation); + bool addNode(NodeId nodeid, const Delegation &delegation); bool forNode(NodeId nodeid, std::function func) const; CPubKey getSessionPubKey() const; diff --git a/src/avalanche/processor.cpp b/src/avalanche/processor.cpp --- a/src/avalanche/processor.cpp +++ b/src/avalanche/processor.cpp @@ -518,10 +518,9 @@ return true; } -bool Processor::addNode(NodeId nodeid, const std::shared_ptr &proof, - const Delegation &delegation) { +bool Processor::addNode(NodeId nodeid, const Delegation &delegation) { LOCK(cs_peerManager); - return peerManager->addNode(nodeid, proof, delegation); + return peerManager->addNode(nodeid, delegation); } bool Processor::forNode(NodeId nodeid, 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 @@ -158,6 +158,14 @@ return std::make_shared(buildRandomProof(score)); } +static void addNodeWithScore(avalanche::PeerManager &pm, NodeId node, + uint32_t score) { + auto proof = getRandomProofPtr(score); + BOOST_CHECK_NE(pm.getPeerId(proof), NO_PEER); + Delegation dg = DelegationBuilder(*proof).build(); + BOOST_CHECK(pm.addNode(node, dg)); +}; + BOOST_AUTO_TEST_CASE(peer_probabilities) { // No peers. avalanche::PeerManager pm; @@ -166,15 +174,11 @@ const NodeId node0 = 42, node1 = 69, node2 = 37; // One peer, we always return it. - auto proof0 = getRandomProofPtr(MIN_VALID_PROOF_SCORE); - Delegation dg0 = DelegationBuilder(*proof0).build(); - pm.addNode(node0, proof0, dg0); + addNodeWithScore(pm, node0, MIN_VALID_PROOF_SCORE); BOOST_CHECK_EQUAL(pm.selectNode(), node0); // Two peers, verify ratio. - auto proof1 = getRandomProofPtr(2 * MIN_VALID_PROOF_SCORE); - Delegation dg1 = DelegationBuilder(*proof1).build(); - pm.addNode(node1, proof1, dg1); + addNodeWithScore(pm, node1, 2 * MIN_VALID_PROOF_SCORE); std::unordered_map results = {}; for (int i = 0; i < 10000; i++) { @@ -186,9 +190,7 @@ BOOST_CHECK(abs(2 * results[0] - results[1]) < 500); // Three peers, verify ratio. - auto proof2 = getRandomProofPtr(MIN_VALID_PROOF_SCORE); - Delegation dg2 = DelegationBuilder(*proof2).build(); - pm.addNode(node2, proof2, dg2); + addNodeWithScore(pm, node2, MIN_VALID_PROOF_SCORE); results.clear(); for (int i = 0; i < 10000; i++) { @@ -211,7 +213,7 @@ auto p = getRandomProofPtr(100); peerids[i] = pm.getPeerId(p); BOOST_CHECK( - pm.addNode(InsecureRand32(), p, DelegationBuilder(*p).build())); + pm.addNode(InsecureRand32(), DelegationBuilder(*p).build())); } BOOST_CHECK_EQUAL(pm.getSlotCount(), 400); @@ -244,7 +246,7 @@ auto p = getRandomProofPtr(100); peerids[i + 4] = pm.getPeerId(p); BOOST_CHECK( - pm.addNode(InsecureRand32(), p, DelegationBuilder(*p).build())); + pm.addNode(InsecureRand32(), DelegationBuilder(*p).build())); } BOOST_CHECK_EQUAL(pm.getSlotCount(), 700); @@ -287,7 +289,7 @@ auto p = getRandomProofPtr(100); peerids[i] = pm.getPeerId(p); BOOST_CHECK( - pm.addNode(InsecureRand32(), p, DelegationBuilder(*p).build())); + pm.addNode(InsecureRand32(), DelegationBuilder(*p).build())); } // Remove all peers. @@ -313,12 +315,13 @@ // Create one peer. auto proof = getRandomProofPtr(10000000 * MIN_VALID_PROOF_SCORE); + BOOST_CHECK_NE(pm.getPeerId(proof), NO_PEER); Delegation dg = DelegationBuilder(*proof).build(); BOOST_CHECK_EQUAL(pm.selectNode(), NO_NODE); // Add 4 nodes. for (int i = 0; i < 4; i++) { - BOOST_CHECK(pm.addNode(i, proof, dg)); + BOOST_CHECK(pm.addNode(i, dg)); } for (int i = 0; i < 100; i++) { @@ -351,9 +354,7 @@ // Move a node from a peer to another. This peer has a very low score such // as chances of being picked are 1 in 10 million. - auto altproof = getRandomProofPtr(MIN_VALID_PROOF_SCORE); - Delegation altdg = DelegationBuilder(*altproof).build(); - BOOST_CHECK(pm.addNode(3, altproof, altdg)); + addNodeWithScore(pm, 3, MIN_VALID_PROOF_SCORE); int node3selected = 0; for (int i = 0; i < 100; i++) { 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 @@ -127,18 +127,19 @@ bool addNode(NodeId nodeid) { auto proof = GetProof(); - return m_processor->addNode(nodeid, proof, - DelegationBuilder(*proof).build()); + BOOST_CHECK(m_processor->addProof(proof)); + return m_processor->addNode(nodeid, DelegationBuilder(*proof).build()); } std::array ConnectNodes() { auto proof = GetProof(); + BOOST_CHECK(m_processor->addProof(proof)); Delegation dg = DelegationBuilder(*proof).build(); std::array nodes; for (CNode *&n : nodes) { n = ConnectNode(NODE_AVALANCHE); - BOOST_CHECK(m_processor->addNode(n->GetId(), proof, dg)); + BOOST_CHECK(m_processor->addNode(n->GetId(), dg)); } return nodes; @@ -737,12 +738,13 @@ BOOST_AUTO_TEST_CASE(poll_inflight_count) { // Create enough nodes so that we run into the inflight request limit. auto proof = GetProof(); + BOOST_CHECK(m_processor->addProof(proof)); Delegation dg = DelegationBuilder(*proof).build(); std::array nodes; for (auto &n : nodes) { n = ConnectNode(NODE_AVALANCHE); - BOOST_CHECK(m_processor->addNode(n->GetId(), proof, dg)); + BOOST_CHECK(m_processor->addNode(n->GetId(), dg)); } // Add a block to poll diff --git a/src/rpc/avalanche.cpp b/src/rpc/avalanche.cpp --- a/src/rpc/avalanche.cpp +++ b/src/rpc/avalanche.cpp @@ -94,12 +94,17 @@ return false; } - if (!g_avalanche->addNode(nodeid, proof, + const avalanche::ProofId &proofid = proof->getId(); + if (!g_avalanche->getProof(proofid) && !g_avalanche->addProof(proof)) { + return false; + } + + if (!g_avalanche->addNode(nodeid, avalanche::DelegationBuilder(*proof).build())) { return false; } - g_avalanche->addUnbroadcastProof(proof->getId()); + g_avalanche->addUnbroadcastProof(proofid); return true; }