diff --git a/src/avalanche/peermanager.h b/src/avalanche/peermanager.h --- a/src/avalanche/peermanager.h +++ b/src/avalanche/peermanager.h @@ -200,6 +200,9 @@ return getProof(proofid) != nullptr; } + bool acceptProof(const ProofRef &proof); + bool rejectProof(const ProofRef &proof); + template bool forPeer(const ProofId &proofid, Callable &&func) const { auto &pview = peers.get(); diff --git a/src/avalanche/peermanager.cpp b/src/avalanche/peermanager.cpp --- a/src/avalanche/peermanager.cpp +++ b/src/avalanche/peermanager.cpp @@ -172,6 +172,73 @@ return createPeer(proof); } +bool PeerManager::acceptProof(const ProofRef &proof) { + const ProofId &proofid = proof->getId(); + + if (isBoundToPeer(proofid)) { + // Nothing to do + return true; + } + + if (!isInConflictingPool(proofid)) { + return false; + } + + // Remove our proof from the conflicting pool + conflictingProofPool.removeProof(proof); + + // Move the conflicting proofs from the valid pool to the conflicting pool + for (const SignedStake &ss : proof->getStakes()) { + auto conflictingProof = + validProofPool.getProof(ss.getStake().getUTXO()); + if (!conflictingProof) { + // No conflict on this UTXO + continue; + } + + moveToConflictingPool(conflictingProof); + } + + // Register our proof again + return registerProof(proof); +} + +bool PeerManager::rejectProof(const ProofRef &proof) { + const ProofId &proofid = proof->getId(); + + if (isInConflictingPool(proofid)) { + // Nothing to do; + return true; + } + + if (!isBoundToPeer(proofid)) { + return false; + } + + // Remove our proof from the peer set + auto &pview = peers.get(); + auto it = pview.find(proof->getId()); + assert(it != pview.end()); + removePeer(it->peerid); + + // Attempt to register the conflicting proofs again + for (const SignedStake &ss : proof->getStakes()) { + const ProofRef conflictingProof = + conflictingProofPool.getProof(ss.getStake().getUTXO()); + if (!conflictingProof) { + // No conflict on this UTXO + continue; + } + + conflictingProofPool.removeProof(conflictingProof); + registerProof(conflictingProof); + } + + // Move our proof to the conflicting pool + return conflictingProofPool.addProofIfPreferred(proof) == + ProofPool::AddProofStatus::SUCCEED; +} + NodeId PeerManager::selectNode() { for (int retry = 0; retry < SELECT_NODE_MAX_RETRY; retry++) { const PeerId p = selectPeer(); 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 @@ -1151,4 +1151,166 @@ gArgs.ClearForcedArg("-avalancheconflictingproofcooldown"); } +BOOST_FIXTURE_TEST_CASE(accept_reject_conflicting_proof, + NoConflictingCooldownFixture) { + avalanche::PeerManager pm; + + const CKey key = CKey::MakeCompressedKey(); + + uint64_t sequence = 42; + const int64_t expiration = 0; + const Amount amount = 10 * COIN; + const uint32_t height = 100; + const bool is_coinbase = false; + + CScript script = GetScriptForDestination(PKHash(key.GetPubKey())); + + auto addCoin = [&]() { + LOCK(cs_main); + COutPoint outpoint(TxId(GetRandHash()), 0); + CCoinsViewCache &coins = ::ChainstateActive().CoinsTip(); + coins.AddCoin(outpoint, + Coin(CTxOut(amount, script), height, is_coinbase), false); + + return outpoint; + }; + + gArgs.ForceSetArg("-enableavalancheproofreplacement", "1"); + + // Create a bunch of proofs with a single utxo, and remember these utxos so + // we can create conflicting proofs + std::vector conflictingOutpoints; + std::set conflictingProofs; + for (size_t i = 0; i < 10; i++) { + COutPoint outpoint = addCoin(); + conflictingOutpoints.push_back(outpoint); + + ProofBuilder pb(sequence, expiration, key); + BOOST_CHECK( + pb.addUTXO(std::move(outpoint), amount, height, is_coinbase, key)); + auto proof = pb.build(); + + BOOST_CHECK(pm.registerProof(proof)); + + conflictingProofs.insert(std::move(proof)); + }; + + // Add a few more proofs for good measure + for (size_t i = 0; i < 5; i++) { + ProofBuilder pb(0, 0, key); + BOOST_CHECK(pb.addUTXO(addCoin(), amount, height, is_coinbase, key)); + BOOST_CHECK(pm.registerProof(pb.build())); + }; + + auto buildConflictingProofWithSequence = [&](uint64_t proofSequence) { + // Create a proof that conflicts with all the conflicting outpoints + ProofBuilder pb(proofSequence, expiration, key); + for (const COutPoint &outpoint : conflictingOutpoints) { + BOOST_CHECK(pb.addUTXO(outpoint, amount, height, is_coinbase, key)); + } + + // Add a few other utxos for good measure + for (size_t i = 0; i < 5; i++) { + BOOST_CHECK( + pb.addUTXO(addCoin(), amount, height, is_coinbase, key)); + }; + + return pb.build(); + }; + + auto lowSeqProof = buildConflictingProofWithSequence(sequence - 1); + // It has conflicts and is not the best candidate + BOOST_CHECK(!pm.registerProof(lowSeqProof)); + BOOST_CHECK(pm.isInConflictingPool(lowSeqProof->getId())); + + auto checkAcceptance = [&](const ProofRef &proof) { + BOOST_CHECK(pm.acceptProof(proof)); + BOOST_CHECK(pm.isBoundToPeer(proof->getId())); + + for (const ProofRef &p : conflictingProofs) { + BOOST_CHECK(pm.isInConflictingPool(p->getId())); + } + }; + + auto checkRejection = [&](const ProofRef &proof) { + BOOST_CHECK(pm.rejectProof(proof)); + BOOST_CHECK(pm.isInConflictingPool(proof->getId())); + + for (const ProofRef &p : conflictingProofs) { + BOOST_CHECK(pm.isBoundToPeer(p->getId())); + } + }; + + auto checkRescan = [&](const ProofRef &proof, bool expectValid) { + pm.updatedBlockTip(); + + const ProofId &proofid = proof->getId(); + BOOST_CHECK_EQUAL(pm.isBoundToPeer(proofid), expectValid); + BOOST_CHECK_EQUAL(pm.isInConflictingPool(proofid), !expectValid); + + for (const ProofRef &p : conflictingProofs) { + BOOST_CHECK_EQUAL(pm.isBoundToPeer(p->getId()), !expectValid); + BOOST_CHECK_EQUAL(pm.isInConflictingPool(p->getId()), expectValid); + } + }; + + // Accept the proof + checkAcceptance(lowSeqProof); + + // Accepting a few more times has no effect + for (size_t i = 0; i < 10; i++) { + checkAcceptance(lowSeqProof); + } + + // Make sure that a rescan will not change the state + checkRescan(lowSeqProof, true); + + // Reject the proof + checkRejection(lowSeqProof); + + // Rejecting a few more times has no effect + for (size_t i = 0; i < 10; i++) { + checkRejection(lowSeqProof); + } + + checkRescan(lowSeqProof, false); + + // Finally accept the proof + checkAcceptance(lowSeqProof); + + auto highSeqProof = buildConflictingProofWithSequence(sequence + 1); + + BOOST_CHECK(pm.registerProof(highSeqProof)); + BOOST_CHECK(pm.isBoundToPeer(highSeqProof->getId())); + + // The lowSeqProof is now evicted, because the conflicting proofs are better + // candidates. + BOOST_CHECK(!pm.exists(lowSeqProof->getId())); + + // Reject the proof + checkRejection(highSeqProof); + + // Rejecting a few more times has no effect + for (size_t i = 0; i < 10; i++) { + checkRejection(highSeqProof); + } + + checkRescan(highSeqProof, false); + + // Accept the proof + checkAcceptance(highSeqProof); + + // Accepting a few more times has no effect + for (size_t i = 0; i < 10; i++) { + checkAcceptance(highSeqProof); + } + + checkRescan(highSeqProof, true); + + // Sanity check that nothing is broken internally + BOOST_CHECK(pm.verify()); + + gArgs.ClearForcedArg("-enableavalancheproofreplacement"); +} + BOOST_AUTO_TEST_SUITE_END()