Check if a vote is for a recently confirmed block (#2663)

* Check if a vote is for a recently confirmed block

* Formatting

* Use count() as the iterator is not necessary (Wes comment)
This commit is contained in:
Guilherme Lawless 2020-03-18 15:59:10 +00:00 committed by GitHub
commit 09fd2b1d45
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 52 additions and 29 deletions

View file

@ -636,6 +636,8 @@ TEST (active_transactions, update_difficulty)
}
}
namespace nano
{
TEST (active_transactions, vote_replays)
{
nano::system system;
@ -659,24 +661,16 @@ TEST (active_transactions, vote_replays)
ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote_send1));
ASSERT_EQ (2, node.active.size ());
ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote_send1));
// Wait until the election is removed, at which point the vote should be indeterminate
system.deadline_set (3s);
while (node.active.size () != 1)
{
ASSERT_NO_ERROR (system.poll ());
}
ASSERT_EQ (nano::vote_code::indeterminate, node.active.vote (vote_send1));
// Wait until the election is removed, at which point the vote is still a replay since it's been recently confirmed
ASSERT_TIMELY (3s, node.active.size () == 1);
ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote_send1));
// Open new account
auto vote_open1 (std::make_shared<nano::vote> (nano::test_genesis_key.pub, nano::test_genesis_key.prv, 0, open1));
ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote_open1));
ASSERT_EQ (1, node.active.size ());
ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote_open1));
system.deadline_set (3s);
while (!node.active.empty ())
{
ASSERT_NO_ERROR (system.poll ());
}
ASSERT_EQ (nano::vote_code::indeterminate, node.active.vote (vote_open1));
ASSERT_TIMELY (3s, node.active.empty ());
ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote_open1));
ASSERT_EQ (nano::Gxrb_ratio, node.ledger.weight (key.pub));
auto send2 (std::make_shared<nano::state_block> (key.pub, open1->hash (), key.pub, nano::Gxrb_ratio - 1, key.pub, key.prv, key.pub, *system.work.generate (open1->hash ())));
@ -693,14 +687,22 @@ TEST (active_transactions, vote_replays)
ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote1_send2));
ASSERT_EQ (1, node.active.size ());
ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote1_send2));
while (!node.active.empty ())
{
ASSERT_NO_ERROR (system.poll ());
}
ASSERT_TIMELY (3s, node.active.empty ());
ASSERT_EQ (0, node.active.size ());
ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote1_send2));
ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote2_send2));
// Removing blocks as recently confirmed makes every vote indeterminate
{
nano::lock_guard<std::mutex> guard (node.active.mutex);
node.active.recently_confirmed.clear ();
}
ASSERT_EQ (nano::vote_code::indeterminate, node.active.vote (vote_send1));
ASSERT_EQ (nano::vote_code::indeterminate, node.active.vote (vote_open1));
ASSERT_EQ (nano::vote_code::indeterminate, node.active.vote (vote1_send2));
ASSERT_EQ (nano::vote_code::indeterminate, node.active.vote (vote2_send2));
}
}
TEST (active_transactions, activate_dependencies)
{
@ -846,7 +848,7 @@ TEST (active_transactions, confirmation_consistency)
}
nano::lock_guard<std::mutex> guard (node.active.mutex);
ASSERT_EQ (i + 1, node.active.recently_confirmed.size ());
ASSERT_EQ (block->qualified_root (), node.active.recently_confirmed.back ());
ASSERT_EQ (block->qualified_root (), node.active.recently_confirmed.back ().first);
ASSERT_EQ (i + 1, node.active.recently_cemented.size ());
}
}

View file

