diff --git a/src/avalanche/peermanager.h b/src/avalanche/peermanager.h --- a/src/avalanche/peermanager.h +++ b/src/avalanche/peermanager.h @@ -197,6 +197,12 @@ */ std::unordered_set m_unbroadcast_proofids; + /** + * Quorum management. + */ + uint32_t totalPeersScore = 0; + uint32_t connectedPeersScore = 0; + public: /** * Node API. @@ -310,6 +316,12 @@ void removeUnbroadcastProof(const ProofId &proofid); auto getUnbroadcastProofs() const { return m_unbroadcast_proofids; } + /* + * Quorum management + */ + uint32_t getTotalPeersScore() const { return totalPeersScore; } + uint32_t getConnectedPeersScore() const { return connectedPeersScore; } + /**************************************************** * Functions which are public for testing purposes. * ****************************************************/ diff --git a/src/avalanche/peermanager.cpp b/src/avalanche/peermanager.cpp --- a/src/avalanche/peermanager.cpp +++ b/src/avalanche/peermanager.cpp @@ -74,6 +74,9 @@ const uint64_t start = slotCount; slots.emplace_back(start, score, it->peerid); slotCount = start + score; + + // Add to our allocated score when we allocate a new peer in the slots + connectedPeersScore += score; }); } @@ -120,9 +123,13 @@ return true; } - // There are no more node left, we need to cleanup. + // There are no more nodes left, we need to clean up. Subtract allocated + // score and remove from slots. const size_t i = it->index; assert(i < slots.size()); + assert(connectedPeersScore >= slots[i].getScore()); + connectedPeersScore -= slots[i].getScore(); + if (i + 1 == slots.size()) { slots.pop_back(); slotCount = slots.empty() ? 0 : slots.back().getStop(); @@ -311,6 +318,9 @@ auto inserted = peers.emplace(peerid, proof, nextCooldownTimePoint); assert(inserted.second); + // Add to our registered score when adding to the peer list + totalPeersScore += proof->getScore(); + // If there are nodes waiting for this proof, add them auto &pendingNodesView = pendingNodes.get(); auto range = pendingNodesView.equal_range(proofid); @@ -500,6 +510,10 @@ m_unbroadcast_proofids.erase(it->getProofId()); + // Remove the peer from the PeerSet and remove its score from the registered + // score total. + assert(totalPeersScore >= it->getScore()); + totalPeersScore -= it->getScore(); peers.erase(it); return true; } @@ -554,6 +568,7 @@ bool PeerManager::verify() const { uint64_t prevStop = 0; + uint32_t scoreFromSlots = 0; for (size_t i = 0; i < slots.size(); i++) { const Slot &s = slots[i]; @@ -574,10 +589,24 @@ if (it == peers.end() || it->index != i) { return false; } + + // Accumulate score across slots + scoreFromSlots += slots[i].getScore(); + } + + // Score across slots must be the same as our allocated score + if (scoreFromSlots != connectedPeersScore) { + return false; } + uint32_t scoreFromAllPeers = 0; + uint32_t scoreFromPeersWithNodes = 0; + std::unordered_set peersUtxos; for (const auto &p : peers) { + // Accumulate the score across peers to compare with total known score + scoreFromAllPeers += p.getScore(); + // A peer should have a proof attached if (!p.proof) { return false; @@ -628,6 +657,7 @@ continue; } + scoreFromPeersWithNodes += p.getScore(); // The index must point to a slot refering to this peer. if (p.index >= slots.size() || slots[p.index].getPeerId() != p.peerid) { return false; @@ -639,6 +669,14 @@ } } + // Check our accumulated scores against our registred and allocated scores + if (scoreFromAllPeers != totalPeersScore) { + return false; + } + if (scoreFromPeersWithNodes != connectedPeersScore) { + return false; + } + // We checked the utxo consistency for all our peers utxos already, so if // the pool size differs from the expected one there are dangling utxos. if (validProofPool.size() != peersUtxos.size()) { 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 @@ -33,13 +33,17 @@ return pendingNodesView.find(nodeid) != pendingNodesView.end(); } + static PeerId getPeerIdForProofId(PeerManager &pm, + const ProofId &proofid) { + auto &pview = pm.peers.get(); + auto it = pview.find(proofid); + return it == pview.end() ? NO_PEER : it->peerid; + } + static PeerId registerAndGetPeerId(PeerManager &pm, const ProofRef &proof) { pm.registerProof(proof); - - auto &pview = pm.peers.get(); - auto it = pview.find(proof->getId()); - return it == pview.end() ? NO_PEER : it->peerid; + return getPeerIdForProofId(pm, proof->getId()); } static std::vector getOrderedScores(const PeerManager &pm) { @@ -1565,4 +1569,228 @@ expectedScores.begin(), expectedScores.end()); } +BOOST_FIXTURE_TEST_CASE(known_score_tracking, NoCoolDownFixture) { + avalanche::PeerManager pm; + + const CKey key = CKey::MakeCompressedKey(); + + const Amount amount10(10 * COIN); + const Amount amount20(20 * COIN); + const uint32_t height = 100; + const bool is_coinbase = false; + CScript script = GetScriptForDestination(PKHash(key.GetPubKey())); + + const COutPoint peer1ConflictingOutput(TxId(GetRandHash()), 0); + { + LOCK(cs_main); + CCoinsViewCache &coins = ::ChainstateActive().CoinsTip(); + coins.AddCoin(peer1ConflictingOutput, + Coin(CTxOut(amount10, script), height, is_coinbase), + false); + } + + const COutPoint peer1SecondaryOutpoint(TxId(GetRandHash()), 1); + { + LOCK(cs_main); + CCoinsViewCache &coins = ::ChainstateActive().CoinsTip(); + coins.AddCoin(peer1SecondaryOutpoint, + Coin(CTxOut(amount20, script), height, is_coinbase), + false); + } + + auto buildProofWithSequenceAndOutpoints = + [&](int64_t sequence, + const std::vector> &outpoints) { + ProofBuilder pb(sequence, 0, key); + for (const auto &outpoint : outpoints) { + BOOST_CHECK(pb.addUTXO(std::get<0>(outpoint), + std::get<1>(outpoint), height, + is_coinbase, key)); + } + return pb.build(); + }; + + auto peer1Proof1 = buildProofWithSequenceAndOutpoints( + 10, {{peer1ConflictingOutput, amount10}, + {peer1SecondaryOutpoint, amount20}}); + auto peer1Proof2 = buildProofWithSequenceAndOutpoints( + 20, {{peer1ConflictingOutput, amount10}}); + auto peer1Proof3 = buildProofWithSequenceAndOutpoints( + 30, {{peer1ConflictingOutput, amount10}, + {COutPoint{TxId(GetRandHash()), 0}, amount10}}); + + const uint32_t peer1Score1 = Proof::amountToScore(amount10 + amount20); + const uint32_t peer1Score2 = Proof::amountToScore(amount10); + + // Add first peer and check that we have its score tracked + BOOST_CHECK_EQUAL(pm.getTotalPeersScore(), 0); + BOOST_CHECK(pm.registerProof(peer1Proof2)); + BOOST_CHECK_EQUAL(pm.getTotalPeersScore(), peer1Score2); + + // Ensure failing to add conflicting proofs doesn't affect the score, the + // first proof stays bound and counted + BOOST_CHECK(!pm.registerProof(peer1Proof1)); + BOOST_CHECK(!pm.registerProof(peer1Proof3)); + + BOOST_CHECK(pm.isBoundToPeer(peer1Proof2->getId())); + BOOST_CHECK(pm.isInConflictingPool(peer1Proof1->getId())); + BOOST_CHECK(pm.isOrphan(peer1Proof3->getId())); + + BOOST_CHECK_EQUAL(pm.getTotalPeersScore(), peer1Score2); + + auto checkRejectDefault = [&](const ProofId &proofid) { + BOOST_CHECK(pm.exists(proofid)); + const bool isOrphan = pm.isOrphan(proofid); + BOOST_CHECK(pm.rejectProof( + proofid, avalanche::PeerManager::RejectionMode::DEFAULT)); + BOOST_CHECK(!pm.isBoundToPeer(proofid)); + BOOST_CHECK_EQUAL(pm.exists(proofid), !isOrphan); + }; + + auto checkRejectInvalidate = [&](const ProofId &proofid) { + BOOST_CHECK(pm.exists(proofid)); + BOOST_CHECK(pm.rejectProof( + proofid, avalanche::PeerManager::RejectionMode::INVALIDATE)); + }; + + // Reject from the orphan pool doesn't affect tracked score + checkRejectDefault(peer1Proof3->getId()); + BOOST_CHECK(!pm.registerProof(peer1Proof3)); + BOOST_CHECK(pm.isOrphan(peer1Proof3->getId())); + BOOST_CHECK_EQUAL(pm.getTotalPeersScore(), peer1Score2); + checkRejectInvalidate(peer1Proof3->getId()); + BOOST_CHECK_EQUAL(pm.getTotalPeersScore(), peer1Score2); + + // Reject from the conflicting pool + checkRejectDefault(peer1Proof1->getId()); + checkRejectInvalidate(peer1Proof1->getId()); + + // Add again a proof to the conflicting pool + BOOST_CHECK(!pm.registerProof(peer1Proof1)); + BOOST_CHECK(pm.isInConflictingPool(peer1Proof1->getId())); + BOOST_CHECK_EQUAL(pm.getTotalPeersScore(), peer1Score2); + + // Reject from the valid pool, default mode + // Now the score should change as the new peer is promoted + checkRejectDefault(peer1Proof2->getId()); + BOOST_CHECK(!pm.isInConflictingPool(peer1Proof1->getId())); + BOOST_CHECK(pm.isBoundToPeer(peer1Proof1->getId())); + BOOST_CHECK_EQUAL(pm.getTotalPeersScore(), peer1Score1); + + // Reject from the valid pool, invalidate mode + // Now the score should change as the old peer is re-promoted + checkRejectInvalidate(peer1Proof1->getId()); + + // The conflicting proof should also be promoted to a peer + BOOST_CHECK(!pm.isInConflictingPool(peer1Proof2->getId())); + BOOST_CHECK(pm.isBoundToPeer(peer1Proof2->getId())); + BOOST_CHECK_EQUAL(pm.getTotalPeersScore(), peer1Score2); + + // Now add another peer and check that combined scores are correct + uint32_t peer2Score = 1 * MIN_VALID_PROOF_SCORE; + auto peer2Proof1 = buildRandomProof(peer2Score); + PeerId peerid2 = TestPeerManager::registerAndGetPeerId(pm, peer2Proof1); + BOOST_CHECK_EQUAL(pm.getTotalPeersScore(), peer1Score2 + peer2Score); + + // Trying to remove non-existent peer doesn't affect score + BOOST_CHECK(!pm.removePeer(1234)); + BOOST_CHECK_EQUAL(pm.getTotalPeersScore(), peer1Score2 + peer2Score); + + // Removing new peer removes its score + BOOST_CHECK(pm.removePeer(peerid2)); + BOOST_CHECK_EQUAL(pm.getTotalPeersScore(), peer1Score2); + PeerId peerid1 = + TestPeerManager::getPeerIdForProofId(pm, peer1Proof2->getId()); + BOOST_CHECK(pm.removePeer(peerid1)); + BOOST_CHECK_EQUAL(pm.getTotalPeersScore(), 0); +} + +BOOST_AUTO_TEST_CASE(connected_score_tracking) { + avalanche::PeerManager pm; + + const auto checkScores = [&pm](uint32_t known, uint32_t connected) { + BOOST_CHECK_EQUAL(pm.getTotalPeersScore(), known); + BOOST_CHECK_EQUAL(pm.getConnectedPeersScore(), connected); + }; + + // Start out with 0s + checkScores(0, 0); + + // Create one peer without a node. Its score should be registered but not + // connected + uint32_t score1 = 10000000 * MIN_VALID_PROOF_SCORE; + auto proof1 = buildRandomProof(score1); + PeerId peerid1 = TestPeerManager::registerAndGetPeerId(pm, proof1); + checkScores(score1, 0); + + // Add nodes. We now have a connected score, but it doesn't matter how many + // nodes we add the score is the same + const ProofId &proofid1 = proof1->getId(); + const uint8_t nodesToAdd = 10; + for (int i = 0; i < nodesToAdd; i++) { + BOOST_CHECK(pm.addNode(i, proofid1)); + checkScores(score1, score1); + } + + // Remove all but 1 node and ensure the score doesn't change + for (int i = 0; i < nodesToAdd - 1; i++) { + BOOST_CHECK(pm.removeNode(i)); + checkScores(score1, score1); + } + + // Removing the last node should remove the score from the connected count + BOOST_CHECK(pm.removeNode(nodesToAdd - 1)); + checkScores(score1, 0); + + // Add 2 nodes to peer and create peer2. Without a node peer2 has no + // connected score but after adding a node it does. + BOOST_CHECK(pm.addNode(0, proofid1)); + BOOST_CHECK(pm.addNode(1, proofid1)); + checkScores(score1, score1); + + uint32_t score2 = 1 * MIN_VALID_PROOF_SCORE; + auto proof2 = buildRandomProof(score2); + PeerId peerid2 = TestPeerManager::registerAndGetPeerId(pm, proof2); + checkScores(score1 + score2, score1); + BOOST_CHECK(pm.addNode(2, proof2->getId())); + checkScores(score1 + score2, score1 + score2); + + // The first peer has two nodes left. Remove one and nothing happens, remove + // the other and its score is no longer in the connected counter.. + BOOST_CHECK(pm.removeNode(0)); + checkScores(score1 + score2, score1 + score2); + BOOST_CHECK(pm.removeNode(1)); + checkScores(score1 + score2, score2); + + // Removing a peer with no allocated score has no affect. + BOOST_CHECK(pm.removePeer(peerid1)); + checkScores(score2, score2); + + // Remove the second peer's node removes its allocated score. + BOOST_CHECK(pm.removeNode(2)); + checkScores(score2, 0); + + // Removing the second peer takes us back to 0. + BOOST_CHECK(pm.removePeer(peerid2)); + checkScores(0, 0); + + // Add 2 peers with nodes and remove them without removing the nodes first. + // Both score counters should be reduced by each peer's score when it's + // removed. + peerid1 = TestPeerManager::registerAndGetPeerId(pm, proof1); + checkScores(score1, 0); + peerid2 = TestPeerManager::registerAndGetPeerId(pm, proof2); + checkScores(score1 + score2, 0); + BOOST_CHECK(pm.addNode(0, proof1->getId())); + checkScores(score1 + score2, score1); + BOOST_CHECK(pm.addNode(1, proof2->getId())); + checkScores(score1 + score2, score1 + score2); + + BOOST_CHECK(pm.removePeer(peerid2)); + checkScores(score1, score1); + + BOOST_CHECK(pm.removePeer(peerid1)); + checkScores(0, 0); +} + BOOST_AUTO_TEST_SUITE_END()