diff --git a/src/avalanche/avalanche.h b/src/avalanche/avalanche.h --- a/src/avalanche/avalanche.h +++ b/src/avalanche/avalanche.h @@ -24,6 +24,13 @@ */ static constexpr bool AVALANCHE_DEFAULT_PEER_DISCOVERY_ENABLED = false; +/** + * Conflicting proofs cooldown time default value in seconds. + * After a proof has been registered, this cooldown delay should elapse before + * another proof with a conflicting UTXO can be considered. + */ +static constexpr size_t AVALANCHE_DEFAULT_CONFLICTING_PROOF_COOLDOWN = 60 * 60; + /** * Avalanche default cooldown in milliseconds. */ diff --git a/src/avalanche/peermanager.cpp b/src/avalanche/peermanager.cpp --- a/src/avalanche/peermanager.cpp +++ b/src/avalanche/peermanager.cpp @@ -4,6 +4,7 @@ #include +#include #include #include #include @@ -279,12 +280,35 @@ bool PeerManager::createPeer(const ProofRef &proof) { assert(proof); - switch (validProofPool.addProofIfNoConflict(proof)) { - case ProofPool::AddProofStatus::REJECTED: + ProofPool::ConflictingProofSet conflictingProofs; + switch (validProofPool.addProofIfNoConflict(proof, conflictingProofs)) { + case ProofPool::AddProofStatus::REJECTED: { + // If the cooldown time has not elapsed, just reject the proof. + auto &pview = peers.get(); + std::chrono::seconds lastConflictingRegistrationTime = + std::chrono::seconds::min(); + for (auto &conflictingProof : conflictingProofs) { + auto it = pview.find(conflictingProof->getId()); + assert(it != pview.end()); + + lastConflictingRegistrationTime = std::max( + lastConflictingRegistrationTime, it->registration_time); + } + + std::chrono::seconds cooldown{ + gArgs.GetArg("-avalancheconflictingproofcooldown", + AVALANCHE_DEFAULT_CONFLICTING_PROOF_COOLDOWN)}; + if (lastConflictingRegistrationTime + cooldown > + GetTime()) { + // If the cooldown is not elapsed, reject the proof + return false; + } + // 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; + } case ProofPool::AddProofStatus::DUPLICATED: // If the proof was already in the pool, don't duplicate the peer. 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 @@ -15,6 +15,8 @@ #include +#include + using namespace avalanche; namespace avalanche { @@ -44,6 +46,17 @@ } // namespace } // namespace avalanche +namespace { +struct NoConflictingCooldownFixture : public TestingSetup { + NoConflictingCooldownFixture() { + gArgs.ForceSetArg("-avalancheconflictingproofcooldown", "0"); + } + ~NoConflictingCooldownFixture() { + gArgs.ClearForcedArg("-avalancheconflictingproofcooldown"); + } +}; +} // namespace + BOOST_FIXTURE_TEST_SUITE(peermanager_tests, TestingSetup) BOOST_AUTO_TEST_CASE(select_peer_linear) { @@ -794,7 +807,8 @@ !pm.registerProof(std::make_shared(std::move(badProof)))); } -BOOST_AUTO_TEST_CASE(conflicting_proof_rescan) { +BOOST_FIXTURE_TEST_CASE(conflicting_proof_rescan, + NoConflictingCooldownFixture) { avalanche::PeerManager pm; const CKey key = CKey::MakeCompressedKey(); @@ -971,7 +985,8 @@ BOOST_CHECK(!pm.exists(orphan10->getId())); } -BOOST_AUTO_TEST_CASE(preferred_conflicting_proof) { +BOOST_FIXTURE_TEST_CASE(preferred_conflicting_proof, + NoConflictingCooldownFixture) { avalanche::PeerManager pm; const CKey key = CKey::MakeCompressedKey(); @@ -1022,4 +1037,106 @@ BOOST_CHECK(!pm.exists(proofSeq10->getId())); } +BOOST_AUTO_TEST_CASE(conflicting_proofs_cooldown) { + int64_t cooldown = 100; + + gArgs.ForceSetArg("-avalancheconflictingproofcooldown", + strprintf("%d", cooldown)); + + int64_t mockTime = 100; + SetMockTime(mockTime); + + 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())); + + auto addCoin = [&](const COutPoint &outpoint) { + LOCK(cs_main); + + CCoinsViewCache &coins = ::ChainstateActive().CoinsTip(); + coins.AddCoin(outpoint, + Coin(CTxOut(amount, script), height, is_coinbase), false); + }; + + auto buildProofWithSequenceAndOutpoints = + [&](uint64_t sequence, const std::vector &outpoints) { + ProofBuilder pb(sequence, + GetRandInt(std::numeric_limits::max()), key); + for (const COutPoint &outpoint : outpoints) { + BOOST_CHECK( + pb.addUTXO(outpoint, amount, height, is_coinbase, key)); + } + + return pb.build(); + }; + + const COutPoint conflictingOutpoint{TxId(GetRandHash()), 0}; + addCoin(conflictingOutpoint); + auto proof_seq10 = + buildProofWithSequenceAndOutpoints(10, {conflictingOutpoint}); + auto proof_seq20 = + buildProofWithSequenceAndOutpoints(20, {conflictingOutpoint}); + + const COutPoint unknownOutpoint{TxId(GetRandHash()), 0}; + auto proof_seq30 = buildProofWithSequenceAndOutpoints( + 30, {conflictingOutpoint, unknownOutpoint}); + + { + avalanche::PeerManager pm; + + // No conflict means no cooldown + BOOST_CHECK(pm.registerProof(proof_seq10)); + BOOST_CHECK(pm.isBoundToPeer(proof_seq10->getId())); + BOOST_CHECK(!pm.isInConflictingPool(proof_seq10->getId())); + + // Cooldown not elapsed, the proof is rejected + mockTime += cooldown - 1; + SetMockTime(mockTime); + BOOST_CHECK(!pm.registerProof(proof_seq20)); + BOOST_CHECK(!pm.exists(proof_seq20->getId())); + + // Cooldown elapsed, the conflicting proof can be registered. + mockTime += 1; + SetMockTime(mockTime); + BOOST_CHECK(!pm.registerProof(proof_seq20)); + BOOST_CHECK(pm.isInConflictingPool(proof_seq20->getId())); + } + + { + avalanche::PeerManager pm; + + // No conflict means no cooldown + BOOST_CHECK(pm.registerProof(proof_seq10)); + BOOST_CHECK(pm.isBoundToPeer(proof_seq10->getId())); + BOOST_CHECK(!pm.isInConflictingPool(proof_seq10->getId())); + + // Register proof_seq30, which is orphaned due to unknownOutpoint + BOOST_CHECK(!pm.registerProof(proof_seq30)); + BOOST_CHECK(pm.isOrphan(proof_seq30->getId())); + + // Add the unknownOutpoint to the utxo set, so proof_seq30 is now valid + addCoin(unknownOutpoint); + + // Promoting an orphan will not bypass the cooldown + pm.updatedBlockTip(); + + BOOST_CHECK(pm.isBoundToPeer(proof_seq10->getId())); + BOOST_CHECK(!pm.exists(proof_seq30->getId())); + + // Wait for the cooldown, now proof_seq30 can be registered + mockTime += cooldown; + SetMockTime(mockTime); + + BOOST_CHECK(!pm.registerProof(proof_seq30)); + BOOST_CHECK(pm.isBoundToPeer(proof_seq10->getId())); + BOOST_CHECK(pm.isInConflictingPool(proof_seq30->getId())); + } + + gArgs.ClearForcedArg("-avalancheconflictingproofcooldown"); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/init.cpp b/src/init.cpp --- a/src/init.cpp +++ b/src/init.cpp @@ -1270,6 +1270,12 @@ strprintf("Enable avalanche peer discovery (default: %u)", AVALANCHE_DEFAULT_PEER_DISCOVERY_ENABLED), ArgsManager::ALLOW_ANY, OptionsCategory::AVALANCHE); + argsman.AddArg("-avalancheconflictingproofcooldown", + strprintf("Mandatory cooldown before a proof conflicting " + "with an already registered one can be considered " + "in seconds (default: %u)", + AVALANCHE_DEFAULT_CONFLICTING_PROOF_COOLDOWN), + ArgsManager::ALLOW_INT, OptionsCategory::AVALANCHE); argsman.AddArg( "-avacooldown", strprintf("Mandatory cooldown between two avapoll (default: %u)",