diff --git a/src/avalanche/peermanager.h b/src/avalanche/peermanager.h --- a/src/avalanche/peermanager.h +++ b/src/avalanche/peermanager.h @@ -11,7 +11,6 @@ #include #include #include -#include #include #include @@ -73,12 +72,8 @@ ProofRef proof; - // The network stack uses timestamp in seconds, so we oblige. - std::chrono::seconds registration_time; - Peer(PeerId peerid_, ProofRef proof_) - : peerid(peerid_), proof(std::move(proof_)), - registration_time(GetTime()) {} + : peerid(peerid_), proof(std::move(proof_)) {} const ProofId &getProofId() const { return proof->getId(); } uint32_t getScore() const { return proof->getScore(); } @@ -198,6 +193,9 @@ bool exists(const ProofId &proofid) const { return getProof(proofid) != nullptr; } + std::chrono::seconds getRegistrationTime(const ProofId &proofid) const { + return validProofPool.getRegistrationTime(proofid); + } template bool forPeer(const ProofId &proofid, Callable &&func) const { diff --git a/src/avalanche/proofpool.h b/src/avalanche/proofpool.h --- a/src/avalanche/proofpool.h +++ b/src/avalanche/proofpool.h @@ -15,6 +15,7 @@ #include #include +#include #include namespace avalanche { @@ -64,6 +65,9 @@ SaltedProofIdHasher>>> pool; + std::unordered_map + registrationTime; + public: enum AddProofStatus { REJECTED = 0, //!< Rejected due to conflicts @@ -102,6 +106,8 @@ ProofRef getProof(const ProofId &proofid) const; ProofRef getProof(const COutPoint &outpoint) const; + std::chrono::seconds getRegistrationTime(const ProofId &proofid) const; + size_t size() const { return pool.size(); } }; diff --git a/src/avalanche/proofpool.cpp b/src/avalanche/proofpool.cpp --- a/src/avalanche/proofpool.cpp +++ b/src/avalanche/proofpool.cpp @@ -6,6 +6,7 @@ #include #include +#include namespace avalanche { @@ -46,6 +47,8 @@ return AddProofStatus::REJECTED; } + registrationTime.emplace(proofid, GetTime()); + return AddProofStatus::SUCCEED; } @@ -74,8 +77,12 @@ // pointer, the copy is cheap enough and should not have any significant impact // on performance. bool ProofPool::removeProof(ProofRef proof) { + const ProofId &proofid = proof->getId(); + + registrationTime.erase(proofid); + auto &poolView = pool.get(); - return poolView.erase(proof->getId()); + return poolView.erase(proofid); } void ProofPool::rescan(PeerManager &peerManager) { @@ -84,6 +91,13 @@ for (auto &entry : previousPool) { peerManager.registerProof(entry.proof); + + // If the registration caused the proof to be removed from the pool, we + // need to delete its registration time. + const ProofId &proofid = entry.proof->getId(); + if (!getProof(proofid)) { + registrationTime.erase(proofid); + } } } @@ -98,4 +112,11 @@ return it == pool.end() ? nullptr : it->proof; } +std::chrono::seconds +ProofPool::getRegistrationTime(const ProofId &proofid) const { + auto it = registrationTime.find(proofid); + return it == registrationTime.end() ? std::chrono::seconds::max() + : it->second; +} + } // namespace avalanche diff --git a/src/avalanche/test/proofpool_tests.cpp b/src/avalanche/test/proofpool_tests.cpp --- a/src/avalanche/test/proofpool_tests.cpp +++ b/src/avalanche/test/proofpool_tests.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -278,4 +279,99 @@ } } +BOOST_AUTO_TEST_CASE(get_registration_time) { + ProofPool testPool; + + // If not registered, the time is set to an unrealistic value + for (size_t i = 0; i < 10; i++) { + // FIXME Use BOOST_CHECK_EQUAL after C++20, which implements operator<< + // for std::chrono::duration. + BOOST_CHECK(testPool.getRegistrationTime(ProofId(GetRandHash())) == + std::chrono::seconds::max()); + } + + SetMockTime(100); + std::vector proofs100; + for (size_t i = 0; i < 10; i++) { + auto proof = buildRandomProof(MIN_VALID_PROOF_SCORE); + BOOST_CHECK_EQUAL(testPool.addProofIfNoConflict(proof), + ProofPool::AddProofStatus::SUCCEED); + proofs100.push_back(std::move(proof)); + } + + SetMockTime(200); + std::vector proofs200; + for (size_t i = 0; i < 10; i++) { + auto proof = buildRandomProof(MIN_VALID_PROOF_SCORE); + BOOST_CHECK_EQUAL(testPool.addProofIfNoConflict(proof), + ProofPool::AddProofStatus::SUCCEED); + proofs200.push_back(std::move(proof)); + } + + SetMockTime(300); + + for (const auto &proof : proofs100) { + BOOST_CHECK(testPool.getRegistrationTime(proof->getId()) == + std::chrono::seconds{100}); + + // Attempting to register again (duplicated) does not update the time + BOOST_CHECK_EQUAL(testPool.addProofIfNoConflict(proof), + ProofPool::AddProofStatus::DUPLICATED); + BOOST_CHECK(testPool.getRegistrationTime(proof->getId()) == + std::chrono::seconds{100}); + } + for (const auto &proof : proofs200) { + BOOST_CHECK(testPool.getRegistrationTime(proof->getId()) == + std::chrono::seconds{200}); + + // Attempting to register again (duplicated) does not update the time + BOOST_CHECK_EQUAL(testPool.addProofIfNoConflict(proof), + ProofPool::AddProofStatus::DUPLICATED); + BOOST_CHECK(testPool.getRegistrationTime(proof->getId()) == + std::chrono::seconds{200}); + } + + const CKey key = CKey::MakeCompressedKey(); + const COutPoint conflictingOutpoint{TxId(GetRandHash()), 0}; + + auto buildProofWithSequence = [&](uint64_t sequence) { + ProofBuilder pb(sequence, 0, key); + BOOST_CHECK( + pb.addUTXO(conflictingOutpoint, 10 * COIN, 123456, false, key)); + return pb.build(); + }; + + SetMockTime(400); + auto proof400 = buildProofWithSequence(10); + BOOST_CHECK_EQUAL(testPool.addProofIfNoConflict(proof400), + ProofPool::AddProofStatus::SUCCEED); + BOOST_CHECK(testPool.getRegistrationTime(proof400->getId()) == + std::chrono::seconds{400}); + + // Build a conflicting proof and check the registration time is updated as + // needed + SetMockTime(500); + auto proof500 = buildProofWithSequence(20); + BOOST_CHECK_EQUAL(testPool.addProofIfNoConflict(proof500), + ProofPool::AddProofStatus::REJECTED); + BOOST_CHECK(testPool.getRegistrationTime(proof400->getId()) == + std::chrono::seconds{400}); + BOOST_CHECK(testPool.getRegistrationTime(proof500->getId()) == + std::chrono::seconds::max()); + + BOOST_CHECK_EQUAL(testPool.addProofIfPreferred(proof500), + ProofPool::AddProofStatus::SUCCEED); + BOOST_CHECK(testPool.getRegistrationTime(proof400->getId()) == + std::chrono::seconds::max()); + BOOST_CHECK(testPool.getRegistrationTime(proof500->getId()) == + std::chrono::seconds{500}); + + SetMockTime(600); + avalanche::PeerManager pm; + testPool.rescan(pm); + BOOST_CHECK(!testPool.getProof(proof500->getId())); + BOOST_CHECK(testPool.getRegistrationTime(proof500->getId()) == + std::chrono::seconds::max()); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/net_processing.cpp b/src/net_processing.cpp --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2144,7 +2144,7 @@ // If we know that proof for long enough, allow for requesting // it. - return peer.registration_time <= + return pm.getRegistrationTime(peer.getProofId()) <= now - UNCONDITIONAL_RELAY_DELAY; }); });