diff --git a/src/avalanche/peermanager.h b/src/avalanche/peermanager.h --- a/src/avalanche/peermanager.h +++ b/src/avalanche/peermanager.h @@ -205,7 +205,20 @@ bool updateNextPossibleConflictTime(PeerId peerid, const std::chrono::seconds &nextTime); - bool registerProof(const ProofRef &proof); + /** + * Registration mode + * - DEFAULT: Default policy, register only if the proof is unknown and has + * no conflict. + * - FORCE_ACCEPT: Turn a valid proof into a peer even if it has conflicts + * and is not the best candidate. + */ + enum class RegistrationMode { + DEFAULT, + FORCE_ACCEPT, + }; + + bool registerProof(const ProofRef &proof, + RegistrationMode mode = RegistrationMode::DEFAULT); bool exists(const ProofId &proofid) const { return getProof(proofid) != nullptr; } diff --git a/src/avalanche/peermanager.cpp b/src/avalanche/peermanager.cpp --- a/src/avalanche/peermanager.cpp +++ b/src/avalanche/peermanager.cpp @@ -164,13 +164,17 @@ return it->nextPossibleConflictTime == nextTime; } -bool PeerManager::registerProof(const ProofRef &proof) { +bool PeerManager::registerProof(const ProofRef &proof, RegistrationMode mode) { assert(proof); const ProofId &proofid = proof->getId(); - if (exists(proofid)) { - // The proof is already registered, or orphaned. + if ((mode != RegistrationMode::FORCE_ACCEPT || + !isInConflictingPool(proofid)) && + exists(proofid)) { + // In default mode, we expect the proof to be unknown, i.e. in none of + // the pools. + // In forced accept mode, the proof can be in the conflicting pool. return false; } @@ -189,12 +193,36 @@ return false; } - switch (validProofPool.addProofIfNoConflict(proof)) { - 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.addProofIfPreferred(proof); - return false; + ProofPool::ConflictingProofSet conflictingProofs; + switch (validProofPool.addProofIfNoConflict(proof, conflictingProofs)) { + case ProofPool::AddProofStatus::REJECTED: { + if (mode != RegistrationMode::FORCE_ACCEPT) { + // The proof has conflicts, move it to the conflicting proof + // pool so it can be pulled back if the conflicting ones are + // invalidated. + conflictingProofPool.addProofIfPreferred(proof); + return false; + } + + conflictingProofPool.removeProof(proofid); + + // Move the conflicting proofs from the valid pool to the + // conflicting pool + auto &peersView = peers.get(); + for (const ProofRef &conflictingProof : conflictingProofs) { + auto it = peersView.find(conflictingProof->getId()); + if (it != peersView.end()) { + removePeer(it->peerid); + } + + conflictingProofPool.addProofIfPreferred(conflictingProof); + } + + auto status = validProofPool.addProofIfNoConflict(proof); + assert(status == ProofPool::AddProofStatus::SUCCEED); + + break; + } case ProofPool::AddProofStatus::DUPLICATED: // If the proof was already in the pool, don't duplicate the peer. return false; @@ -204,6 +232,8 @@ // No default case, so the compiler can warn about missing cases } + // At this stage we are going to create a peer so the proof should never + // exist in the conflicting pool, but use belt and suspenders. conflictingProofPool.removeProof(proofid); // New peer means new peerid! 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 @@ -1095,4 +1095,87 @@ checkNextPossibleConflictTime(now + std::chrono::seconds{1}); } +BOOST_AUTO_TEST_CASE(register_force_accept) { + 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.isInConflictingPool(proofSeq30->getId())); + + // proofSeq20 is a worst candidate than proofSeq30, so it goes to the + // conflicting pool. + BOOST_CHECK(!pm.registerProof(proofSeq20)); + BOOST_CHECK(pm.isBoundToPeer(proofSeq30->getId())); + BOOST_CHECK(pm.isInConflictingPool(proofSeq20->getId())); + + // We can force the acceptance of proofSeq20 + using RegistrationMode = avalanche::PeerManager::RegistrationMode; + BOOST_CHECK(pm.registerProof(proofSeq20, RegistrationMode::FORCE_ACCEPT)); + BOOST_CHECK(pm.isBoundToPeer(proofSeq20->getId())); + BOOST_CHECK(pm.isInConflictingPool(proofSeq30->getId())); + + // We can also force the acceptance of a proof which is not already in the + // conflicting pool. + BOOST_CHECK(!pm.registerProof(proofSeq10)); + BOOST_CHECK(!pm.exists(proofSeq10->getId())); + + BOOST_CHECK(pm.registerProof(proofSeq10, RegistrationMode::FORCE_ACCEPT)); + BOOST_CHECK(pm.isBoundToPeer(proofSeq10->getId())); + BOOST_CHECK(!pm.exists(proofSeq20->getId())); + BOOST_CHECK(pm.isInConflictingPool(proofSeq30->getId())); + + // Attempting to register again fails, and has no impact on the pools + for (size_t i = 0; i < 10; i++) { + BOOST_CHECK(!pm.registerProof(proofSeq10)); + BOOST_CHECK( + !pm.registerProof(proofSeq10, RegistrationMode::FORCE_ACCEPT)); + + BOOST_CHECK(pm.isBoundToPeer(proofSeq10->getId())); + BOOST_CHECK(!pm.exists(proofSeq20->getId())); + BOOST_CHECK(pm.isInConflictingPool(proofSeq30->getId())); + } + + // Revert between proofSeq10 and proofSeq30 a few times + for (size_t i = 0; i < 10; i++) { + BOOST_CHECK( + pm.registerProof(proofSeq30, RegistrationMode::FORCE_ACCEPT)); + + BOOST_CHECK(pm.isBoundToPeer(proofSeq30->getId())); + BOOST_CHECK(pm.isInConflictingPool(proofSeq10->getId())); + + BOOST_CHECK( + pm.registerProof(proofSeq10, RegistrationMode::FORCE_ACCEPT)); + + BOOST_CHECK(pm.isBoundToPeer(proofSeq10->getId())); + BOOST_CHECK(pm.isInConflictingPool(proofSeq30->getId())); + } +} + BOOST_AUTO_TEST_SUITE_END()