Skip to content

Commit

Permalink
Use qualified root for vote tracking
Browse files Browse the repository at this point in the history
  • Loading branch information
pwojcikdev committed Feb 16, 2025
1 parent b6548d7 commit cfded44
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 46 deletions.
2 changes: 1 addition & 1 deletion nano/core_test/node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2003,7 +2003,7 @@ TEST (node, vote_by_hash_bundle)

for (auto const & block : blocks)
{
system.nodes[0]->generator.add (block->root (), block->hash ());
system.nodes[0]->generator.add (*block);
}

// Verify that bundling occurs. While reaching 12 should be common on most hardware in release mode,
Expand Down
24 changes: 12 additions & 12 deletions nano/core_test/voting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ TEST (local_vote_history, basic)
TEST (vote_spacing, basic)
{
nano::vote_spacing spacing{ std::chrono::milliseconds{ 100 } };
nano::root root1{ 1 };
nano::root root2{ 2 };
nano::qualified_root root1{ 1 };
nano::qualified_root root2{ 2 };
nano::block_hash hash3{ 3 };
nano::block_hash hash4{ 4 };
nano::block_hash hash5{ 5 };
Expand All @@ -88,8 +88,8 @@ TEST (vote_spacing, prune)

auto length = std::chrono::milliseconds{ 100 };
nano::vote_spacing spacing{ length };
nano::root root1{ 1 };
nano::root root2{ 2 };
nano::qualified_root root1{ 1 };
nano::qualified_root root2{ 2 };
nano::block_hash hash3{ 3 };
nano::block_hash hash4{ 4 };
spacing.flag (root1, hash3, now);
Expand All @@ -109,7 +109,7 @@ TEST (vote_generator, cache)
auto & node (*system.nodes[0]);
auto epoch1 = system.upgrade_genesis_epoch (node, nano::epoch::epoch_1);
system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv);
node.generator.add (epoch1->root (), epoch1->hash ());
node.generator.add (*epoch1);
ASSERT_TIMELY (1s, !node.history.votes (epoch1->root (), epoch1->hash ()).empty ());
auto votes (node.history.votes (epoch1->root (), epoch1->hash ()));
ASSERT_FALSE (votes.empty ());
Expand Down Expand Up @@ -185,15 +185,15 @@ TEST (vote_generator, vote_spacing)
.build ();
ASSERT_EQ (nano::block_status::progress, node.ledger.process (node.ledger.tx_begin_write (), send1));
ASSERT_EQ (0, node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_broadcasts));
node.generator.add (nano::dev::genesis->hash (), send1->hash ());
node.generator.add (*send1);
ASSERT_TIMELY_EQ (3s, 1, node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_broadcasts));

ASSERT_FALSE (node.ledger.rollback (node.ledger.tx_begin_write (), send1->hash ()));
ASSERT_EQ (nano::block_status::progress, node.ledger.process (node.ledger.tx_begin_write (), send2));
node.generator.add (nano::dev::genesis->hash (), send2->hash ());
node.generator.add (*send2);
ASSERT_TIMELY_EQ (3s, 2, node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_broadcasts));
// vote on send2 second time (rapid succession) - expect spacing
node.generator.add (nano::dev::genesis->hash (), send2->hash ());
node.generator.add (*send2);
ASSERT_TIMELY_EQ (3s, 1, node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_spacing));
ASSERT_TIMELY_EQ (3s, 2, node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_broadcasts));
}
Expand Down Expand Up @@ -229,22 +229,22 @@ TEST (vote_generator, vote_spacing_rapid)
.work (*system.work.generate (nano::dev::genesis->hash ()))
.build ();
ASSERT_EQ (nano::block_status::progress, node.ledger.process (node.ledger.tx_begin_write (), send1));
node.generator.add (nano::dev::genesis->hash (), send1->hash ());
node.generator.add (*send1);
ASSERT_TIMELY_EQ (3s, 1, node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_broadcasts));
ASSERT_FALSE (node.ledger.rollback (node.ledger.tx_begin_write (), send1->hash ()));
ASSERT_EQ (nano::block_status::progress, node.ledger.process (node.ledger.tx_begin_write (), send2));
// vote on send2 first time - expect : new broadcast & no spacing
node.generator.add (nano::dev::genesis->hash (), send2->hash ());
node.generator.add (*send2);
ASSERT_TIMELY_EQ (3s, 2, node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_broadcasts));
ASSERT_TIMELY_EQ (3s, 0, node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_spacing));

