diff --git a/src/avalanche/processor.cpp b/src/avalanche/processor.cpp --- a/src/avalanche/processor.cpp +++ b/src/avalanche/processor.cpp @@ -533,18 +533,32 @@ std::vector Processor::getInvsForNextPoll(bool forPoll) { std::vector invs; - auto proofVoteRecordsReadView = proofVoteRecords.getReadView(); - - auto pit = proofVoteRecordsReadView.begin(); - // TODO Factorize the proof and block loops - while (pit != proofVoteRecordsReadView.end() && - invs.size() < AVALANCHE_MAX_ELEMENT_POLL) { - const bool shouldPoll = - forPoll ? pit->second.registerPoll() : pit->second.shouldPoll(); - if (shouldPoll) { - invs.emplace_back(MSG_AVA_PROOF, pit->first->getId()); + auto extractVoteRecordsToInvs = [&](const auto &itemVoteRecordRange, + auto buildInvFromVoteItem) { + for (const auto &[item, voteRecord] : itemVoteRecordRange) { + const bool shouldPoll = + forPoll ? voteRecord.registerPoll() : voteRecord.shouldPoll(); + + if (!shouldPoll) { + continue; + } + + invs.emplace_back(buildInvFromVoteItem(item)); + if (invs.size() >= AVALANCHE_MAX_ELEMENT_POLL) { + // Make sure we do not produce more invs than specified by the + // protocol. + return true; + } } - ++pit; + + return false; + }; + + if (extractVoteRecordsToInvs(proofVoteRecords.getReadView(), + [](const ProofRef &proof) { + return CInv(MSG_AVA_PROOF, proof->getId()); + })) { + return invs; } // First remove all blocks that are not worth polling. @@ -562,23 +576,9 @@ } auto r = blockVoteRecords.getReadView(); - for (const std::pair &p : - reverse_iterate(r)) { - // Check if we can run poll. - const bool shouldPoll = - forPoll ? p.second.registerPoll() : p.second.shouldPoll(); - if (!shouldPoll) { - continue; - } - - // We don't have a decision, we need more votes. - invs.emplace_back(MSG_BLOCK, p.first->GetBlockHash()); - if (invs.size() >= AVALANCHE_MAX_ELEMENT_POLL) { - // Make sure we do not produce more invs than specified by the - // protocol. - return invs; - } - } + extractVoteRecordsToInvs(reverse_iterate(r), [](const CBlockIndex *pindex) { + return CInv(MSG_BLOCK, pindex->GetBlockHash()); + }); return invs; } diff --git a/src/avalanche/test/processor_tests.cpp b/src/avalanche/test/processor_tests.cpp --- a/src/avalanche/test/processor_tests.cpp +++ b/src/avalanche/test/processor_tests.cpp @@ -41,6 +41,16 @@ } static uint64_t getRound(const Processor &p) { return p.round; } + + static void clearProofVoteRecords(Processor &p) { + auto w = p.proofVoteRecords.getWriteView(); + w->erase(w.begin(), w.end()); + } + + static void clearBlockVoteRecords(Processor &p) { + auto w = p.blockVoteRecords.getWriteView(); + w->erase(w.begin(), w.end()); + } }; } // namespace } // namespace avalanche @@ -1178,4 +1188,69 @@ BOOST_CHECK_EQUAL(m_processor->getConfidence(proofB), 0); } +BOOST_AUTO_TEST_CASE(max_element_poll) { + auto invs = AvalancheTest::getInvsForNextPoll(*m_processor); + BOOST_CHECK_EQUAL(invs.size(), 0); + + auto checkMaxElementPoll = [&](auto &provider) { + // Fill the invs up to the limit + for (size_t i = 0; i < AVALANCHE_MAX_ELEMENT_POLL; i++) { + auto item = provider.buildVoteItem(); + BOOST_CHECK(provider.addToReconcile(item)); + invs = AvalancheTest::getInvsForNextPoll(*m_processor); + BOOST_CHECK_EQUAL(invs.size(), i + 1); + } + + // More items will not result in more invs + for (size_t i = 0; i < 10; i++) { + auto item = provider.buildVoteItem(); + BOOST_CHECK(provider.addToReconcile(item)); + invs = AvalancheTest::getInvsForNextPoll(*m_processor); + BOOST_CHECK_EQUAL(invs.size(), AVALANCHE_MAX_ELEMENT_POLL); + } + }; + + // Check the inv limit when filled with proofs + ProofProvider proofProvider(this); + checkMaxElementPoll(proofProvider); + AvalancheTest::clearProofVoteRecords(*m_processor); + invs = AvalancheTest::getInvsForNextPoll(*m_processor); + BOOST_CHECK_EQUAL(invs.size(), 0); + + // Check the inv limit when filled with blocks + BlockProvider blockProvider(this); + checkMaxElementPoll(blockProvider); + AvalancheTest::clearBlockVoteRecords(*m_processor); + invs = AvalancheTest::getInvsForNextPoll(*m_processor); + BOOST_CHECK_EQUAL(invs.size(), 0); + + // Check the inv limit with both proofs and blocks + for (size_t i = 0; i < AVALANCHE_MAX_ELEMENT_POLL / 2; i++) { + auto proof = proofProvider.buildVoteItem(); + BOOST_CHECK(proofProvider.addToReconcile(proof)); + invs = AvalancheTest::getInvsForNextPoll(*m_processor); + BOOST_CHECK_EQUAL(invs.size(), i + 1); + } + for (size_t i = AVALANCHE_MAX_ELEMENT_POLL / 2; + i < AVALANCHE_MAX_ELEMENT_POLL; i++) { + auto blockIndex = blockProvider.buildVoteItem(); + BOOST_CHECK(blockProvider.addToReconcile(blockIndex)); + invs = AvalancheTest::getInvsForNextPoll(*m_processor); + BOOST_CHECK_EQUAL(invs.size(), i + 1); + } + + // Add enough proofs and block indexes so that there is more than + // AVALANCHE_MAX_ELEMENT_POLL of each type. + for (size_t i = 0; i < AVALANCHE_MAX_ELEMENT_POLL; i++) { + auto proof = proofProvider.buildVoteItem(); + BOOST_CHECK(proofProvider.addToReconcile(proof)); + + auto blockIndex = blockProvider.buildVoteItem(); + BOOST_CHECK(blockProvider.addToReconcile(blockIndex)); + + invs = AvalancheTest::getInvsForNextPoll(*m_processor); + BOOST_CHECK_EQUAL(invs.size(), AVALANCHE_MAX_ELEMENT_POLL); + } +} + BOOST_AUTO_TEST_SUITE_END()