diff --git a/src/avalanche/peermanager.h b/src/avalanche/peermanager.h --- a/src/avalanche/peermanager.h +++ b/src/avalanche/peermanager.h @@ -29,13 +29,6 @@ namespace avalanche { -/** - * 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; - class Delegation; namespace { @@ -133,6 +126,7 @@ PeerSet peers; ProofPool validProofPool; + ProofPool orphanProofPool; using NodeSet = boost::multi_index_container< Node, @@ -165,11 +159,6 @@ 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 */ diff --git a/src/avalanche/peermanager.cpp b/src/avalanche/peermanager.cpp --- a/src/avalanche/peermanager.cpp +++ b/src/avalanche/peermanager.cpp @@ -161,7 +161,7 @@ cs_main, return proof->verify(state, ::ChainstateActive().CoinsTip())); if (!valid) { if (isOrphanState(state)) { - orphanProofs.addProof(proof); + orphanProofPool.addProof(proof); } // Reject invalid proof. @@ -220,10 +220,14 @@ removePeer(pid); } - orphanProofs.rescan(*this); + ProofPool previousOrphans = std::move(orphanProofPool); + orphanProofPool.pool.clear(); + for (auto &entry : previousOrphans.pool) { + registerProof(entry.proof); + } for (auto &p : newOrphans) { - orphanProofs.addProof(p); + orphanProofPool.addProof(p); } } @@ -235,11 +239,7 @@ return true; }); - if (!proof) { - proof = orphanProofs.getProof(proofid); - } - - return proof; + return !!proof ? proof : orphanProofPool.getProof(proofid); } bool PeerManager::isBoundToPeer(const ProofId &proofid) const { @@ -248,7 +248,7 @@ } bool PeerManager::isOrphan(const ProofId &proofid) const { - return orphanProofs.getProof(proofid) != nullptr; + return !!orphanProofPool.getProof(proofid); } bool PeerManager::createPeer(const ProofRef &proof) { @@ -259,7 +259,7 @@ if (!added) { // The proof has conflicts, orphan the proof so it can be pulled back if // the conflicting ones are invalidated. - orphanProofs.addProof(proof); + orphanProofPool.addProof(proof); return false; } 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 @@ -942,4 +942,35 @@ } } +BOOST_AUTO_TEST_CASE(conflicting_orphans) { + avalanche::PeerManager pm; + + const CKey key = CKey::MakeCompressedKey(); + + const Amount amount(10 * COIN); + const uint32_t height = 100; + const bool is_coinbase = false; + + const COutPoint conflictingOutpoint(TxId(GetRandHash()), 0); + + auto buildProofWithSequence = [&](uint64_t sequence) { + ProofBuilder pb(sequence, 0, key); + BOOST_CHECK( + pb.addUTXO(conflictingOutpoint, amount, height, is_coinbase, key)); + + return pb.build(); + }; + + auto orphan10 = buildProofWithSequence(10); + auto orphan20 = buildProofWithSequence(20); + + BOOST_CHECK(!pm.registerProof(orphan10)); + BOOST_CHECK(pm.isOrphan(orphan10->getId())); + + BOOST_CHECK(!pm.registerProof(orphan20)); + BOOST_CHECK(!pm.isOrphan(orphan20->getId())); + BOOST_CHECK(!pm.exists(orphan20->getId())); + BOOST_CHECK(pm.isOrphan(orphan10->getId())); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/test/functional/abc_rpc_avalancheproof.py b/test/functional/abc_rpc_avalancheproof.py --- a/test/functional/abc_rpc_avalancheproof.py +++ b/test/functional/abc_rpc_avalancheproof.py @@ -388,6 +388,9 @@ assert_raises_rpc_error(-8, "The proof has conflicting utxo with an existing proof", node.sendavalancheproof, conflicting_utxo) + # Clear the proof pool + self.restart_node(0) + # Good proof assert node.verifyavalancheproof(proof)