From 3b22533c0ccd6578f52a3ea8f995c9e71f03d410 Mon Sep 17 00:00:00 2001 From: gr0vity-dev <85646666+gr0vity-dev@users.noreply.github.com> Date: Wed, 4 Oct 2023 13:55:36 +0200 Subject: [PATCH] Refactor block_cemented_callback for readability (#4306) - Modularized the block confirmation logic into separate functions. - Separated active and inactive block confirmation processes. - Simplified the main block_cemented_callback() function for easier understanding. - Reduced nested conditionals by early returning and using helper methods. - Improved code readability and maintainability. --- nano/node/active_transactions.cpp | 207 +++++++++++++++++++----------- nano/node/active_transactions.hpp | 10 ++ 2 files changed, 142 insertions(+), 75 deletions(-) diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index 05972cdc66..bffa0b6243 100644 --- a/nano/node/active_transactions.cpp +++ b/nano/node/active_transactions.cpp @@ -68,98 +68,155 @@ void nano::active_transactions::stop () void nano::active_transactions::block_cemented_callback (std::shared_ptr const & block_a) { auto transaction = node.store.tx_begin_read (); + auto status_type = election_status (transaction, block_a); - boost::optional election_status_type; - if (!confirmation_height_processor.is_processing_added_block (block_a->hash ())) + if (!status_type) + return; + + switch (*status_type) + { + 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; + } + + handle_final_votes_confirmation (block_a, transaction, *status_type); +} + +boost::optional nano::active_transactions::election_status (nano::store::read_transaction const & transaction, std::shared_ptr const & block) +{ + boost::optional status_type; + + if (!confirmation_height_processor.is_processing_added_block (block->hash ())) { - election_status_type = confirm_block (transaction, block_a); + status_type = confirm_block (transaction, block); } else { - // This block was explicitly added to the confirmation height_processor - election_status_type = nano::election_status_type::active_confirmed_quorum; + status_type = nano::election_status_type::active_confirmed_quorum; } - if (election_status_type.is_initialized ()) + return status_type; +} + +void nano::active_transactions::process_inactive_confirmation (nano::store::read_transaction const & transaction, std::shared_ptr const & block) +{ + nano::account account; + nano::uint128_t amount{ 0 }; + bool is_state_send = false; + bool is_state_epoch = false; + nano::account pending_account{}; + node.process_confirmed_data (transaction, block, block->hash (), account, amount, is_state_send, is_state_epoch, pending_account); + node.observers.blocks.notify (nano::election_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 }, {}, account, amount, is_state_send, is_state_epoch); +} + +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 ()) { - if (election_status_type == nano::election_status_type::inactive_confirmation_height) - { - nano::account account{}; - nano::uint128_t amount (0); - bool is_state_send (false); - bool is_state_epoch (false); - nano::account pending_account{}; - node.process_confirmed_data (transaction, block_a, block_a->hash (), account, amount, is_state_send, is_state_epoch, pending_account); - node.observers.blocks.notify (nano::election_status{ block_a, 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 }, {}, account, amount, is_state_send, is_state_epoch); - } - else + auto election = existing->second; + election_winner_details.erase (hash); + election_winners_lk.unlock (); + if (election->confirmed () && election->winner ()->hash () == hash) { - auto hash (block_a->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) - { - nano::unique_lock election_lk{ election->mutex }; - auto status_l = election->status; - election_lk.unlock (); - recently_cemented.put (status_l); - auto destination (block_a->link ().is_zero () ? block_a->destination () : block_a->link ().as_account ()); - node.receive_confirmed (transaction, hash, destination); - nano::account account{}; - nano::uint128_t amount (0); - bool is_state_send (false); - bool is_state_epoch (false); - nano::account pending_account{}; - node.process_confirmed_data (transaction, block_a, hash, account, amount, is_state_send, is_state_epoch, pending_account); - election_lk.lock (); - election->status.type = *election_status_type; - election->status.confirmation_request_count = election->confirmation_request_count; - status_l = election->status; - election_lk.unlock (); - auto votes (election->votes_with_weight ()); - node.observers.blocks.notify (status_l, votes, account, amount, is_state_send, is_state_epoch); - if (amount > 0) - { - node.observers.account_balance.notify (account, false); - if (!pending_account.is_zero ()) - { - node.observers.account_balance.notify (pending_account, true); - } - } - } - } + 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 (); + update_recently_cemented (election); + + nano::account account; + nano::uint128_t amount (0); + bool is_state_send = false; + bool is_state_epoch = false; + nano::account pending_account; + + handle_block_confirmation (transaction, block, hash, account, amount, is_state_send, is_state_epoch, pending_account); - auto const & account (!block_a->account ().is_zero () ? block_a->account () : block_a->sideband ().account); - debug_assert (!account.is_zero ()); - if (!node.ledger.cache.final_votes_confirmation_canary.load () && account == node.network_params.ledger.final_votes_canary_account && block_a->sideband ().height >= node.network_params.ledger.final_votes_canary_height) + update_election_status (election, status_type); + notify_observers (election, account, amount, is_state_send, is_state_epoch, pending_account); +} + +void nano::active_transactions::update_recently_cemented (std::shared_ptr const & election) +{ + nano::unique_lock election_lk{ election->mutex }; + recently_cemented.put (election->status); +} + +void nano::active_transactions::handle_block_confirmation (nano::store::read_transaction const & transaction, std::shared_ptr const & block, nano::block_hash const & hash, nano::account & account, nano::uint128_t & amount, bool & is_state_send, bool & is_state_epoch, nano::account & pending_account) +{ + auto destination = block->link ().is_zero () ? block->destination () : block->link ().as_account (); + node.receive_confirmed (transaction, hash, destination); + node.process_confirmed_data (transaction, block, hash, account, amount, is_state_send, is_state_epoch, pending_account); +} + +void nano::active_transactions::update_election_status (std::shared_ptr election, nano::election_status_type status_type) +{ + nano::unique_lock election_lk{ election->mutex }; + election->status.type = status_type; + election->status.confirmation_request_count = election->confirmation_request_count; +} + +void nano::active_transactions::notify_observers (std::shared_ptr const & election, nano::account const & account, nano::uint128_t amount, bool is_state_send, bool is_state_epoch, nano::account const & pending_account) +{ + auto status = election->status; + auto votes = election->votes_with_weight (); + + node.observers.blocks.notify (status, votes, account, amount, is_state_send, is_state_epoch); + + if (amount > 0) + { + node.observers.account_balance.notify (account, false); + if (!pending_account.is_zero ()) { - node.ledger.cache.final_votes_confirmation_canary.store (true); + node.observers.account_balance.notify (pending_account, true); } + } +} - // Next-block activations are done after cementing hardcoded bootstrap count to allow confirming very large chains without interference - bool const cemented_bootstrap_count_reached{ node.ledger.cache.cemented_count >= node.ledger.bootstrap_weight_max_blocks }; +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 const & account = !block->account ().is_zero () ? block->account () : block->sideband ().account; - // Next-block activations are only done for blocks with previously active elections - bool const was_active{ *election_status_type == nano::election_status_type::active_confirmed_quorum || *election_status_type == nano::election_status_type::active_confirmation_height }; + bool is_canary_not_set = !node.ledger.cache.final_votes_confirmation_canary.load (); + bool is_canary_account = account == node.network_params.ledger.final_votes_canary_account; + bool is_height_above_threshold = block->sideband ().height >= node.network_params.ledger.final_votes_canary_height; - if (cemented_bootstrap_count_reached && was_active) - { - // Start or vote for the next unconfirmed block - node.scheduler.priority.activate (account, transaction); + if (is_canary_not_set && is_canary_account && is_height_above_threshold) + { + node.ledger.cache.final_votes_confirmation_canary.store (true); + } - // Start or vote for the next unconfirmed block in the destination account - auto const & destination (node.ledger.block_destination (transaction, *block_a)); - if (!destination.is_zero () && destination != account) - { - node.scheduler.priority.activate (destination, transaction); - } - } + 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 (account, block, transaction); + } +} + +void nano::active_transactions::activate_successors (const nano::account & account, std::shared_ptr const & block, nano::store::read_transaction const & transaction) +{ + node.scheduler.priority.activate (account, transaction); + auto const & destination = node.ledger.block_destination (transaction, *block); + + // Start or vote for the next unconfirmed block in the destination account + if (!destination.is_zero () && destination != account) + { + node.scheduler.priority.activate (destination, transaction); } } diff --git a/nano/node/active_transactions.hpp b/nano/node/active_transactions.hpp index bf54078db6..d62e8631fb 100644 --- a/nano/node/active_transactions.hpp +++ b/nano/node/active_transactions.hpp @@ -197,6 +197,16 @@ class active_transactions final * TODO: Should be moved to `vote_cache` class */ void add_inactive_vote_cache (nano::block_hash const & hash, std::shared_ptr vote); + boost::optional election_status (nano::store::read_transaction const & transaction, 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 (const nano::account & account, std::shared_ptr const & block, nano::store::read_transaction const & transaction); + void update_recently_cemented (std::shared_ptr const & election); + void handle_block_confirmation (nano::store::read_transaction const & transaction, std::shared_ptr const & block, nano::block_hash const & hash, nano::account & account, nano::uint128_t & amount, bool & is_state_send, bool & is_state_epoch, nano::account & pending_account); + void update_election_status (std::shared_ptr election, nano::election_status_type status); + void notify_observers (std::shared_ptr const & election, nano::account const & account, nano::uint128_t amount, bool is_state_send, bool is_state_epoch, nano::account const & pending_account); private: // Dependencies nano::confirmation_height_processor & confirmation_height_processor;