diff --git a/src/avalanche/peermanager.h b/src/avalanche/peermanager.h --- a/src/avalanche/peermanager.h +++ b/src/avalanche/peermanager.h @@ -231,6 +231,12 @@ return it != pview.end() && func(*it); } + template + bool forPeer(const PeerId &peerid, Callable &&func) const { + auto it = peers.find(peerid); + return it != peers.end() && func(*it); + } + template void forEachPeer(Callable &&func) const { for (const auto &p : peers) { func(p); diff --git a/src/avalanche/peermanager.cpp b/src/avalanche/peermanager.cpp --- a/src/avalanche/peermanager.cpp +++ b/src/avalanche/peermanager.cpp @@ -251,6 +251,29 @@ return proof; } +static bool preferConflictingProof(const std::shared_ptr &conflicting, + const std::shared_ptr ¤t) { + // If the proof master is the same, assume the sequence number is the + // righteous discriminant; otherwise, use costly parameters. + // This is so to prevent a user participating in an aggregated proof with + // other users from being able to invalidate the proof for free and make the + // aggregation mechanism inefficient. + // TODO this only makes sense if the staked coins are locked. + if (conflicting->getMaster() == current->getMaster()) { + return conflicting->getSequence() > current->getSequence(); + } + + if (conflicting->getScore() != current->getScore()) { + return conflicting->getScore() > current->getScore(); + } + + if (conflicting->getStakes().size() != current->getStakes().size()) { + return conflicting->getStakes().size() < current->getStakes().size(); + } + + return conflicting->getId() < current->getId(); +} + PeerManager::PeerSet::iterator PeerManager::fetchOrCreatePeer(const std::shared_ptr &proof) { assert(proof); @@ -288,11 +311,17 @@ // Attach UTXOs to this proof. std::unordered_set conflicting_peerids; + bool accepted = true; for (const auto &s : proof->getStakes()) { auto p = utxos.emplace(s.getStake().getUTXO(), peerid); if (!p.second) { // We have a collision with an existing proof. conflicting_peerids.insert(p.first->second); + + accepted = + accepted && forPeer(p.first->second, [&](const Peer &peer) { + return preferConflictingProof(proof, peer.proof); + }); } } @@ -309,7 +338,7 @@ } for (auto &cb : conflictingProofHandlers) { - cb->onConflictingProof(proof, false); + cb->onConflictingProof(proof, accepted); } return 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 @@ -36,10 +36,12 @@ : public PeerManager::ConflictingProofHandler { public: std::shared_ptr lastProof; + bool lastVote; void onConflictingProof(const std::shared_ptr &proof, bool accepted) override { lastProof = proof; + lastVote = accepted; } }; @@ -849,4 +851,136 @@ } } +BOOST_AUTO_TEST_CASE(conflicting_proof_same_pubkey) { + avalanche::PeerManager pm; + + auto conflictingProofHandler = + std::make_shared(); + auto handler = pm.handleConflictingProof(conflictingProofHandler); + + const CKey key = CKey::MakeCompressedKey(); + + const COutPoint outpoint(TxId(GetRandHash()), 0); + const Amount amount(10 * COIN); + const uint32_t height = 100; + const bool is_coinbase = false; + + CScript script = GetScriptForDestination(PKHash(key.GetPubKey())); + + { + LOCK(cs_main); + CCoinsViewCache &coins = ::ChainstateActive().CoinsTip(); + coins.AddCoin(outpoint, + Coin(CTxOut(amount, script), height, is_coinbase), false); + } + + auto buildProofWithSequence = [&](uint64_t sequence) { + ProofBuilder pb(sequence, GetRandInt(std::numeric_limits::max()), + key); + BOOST_CHECK(pb.addUTXO(outpoint, amount, height, is_coinbase, key)); + return std::make_shared(pb.build()); + }; + + auto proof_base = buildProofWithSequence(10); + BOOST_CHECK_NE(pm.getPeerId(proof_base), NO_PEER); + BOOST_CHECK(!conflictingProofHandler->lastProof); + + auto proof_lowSequence = buildProofWithSequence(9); + BOOST_CHECK_EQUAL(pm.getPeerId(proof_lowSequence), NO_PEER); + BOOST_CHECK_EQUAL(conflictingProofHandler->lastProof->getId(), + proof_lowSequence->getId()); + BOOST_CHECK_EQUAL(conflictingProofHandler->lastVote, false); + + auto proof_sameSequence = buildProofWithSequence(10); + // Collision might occur here, but it's so unlikely that it's not a concern + BOOST_CHECK_EQUAL(pm.getPeerId(proof_sameSequence), NO_PEER); + BOOST_CHECK_EQUAL(conflictingProofHandler->lastProof->getId(), + proof_sameSequence->getId()); + BOOST_CHECK_EQUAL(conflictingProofHandler->lastVote, false); + + auto proof_highSequence = buildProofWithSequence(11); + BOOST_CHECK_EQUAL(pm.getPeerId(proof_highSequence), NO_PEER); + BOOST_CHECK_EQUAL(conflictingProofHandler->lastProof->getId(), + proof_highSequence->getId()); + BOOST_CHECK_EQUAL(conflictingProofHandler->lastVote, true); +} + +BOOST_AUTO_TEST_CASE(conflicting_proof_different_pubkey) { + avalanche::PeerManager pm; + + auto conflictingProofHandler = + std::make_shared(); + auto handler = pm.handleConflictingProof(conflictingProofHandler); + + const CKey key = CKey::MakeCompressedKey(); + + const uint32_t height = 100; + const bool is_coinbase = false; + + CScript script = GetScriptForDestination(PKHash(key.GetPubKey())); + + auto addCoin = [&](const Amount &amount) { + 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; + }; + + const Amount conflictingOutpointAmount = 10 * COIN; + // This is the outpoint that will conflict through the test + COutPoint conflictingOutpoint = addCoin(conflictingOutpointAmount); + + auto buildProofFromAmounts = [&](std::vector amounts) { + CKey master = CKey::MakeCompressedKey(); + ProofBuilder pb(0, 0, std::move(master)); + BOOST_CHECK(pb.addUTXO(conflictingOutpoint, conflictingOutpointAmount, + height, is_coinbase, key)); + for (Amount &amount : amounts) { + auto outpoint = addCoin(amount); + BOOST_CHECK(pb.addUTXO(std::move(outpoint), amount, height, + is_coinbase, key)); + } + return std::make_shared(pb.build()); + }; + + auto proof_base = buildProofFromAmounts({10 * COIN, 10 * COIN}); + BOOST_CHECK_NE(pm.getPeerId(proof_base), NO_PEER); + BOOST_CHECK(!conflictingProofHandler->lastProof); + + auto proof_lowAmount = buildProofFromAmounts({10 * COIN, 9 * COIN}); + BOOST_CHECK_EQUAL(pm.getPeerId(proof_lowAmount), NO_PEER); + BOOST_CHECK_EQUAL(conflictingProofHandler->lastProof->getId(), + proof_lowAmount->getId()); + BOOST_CHECK_EQUAL(conflictingProofHandler->lastVote, false); + + auto proof_highAmount = buildProofFromAmounts({10 * COIN, 11 * COIN}); + BOOST_CHECK_EQUAL(pm.getPeerId(proof_highAmount), NO_PEER); + BOOST_CHECK_EQUAL(conflictingProofHandler->lastProof->getId(), + proof_highAmount->getId()); + BOOST_CHECK_EQUAL(conflictingProofHandler->lastVote, true); + + auto proof_lowStakeCount = buildProofFromAmounts({20 * COIN}); + BOOST_CHECK_EQUAL(pm.getPeerId(proof_lowStakeCount), NO_PEER); + BOOST_CHECK_EQUAL(conflictingProofHandler->lastProof->getId(), + proof_lowStakeCount->getId()); + BOOST_CHECK_EQUAL(conflictingProofHandler->lastVote, true); + + auto proof_highStakeCount = + buildProofFromAmounts({10 * COIN, 5 * COIN, 5 * COIN}); + BOOST_CHECK_EQUAL(pm.getPeerId(proof_highStakeCount), NO_PEER); + BOOST_CHECK_EQUAL(conflictingProofHandler->lastProof->getId(), + proof_highStakeCount->getId()); + BOOST_CHECK_EQUAL(conflictingProofHandler->lastVote, false); + + auto proof_similar = buildProofFromAmounts({10 * COIN, 10 * COIN}); + BOOST_CHECK_EQUAL(pm.getPeerId(proof_similar), NO_PEER); + BOOST_CHECK_EQUAL(conflictingProofHandler->lastProof->getId(), + proof_similar->getId()); + BOOST_CHECK_EQUAL(conflictingProofHandler->lastVote, + proof_similar->getId() < proof_base->getId()); +} + BOOST_AUTO_TEST_SUITE_END()