@ -517,9 +517,10 @@ std::pair<std::shared_ptr<nano::election>, bool> nano::active_transactions::inse
// Validate a vote and apply it to the current election if one exists
nano::vote_code nano::active_transactions::vote (std::shared_ptr<nano::vote> vote_a)
{
// If none of the hashes are active, it is unknown whether it's a replay
// In this case, votes are also not republished
// If none of the hashes are active, votes are not republished
bool at_least_one (false);
// If all hashes were recently confirmed then it is a replay
unsigned recently_confirmed_counter (0);
bool replay (false);
bool processed (false);
{
@ -527,6 +528,7 @@ nano::vote_code nano::active_transactions::vote (std::shared_ptr<nano::vote> vot
for (auto vote_block : vote_a->blocks)
{
nano::election_vote_result result;
auto & recently_confirmed_by_hash (recently_confirmed.get<tag_hash> ());
if (vote_block.which ())
{
auto block_hash (boost::get<nano::block_hash> (vote_block));
@ -536,10 +538,14 @@ nano::vote_code nano::active_transactions::vote (std::shared_ptr<nano::vote> vot
at_least_one = true;
result = existing->second->vote (vote_a->account, vote_a->sequence, block_hash);
}
else // possibly a vote for a recently confirmed election
else if (recently_confirmed_by_hash.count (block_hash) == 0)
{
add_inactive_votes_cache (block_hash, vote_a->account);
}
else
{
++recently_confirmed_counter;
}
}
else
{
@ -550,23 +556,33 @@ nano::vote_code nano::active_transactions::vote (std::shared_ptr<nano::vote> vot
at_least_one = true;
result = existing->election->vote (vote_a->account, vote_a->sequence, block->hash ());
}
else
else if (recently_confirmed_by_hash.count (block->hash ()) == 0)
{
add_inactive_votes_cache (block->hash (), vote_a->account);
}
else
{
++recently_confirmed_counter;
}
}
processed = processed || result.processed;
replay = replay || result.replay;
}
}
if (at_least_one)
{
// Republish vote if it is new and the node does not host a principal representative (or close to)
if (processed && !node.wallets.rep_counts ().have_half_rep ())
{
node.network.flood_vote (vote_a, 0.5f);
}
return replay ? nano::vote_code::replay : nano::vote_code::vote;
}
else if (recently_confirmed_counter == vote_a->blocks.size ())
{
return nano::vote_code::replay;
}
else
{
return nano::vote_code::indeterminate;
@ -792,10 +808,10 @@ void nano::active_transactions::add_recently_cemented (nano::election_status con
}
}
void nano::active_transactions::add_recently_confirmed (nano::qualified_root const & root_a)
void nano::active_transactions::add_recently_confirmed (nano::qualified_root const & root_a, nano::block_hash const & hash_a)
{
recently_confirmed.get<tag_sequence> ().push_back (root_a);
if (recently_confirmed.size () > node.config.confirmation_history_size)
recently_confirmed.get<tag_sequence> ().emplace_back (root_a, hash_a);
if (recently_confirmed.size () > recently_confirmed_size)
{
recently_confirmed.get<tag_sequence> ().pop_front ();
}

View file

@ -124,7 +124,7 @@ public:
std::deque<nano::election_status> recently_cemented;
void add_recently_cemented (nano::election_status const &);
void add_recently_confirmed (nano::qualified_root const &);
void add_recently_confirmed (nano::qualified_root const &, nano::block_hash const &);
void add_inactive_votes_cache (nano::block_hash const &, nano::account const &);
nano::inactive_cache_information find_inactive_votes_cache (nano::block_hash const &);
void erase_inactive_votes_cache (nano::block_hash const &);
@ -164,12 +164,16 @@ private:
// Maximum time an election can be kept active if it is extending the container
std::chrono::seconds const election_time_to_live;
static size_t constexpr recently_confirmed_size{ 65536 };
using recent_confirmation = std::pair<nano::qualified_root, nano::block_hash>;
// clang-format off
boost::multi_index_container<nano::qualified_root,
boost::multi_index_container<recent_confirmation,
mi::indexed_by<
mi::sequenced<mi::tag<tag_sequence>>,
mi::hashed_unique<mi::tag<tag_root>,
mi::identity<nano::qualified_root>>>>
mi::member<recent_confirmation, nano::qualified_root, &recent_confirmation::first>>,
mi::hashed_unique<mi::tag<tag_hash>,
mi::member<recent_confirmation, nano::block_hash, &recent_confirmation::second>>>>
recently_confirmed;
using prioritize_num_uncemented = boost::multi_index_container<nano::cementable_account,
mi::indexed_by<
@ -202,6 +206,7 @@ private:
boost::thread thread;
friend class active_transactions_dropped_cleanup_Test;
friend class active_transactions_vote_replays_Test;
friend class confirmation_height_prioritize_frontiers_Test;
friend class confirmation_height_prioritize_frontiers_overwrite_Test;
friend class active_transactions_confirmation_consistency_Test;

View file

@ -54,7 +54,7 @@ void nano::election::confirm_once (nano::election_status_type type_a)
// election winner details can be cleared consistently sucessfully in active_transactions::block_cemented_callback
node.active.add_election_winner_details (status_l.winner->hash (), this_l);
}
node.active.add_recently_confirmed (status_l.winner->qualified_root ());
node.active.add_recently_confirmed (status_l.winner->qualified_root (), status_l.winner->hash ());
node.background ([node_l, status_l, confirmation_action_l, this_l]() {
node_l->process_confirmed (status_l, this_l);
confirmation_action_l (status_l.winner);