diff --git a/src/avalanche/peermanager.h b/src/avalanche/peermanager.h --- a/src/avalanche/peermanager.h +++ b/src/avalanche/peermanager.h @@ -128,7 +128,7 @@ * Node API. */ bool addNode(NodeId nodeid, const Proof &proof, - const Delegation &delegation); + const Delegation &delegation, const CCoinsViewCache &coins); bool removeNode(NodeId nodeid); bool forNode(NodeId nodeid, std::function func) const; @@ -142,7 +142,7 @@ /** * Update the peer set when a nw block is connected. */ - void updatedBlockTip(); + void updatedBlockTip(const CCoinsViewCache &coins); /**************************************************** * Functions which are public for testing purposes. * @@ -151,7 +151,7 @@ * Provide the PeerId associated with the given proof. If the peer does not * exists, then it is created. */ - PeerId getPeerId(const Proof &proof); + PeerId getPeerId(const Proof &proof, const CCoinsViewCache &coins); /** * Remove an existing peer. @@ -182,7 +182,8 @@ std::vector getNodeIdsForPeer(PeerId peerId) const; private: - PeerSet::iterator fetchOrCreatePeer(const Proof &proof); + PeerSet::iterator fetchOrCreatePeer(const Proof &proof, + const CCoinsViewCache &coins); bool addNodeToPeer(const PeerSet::iterator &it); bool removeNodeFromPeer(const PeerSet::iterator &it, uint32_t count = 1); }; diff --git a/src/avalanche/peermanager.cpp b/src/avalanche/peermanager.cpp --- a/src/avalanche/peermanager.cpp +++ b/src/avalanche/peermanager.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include // For ChainstateActive() @@ -14,8 +15,9 @@ namespace avalanche { bool PeerManager::addNode(NodeId nodeid, const Proof &proof, - const Delegation &delegation) { - auto it = fetchOrCreatePeer(proof); + const Delegation &delegation, + const CCoinsViewCache &coins) { + auto it = fetchOrCreatePeer(proof, coins); if (it == peers.end()) { return false; } @@ -160,18 +162,13 @@ return NO_NODE; } -void PeerManager::updatedBlockTip() { +void PeerManager::updatedBlockTip(const CCoinsViewCache &coins) { std::vector invalidPeers; - { - LOCK(cs_main); - - const CCoinsViewCache &coins = ::ChainstateActive().CoinsTip(); - for (const auto &p : peers) { - ProofValidationState state; - if (!p.proof.verify(state, coins)) { - invalidPeers.push_back(p.peerid); - } + for (const auto &p : peers) { + ProofValidationState state; + if (!p.proof.verify(state, coins)) { + invalidPeers.push_back(p.peerid); } } @@ -180,13 +177,15 @@ } } -PeerId PeerManager::getPeerId(const Proof &proof) { - auto it = fetchOrCreatePeer(proof); +PeerId PeerManager::getPeerId(const Proof &proof, + const CCoinsViewCache &coins) { + auto it = fetchOrCreatePeer(proof, coins); return it == peers.end() ? NO_PEER : it->peerid; } PeerManager::PeerSet::iterator -PeerManager::fetchOrCreatePeer(const Proof &proof) { +PeerManager::fetchOrCreatePeer(const Proof &proof, + const CCoinsViewCache &coins) { { // Check if we already know of that peer. auto &pview = peers.get(); @@ -196,15 +195,10 @@ } } - { - // Reject invalid proof. - LOCK(cs_main); - const CCoinsViewCache &coins = ::ChainstateActive().CoinsTip(); - - ProofValidationState state; - if (!proof.verify(state, coins)) { - return peers.end(); - } + // Reject invalid proof. + ProofValidationState state; + if (!proof.verify(state, coins)) { + return peers.end(); } // New peer means new peerid! diff --git a/src/avalanche/processor.cpp b/src/avalanche/processor.cpp --- a/src/avalanche/processor.cpp +++ b/src/avalanche/processor.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include // For DecodeSecret #include // For ::PeerManager #include @@ -150,15 +151,19 @@ NotificationsHandler(Processor *p) : m_processor(p) {} void updatedBlockTip() override { - LOCK(m_processor->cs_peerManager); + bool isInitialBlockDownload = + ::ChainstateActive().IsInitialBlockDownload(); + LOCK(cs_main); + const ::CCoinsViewCache &coins = ::ChainstateActive().CoinsTip(); - if (m_processor->mustRegisterProof && - !::ChainstateActive().IsInitialBlockDownload()) { - m_processor->peerManager->getPeerId(m_processor->peerData->proof); + LOCK(m_processor->cs_peerManager); + if (m_processor->mustRegisterProof && !isInitialBlockDownload) { + m_processor->peerManager->getPeerId(m_processor->peerData->proof, + coins); m_processor->mustRegisterProof = false; } - m_processor->peerManager->updatedBlockTip(); + m_processor->peerManager->updatedBlockTip(coins); } }; @@ -444,8 +449,10 @@ bool Processor::addNode(NodeId nodeid, const Proof &proof, const Delegation &delegation) { + LOCK(cs_main); + const ::CCoinsViewCache &coins = ::ChainstateActive().CoinsTip(); LOCK(cs_peerManager); - return peerManager->addNode(nodeid, proof, delegation); + return peerManager->addNode(nodeid, proof, delegation, coins); } 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 @@ -161,16 +161,19 @@ const NodeId node0 = 42, node1 = 69, node2 = 37; + LOCK(cs_main); + const CCoinsViewCache &coins = ::ChainstateActive().CoinsTip(); + // One peer, we always return it. Proof proof0 = buildRandomProof(MIN_VALID_PROOF_SCORE); Delegation dg0 = DelegationBuilder(proof0).build(); - pm.addNode(node0, proof0, dg0); + pm.addNode(node0, proof0, dg0, coins); BOOST_CHECK_EQUAL(pm.selectNode(), node0); // Two peers, verify ratio. Proof proof1 = buildRandomProof(2 * MIN_VALID_PROOF_SCORE); Delegation dg1 = DelegationBuilder(proof1).build(); - pm.addNode(node1, proof1, dg1); + pm.addNode(node1, proof1, dg1, coins); std::unordered_map results = {}; for (int i = 0; i < 10000; i++) { @@ -184,7 +187,7 @@ // Three peers, verify ratio. Proof proof2 = buildRandomProof(MIN_VALID_PROOF_SCORE); Delegation dg2 = DelegationBuilder(proof2).build(); - pm.addNode(node2, proof2, dg2); + pm.addNode(node2, proof2, dg2, coins); results.clear(); for (int i = 0; i < 10000; i++) { @@ -201,13 +204,16 @@ avalanche::PeerManager pm; BOOST_CHECK_EQUAL(pm.selectPeer(), NO_PEER); + LOCK(cs_main); + const CCoinsViewCache &coins = ::ChainstateActive().CoinsTip(); + // Add 4 peers. std::array peerids; for (int i = 0; i < 4; i++) { Proof p = buildRandomProof(100); - peerids[i] = pm.getPeerId(p); - BOOST_CHECK( - pm.addNode(InsecureRand32(), p, DelegationBuilder(p).build())); + peerids[i] = pm.getPeerId(p, coins); + BOOST_CHECK(pm.addNode(InsecureRand32(), p, + DelegationBuilder(p).build(), coins)); } BOOST_CHECK_EQUAL(pm.getSlotCount(), 400); @@ -238,9 +244,9 @@ // Add 4 more peers. for (int i = 0; i < 4; i++) { Proof p = buildRandomProof(100); - peerids[i + 4] = pm.getPeerId(p); - BOOST_CHECK( - pm.addNode(InsecureRand32(), p, DelegationBuilder(p).build())); + peerids[i + 4] = pm.getPeerId(p, coins); + BOOST_CHECK(pm.addNode(InsecureRand32(), p, + DelegationBuilder(p).build(), coins)); } BOOST_CHECK_EQUAL(pm.getSlotCount(), 700); @@ -277,13 +283,16 @@ BOOST_AUTO_TEST_CASE(compact_slots) { avalanche::PeerManager pm; + LOCK(cs_main); + const CCoinsViewCache &coins = ::ChainstateActive().CoinsTip(); + // Add 4 peers. std::array peerids; for (int i = 0; i < 4; i++) { Proof p = buildRandomProof(100); - peerids[i] = pm.getPeerId(p); - BOOST_CHECK( - pm.addNode(InsecureRand32(), p, DelegationBuilder(p).build())); + peerids[i] = pm.getPeerId(p, coins); + BOOST_CHECK(pm.addNode(InsecureRand32(), p, + DelegationBuilder(p).build(), coins)); } // Remove all peers. @@ -307,6 +316,9 @@ BOOST_AUTO_TEST_CASE(node_crud) { avalanche::PeerManager pm; + LOCK(cs_main); + const CCoinsViewCache &coins = ::ChainstateActive().CoinsTip(); + // Create one peer. Proof proof = buildRandomProof(10000000 * MIN_VALID_PROOF_SCORE); Delegation dg = DelegationBuilder(proof).build(); @@ -314,7 +326,7 @@ // Add 4 nodes. for (int i = 0; i < 4; i++) { - BOOST_CHECK(pm.addNode(i, proof, dg)); + BOOST_CHECK(pm.addNode(i, proof, dg, coins)); } for (int i = 0; i < 100; i++) { @@ -349,7 +361,7 @@ // as chances of being picked are 1 in 10 million. Proof altproof = buildRandomProof(MIN_VALID_PROOF_SCORE); Delegation altdg = DelegationBuilder(altproof).build(); - BOOST_CHECK(pm.addNode(3, altproof, altdg)); + BOOST_CHECK(pm.addNode(3, altproof, altdg, coins)); int node3selected = 0; for (int i = 0; i < 100; i++) { @@ -377,16 +389,14 @@ const Amount v = 5 * COIN; const int height = 1234; - { - LOCK(cs_main); - CCoinsViewCache &coins = ::ChainstateActive().CoinsTip(); + LOCK(cs_main); + CCoinsViewCache &coins = ::ChainstateActive().CoinsTip(); - for (int i = 0; i < 10; i++) { - coins.AddCoin(COutPoint(txid1, i), - Coin(CTxOut(v, script), height, false), false); - coins.AddCoin(COutPoint(txid2, i), - Coin(CTxOut(v, script), height, false), false); - } + for (int i = 0; i < 10; i++) { + coins.AddCoin(COutPoint(txid1, i), + Coin(CTxOut(v, script), height, false), false); + coins.AddCoin(COutPoint(txid2, i), + Coin(CTxOut(v, script), height, false), false); } avalanche::PeerManager pm; @@ -396,7 +406,7 @@ pb.addUTXO(o, v, height, false, key); } - return pm.getPeerId(pb.build()); + return pm.getPeerId(pb.build(), coins); }; // Add one peer. 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 @@ -138,10 +138,12 @@ Proof proof = GetProof(); Delegation dg = DelegationBuilder(proof).build(); + LOCK(cs_main); + const CCoinsViewCache &coins = ::ChainstateActive().CoinsTip(); std::array nodes; for (CNode *&n : nodes) { n = ConnectNode(NODE_AVALANCHE); - BOOST_CHECK(pm.addNode(n->GetId(), proof, dg)); + BOOST_CHECK(pm.addNode(n->GetId(), proof, dg, coins)); } return nodes; @@ -747,10 +749,14 @@ Proof proof = GetProof(); Delegation dg = DelegationBuilder(proof).build(); - std::array nodes; - for (auto &n : nodes) { - n = ConnectNode(NODE_AVALANCHE); - BOOST_CHECK(pm.addNode(n->GetId(), proof, dg)); + { + LOCK(cs_main); + const CCoinsViewCache &coins = ::ChainstateActive().CoinsTip(); + std::array nodes; + for (auto &n : nodes) { + n = ConnectNode(NODE_AVALANCHE); + BOOST_CHECK(pm.addNode(n->GetId(), proof, dg, coins)); + } } // Add a block to poll