From e77f9fb0d11a9944f48cef104ebe93b1ba4b5c0e Mon Sep 17 00:00:00 2001 From: Guilherme Lawless Date: Tue, 28 Apr 2020 12:03:04 +0100 Subject: [PATCH] Tally votes on conflicting block with no inactive votes Mostly applicable to tests, but consider this sequence of events: - An election gets created for a processed block - A vote arrives for a conflicting block; gets added to election, not inactive - The conflicting block gets processed Currently, `election::publish` calls `insert_inactive_votes_cache` but votes are not tallied since there was no inactive vote. This fixes the above situation by calling `confirm_if_quorum` if no votes were cached when a new conflicting block is inserted. --- nano/core_test/active_transactions.cpp | 30 +++++++++++++++++++++++++- nano/node/election.cpp | 11 +++++++--- nano/node/election.hpp | 2 +- 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/nano/core_test/active_transactions.cpp b/nano/core_test/active_transactions.cpp index 118910126b..b704f40d97 100644 --- a/nano/core_test/active_transactions.cpp +++ b/nano/core_test/active_transactions.cpp @@ -1088,4 +1088,32 @@ TEST (active_transactions, restart_dropped) ASSERT_EQ (2, node.stats.count (nano::stat::type::election, nano::stat::detail::election_restart)); // Wait for the election to complete ASSERT_TIMELY (5s, node.ledger.cache.cemented_count == 2); -} \ No newline at end of file +} + +// Ensures votes are tallied on election::publish even if no vote is inserted through inactive_votes_cache +TEST (active_transactions, conflicting_block_vote_existing_election) +{ + nano::system system; + nano::node_flags node_flags; + node_flags.disable_request_loop = true; + auto & node = *system.add_node (node_flags); + nano::genesis genesis; + nano::keypair key; + auto send (std::make_shared (nano::test_genesis_key.pub, genesis.hash (), nano::test_genesis_key.pub, nano::genesis_amount - 100, key.pub, nano::test_genesis_key.prv, nano::test_genesis_key.pub, *system.work.generate (genesis.hash ()))); + auto fork (std::make_shared (nano::test_genesis_key.pub, genesis.hash (), nano::test_genesis_key.pub, nano::genesis_amount - 200, key.pub, nano::test_genesis_key.prv, nano::test_genesis_key.pub, *system.work.generate (genesis.hash ()))); + auto vote_fork (std::make_shared (nano::test_genesis_key.pub, nano::test_genesis_key.prv, 0, fork)); + + ASSERT_EQ (nano::process_result::progress, node.process_local (send).code); + ASSERT_EQ (1, node.active.size ()); + + // Vote for conflicting block, but the block does not yet exist in the ledger + node.active.vote (vote_fork); + + // Block now gets processed + ASSERT_EQ (nano::process_result::fork, node.process_local (fork).code); + + // Election must be confirmed + auto election (node.active.election (fork->qualified_root ())); + ASSERT_NE (nullptr, election); + ASSERT_TRUE (election->confirmed ()); +} diff --git a/nano/node/election.cpp b/nano/node/election.cpp index a751f0b7e7..92e86a691b 100644 --- a/nano/node/election.cpp +++ b/nano/node/election.cpp @@ -481,7 +481,11 @@ bool nano::election::publish (std::shared_ptr block_a) if (existing == blocks.end ()) { blocks.emplace (std::make_pair (block_a->hash (), block_a)); - insert_inactive_votes_cache (block_a->hash ()); + if (!insert_inactive_votes_cache (block_a->hash ())) + { + // Even if no votes were in cache, they could be in the election + confirm_if_quorum (); + } node.network.flood_block (block_a, nano::buffer_drop_policy::no_limiter_drop); } else @@ -576,10 +580,10 @@ void nano::election::cleanup () } } -void nano::election::insert_inactive_votes_cache (nano::block_hash const & hash_a) +size_t nano::election::insert_inactive_votes_cache (nano::block_hash const & hash_a) { auto cache (node.active.find_inactive_votes_cache (hash_a)); - for (auto & rep : cache.voters) + for (auto const & rep : cache.voters) { auto inserted (last_votes.emplace (rep, nano::vote_info{ std::chrono::steady_clock::time_point::min (), 0, hash_a })); if (inserted.second) @@ -597,6 +601,7 @@ void nano::election::insert_inactive_votes_cache (nano::block_hash const & hash_ } confirm_if_quorum (); } + return cache.voters.size (); } bool nano::election::prioritized () const diff --git a/nano/node/election.hpp b/nano/node/election.hpp index 804d2b9b9e..6ceb7f298b 100644 --- a/nano/node/election.hpp +++ b/nano/node/election.hpp @@ -84,7 +84,7 @@ class election final : public std::enable_shared_from_this size_t last_votes_size (); void update_dependent (); void adjust_dependent_difficulty (); - void insert_inactive_votes_cache (nano::block_hash const &); + size_t insert_inactive_votes_cache (nano::block_hash const &); bool prioritized () const; void prioritize_election (nano::vote_generator_session &); // Erase all blocks from active and, if not confirmed, clear digests from network filters