diff --git a/src/avalanche/peermanager.h b/src/avalanche/peermanager.h --- a/src/avalanche/peermanager.h +++ b/src/avalanche/peermanager.h @@ -37,14 +37,7 @@ * Maximum number of proofs to store per utxo. This will allow for conflict * resolution when several proofs claim the same utxo. */ -static constexpr size_t AVALANCHE_MAXPROOF_PER_UTXO = 1; - -/** - * Maximum number of stakes in the orphanProofs. - * Benchmarking on a consumer grade computer shows that 10000 stakes can be - * verified in less than 1 second. - */ -static constexpr size_t AVALANCHE_ORPHANPROOFPOOL_SIZE = 10000; +static constexpr size_t AVALANCHE_MAXPROOF_PER_UTXO = 2; class Delegation; @@ -175,18 +168,12 @@ static constexpr int SELECT_PEER_MAX_RETRY = 3; static constexpr int SELECT_NODE_MAX_RETRY = 3; - /** - * Tracks proof which for which the UTXO are unavailable. - */ - OrphanProofPool orphanProofs{AVALANCHE_ORPHANPROOFPOOL_SIZE}; - /** * Track proof ids to broadcast */ std::unordered_set m_unbroadcast_proofids; public: - // Limit the pool to a depth to a single proof per utxo PeerManager(size_t maxProofsPerUtxo = AVALANCHE_MAXPROOF_PER_UTXO) : pool(maxProofsPerUtxo) {} diff --git a/src/avalanche/peermanager.cpp b/src/avalanche/peermanager.cpp --- a/src/avalanche/peermanager.cpp +++ b/src/avalanche/peermanager.cpp @@ -277,7 +277,8 @@ if (!valid) { if (isOrphanState(state)) { - orphanProofs.addProof(proof); + // Don't create a peer but keep the utxos around. + addProofToPool(proof, false /* allowPeerOverride */); } // Reject invalid proof. @@ -289,10 +290,18 @@ gArgs.GetBoolArg("-enableavalancheproofreplacement", AVALANCHE_DEFAULT_PROOF_REPLACEMENT_ENABLED))) { // Rejected due to conflicting proofs - orphanProofs.addProof(proof); return false; } + if (!isPeerCandidate(proof)) { + // Orphaned due to conflicting proofs + return false; + } + + // This is the best candidate, override the conflicting peers + pool.forEachConflictingProof(proof, + [&](const ProofRef &p) { removePeer(p); }); + return createPeer(proof); } @@ -321,7 +330,6 @@ void PeerManager::updatedBlockTip() { std::vector invalidPeers; - std::vector newOrphans; { LOCK(cs_main); @@ -330,9 +338,6 @@ for (const auto &p : peers) { ProofValidationState state; if (!p.proof->verify(state, coins)) { - if (isOrphanState(state)) { - newOrphans.push_back(p.proof); - } invalidPeers.push_back(p.peerid); } } @@ -345,10 +350,23 @@ removePeer(pid); } - orphanProofs.rescan(*this); + // Build a candidate set from the pool + std::set candidates; + pool.forEachUtxo([&](const COutPoint &outpoint, const auto &proofs) { + for (const ProofRef &candidate : proofs) { + if (isValid(candidate->getId())) { + return; + } - for (auto &p : newOrphans) { - orphanProofs.addProof(p); + candidates.emplace(candidate); + } + }); + + for (const ProofRef &candidate : candidates) { + // Temporarly remove the candidate from the orphan state. Registration + // will eventually turn it into a peer or make it an orphan again. + pool.removeProof(candidate); + registerProof(candidate); } } @@ -361,7 +379,14 @@ }); if (!proof) { - proof = orphanProofs.getProof(proofid); + pool.forEachUtxo([&](const COutPoint &outpoint, const auto &proofs) { + for (const ProofRef &p : proofs) { + if (p->getId() == proofid) { + proof = p; + return; + } + } + }); } return proof; @@ -373,7 +398,7 @@ } bool PeerManager::isOrphan(const ProofId &proofid) const { - return orphanProofs.getProof(proofid) != nullptr; + return !isValid(proofid) && getProof(proofid); } bool PeerManager::createPeer(const ProofRef &proof) { @@ -440,9 +465,6 @@ nview.upper_bound(boost::make_tuple( peerid, std::chrono::steady_clock::now()))); - bool removed = pool.removeProof(it->proof); - assert(removed); - m_unbroadcast_proofids.erase(it->proof->getId()); peers.erase(it); @@ -579,14 +601,7 @@ } } - // Check there is no dangling utxo in the pool - bool danglingUtxo = false; - pool.forEachUtxo([&](const COutPoint &outpoint, const auto &proof) { - danglingUtxo = - danglingUtxo || peersUtxos.find(outpoint) == peersUtxos.end(); - }); - - return !danglingUtxo; + return true; } PeerId selectPeerImpl(const std::vector &slots, const uint64_t slot, diff --git a/src/avalanche/proofpool.h b/src/avalanche/proofpool.h --- a/src/avalanche/proofpool.h +++ b/src/avalanche/proofpool.h @@ -51,6 +51,22 @@ fun(outpoint, proofs); } } + + template + void forEachConflictingProof(const ProofRef &proof, Callable fun) const { + for (const SignedStake &ss : proof->getStakes()) { + auto it = utxos.find(ss.getStake().getUTXO()); + if (it == utxos.end()) { + continue; + } + + for (const ProofRef &p : it->second) { + if (p->getId() != proof->getId()) { + fun(p); + } + } + } + } }; } // namespace avalanche 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 @@ -483,7 +483,7 @@ // Remove the peer, the nodes should be pending again BOOST_CHECK(pm.removePeer(peerid)); - BOOST_CHECK(!pm.exists(proof->getId())); + BOOST_CHECK(!pm.isValid(proof->getId())); for (int i = 0; i < 10; i++) { BOOST_CHECK(TestPeerManager::isNodePending(pm, i)); BOOST_CHECK(!TestPeerManager::nodeBelongToPeer(pm, i, peerid));