// vote on send2 again rapidly - expect : no broadcast & new spacing
node.generator.add (nano::dev::genesis->hash (), send2->hash ());
node.generator.add (*send2);
ASSERT_TIMELY_EQ (3s, 2, node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_broadcasts));
ASSERT_TIMELY_EQ (3s, 1, node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_spacing));
std::this_thread::sleep_for (config.network_params.voting.delay);
// vote on send2 again after the spacing delay - expect : new broadcast & no spacing
node.generator.add (nano::dev::genesis->hash (), send2->hash ());
node.generator.add (*send2);
ASSERT_TIMELY_EQ (3s, 3, node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_broadcasts));
ASSERT_TIMELY_EQ (3s, 1, node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_spacing));
}
6 changes: 3 additions & 3 deletions nano/node/election.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ void nano::election::confirm_if_quorum (nano::unique_lock<nano::mutex> & lock_a)
if (!is_quorum.exchange (true) && node.config.enable_voting && node.wallets.reps ().voting > 0)
{
++vote_broadcast_count;
node.final_generator.add (root, status.winner->hash ());
node.final_generator.add (*status.winner);
}
if (final_weight >= node.online_reps.delta ())
{
Expand Down Expand Up @@ -668,7 +668,7 @@ void nano::election::broadcast_vote_locked (nano::unique_lock<nano::mutex> & loc
nano::log::arg{ "winner", status.winner },
nano::log::arg{ "type", "final" });

node.final_generator.add (root, status.winner->hash ()); // Broadcasts vote to the network
node.final_generator.add (*status.winner); // Broadcasts vote to the network
}
else
{
Expand All @@ -679,7 +679,7 @@ void nano::election::broadcast_vote_locked (nano::unique_lock<nano::mutex> & loc
nano::log::arg{ "winner", status.winner },
nano::log::arg{ "type", "normal" });

node.generator.add (root, status.winner->hash ()); // Broadcasts vote to the network
node.generator.add (*status.winner); // Broadcasts vote to the network
}
}
}
Expand Down
18 changes: 9 additions & 9 deletions nano/node/vote_generator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ nano::vote_generator::~vote_generator ()
debug_assert (!thread.joinable ());
}

bool nano::vote_generator::should_vote (transaction_variant_t const & transaction_variant, nano::root const & root_a, nano::block_hash const & hash_a) const
bool nano::vote_generator::should_vote (transaction_variant_t const & transaction_variant, nano::qualified_root const & root_a, nano::block_hash const & hash_a) const
{
bool should_vote = false;
std::shared_ptr<nano::block> block;
Expand All @@ -54,7 +54,7 @@ bool nano::vote_generator::should_vote (transaction_variant_t const & transactio

block = ledger.any.block_get (transaction, hash_a);
should_vote = block != nullptr && ledger.dependents_confirmed (transaction, *block) && ledger.store.final_vote.put (transaction, block->qualified_root (), hash_a);
debug_assert (block == nullptr || root_a == block->root ());
debug_assert (block == nullptr || root_a == block->qualified_root ());
}
else
{
Expand Down Expand Up @@ -98,9 +98,9 @@ void nano::vote_generator::stop ()
}
}

void nano::vote_generator::add (const root & root, const block_hash & hash)
void nano::vote_generator::add (nano::block const & block)
{
vote_generation_queue.add (std::make_pair (root, hash));
vote_generation_queue.add (std::make_pair (block.qualified_root (), block.hash ()));
}

