diff --git a/src/avalanche/peermanager.h b/src/avalanche/peermanager.h --- a/src/avalanche/peermanager.h +++ b/src/avalanche/peermanager.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -104,6 +105,18 @@ struct by_proofid; struct by_nodeid; +enum class ProofRegistrationResult { + NONE = 0, + ALREADY_REGISTERED, + ORPHAN, + INVALID, + CONFLICTING, + REJECTED, +}; + +class ProofRegistrationState : public ValidationState { +}; + namespace bmi = boost::multi_index; class PeerManager { @@ -218,7 +231,14 @@ }; bool registerProof(const ProofRef &proof, + ProofRegistrationState ®istrationState, RegistrationMode mode = RegistrationMode::DEFAULT); + bool registerProof(const ProofRef &proof, + RegistrationMode mode = RegistrationMode::DEFAULT) { + ProofRegistrationState dummy; + return registerProof(proof, dummy, mode); + } + bool exists(const ProofId &proofid) const { return getProof(proofid) != nullptr; } diff --git a/src/avalanche/peermanager.cpp b/src/avalanche/peermanager.cpp --- a/src/avalanche/peermanager.cpp +++ b/src/avalanche/peermanager.cpp @@ -164,33 +164,44 @@ return it->nextPossibleConflictTime == nextTime; } -bool PeerManager::registerProof(const ProofRef &proof, RegistrationMode mode) { +bool PeerManager::registerProof(const ProofRef &proof, + ProofRegistrationState ®istrationState, + RegistrationMode mode) { assert(proof); const ProofId &proofid = proof->getId(); + auto invalidate = [&](ProofRegistrationResult result, + const std::string &message) { + return registrationState.Invalid( + result, message, strprintf("proofid: %s", proofid.ToString())); + }; + if ((mode != RegistrationMode::FORCE_ACCEPT || !isInConflictingPool(proofid)) && exists(proofid)) { // In default mode, we expect the proof to be unknown, i.e. in none of // the pools. // In forced accept mode, the proof can be in the conflicting pool. - return false; + return invalidate(ProofRegistrationResult::ALREADY_REGISTERED, + "proof-already-registered"); } // Check the proof's validity. - ProofValidationState state; + ProofValidationState validationState; // Using WITH_LOCK directly inside the if statement will trigger a cppcheck // false positive syntax error const bool valid = WITH_LOCK( - cs_main, return proof->verify(state, ::ChainstateActive().CoinsTip())); + cs_main, + return proof->verify(validationState, ::ChainstateActive().CoinsTip())); if (!valid) { - if (isOrphanState(state)) { + if (isOrphanState(validationState)) { orphanProofPool.addProofIfPreferred(proof); + return invalidate(ProofRegistrationResult::ORPHAN, "orphan-proof"); } // Reject invalid proof. - return false; + return invalidate(ProofRegistrationResult::INVALID, "invalid-proof"); } ProofPool::ConflictingProofSet conflictingProofs; @@ -200,8 +211,12 @@ // 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; + return conflictingProofPool.addProofIfPreferred(proof) == + ProofPool::AddProofStatus::REJECTED + ? invalidate(ProofRegistrationResult::REJECTED, + "rejected-proof") + : invalidate(ProofRegistrationResult::CONFLICTING, + "conflicting-utxos"); } conflictingProofPool.removeProof(proofid); @@ -225,7 +240,8 @@ } case ProofPool::AddProofStatus::DUPLICATED: // If the proof was already in the pool, don't duplicate the peer. - return false; + return invalidate(ProofRegistrationResult::ALREADY_REGISTERED, + "proof-already-registered"); case ProofPool::AddProofStatus::SUCCEED: break; 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 @@ -642,8 +642,15 @@ // Add the proofs BOOST_CHECK(pm.registerProof(proof1)); - BOOST_CHECK(!pm.registerProof(proof2)); - BOOST_CHECK(!pm.registerProof(proof3)); + + auto registerOrphan = [&](const ProofRef &proof) { + ProofRegistrationState state; + BOOST_CHECK(!pm.registerProof(proof, state)); + BOOST_CHECK(state.GetResult() == ProofRegistrationResult::ORPHAN); + }; + + registerOrphan(proof2); + registerOrphan(proof3); auto checkOrphan = [&](const ProofRef &proof, bool expectedOrphan) { const ProofId &proofid = proof->getId(); @@ -771,8 +778,14 @@ for (int i = 0; i < numProofs; i++) { BOOST_CHECK(pm.registerProof(proofs[i])); - // Fail to add an existing proof - BOOST_CHECK(!pm.registerProof(proofs[i])); + + { + ProofRegistrationState state; + // Fail to add an existing proof + BOOST_CHECK(!pm.registerProof(proofs[i], state)); + BOOST_CHECK(state.GetResult() == + ProofRegistrationResult::ALREADY_REGISTERED); + } for (int added = 0; added <= i; added++) { auto proof = pm.getProof(proofs[added]->getId()); @@ -790,8 +803,11 @@ bilingual_str error; Proof badProof; BOOST_CHECK(Proof::FromHex(badProof, badProofHex, error)); + + ProofRegistrationState state; BOOST_CHECK( - !pm.registerProof(std::make_shared(std::move(badProof)))); + !pm.registerProof(std::make_shared(std::move(badProof)), state)); + BOOST_CHECK(state.GetResult() == ProofRegistrationResult::INVALID); } BOOST_AUTO_TEST_CASE(conflicting_proof_rescan) { @@ -839,7 +855,9 @@ conflictingProof = pb.build(); } - BOOST_CHECK(!pm.registerProof(conflictingProof)); + ProofRegistrationState state; + BOOST_CHECK(!pm.registerProof(conflictingProof, state)); + BOOST_CHECK(state.GetResult() == ProofRegistrationResult::CONFLICTING); BOOST_CHECK(pm.isInConflictingPool(conflictingProof->getId())); { @@ -1178,4 +1196,52 @@ } } +BOOST_AUTO_TEST_CASE(evicted_proof) { + avalanche::PeerManager pm; + + 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())); + + const COutPoint conflictingOutpoint(TxId(GetRandHash()), 0); + { + LOCK(cs_main); + CCoinsViewCache &coins = ::ChainstateActive().CoinsTip(); + coins.AddCoin(conflictingOutpoint, + Coin(CTxOut(amount, script), height, is_coinbase), false); + } + + auto buildProofWithSequence = [&](uint64_t sequence) { + ProofBuilder pb(sequence, 0, key); + BOOST_CHECK( + pb.addUTXO(conflictingOutpoint, amount, height, is_coinbase, key)); + return pb.build(); + }; + + auto proofSeq10 = buildProofWithSequence(10); + auto proofSeq20 = buildProofWithSequence(20); + auto proofSeq30 = buildProofWithSequence(30); + + { + ProofRegistrationState state; + BOOST_CHECK(pm.registerProof(proofSeq30, state)); + BOOST_CHECK(state.IsValid()); + } + + { + ProofRegistrationState state; + BOOST_CHECK(!pm.registerProof(proofSeq20, state)); + BOOST_CHECK(state.GetResult() == ProofRegistrationResult::CONFLICTING); + } + + { + ProofRegistrationState state; + BOOST_CHECK(!pm.registerProof(proofSeq10, state)); + BOOST_CHECK(state.GetResult() == ProofRegistrationResult::REJECTED); + } +} + BOOST_AUTO_TEST_SUITE_END()