From 13af56e6757b1d431e7641633e8fe027bb94b0dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Wed, 8 Nov 2023 18:47:32 +0100 Subject: [PATCH 1/3] Support for larger `confirm_req` & `confirm_ack` messages --- nano/core_test/message.cpp | 53 +++++++++++++++ nano/node/messages.cpp | 136 ++++++++++++++++++++++++++++++++----- nano/node/messages.hpp | 56 +++++++++++---- nano/node/network.hpp | 1 + nano/secure/vote.cpp | 13 +++- nano/secure/vote.hpp | 15 ++-- 6 files changed, 238 insertions(+), 36 deletions(-) diff --git a/nano/core_test/message.cpp b/nano/core_test/message.cpp index f2760bb434..0142549254 100644 --- a/nano/core_test/message.cpp +++ b/nano/core_test/message.cpp @@ -90,6 +90,59 @@ TEST (message, publish_serialization) ASSERT_EQ (nano::message_type::publish, header.type); } +TEST (message, confirm_header_flags) +{ + nano::message_header header_v2{ nano::dev::network_params.network, nano::message_type::confirm_req }; + header_v2.confirm_set_v2 (true); + + const uint8_t value = 0b0110'1001; + + header_v2.count_v2_set (value); // Max count value + + ASSERT_TRUE (header_v2.confirm_is_v2 ()); + ASSERT_EQ (header_v2.count_v2_get (), value); + + std::vector bytes; + { + nano::vectorstream stream (bytes); + header_v2.serialize (stream); + } + nano::bufferstream stream (bytes.data (), bytes.size ()); + + bool error = false; + nano::message_header header (error, stream); + ASSERT_FALSE (error); + ASSERT_EQ (nano::message_type::confirm_req, header.type); + + ASSERT_TRUE (header.confirm_is_v2 ()); + ASSERT_EQ (header.count_v2_get (), value); +} + +TEST (message, confirm_header_flags_max) +{ + nano::message_header header_v2{ nano::dev::network_params.network, nano::message_type::confirm_req }; + header_v2.confirm_set_v2 (true); + header_v2.count_v2_set (255); // Max count value + + ASSERT_TRUE (header_v2.confirm_is_v2 ()); + ASSERT_EQ (header_v2.count_v2_get (), 255); + + std::vector bytes; + { + nano::vectorstream stream (bytes); + header_v2.serialize (stream); + } + nano::bufferstream stream (bytes.data (), bytes.size ()); + + bool error = false; + nano::message_header header (error, stream); + ASSERT_FALSE (error); + ASSERT_EQ (nano::message_type::confirm_req, header.type); + + ASSERT_TRUE (header.confirm_is_v2 ()); + ASSERT_EQ (header.count_v2_get (), 255); +} + TEST (message, confirm_ack_hash_serialization) { std::vector hashes; diff --git a/nano/node/messages.cpp b/nano/node/messages.cpp index dce563cef9..3b6079b432 100644 --- a/nano/node/messages.cpp +++ b/nano/node/messages.cpp @@ -189,26 +189,71 @@ nano::block_type nano::message_header::block_type () const void nano::message_header::block_type_set (nano::block_type type_a) { extensions &= ~block_type_mask; - extensions |= std::bitset<16> (static_cast (type_a) << 8); + extensions |= (extensions_bitset_t{ static_cast (type_a) } << 8); } uint8_t nano::message_header::count_get () const { + debug_assert (type == nano::message_type::confirm_ack || type == nano::message_type::confirm_req); + debug_assert (!flag_test (confirm_v2_flag)); // Only valid for v1 + return static_cast (((extensions & count_mask) >> 12).to_ullong ()); } void nano::message_header::count_set (uint8_t count_a) { - debug_assert (count_a < 16); + debug_assert (type == nano::message_type::confirm_ack || type == nano::message_type::confirm_req); + debug_assert (!flag_test (confirm_v2_flag)); // Only valid for v1 + debug_assert (count_a < 16); // Max 4 bits + extensions &= ~count_mask; - extensions |= std::bitset<16> (static_cast (count_a) << 12); + extensions |= ((extensions_bitset_t{ count_a } << 12) & count_mask); +} + +/* + * We need those shenanigans because we need to keep compatibility with previous protocol versions (<= V25.1) + */ + +uint8_t nano::message_header::count_v2_get () const +{ + debug_assert (type == nano::message_type::confirm_ack || type == nano::message_type::confirm_req); + debug_assert (flag_test (confirm_v2_flag)); // Only valid for v2 + + // Extract 2 parts of 4 bits + auto left = (extensions & count_v2_mask_left) >> 12; + auto right = (extensions & count_v2_mask_right) >> 4; + + return static_cast (((left << 4) | right).to_ullong ()); +} + +void nano::message_header::count_v2_set (uint8_t count) +{ + debug_assert (type == nano::message_type::confirm_ack || type == nano::message_type::confirm_req); + debug_assert (flag_test (confirm_v2_flag)); // Only valid for v2 + debug_assert (count < 256); // Max 8 bits + + extensions &= ~(count_v2_mask_left | count_v2_mask_right); + + // Split count into 2 parts of 4 bits + extensions_bitset_t trim_mask{ 0xf }; + auto left = (extensions_bitset_t{ count } >> 4) & trim_mask; + auto right = (extensions_bitset_t{ count }) & trim_mask; + + extensions |= (left << 12) | (right << 4); } -void nano::message_header::flag_set (uint8_t flag_a, bool enable) +bool nano::message_header::flag_test (uint8_t flag) const { - // Flags from 8 are block_type & count - debug_assert (flag_a < 8); - extensions.set (flag_a, enable); + // Extension bits at index >= 8 are block type & count + debug_assert (flag < 8); + return extensions.test (flag); +} + +void nano::message_header::flag_set (uint8_t flag, bool enable) +{ + // Extension bits at index >= 8 are block type & count + debug_assert (flag < 8); + extensions.set (flag, enable); } bool nano::message_header::bulk_pull_is_count_present () const @@ -250,6 +295,18 @@ bool nano::message_header::frontier_req_is_only_confirmed_present () const return result; } +bool nano::message_header::confirm_is_v2 () const +{ + debug_assert (type == nano::message_type::confirm_ack || type == nano::message_type::confirm_req); + return flag_test (confirm_v2_flag); +} + +void nano::message_header::confirm_set_v2 (bool value) +{ + debug_assert (type == nano::message_type::confirm_ack || type == nano::message_type::confirm_req); + flag_set (confirm_v2_flag, value); +} + std::size_t nano::message_header::payload_length_bytes () const { switch (type) @@ -282,7 +339,7 @@ std::size_t nano::message_header::payload_length_bytes () const } case nano::message_type::confirm_ack: { - return nano::confirm_ack::size (count_get ()); + return nano::confirm_ack::size (*this); } case nano::message_type::confirm_req: { @@ -520,12 +577,22 @@ nano::confirm_req::confirm_req (nano::network_constants const & constants, std:: roots_hashes (roots_hashes_a) { debug_assert (!roots_hashes.empty ()); - debug_assert (roots_hashes.size () < 16); + debug_assert (roots_hashes.size () < 256); // Set `not_a_block` (1) block type for hashes + roots request // This is needed to keep compatibility with previous protocol versions (<= V25.1) header.block_type_set (nano::block_type::not_a_block); - header.count_set (static_cast (roots_hashes.size ())); + + if (roots_hashes.size () >= 16) + { + // Set v2 flag and use extended count if there are more than 15 hash + root pairs + header.confirm_set_v2 (true); + header.count_v2_set (static_cast (roots_hashes.size ())); + } + else + { + header.count_set (static_cast (roots_hashes.size ())); + } } nano::confirm_req::confirm_req (nano::network_constants const & constants, nano::block_hash const & hash_a, nano::root const & root_a) : @@ -559,7 +626,7 @@ bool nano::confirm_req::deserialize (nano::stream & stream_a) bool result = false; try { - uint8_t const count = header.count_get (); + uint8_t const count = hash_count (header); for (auto i (0); i != count && !result; ++i) { nano::block_hash block_hash (0); @@ -605,9 +672,21 @@ std::string nano::confirm_req::roots_string () const return result; } +uint8_t nano::confirm_req::hash_count (const nano::message_header & header) +{ + if (header.confirm_is_v2 ()) + { + return header.count_v2_get (); + } + else + { + return header.count_get (); + } +} + std::size_t nano::confirm_req::size (nano::message_header const & header) { - auto const count = header.count_get (); + auto const count = hash_count (header); return count * (sizeof (decltype (roots_hashes)::value_type::first) + sizeof (decltype (roots_hashes)::value_type::second)); } @@ -641,9 +720,20 @@ nano::confirm_ack::confirm_ack (nano::network_constants const & constants, std:: message (constants, nano::message_type::confirm_ack), vote (vote_a) { - debug_assert (vote_a->hashes.size () < 16); + debug_assert (vote->hashes.size () < 256); - header.count_set (static_cast (vote_a->hashes.size ())); + header.block_type_set (nano::block_type::not_a_block); + + if (vote->hashes.size () >= 16) + { + // Set v2 flag and use extended count if there are more than 15 hashes + header.confirm_set_v2 (true); + header.count_v2_set (static_cast (vote->hashes.size ())); + } + else + { + header.count_set (static_cast (vote->hashes.size ())); + } } void nano::confirm_ack::serialize (nano::stream & stream_a) const @@ -663,10 +753,22 @@ void nano::confirm_ack::visit (nano::message_visitor & visitor_a) const visitor_a.confirm_ack (*this); } -std::size_t nano::confirm_ack::size (std::size_t count) +uint8_t nano::confirm_ack::hash_count (const nano::message_header & header) { - std::size_t result = sizeof (nano::account) + sizeof (nano::signature) + sizeof (uint64_t) + count * sizeof (nano::block_hash); - return result; + if (header.confirm_is_v2 ()) + { + return header.count_v2_get (); + } + else + { + return header.count_get (); + } +} + +std::size_t nano::confirm_ack::size (const nano::message_header & header) +{ + auto const count = hash_count (header); + return nano::vote::size (count); } std::string nano::confirm_ack::to_string () const diff --git a/nano/node/messages.hpp b/nano/node/messages.hpp index ae4e9b1153..d808f8a7ee 100644 --- a/nano/node/messages.hpp +++ b/nano/node/messages.hpp @@ -60,40 +60,59 @@ class message_visitor; class message_header final { public: + using extensions_bitset_t = std::bitset<16>; + message_header (nano::network_constants const &, nano::message_type); message_header (bool &, nano::stream &); + void serialize (nano::stream &) const; bool deserialize (nano::stream &); - nano::block_type block_type () const; - void block_type_set (nano::block_type); - uint8_t count_get () const; - void count_set (uint8_t); + + std::string to_string () const; + +public: // Payload nano::networks network; uint8_t version_max; uint8_t version_using; uint8_t version_min; - std::string to_string () const; + nano::message_type type; + extensions_bitset_t extensions; public: - nano::message_type type; - std::bitset<16> extensions; static std::size_t constexpr size = sizeof (nano::networks) + sizeof (version_max) + sizeof (version_using) + sizeof (version_min) + sizeof (type) + sizeof (/* extensions */ uint16_t); - void flag_set (uint8_t, bool enable = true); + bool flag_test (uint8_t flag) const; + void flag_set (uint8_t flag, bool enable = true); + + nano::block_type block_type () const; + void block_type_set (nano::block_type); + + uint8_t count_get () const; + void count_set (uint8_t); + uint8_t count_v2_get () const; + void count_v2_set (uint8_t); + static uint8_t constexpr bulk_pull_count_present_flag = 0; static uint8_t constexpr bulk_pull_ascending_flag = 1; bool bulk_pull_is_count_present () const; bool bulk_pull_ascending () const; + static uint8_t constexpr frontier_req_only_confirmed = 1; bool frontier_req_is_only_confirmed_present () const; + static uint8_t constexpr confirm_v2_flag = 0; + bool confirm_is_v2 () const; + void confirm_set_v2 (bool); + /** Size of the payload in bytes. For some messages, the payload size is based on header flags. */ std::size_t payload_length_bytes () const; bool is_valid_message_type () const; - static std::bitset<16> constexpr block_type_mask{ 0x0f00 }; - static std::bitset<16> constexpr count_mask{ 0xf000 }; - static std::bitset<16> constexpr telemetry_size_mask{ 0x3ff }; + static extensions_bitset_t constexpr block_type_mask{ 0x0f00 }; + static extensions_bitset_t constexpr count_mask{ 0xf000 }; + static extensions_bitset_t constexpr count_v2_mask_left{ 0xf000 }; + static extensions_bitset_t constexpr count_v2_mask_right{ 0x00f0 }; + static extensions_bitset_t constexpr telemetry_size_mask{ 0x3ff }; }; class message @@ -148,6 +167,7 @@ class confirm_req final : public message confirm_req (bool & error, nano::stream &, nano::message_header const &); confirm_req (nano::network_constants const & constants, std::vector> const &); confirm_req (nano::network_constants const & constants, nano::block_hash const &, nano::root const &); + void serialize (nano::stream &) const override; bool deserialize (nano::stream &); void visit (nano::message_visitor &) const override; @@ -157,6 +177,9 @@ class confirm_req final : public message static std::size_t size (nano::message_header const &); +private: + static uint8_t hash_count (nano::message_header const &); + public: // Payload std::vector> roots_hashes; }; @@ -164,13 +187,20 @@ class confirm_req final : public message class confirm_ack final : public message { public: - confirm_ack (bool &, nano::stream &, nano::message_header const &, nano::vote_uniquer * = nullptr); + confirm_ack (bool & error, nano::stream &, nano::message_header const &, nano::vote_uniquer * = nullptr); confirm_ack (nano::network_constants const & constants, std::shared_ptr const &); + void serialize (nano::stream &) const override; void visit (nano::message_visitor &) const override; bool operator== (nano::confirm_ack const &) const; - static std::size_t size (std::size_t count); std::string to_string () const; + + static std::size_t size (nano::message_header const &); + +private: + static uint8_t hash_count (nano::message_header const &); + +public: // Payload std::shared_ptr vote; }; diff --git a/nano/node/network.hpp b/nano/node/network.hpp index 0253496e95..56f6f23b86 100644 --- a/nano/node/network.hpp +++ b/nano/node/network.hpp @@ -154,6 +154,7 @@ class network final std::atomic stopped{ false }; static unsigned const broadcast_interval_ms = 10; static std::size_t const buffer_size = 512; + static std::size_t const confirm_req_hashes_max = 7; static std::size_t const confirm_ack_hashes_max = 12; }; diff --git a/nano/secure/vote.cpp b/nano/secure/vote.cpp index 2a7a669334..2f19c9748e 100644 --- a/nano/secure/vote.cpp +++ b/nano/secure/vote.cpp @@ -1,3 +1,4 @@ +#include #include #include @@ -13,11 +14,15 @@ nano::vote::vote (nano::account const & account_a, nano::raw_key const & prv_a, timestamp_m{ packed_timestamp (timestamp_a, duration) }, account{ account_a } { + debug_assert (hashes.size () <= max_hashes); + signature = nano::sign_message (prv_a, account_a, hash ()); } void nano::vote::serialize (nano::stream & stream_a) const { + debug_assert (hashes.size () <= max_hashes); + write (stream_a, account); write (stream_a, signature); write (stream_a, boost::endian::native_to_little (timestamp_m)); @@ -36,7 +41,7 @@ bool nano::vote::deserialize (nano::stream & stream_a) nano::read (stream_a, signature.bytes); nano::read (stream_a, timestamp_m); - while (stream_a.in_avail () > 0) + while (stream_a.in_avail () > 0 && hashes.size () < max_hashes) { nano::block_hash block_hash; nano::read (stream_a, block_hash); @@ -50,6 +55,12 @@ bool nano::vote::deserialize (nano::stream & stream_a) return error; } +std::size_t nano::vote::size (uint8_t count) +{ + debug_assert (count <= max_hashes); + return partial_size + count * sizeof (nano::block_hash); +} + std::string const nano::vote::hash_prefix = "vote "; nano::block_hash nano::vote::hash () const diff --git a/nano/secure/vote.hpp b/nano/secure/vote.hpp index b8284c19dc..35d7308a13 100644 --- a/nano/secure/vote.hpp +++ b/nano/secure/vote.hpp @@ -34,6 +34,7 @@ class vote final * @returns true if there was an error */ bool deserialize (nano::stream &); + static std::size_t size (uint8_t count); nano::block_hash hash () const; nano::block_hash full_hash () const; @@ -58,14 +59,11 @@ class vote final static uint64_t constexpr timestamp_min = { 0x0000'0000'0000'0010ULL }; static uint8_t constexpr duration_max = { 0x0fu }; + static std::size_t constexpr max_hashes = 255; + /* Check if timestamp represents a final vote */ static bool is_final_timestamp (uint64_t timestamp); -private: - static std::string const hash_prefix; - - static uint64_t packed_timestamp (uint64_t timestamp, uint8_t duration); - public: // Payload // The hashes for which this vote directly covers std::vector hashes; @@ -77,6 +75,13 @@ class vote final private: // Payload // Vote timestamp uint64_t timestamp_m{ 0 }; + +private: + // Size of vote payload without hashes + static std::size_t constexpr partial_size = sizeof (account) + sizeof (signature) + sizeof (timestamp_m); + static std::string const hash_prefix; + + static uint64_t packed_timestamp (uint64_t timestamp, uint8_t duration); }; using vote_uniquer = nano::uniquer; From 632b73e3e38219b9a5cf12a41db44c5ace44399a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 11 Dec 2023 19:47:53 +0100 Subject: [PATCH 2/3] Large vote tests --- nano/core_test/message.cpp | 99 ++++++++++++++++++++++++++++++- nano/core_test/vote_processor.cpp | 22 +++++++ nano/node/request_aggregator.cpp | 1 + nano/test_common/chains.cpp | 2 +- 4 files changed, 122 insertions(+), 2 deletions(-) diff --git a/nano/core_test/message.cpp b/nano/core_test/message.cpp index 0142549254..29e70e371c 100644 --- a/nano/core_test/message.cpp +++ b/nano/core_test/message.cpp @@ -179,10 +179,51 @@ TEST (message, confirm_ack_hash_serialization) ASSERT_FALSE (error); ASSERT_EQ (con1, con2); ASSERT_EQ (hashes, con2.vote->hashes); - // Check overflow with max hashes + ASSERT_FALSE (header.confirm_is_v2 ()); ASSERT_EQ (header.count_get (), hashes.size ()); } +TEST (message, confirm_ack_hash_serialization_v2) +{ + std::vector hashes; + for (auto i (hashes.size ()); i < 255; i++) + { + nano::keypair key1; + nano::block_hash previous; + nano::random_pool::generate_block (previous.bytes.data (), previous.bytes.size ()); + nano::block_builder builder; + auto block = builder + .state () + .account (key1.pub) + .previous (previous) + .representative (key1.pub) + .balance (2) + .link (4) + .sign (key1.prv, key1.pub) + .work (5) + .build (); + hashes.push_back (block->hash ()); + } + + nano::keypair representative1; + auto vote (std::make_shared (representative1.pub, representative1.prv, 0, 0, hashes)); + nano::confirm_ack con1{ nano::dev::network_params.network, vote }; + std::vector bytes; + { + nano::vectorstream stream1 (bytes); + con1.serialize (stream1); + } + nano::bufferstream stream2 (bytes.data (), bytes.size ()); + bool error (false); + nano::message_header header (error, stream2); + nano::confirm_ack con2 (error, stream2, header); + ASSERT_FALSE (error); + ASSERT_EQ (con1, con2); + ASSERT_EQ (hashes, con2.vote->hashes); + ASSERT_TRUE (header.confirm_is_v2 ()); + ASSERT_EQ (header.count_v2_get (), hashes.size ()); +} + TEST (message, confirm_req_hash_serialization) { nano::keypair key1; @@ -263,6 +304,62 @@ TEST (message, confirm_req_hash_batch_serialization) ASSERT_EQ (req.roots_hashes, roots_hashes); ASSERT_EQ (req2.roots_hashes, roots_hashes); ASSERT_EQ (header.count_get (), req.roots_hashes.size ()); + ASSERT_FALSE (header.confirm_is_v2 ()); +} + +TEST (message, confirm_req_hash_batch_serialization_v2) +{ + nano::keypair key; + nano::keypair representative; + nano::block_builder builder; + auto open = builder + .state () + .account (key.pub) + .previous (0) + .representative (representative.pub) + .balance (2) + .link (4) + .sign (key.prv, key.pub) + .work (5) + .build (); + + std::vector> roots_hashes; + roots_hashes.push_back (std::make_pair (open->hash (), open->root ())); + for (auto i (roots_hashes.size ()); i < 255; i++) + { + nano::keypair key1; + nano::block_hash previous; + nano::random_pool::generate_block (previous.bytes.data (), previous.bytes.size ()); + auto block = builder + .state () + .account (key1.pub) + .previous (previous) + .representative (representative.pub) + .balance (2) + .link (4) + .sign (key1.prv, key1.pub) + .work (5) + .build (); + roots_hashes.push_back (std::make_pair (block->hash (), block->root ())); + } + + nano::confirm_req req{ nano::dev::network_params.network, roots_hashes }; + std::vector bytes; + { + nano::vectorstream stream (bytes); + req.serialize (stream); + } + auto error (false); + nano::bufferstream stream2 (bytes.data (), bytes.size ()); + nano::message_header header (error, stream2); + nano::confirm_req req2 (error, stream2, header); + ASSERT_FALSE (error); + ASSERT_EQ (req, req2); + ASSERT_EQ (req.roots_hashes, req2.roots_hashes); + ASSERT_EQ (req.roots_hashes, roots_hashes); + ASSERT_EQ (req2.roots_hashes, roots_hashes); + ASSERT_EQ (header.count_v2_get (), req.roots_hashes.size ()); + ASSERT_TRUE (header.confirm_is_v2 ()); } // this unit test checks that conversion of message_header to string works as expected diff --git a/nano/core_test/vote_processor.cpp b/nano/core_test/vote_processor.cpp index a444719b01..48bdc4ec9f 100644 --- a/nano/core_test/vote_processor.cpp +++ b/nano/core_test/vote_processor.cpp @@ -326,6 +326,28 @@ TEST (vote_processor, no_broadcast_local_with_a_principal_representative) ASSERT_EQ (1, node.stats.count (nano::stat::type::message, nano::stat::detail::publish, nano::stat::dir::out)); } +/** + * Ensure that node behaves well with votes larger than 12 hashes, which was maximum before V26 + */ +TEST (vote_processor, large_votes) +{ + nano::test::system system (1); + auto & node = *system.nodes[0]; + + const int count = 32; + auto blocks = nano::test::setup_chain (system, node, count, nano::dev::genesis_key, /* do not confirm */ false); + + ASSERT_TRUE (nano::test::start_elections (system, node, blocks)); + ASSERT_TIMELY (5s, nano::test::active (node, blocks)); + + auto vote = nano::test::make_final_vote (nano::dev::genesis_key, blocks); + ASSERT_TRUE (vote->hashes.size () == count); + + node.vote_processor.vote (vote, nano::test::fake_channel (node)); + + ASSERT_TIMELY (5s, nano::test::confirmed (node, blocks)); +} + /** * basic test to check that the timestamp mask is applied correctly on vote timestamp and duration fields */ diff --git a/nano/node/request_aggregator.cpp b/nano/node/request_aggregator.cpp index cb84ba6045..7f6aa2d2e1 100644 --- a/nano/node/request_aggregator.cpp +++ b/nano/node/request_aggregator.cpp @@ -34,6 +34,7 @@ nano::request_aggregator::request_aggregator (nano::node_config const & config_a condition.wait (lock, [&started = started] { return started; }); } +// TODO: This is badly implemented, will prematurely drop large vote requests void nano::request_aggregator::add (std::shared_ptr const & channel_a, std::vector> const & hashes_roots_a) { debug_assert (wallets.reps ().voting > 0); diff --git a/nano/test_common/chains.cpp b/nano/test_common/chains.cpp index af528e92a0..bb77caa067 100644 --- a/nano/test_common/chains.cpp +++ b/nano/test_common/chains.cpp @@ -18,7 +18,7 @@ nano::block_list_t nano::test::setup_chain (nano::test::system & system, nano::n .state () .account (target.pub) .previous (latest) - .representative (throwaway.pub) + .representative (target.pub) .balance (balance) .link (throwaway.pub) .sign (target.prv, target.pub) From a770ea15fbe3d4f9a7bd0027a1918e3b896bd88e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Tue, 12 Dec 2023 16:47:19 +0100 Subject: [PATCH 3/3] Fix `vote_processor.codes` test --- nano/core_test/vote_processor.cpp | 5 ++++- nano/node/vote_cache.cpp | 6 ++++++ nano/node/vote_cache.hpp | 3 +++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/nano/core_test/vote_processor.cpp b/nano/core_test/vote_processor.cpp index 48bdc4ec9f..14fea2b451 100644 --- a/nano/core_test/vote_processor.cpp +++ b/nano/core_test/vote_processor.cpp @@ -25,9 +25,12 @@ TEST (vote_processor, codes) // Hint of pre-validation ASSERT_NE (nano::vote_code::invalid, node.vote_processor.vote_blocking (vote_invalid, channel, true)); - // No ongoing election + // No ongoing election (vote goes to vote cache) ASSERT_EQ (nano::vote_code::indeterminate, node.vote_processor.vote_blocking (vote, channel)); + // Clear vote cache before starting election + node.vote_cache.clear (); + // First vote from an account for an ongoing election node.start_election (blocks[0]); std::shared_ptr election; diff --git a/nano/node/vote_cache.cpp b/nano/node/vote_cache.cpp index b6e2bb8040..527ea0a097 100644 --- a/nano/node/vote_cache.cpp +++ b/nano/node/vote_cache.cpp @@ -193,6 +193,12 @@ bool nano::vote_cache::erase (const nano::block_hash & hash) return result; } +void nano::vote_cache::clear () +{ + nano::lock_guard lock{ mutex }; + cache.clear (); +} + std::vector nano::vote_cache::top (const nano::uint128_t & min_tally) { stats.inc (nano::stat::type::vote_cache, nano::stat::detail::top); diff --git a/nano/node/vote_cache.hpp b/nano/node/vote_cache.hpp index 4a353446af..5b88ff1ba6 100644 --- a/nano/node/vote_cache.hpp +++ b/nano/node/vote_cache.hpp @@ -97,15 +97,18 @@ class vote_cache final * Adds a new vote to cache */ void vote (nano::block_hash const & hash, std::shared_ptr vote); + /** * Tries to find an entry associated with block hash */ std::optional find (nano::block_hash const & hash) const; + /** * Removes an entry associated with block hash, does nothing if entry does not exist * @return true if hash existed and was erased, false otherwise */ bool erase (nano::block_hash const & hash); + void clear (); std::size_t size () const; bool empty () const;