diff --git a/src/avalanche/peermanager.h b/src/avalanche/peermanager.h --- a/src/avalanche/peermanager.h +++ b/src/avalanche/peermanager.h @@ -26,6 +26,11 @@ #include #include +/** + * Is proof replacement enabled by default. + */ +static constexpr bool AVALANCHE_DEFAULT_PROOF_REPLACEMENT_ENABLED = false; + namespace avalanche { /** @@ -273,6 +278,7 @@ private: bool createPeer(const ProofRef &proof); + bool removePeer(const ProofRef &proof); bool addOrUpdateNode(const PeerSet::iterator &it, NodeId nodeid); bool addNodeToPeer(const PeerSet::iterator &it); bool removeNodeFromPeer(const PeerSet::iterator &it, uint32_t count = 1); diff --git a/src/avalanche/peermanager.cpp b/src/avalanche/peermanager.cpp --- a/src/avalanche/peermanager.cpp +++ b/src/avalanche/peermanager.cpp @@ -153,24 +153,37 @@ return false; } - // The proof is attached with no conflict - if (shiftedProofs.empty()) { - return true; - } + if (!gArgs.GetBoolArg("-enableavalancheproofreplacement", + AVALANCHE_DEFAULT_PROOF_REPLACEMENT_ENABLED)) { + // The proof is attached with no conflict + if (shiftedProofs.empty()) { + return true; + } + + // For now, if there is a conflict, just cleanup the mess. + pool.removeProof(proof); + + // Restore the previous state. Since we removed the conflicting proof, + // this can never fail. + for (const ProofRef &p : shiftedProofs) { + std::set inconsistent; + const bool attached = pool.addProof(p, inconsistent); + assert(attached); + assert(inconsistent.empty()); + } - // For now, if there is a conflict, just cleanup the mess. - pool.removeProof(proof); + return false; + } - // Restore the previous state. Since we removed the conflicting proof, this - // can never fail. + // If the proof is successfully added to the pool, deal with conflicts. + // The proof might have replaced others, so we need to remove their peers + // as needed. for (const ProofRef &p : shiftedProofs) { - std::set inconsistent; - const bool attached = pool.addProof(p, inconsistent); - assert(attached); - assert(inconsistent.empty()); + removePeer(p); + orphanProofs.addProof(p); } - return false; + return true; } bool PeerManager::registerProof(const ProofRef &proof) { @@ -320,6 +333,11 @@ return true; } +bool PeerManager::removePeer(const ProofRef &proof) { + return forPeer(proof->getId(), + [&](const Peer &peer) { return removePeer(peer.peerid); }); +} + bool PeerManager::removePeer(const PeerId peerid) { auto it = peers.find(peerid); if (it == peers.end()) { 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 @@ -859,8 +859,6 @@ } BOOST_AUTO_TEST_CASE(conflicting_proof_selection) { - avalanche::PeerManager pm; - const CKey key = CKey::MakeCompressedKey(); const Amount amount(10 * COIN); @@ -892,11 +890,21 @@ auto proof_base = buildProofWithSequence(10); + gArgs.ForceSetArg("-enableavalancheproofreplacement", "1"); + ConflictingProofComparator comparator; auto checkPreferred = [&](const ProofRef &candidate, const ProofRef &reference, bool expectAccepted) { BOOST_CHECK_EQUAL(comparator(candidate, reference), expectAccepted); BOOST_CHECK_EQUAL(comparator(reference, candidate), !expectAccepted); + + avalanche::PeerManager pm; + BOOST_CHECK(pm.registerProof(reference)); + BOOST_CHECK(pm.isValid(reference->getId())); + + BOOST_CHECK_EQUAL(pm.registerProof(candidate), expectAccepted); + BOOST_CHECK_EQUAL(pm.isValid(candidate->getId()), expectAccepted); + BOOST_CHECK_EQUAL(pm.isValid(reference->getId()), !expectAccepted); }; // Same master key, lower sequence number @@ -910,7 +918,7 @@ BOOST_CHECK( pb.addUTXO(conflictingOutpoint, amount, height, is_coinbase, key)); for (const Amount &v : amounts) { - auto outpoint = addCoin(amount); + auto outpoint = addCoin(v); BOOST_CHECK( pb.addUTXO(std::move(outpoint), v, height, is_coinbase, key)); } @@ -940,6 +948,8 @@ checkPreferred(proofSimilar, proof_multiUtxo, proofSimilar->getId() < proof_multiUtxo->getId()); } + + gArgs.ClearForcedArg("-enableavalancheproofreplacement"); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/init.cpp b/src/init.cpp --- a/src/init.cpp +++ b/src/init.cpp @@ -12,6 +12,7 @@ #include #include #include +#include // For AVALANCHE_DEFAULT_PROOF_REPLACEMENT_ENABLED #include #include // For AVALANCHE_LEGACY_PROOF_DEFAULT #include @@ -1270,6 +1271,10 @@ strprintf("Enable avalanche peer discovery (default: %u)", AVALANCHE_DEFAULT_PEER_DISCOVERY_ENABLED), ArgsManager::ALLOW_ANY, OptionsCategory::AVALANCHE); + argsman.AddArg("-enableavalancheproofreplacement", + strprintf("Enable avalanche proof replacement (default: %u)", + AVALANCHE_DEFAULT_PROOF_REPLACEMENT_ENABLED), + ArgsManager::ALLOW_BOOL, OptionsCategory::AVALANCHE); argsman.AddArg( "-avacooldown", strprintf("Mandatory cooldown between two avapoll (default: %u)",