From a42f96c4d14fdcb6c389b0f4dc66827f270186b6 Mon Sep 17 00:00:00 2001 From: Dimitrios Siganos Date: Wed, 31 Aug 2022 17:15:48 +0100 Subject: [PATCH] Fix for test election.quorum_minimum_update_weight_before_quorum_checks (#3932) * Fix for test election.quorum_minimum_update_weight_before_quorum_checks The problem was that there was a race condition between the rep crawler background thread and rep_crawler::response(). For the response to be processed as valid, a request must be marked in rep_crawler::active container. I added force flag in rep_crawler::response to insert the vote regardless to be used in testing. --- nano/core_test/election.cpp | 3 ++- nano/node/repcrawler.cpp | 4 ++-- nano/node/repcrawler.hpp | 5 +++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/nano/core_test/election.cpp b/nano/core_test/election.cpp index 49c8f572..0c221f49 100644 --- a/nano/core_test/election.cpp +++ b/nano/core_test/election.cpp @@ -188,6 +188,7 @@ TEST (election, quorum_minimum_confirm_fail) namespace nano { +// FIXME: this test fails on rare occasions. It needs a review. TEST (election, quorum_minimum_update_weight_before_quorum_checks) { nano::test::system system{}; @@ -243,7 +244,7 @@ TEST (election, quorum_minimum_update_weight_before_quorum_checks) ASSERT_NE (channel, nullptr); auto const vote2 = std::make_shared (key1.pub, key1.prv, nano::vote::timestamp_max, nano::vote::duration_max, std::vector{ send1->hash () }); - ASSERT_TIMELY (10s, !node1.rep_crawler.response (channel, vote2)); + ASSERT_FALSE (node1.rep_crawler.response (channel, vote2, true)); ASSERT_FALSE (election->confirmed ()); { diff --git a/nano/node/repcrawler.cpp b/nano/node/repcrawler.cpp index 8c7e8079..daf7754b 100644 --- a/nano/node/repcrawler.cpp +++ b/nano/node/repcrawler.cpp @@ -230,13 +230,13 @@ bool nano::rep_crawler::is_pr (nano::transport::channel const & channel_a) const return result; } -bool nano::rep_crawler::response (std::shared_ptr const & channel_a, std::shared_ptr const & vote_a) +bool nano::rep_crawler::response (std::shared_ptr const & channel_a, std::shared_ptr const & vote_a, bool force) { bool error = true; nano::lock_guard lock (active_mutex); for (auto i = vote_a->hashes.begin (), n = vote_a->hashes.end (); i != n; ++i) { - if (active.count (*i) != 0) + if (force || active.count (*i) != 0) { responses.emplace_back (channel_a, vote_a); error = false; diff --git a/nano/node/repcrawler.hpp b/nano/node/repcrawler.hpp index d6ed45f1..7186c944 100644 --- a/nano/node/repcrawler.hpp +++ b/nano/node/repcrawler.hpp @@ -98,9 +98,10 @@ public: /** * Called when a non-replay vote on a block previously sent by query() is received. This indicates * with high probability that the endpoint is a representative node. - * @return false if the vote corresponded to any active hash. + * The force flag can be set to skip the active check in unit testing when we want to force a vote in the rep crawler. + * @return false if any vote passed the checks and was added to the response queue of the rep crawler */ - bool response (std::shared_ptr const &, std::shared_ptr const &); + bool response (std::shared_ptr const &, std::shared_ptr const &, bool force = false); /** Get total available weight from representatives */ nano::uint128_t total_weight () const;