Skip to content

Commit

Permalink
Currently there is no rate limiter on vote generation for a particula…
Browse files Browse the repository at this point in the history
…r root and this can lead to increased, unnecessary vote traffic.

This patch adds a delay between constructing non-cached votes.
  • Loading branch information
clemahieu committed Nov 18, 2020
1 parent 90ef5c3 commit 9b2870c
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 6 deletions.
43 changes: 43 additions & 0 deletions nano/core_test/voting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,46 @@ TEST (vote_generator, session)
thread.join ();
ASSERT_TIMELY (2s, 1 == node->stats.count (nano::stat::type::vote, nano::stat::detail::vote_indeterminate));
}

namespace nano {
TEST (vote_generator, vote_spacing)
{
nano::node_config config;
config.frontiers_confirmation = nano::frontiers_confirmation_mode::disabled;
nano::system system;
auto & node = *system.add_node (config);
auto & wallet = *system.wallet (0);
wallet.insert_adhoc (nano::dev_genesis_key.prv);
nano::state_block_builder builder;
auto send1 = builder.make_block ()
.account (nano::dev_genesis_key.pub)
.previous (nano::genesis_hash)
.representative (nano::dev_genesis_key.pub)
.balance (nano::genesis_amount - nano::Gxrb_ratio)
.link (nano::dev_genesis_key.pub)
.sign (nano::dev_genesis_key.prv, nano::dev_genesis_key.pub)
.work (*system.work.generate (nano::genesis_hash))
.build_shared ();
auto send2 = builder.make_block ()
.account (nano::dev_genesis_key.pub)
.previous (nano::genesis_hash)
.representative (nano::dev_genesis_key.pub)
.balance (nano::genesis_amount - nano::Gxrb_ratio - 1)
.link (nano::dev_genesis_key.pub)
.sign (nano::dev_genesis_key.prv, nano::dev_genesis_key.pub)
.work (*system.work.generate (nano::genesis_hash))
.build_shared ();
ASSERT_EQ (nano::process_result::progress, node.ledger.process (node.store.tx_begin_write (), *send1).code);
ASSERT_EQ (0, node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_broadcasts));
node.active.generator.add (nano::genesis_hash, send1->hash ());
ASSERT_TIMELY (3s, node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_broadcasts) == 1);
ASSERT_FALSE (node.ledger.rollback (node.store.tx_begin_write (), send1->hash ()));
ASSERT_EQ (nano::process_result::progress, node.ledger.process (node.store.tx_begin_write (), *send2).code);
node.active.generator.add (nano::genesis_hash, send2->hash ());
ASSERT_TIMELY (3s, node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_spacing) == 1);
ASSERT_EQ (1, node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_broadcasts));
std::this_thread::sleep_for (node.active.generator.history.delay);
node.active.generator.add (nano::genesis_hash, send2->hash ());
ASSERT_TIMELY (3s, node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_broadcasts) == 2);
}
}
3 changes: 3 additions & 0 deletions nano/lib/stats.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,9 @@ std::string nano::stat::detail_to_string (uint32_t key)
case nano::stat::detail::generator_replies_discarded:
res = "generator_replies_discarded";
break;
case nano::stat::detail::generator_spacing:
res = "generator_spacing";
break;
}
return res;
}
Expand Down
3 changes: 2 additions & 1 deletion nano/lib/stats.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,8 @@ class stat final
// vote generator
generator_broadcasts,
generator_replies,
generator_replies_discarded
generator_replies_discarded,
generator_spacing
};

/** Direction of the stat. If the direction is irrelevant, use in */
Expand Down
28 changes: 24 additions & 4 deletions nano/node/voting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,17 @@ size_t nano::local_vote_history::size () const
return history.size ();
}

bool nano::local_vote_history::votable (nano::root const & root_a, nano::block_hash const & hash_a) const
{
bool result = true;
for (auto range = history.get<tag_root> ().equal_range (root_a); result && range.first != range.second; ++range.first)
{
auto & item = *range.first;
result = item.time < std::chrono::steady_clock::now () - delay || item.hash == hash_a;
}
return result;
}

std::unique_ptr<nano::container_info_component> nano::collect_container_info (nano::local_vote_history & history, const std::string & name)
{
size_t history_count = history.size ();
Expand Down Expand Up @@ -219,18 +230,27 @@ void nano::vote_generator::broadcast (nano::unique_lock<std::mutex> & lock_a)
}
if (cached_votes.empty ())
{
roots.push_back (root);
hashes.push_back (hash);
if (history.votable (root, hash))
{
roots.push_back (root);
hashes.push_back (hash);
}
else
{
stats.inc (nano::stat::type::vote_generator, nano::stat::detail::generator_spacing);
}
}
candidates.pop_front ();
}
if (!hashes.empty ())
{
lock_a.unlock ();
vote (hashes, roots, [this](auto const & vote_a) { this->broadcast_action (vote_a); });
vote (hashes, roots, [this](auto const & vote_a) {
this->broadcast_action (vote_a);
this->stats.inc (nano::stat::type::vote_generator, nano::stat::detail::generator_broadcasts);
});
lock_a.lock ();
}
stats.inc (nano::stat::type::vote_generator, nano::stat::detail::generator_broadcasts);
}

void nano::vote_generator::reply (nano::unique_lock<std::mutex> & lock_a, request_t && request_a)
Expand Down
6 changes: 5 additions & 1 deletion nano/node/voting.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ class local_vote_history final
}
nano::root root;
nano::block_hash hash;
std::chrono::steady_clock::time_point time{ std::chrono::steady_clock::now () };
std::shared_ptr<nano::vote> vote;
};

Expand All @@ -55,6 +56,7 @@ class local_vote_history final
std::vector<std::shared_ptr<nano::vote>> votes (nano::root const & root_a, nano::block_hash const & hash_a) const;
bool exists (nano::root const &) const;
size_t size () const;
bool votable (nano::root const & root_a, nano::block_hash const & hash_a) const;

private:
// clang-format off
Expand All @@ -72,10 +74,11 @@ class local_vote_history final
// Only used in Debug
bool consistency_check (nano::root const &) const;
mutable std::mutex mutex;
std::chrono::seconds const delay{ nano::network_params ().network.is_dev_network () ? 1 : 15 };

friend std::unique_ptr<container_info_component> collect_container_info (local_vote_history & history, const std::string & name);

friend class local_vote_history_basic_Test;
friend class vote_generator_vote_spacing_Test;
};

std::unique_ptr<container_info_component> collect_container_info (local_vote_history & history, const std::string & name);
Expand Down Expand Up @@ -120,6 +123,7 @@ class vote_generator final
bool started{ false };
std::thread thread;

friend class vote_generator_vote_spacing_Test;
friend std::unique_ptr<container_info_component> collect_container_info (vote_generator & vote_generator, const std::string & name);
};

Expand Down

0 comments on commit 9b2870c

Please sign in to comment.