void nano::vote_generator::process_batch (std::deque<queue_entry_t> & batch)
Expand Down Expand Up @@ -157,7 +157,7 @@ std::size_t nano::vote_generator::generate (std::vector<std::shared_ptr<nano::bl
return this->ledger.dependents_confirmed (transaction, *block_a);
};
auto as_candidate = [] (auto const & block_a) {
return candidate_t{ block_a->root (), block_a->hash () };
return candidate_t{ block_a->qualified_root (), block_a->hash () };
};
nano::transform_if (blocks_a.begin (), blocks_a.end (), std::back_inserter (req_candidates), dependents_confirmed, as_candidate);
}
Expand All @@ -178,7 +178,7 @@ void nano::vote_generator::broadcast (nano::unique_lock<nano::mutex> & lock_a)
debug_assert (lock_a.owns_lock ());

std::vector<nano::block_hash> hashes;
std::vector<nano::root> roots;
std::vector<nano::qualified_root> roots;
hashes.reserve (nano::network::confirm_ack_hashes_max);
roots.reserve (nano::network::confirm_ack_hashes_max);
while (!candidates.empty () && hashes.size () < nano::network::confirm_ack_hashes_max)
Expand Down Expand Up @@ -222,7 +222,7 @@ void nano::vote_generator::reply (nano::unique_lock<nano::mutex> & lock_a, reque
while (i != n && !stopped)
{
std::vector<nano::block_hash> hashes;
std::vector<nano::root> roots;
std::vector<nano::qualified_root> roots;
hashes.reserve (nano::network::confirm_ack_hashes_max);
roots.reserve (nano::network::confirm_ack_hashes_max);
for (; i != n && hashes.size () < nano::network::confirm_ack_hashes_max; ++i)
Expand Down Expand Up @@ -250,7 +250,7 @@ void nano::vote_generator::reply (nano::unique_lock<nano::mutex> & lock_a, reque
lock_a.lock ();
}

void nano::vote_generator::vote (std::vector<nano::block_hash> const & hashes_a, std::vector<nano::root> const & roots_a, std::function<void (std::shared_ptr<nano::vote> const &)> const & action_a)
void nano::vote_generator::vote (std::vector<nano::block_hash> const & hashes_a, std::vector<nano::qualified_root> const & roots_a, std::function<void (std::shared_ptr<nano::vote> const &)> const & action_a)
{
debug_assert (hashes_a.size () == roots_a.size ());
std::vector<std::shared_ptr<nano::vote>> votes_l;
Expand All @@ -263,7 +263,7 @@ void nano::vote_generator::vote (std::vector<nano::block_hash> const & hashes_a,
{
for (std::size_t i (0), n (hashes_a.size ()); i != n; ++i)
{
history.add (roots_a[i], hashes_a[i], vote_l);
history.add (roots_a[i].root (), hashes_a[i], vote_l);
spacing.flag (roots_a[i], hashes_a[i]);
}
action_a (vote_l);
Expand Down
10 changes: 5 additions & 5 deletions nano/node/vote_generator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,17 @@ namespace nano
class vote_generator final
{
private:
using candidate_t = std::pair<nano::root, nano::block_hash>;
using candidate_t = std::pair<nano::qualified_root, nano::block_hash>;
using request_t = std::pair<std::vector<candidate_t>, std::shared_ptr<nano::transport::channel>>;
using queue_entry_t = std::pair<nano::root, nano::block_hash>;
using queue_entry_t = std::pair<nano::qualified_root, nano::block_hash>;
std::chrono::steady_clock::time_point next_broadcast = { std::chrono::steady_clock::now () };

public:
vote_generator (nano::node_config const &, nano::node &, nano::ledger &, nano::wallets &, nano::vote_processor &, nano::local_vote_history &, nano::network &, nano::stats &, nano::logger &, bool is_final);
~vote_generator ();

/** Queue items for vote generation, or broadcast votes already in cache */
void add (nano::root const &, nano::block_hash const &);
void add (nano::block const & block);
/** Queue blocks for vote generation, returning the number of successful candidates.*/
std::size_t generate (std::vector<std::shared_ptr<nano::block>> const & blocks_a, std::shared_ptr<nano::transport::channel> const & channel_a);

Expand All @@ -52,10 +52,10 @@ class vote_generator final
void run ();
void broadcast (nano::unique_lock<nano::mutex> &);
void reply (nano::unique_lock<nano::mutex> &, request_t &&);
void vote (std::vector<nano::block_hash> const &, std::vector<nano::root> const &, std::function<void (std::shared_ptr<nano::vote> const &)> const &);
void vote (std::vector<nano::block_hash> const &, std::vector<nano::qualified_root> const &, std::function<void (std::shared_ptr<nano::vote> const &)> const &);
void broadcast_action (std::shared_ptr<nano::vote> const &) const;
void process_batch (std::deque<queue_entry_t> & batch);
bool should_vote (transaction_variant_t const &, nano::root const &, nano::block_hash const &) const;
bool should_vote (transaction_variant_t const &, nano::qualified_root const &, nano::block_hash const &) const;
bool broadcast_predicate () const;

private: // Dependencies
Expand Down
18 changes: 8 additions & 10 deletions nano/node/vote_spacing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,37 +5,35 @@ void nano::vote_spacing::trim (std::chrono::steady_clock::time_point now)
recent.get<tag_time> ().erase (recent.get<tag_time> ().begin (), recent.get<tag_time> ().upper_bound (now - delay));
}

bool nano::vote_spacing::votable (nano::root const & root_a, nano::block_hash const & hash_a, std::chrono::steady_clock::time_point now) const
bool nano::vote_spacing::votable (nano::qualified_root const & root, nano::block_hash const & hash, std::chrono::steady_clock::time_point now) const
{
for (auto range = recent.get<tag_root> ().equal_range (root_a); range.first != range.second; ++range.first)
if (auto it = recent.get<tag_root> ().find (root); it != recent.end ())
{
auto & item = *range.first;
if (hash_a == item.hash && item.time >= now - delay)
if (hash == it->hash && it->time >= now - delay)
{
return false; // Same hash and not expired -> not votable
}
}
return true; // Either different hash or expired -> votable
}

void nano::vote_spacing::flag (nano::root const & root_a, nano::block_hash const & hash_a, std::chrono::steady_clock::time_point now)
void nano::vote_spacing::flag (nano::qualified_root const & root, nano::block_hash const & hash, std::chrono::steady_clock::time_point now)
{
trim (now);

auto existing = recent.get<tag_root> ().find (root_a);
if (existing != recent.end ())
if (auto it = recent.get<tag_root> ().find (root); it != recent.end ())
{
// We update both timestamp and hash because we want to track which fork
// we most recently voted for. This ensures proper spacing between votes
// for the same block while allowing immediate votes for competing forks.
recent.get<tag_root> ().modify (existing, [now, hash_a] (entry & entry) {
recent.get<tag_root> ().modify (it, [now, hash] (entry & entry) {
entry.hash = hash;
entry.time = now;
entry.hash = hash_a;
});
}
else
{
recent.insert ({ root_a, now, hash_a });
recent.insert ({ root, hash, now });
}
}

Expand Down
12 changes: 6 additions & 6 deletions nano/node/vote_spacing.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ class vote_spacing final
{
}

bool votable (nano::root const & root, nano::block_hash const & hash, std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now ()) const;
void flag (nano::root const & root, nano::block_hash const & hash, std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now ());
bool votable (nano::qualified_root const & root, nano::block_hash const & hash, std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now ()) const;
void flag (nano::qualified_root const & root, nano::block_hash const & hash, std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now ());
std::size_t size () const;

private:
Expand All @@ -34,9 +34,9 @@ class vote_spacing final

struct entry
{
nano::root root;
std::chrono::steady_clock::time_point time;
nano::qualified_root root;
nano::block_hash hash;
std::chrono::steady_clock::time_point time;
};

// clang-format off
Expand All @@ -45,8 +45,8 @@ class vote_spacing final

using ordered_votes = boost::multi_index_container<entry,
mi::indexed_by<
mi::hashed_non_unique<mi::tag<tag_root>,
mi::member<entry, nano::root, &entry::root>>,
mi::hashed_unique<mi::tag<tag_root>,
mi::member<entry, nano::qualified_root, &entry::root>>,
mi::ordered_non_unique<mi::tag<tag_time>,
mi::member<entry, std::chrono::steady_clock::time_point, &entry::time>>
>>;
Expand Down

0 comments on commit cfded44

Please sign in to comment.