diff --git a/src/avalanche/peermanager.cpp b/src/avalanche/peermanager.cpp --- a/src/avalanche/peermanager.cpp +++ b/src/avalanche/peermanager.cpp @@ -283,7 +283,7 @@ case ProofPool::AddProofStatus::REJECTED: // The proof has conflicts, move it to the conflicting proof pool so // it can be pulled back if the conflicting ones are invalidated. - conflictingProofPool.addProofIfNoConflict(proof); + conflictingProofPool.addProofIfPreferred(proof); return false; case ProofPool::AddProofStatus::DUPLICATED: // If the proof was already in the pool, don't duplicate the peer. 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 @@ -972,4 +972,55 @@ BOOST_CHECK(pm.isOrphan(orphan10->getId())); } +BOOST_AUTO_TEST_CASE(preferred_conflicting_proof) { + avalanche::PeerManager pm; + + const CKey key = CKey::MakeCompressedKey(); + + const Amount amount(10 * COIN); + const uint32_t height = 100; + const bool is_coinbase = false; + CScript script = GetScriptForDestination(PKHash(key.GetPubKey())); + + const COutPoint conflictingOutpoint(TxId(GetRandHash()), 0); + { + LOCK(cs_main); + CCoinsViewCache &coins = ::ChainstateActive().CoinsTip(); + coins.AddCoin(conflictingOutpoint, + Coin(CTxOut(amount, script), height, is_coinbase), false); + } + + auto buildProofWithSequence = [&](uint64_t sequence) { + ProofBuilder pb(sequence, 0, key); + BOOST_CHECK( + pb.addUTXO(conflictingOutpoint, amount, height, is_coinbase, key)); + + return pb.build(); + }; + + auto proofSeq10 = buildProofWithSequence(10); + auto proofSeq20 = buildProofWithSequence(20); + auto proofSeq30 = buildProofWithSequence(30); + + BOOST_CHECK(pm.registerProof(proofSeq30)); + BOOST_CHECK(pm.isBoundToPeer(proofSeq30->getId())); + BOOST_CHECK(!pm.isConflicting(proofSeq30->getId())); + + // proofSeq10 is a worst candidate than proofSeq30, so it goes to the + // conflicting pool. + BOOST_CHECK(!pm.registerProof(proofSeq10)); + BOOST_CHECK(pm.isBoundToPeer(proofSeq30->getId())); + BOOST_CHECK(!pm.isBoundToPeer(proofSeq10->getId())); + BOOST_CHECK(pm.isConflicting(proofSeq10->getId())); + + // proofSeq20 is a worst candidate than proofSeq30 but a better one than + // proogSeq10, so it replaces it in the conflicting pool and proofSeq10 is + // evicted. + BOOST_CHECK(!pm.registerProof(proofSeq20)); + BOOST_CHECK(pm.isBoundToPeer(proofSeq30->getId())); + BOOST_CHECK(!pm.isBoundToPeer(proofSeq20->getId())); + BOOST_CHECK(pm.isConflicting(proofSeq20->getId())); + BOOST_CHECK(!pm.exists(proofSeq10->getId())); +} + BOOST_AUTO_TEST_SUITE_END()