diff --git a/nano/core_test/confirmation_height.cpp b/nano/core_test/confirmation_height.cpp index a150d40e..352d04d4 100644 --- a/nano/core_test/confirmation_height.cpp +++ b/nano/core_test/confirmation_height.cpp @@ -1444,8 +1444,9 @@ TEST (confirmation_height, pending_observer_callbacks) node->confirmation_height_processor.add (send1); - // Confirm the callback is not called under this circumstance because there is no election information - ASSERT_TIMELY (10s, node->stats.count (nano::stat::type::http_callback, nano::stat::detail::http_callback, nano::stat::dir::out) == 1 && node->ledger.stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::all, nano::stat::dir::out) == 1); + // Callback is performed for all blocks that are confirmed + ASSERT_TIMELY_EQ (5s, 2, node->stats.count (nano::stat::type::http_callback, nano::stat::detail::http_callback, nano::stat::dir::out)) + ASSERT_TIMELY_EQ (5s, 2, node->ledger.stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::all, nano::stat::dir::out)); ASSERT_EQ (2, node->stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed, nano::stat::dir::in)); ASSERT_EQ (2, node->stats.count (nano::stat::type::confirmation_height, get_stats_detail (mode_a), nano::stat::dir::in)); @@ -1528,7 +1529,8 @@ TEST (confirmation_height, callback_confirmed_history) ASSERT_TIMELY_EQ (10s, node->active.size (), 0); ASSERT_TIMELY_EQ (10s, node->stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::active_quorum, nano::stat::dir::out), 1); - ASSERT_EQ (1, node->active.recently_cemented.list ().size ()); + // Each block that's confirmed is in the recently_cemented history + ASSERT_EQ (2, node->active.recently_cemented.list ().size ()); ASSERT_TRUE (node->active.empty ()); // Confirm the callback is not called under this circumstance diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index d9d88918..350b4c28 100644 --- a/nano/node/active_transactions.cpp +++ b/nano/node/active_transactions.cpp @@ -80,75 +80,44 @@ void nano::active_transactions::stop () clear (); } -void nano::active_transactions::block_cemented_callback (std::shared_ptr const & block_a) +void nano::active_transactions::block_cemented_callback (std::shared_ptr const & block) { - auto status_type = election_status (block_a); - - if (!status_type) - return; - - auto transaction = node.store.tx_begin_read (); - switch (*status_type) + if (auto election_l = election (block->qualified_root ())) { - case nano::election_status_type::inactive_confirmation_height: - process_inactive_confirmation (transaction, block_a); - break; - - default: - process_active_confirmation (transaction, block_a, *status_type); - break; + election_l->try_confirm (block->hash ()); } - - handle_final_votes_confirmation (block_a, transaction, *status_type); -} - -boost::optional nano::active_transactions::election_status (std::shared_ptr const & block) -{ - boost::optional status_type; - - if (!confirmation_height_processor.is_processing_added_block (block->hash ())) + auto election = remove_election_winner_details (block->hash ()); + nano::election_status status; + std::vector votes; + status.winner = block; + if (election) { - status_type = confirm_block (block); + status = election->get_status (); + votes = election->votes_with_weight (); + } + if (confirmation_height_processor.is_processing_added_block (block->hash ())) + { + status.type = nano::election_status_type::active_confirmed_quorum; + } + else if (election) + { + status.type = nano::election_status_type::active_confirmation_height; } else { - status_type = nano::election_status_type::active_confirmed_quorum; + status.type = nano::election_status_type::inactive_confirmation_height; } - - return status_type; -} - -void nano::active_transactions::process_inactive_confirmation (nano::store::read_transaction const & transaction, std::shared_ptr const & block) -{ - nano::election_status status{ block, 0, 0, std::chrono::duration_cast (std::chrono::system_clock::now ().time_since_epoch ()), std::chrono::duration_values::zero (), 0, 1, 0, nano::election_status_type::inactive_confirmation_height }; - notify_observers (transaction, status, {}); -} - -void nano::active_transactions::process_active_confirmation (nano::store::read_transaction const & transaction, std::shared_ptr const & block, nano::election_status_type status_type) -{ - auto hash (block->hash ()); - nano::unique_lock election_winners_lk{ election_winner_details_mutex }; - auto existing = election_winner_details.find (hash); - if (existing != election_winner_details.end ()) - { - auto election = existing->second; - election_winner_details.erase (hash); - election_winners_lk.unlock (); - if (election->confirmed () && election->winner ()->hash () == hash) - { - handle_confirmation (transaction, block, election, status_type); - } - } -} - -void nano::active_transactions::handle_confirmation (nano::store::read_transaction const & transaction, std::shared_ptr const & block, std::shared_ptr election, nano::election_status_type status_type) -{ - nano::block_hash hash = block->hash (); - recently_cemented.put (election->get_status ()); - - auto status = election->set_status_type (status_type); - auto votes = election->votes_with_weight (); + recently_cemented.put (status); + auto transaction = node.store.tx_begin_read (); notify_observers (transaction, status, votes); + bool cemented_bootstrap_count_reached = node.ledger.cache.cemented_count >= node.ledger.bootstrap_weight_max_blocks; + bool was_active = status.type == nano::election_status_type::active_confirmed_quorum || status.type == nano::election_status_type::active_confirmation_height; + + // Next-block activations are only done for blocks with previously active elections + if (cemented_bootstrap_count_reached && was_active) + { + activate_successors (transaction, block); + } } void nano::active_transactions::notify_observers (nano::store::read_transaction const & transaction, nano::election_status const & status, std::vector const & votes) @@ -170,19 +139,6 @@ void nano::active_transactions::notify_observers (nano::store::read_transaction } } -void nano::active_transactions::handle_final_votes_confirmation (std::shared_ptr const & block, nano::store::read_transaction const & transaction, nano::election_status_type status) -{ - auto account = block->account (); - bool cemented_bootstrap_count_reached = node.ledger.cache.cemented_count >= node.ledger.bootstrap_weight_max_blocks; - bool was_active = status == nano::election_status_type::active_confirmed_quorum || status == nano::election_status_type::active_confirmation_height; - - // Next-block activations are only done for blocks with previously active elections - if (cemented_bootstrap_count_reached && was_active) - { - activate_successors (transaction, block); - } -} - void nano::active_transactions::activate_successors (nano::store::read_transaction const & transaction, std::shared_ptr const & block) { node.scheduler.priority.activate (block->account (), transaction); @@ -200,10 +156,17 @@ void nano::active_transactions::add_election_winner_details (nano::block_hash co election_winner_details.emplace (hash_a, election_a); } -void nano::active_transactions::remove_election_winner_details (nano::block_hash const & hash_a) +std::shared_ptr nano::active_transactions::remove_election_winner_details (nano::block_hash const & hash_a) { nano::lock_guard guard{ election_winner_details_mutex }; - election_winner_details.erase (hash_a); + std::shared_ptr result; + auto existing = election_winner_details.find (hash_a); + if (existing != election_winner_details.end ()) + { + result = existing->second; + election_winner_details.erase (existing); + } + return result; } void nano::active_transactions::block_already_cemented_callback (nano::block_hash const & hash_a) @@ -653,33 +616,6 @@ bool nano::active_transactions::publish (std::shared_ptr const & bl return result; } -// Returns the type of election status requiring callbacks calling later -boost::optional nano::active_transactions::confirm_block (std::shared_ptr const & block_a) -{ - auto const hash = block_a->hash (); - std::shared_ptr election = nullptr; - { - nano::lock_guard guard{ mutex }; - auto existing = blocks.find (hash); - if (existing != blocks.end ()) - { - election = existing->second; - } - } - - boost::optional status_type; - if (election) - { - status_type = election->try_confirm (hash); - } - else - { - status_type = nano::election_status_type::inactive_confirmation_height; - } - - return status_type; -} - void nano::active_transactions::add_vote_cache (nano::block_hash const & hash, std::shared_ptr const vote) { if (node.ledger.weight (vote->account) > node.minimum_principal_weight ()) diff --git a/nano/node/active_transactions.hpp b/nano/node/active_transactions.hpp index 9f56d5ca..7323bf51 100644 --- a/nano/node/active_transactions.hpp +++ b/nano/node/active_transactions.hpp @@ -160,7 +160,6 @@ public: bool empty () const; std::size_t size () const; bool publish (std::shared_ptr const &); - boost::optional confirm_block (std::shared_ptr const &); void block_cemented_callback (std::shared_ptr const &); void block_already_cemented_callback (nano::block_hash const &); @@ -177,7 +176,7 @@ public: std::size_t election_winner_details_size (); void add_election_winner_details (nano::block_hash const &, std::shared_ptr const &); - void remove_election_winner_details (nano::block_hash const &); + std::shared_ptr remove_election_winner_details (nano::block_hash const &); private: // Erase elections if we're over capacity @@ -195,11 +194,6 @@ private: * TODO: Should be moved to `vote_cache` class */ void add_vote_cache (nano::block_hash const & hash, std::shared_ptr vote); - boost::optional election_status (std::shared_ptr const & block); - void process_inactive_confirmation (nano::store::read_transaction const & transaction, std::shared_ptr const & block); - void process_active_confirmation (nano::store::read_transaction const & transaction, std::shared_ptr const & block, nano::election_status_type status); - void handle_final_votes_confirmation (std::shared_ptr const & block, nano::store::read_transaction const & transaction, nano::election_status_type status); - void handle_confirmation (nano::store::read_transaction const & transaction, std::shared_ptr const & block, std::shared_ptr election, nano::election_status_type status); void activate_successors (nano::store::read_transaction const & transaction, std::shared_ptr const & block); void notify_observers (nano::store::read_transaction const & transaction, nano::election_status const & status, std::vector const & votes); diff --git a/nano/node/election.cpp b/nano/node/election.cpp index d179dabd..a41136e4 100644 --- a/nano/node/election.cpp +++ b/nano/node/election.cpp @@ -34,7 +34,7 @@ nano::election::election (nano::node & node_a, std::shared_ptr cons last_blocks.emplace (block_a->hash (), block_a); } -void nano::election::confirm_once (nano::unique_lock & lock_a, nano::election_status_type type_a) +void nano::election::confirm_once (nano::unique_lock & lock_a) { debug_assert (lock_a.owns_lock ()); @@ -51,7 +51,6 @@ void nano::election::confirm_once (nano::unique_lock & lock_a, nano status.confirmation_request_count = confirmation_request_count; status.block_count = nano::narrow_cast (last_blocks.size ()); status.voter_count = nano::narrow_cast (last_votes.size ()); - status.type = type_a; auto const status_l = status; node.active.recently_confirmed.put (qualified_root, status_l.winner->hash ()); @@ -403,44 +402,22 @@ void nano::election::confirm_if_quorum (nano::unique_lock & lock_a) } if (final_weight >= node.online_reps.delta ()) { - confirm_once (lock_a, nano::election_status_type::active_confirmed_quorum); + confirm_once (lock_a); } } } -boost::optional nano::election::try_confirm (nano::block_hash const & hash) +void nano::election::try_confirm (nano::block_hash const & hash) { - boost::optional status_type; nano::unique_lock election_lock{ mutex }; auto winner = status.winner; if (winner && winner->hash () == hash) { - // Determine if the block was confirmed explicitly via election confirmation or implicitly via confirmation height if (!confirmed_locked ()) { - confirm_once (election_lock, nano::election_status_type::active_confirmation_height); - status_type = nano::election_status_type::active_confirmation_height; - } - else - { - status_type = nano::election_status_type::active_confirmed_quorum; + confirm_once (election_lock); } } - else - { - status_type = boost::optional{}; - } - return status_type; -} - -nano::election_status nano::election::set_status_type (nano::election_status_type status_type) -{ - nano::unique_lock election_lk{ mutex }; - status.type = status_type; - status.confirmation_request_count = confirmation_request_count; - nano::election_status status_l{ status }; - election_lk.unlock (); - return status_l; } std::shared_ptr nano::election::find (nano::block_hash const & hash_a) const @@ -709,11 +686,11 @@ bool nano::election::replace_by_weight (nano::unique_lock & lock_a, return replaced; } -void nano::election::force_confirm (nano::election_status_type type_a) +void nano::election::force_confirm () { release_assert (node.network_params.network.is_dev_network ()); nano::unique_lock lock{ mutex }; - confirm_once (lock, type_a); + confirm_once (lock); } std::unordered_map> nano::election::blocks () const diff --git a/nano/node/election.hpp b/nano/node/election.hpp index 63e75827..59cf2ea9 100644 --- a/nano/node/election.hpp +++ b/nano/node/election.hpp @@ -146,8 +146,7 @@ public: // Interface bool publish (std::shared_ptr const & block_a); // Confirm this block if quorum is met void confirm_if_quorum (nano::unique_lock &); - boost::optional try_confirm (nano::block_hash const & hash); - nano::election_status set_status_type (nano::election_status_type status_type); + void try_confirm (nano::block_hash const & hash); /** * Broadcasts vote for the current winner of this election @@ -173,7 +172,7 @@ private: bool confirmed_locked () const; nano::election_extended_status current_status_locked () const; // lock_a does not own the mutex on return - void confirm_once (nano::unique_lock & lock_a, nano::election_status_type = nano::election_status_type::active_confirmed_quorum); + void confirm_once (nano::unique_lock & lock_a); bool broadcast_block_predicate () const; void broadcast_block (nano::confirmation_solicitor &); void send_confirm_req (nano::confirmation_solicitor &); @@ -217,7 +216,7 @@ private: // Constants friend class confirmation_solicitor; public: // Only used in tests - void force_confirm (nano::election_status_type = nano::election_status_type::active_confirmed_quorum); + void force_confirm (); std::unordered_map votes () const; std::unordered_map> blocks () const; diff --git a/nano/secure/common.hpp b/nano/secure/common.hpp index 0928289f..d585e1b2 100644 --- a/nano/secure/common.hpp +++ b/nano/secure/common.hpp @@ -360,14 +360,14 @@ class election_status final { public: std::shared_ptr winner; - nano::amount tally; - nano::amount final_tally; - std::chrono::milliseconds election_end; - std::chrono::milliseconds election_duration; - unsigned confirmation_request_count; - unsigned block_count; - unsigned voter_count; - election_status_type type; + nano::amount tally{ 0 }; + nano::amount final_tally{ 0 }; + std::chrono::milliseconds election_end{ std::chrono::duration_cast (std::chrono::system_clock::now ().time_since_epoch ()) }; + std::chrono::milliseconds election_duration{ std::chrono::duration_values::zero () }; + unsigned confirmation_request_count{ 0 }; + unsigned block_count{ 0 }; + unsigned voter_count{ 0 }; + election_status_type type{ nano::election_status_type::inactive_confirmation_height }; }; nano::wallet_id random_wallet_id ();