diff --git a/src/avalanche/peermanager.h b/src/avalanche/peermanager.h --- a/src/avalanche/peermanager.h +++ b/src/avalanche/peermanager.h @@ -197,6 +197,9 @@ return validProofPool.getRegistrationTime(proofid); } + 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,62 @@ return createPeer(proof); } +bool PeerManager::acceptProof(const ProofRef &proof) { + const ProofId &proofid = proof->getId(); + + if (!exists(proofid)) { + return false; + } + + if (isBoundToPeer(proofid)) { + // Nothing to do + return true; + } + + // Remove the accepted proof from the orphan pool + orphanProofPool.removeProof(proof); + + // Orphan the conflicting proofs + validProofPool.forEachConflictingProof( + proof, [&](const ProofRef &conflictingProof) { + moveToOrphan(conflictingProof); + }); + + // Then register the accepted proof + return registerProof(proof); +} + +bool PeerManager::rejectProof(const ProofRef &proof) { + const ProofId &proofid = proof->getId(); + + if (!exists(proofid)) { + return false; + } + + if (isOrphan(proofid)) { + // Nothing to do + return true; + } + + // Remove the conflicting proofs from the orphan pool + ProofPool::ConflictingProofSet conflictingProofs; + orphanProofPool.forEachConflictingProof( + proof, [&](const ProofRef &conflictingProof) { + conflictingProofs.insert(conflictingProof); + orphanProofPool.removeProof(conflictingProof); + }); + + // Orphan the rejected proof + moveToOrphan(proof); + + // Then register the conflicting proofs + for (const ProofRef &conflictingProof : conflictingProofs) { + registerProof(conflictingProof); + } + + return true; +} + NodeId PeerManager::selectNode() { for (int retry = 0; retry < SELECT_NODE_MAX_RETRY; retry++) { const PeerId p = selectPeer(); diff --git a/src/avalanche/proofpool.h b/src/avalanche/proofpool.h --- a/src/avalanche/proofpool.h +++ b/src/avalanche/proofpool.h @@ -102,6 +102,18 @@ ProofRef getProof(const ProofId &proofid) const; ProofRef getProof(const COutPoint &outpoint) const; + template + void forEachConflictingProof(const ProofRef &proof, Callable fun) { + const ProofId &proofid = proof->getId(); + + for (const SignedStake &ss : proof->getStakes()) { + auto conflictingProof = getProof(ss.getStake().getUTXO()); + if (conflictingProof && conflictingProof->getId() != proofid) { + fun(conflictingProof); + } + } + } + std::chrono::seconds getRegistrationTime(const ProofId &proofid) const; size_t size() const { return pool.size(); } 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 @@ -42,7 +42,46 @@ auto it = pview.find(proof->getId()); return it == pview.end() ? NO_PEER : it->peerid; } + + static void checkConflictingProofs( + PeerManager &pm, const ProofRef &proof, const bool expectAccepted, + const std::set &expectedConflictingProofs) { + pm.registerProof(proof); + + const ProofId &proofid = proof->getId(); + + // If not accepted, it should be orphaned or rejected. + BOOST_CHECK_EQUAL(pm.isBoundToPeer(proofid), expectAccepted); + + // Proof pool consistency check. + // If the proof is accepted all the conflicting proofs should have + // been orphaned. The opposite is not true, as there might very well + // be another proof that took precedence. + if (expectAccepted) { + for (const ProofRef &p : expectedConflictingProofs) { + BOOST_CHECK(!pm.isBoundToPeer(p->getId())); + } + } + + auto pool = expectAccepted ? pm.orphanProofPool : pm.validProofPool; + std::set conflictingProofs; + for (const SignedStake &ss : proof->getStakes()) { + auto conflictingProof = pool.getProof(ss.getStake().getUTXO()); + if (conflictingProof && conflictingProof->getId() != proofid) { + conflictingProofs.insert(conflictingProof); + } + } + + BOOST_CHECK_EQUAL_COLLECTIONS(conflictingProofs.begin(), + conflictingProofs.end(), + expectedConflictingProofs.begin(), + expectedConflictingProofs.end()); + + // Mostly for utxos consistency check + BOOST_CHECK(pm.verify()); + } }; + } // namespace } // namespace avalanche @@ -1093,4 +1132,170 @@ BOOST_CHECK(!pm.exists(orphan10->getId())); } +BOOST_AUTO_TEST_CASE(accept_reject_conflicting_proof) { + 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"); + gArgs.ForceSetArg("-avalancheproofreplacementcooldown", "0"); + + // 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); + TestPeerManager::checkConflictingProofs(pm, lowSeqProof, false, + conflictingProofs); + + auto notRegistered = buildRandomProof(MIN_VALID_PROOF_SCORE); + BOOST_CHECK(!pm.acceptProof(notRegistered)); + BOOST_CHECK(!pm.rejectProof(notRegistered)); + + auto expectValid = [&](const ProofRef &proof, bool expectValid) { + const ProofId &proofid = proof->getId(); + BOOST_CHECK(pm.exists(proofid)); + BOOST_CHECK_EQUAL(pm.isBoundToPeer(proofid), expectValid); + BOOST_CHECK_EQUAL(pm.isOrphan(proofid), !expectValid); + }; + + auto checkAcceptance = [&](const ProofRef &proof) { + BOOST_CHECK(pm.acceptProof(proof)); + expectValid(proof, true); + for (const ProofRef &p : conflictingProofs) { + expectValid(p, false); + } + }; + + auto checkRejection = [&](const ProofRef &proof) { + BOOST_CHECK(pm.rejectProof(proof)); + expectValid(proof, false); + for (const ProofRef &p : conflictingProofs) { + expectValid(p, true); + } + }; + + // Accept the proof + checkAcceptance(lowSeqProof); + + // Accepting a few more times has no effect + for (size_t i = 0; i < 10; i++) { + checkAcceptance(lowSeqProof); + } + + // A new block will cause the conflicting proofs to get pulled back and our + // proof to be orphaned. + pm.updatedBlockTip(); + expectValid(lowSeqProof, false); + for (const ProofRef &p : conflictingProofs) { + expectValid(p, true); + } + + // Accept the proof again + checkAcceptance(lowSeqProof); + + // Reject the proof + checkRejection(lowSeqProof); + // Rejecting a few more times has no effect + for (size_t i = 0; i < 10; i++) { + checkRejection(lowSeqProof); + } + + // Finally accept the proof + checkAcceptance(lowSeqProof); + + auto highSeqProof = buildConflictingProofWithSequence(sequence + 1); + TestPeerManager::checkConflictingProofs(pm, highSeqProof, true, + conflictingProofs); + // The low sequence proof is now evicted + 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); + } + + // A new block will bring our proof back + pm.updatedBlockTip(); + expectValid(highSeqProof, true); + for (const ProofRef &p : conflictingProofs) { + expectValid(p, false); + } + + // Reject the proof again + checkRejection(highSeqProof); + + // Acceot the proof + checkAcceptance(highSeqProof); + + // Accepting a few more times has no effect + for (size_t i = 0; i < 10; i++) { + checkAcceptance(highSeqProof); + } + + // Sanity check that nothing is broken internally + BOOST_CHECK(pm.verify()); + + gArgs.ClearForcedArg("-enableavalancheproofreplacement"); + gArgs.ClearForcedArg("-avalancheproofreplacementcooldown"); +} + BOOST_AUTO_TEST_SUITE_END()