Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Block cemented callback simplify #4508

Merged
merged 2 commits into from
Mar 20, 2024
Merged

Block cemented callback simplify #4508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
152f444
Simplify active_transactions::block_cemented_callback
clemahieu Mar 20, 2024
e4b4917
Remove unused code related to trying to externally set election status
clemahieu Mar 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions nano/core_test/confirmation_height.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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
Expand Down
136 changes: 36 additions & 100 deletions nano/node/active_transactions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,77 +80,46 @@ void nano::active_transactions::stop ()
clear ();
}

void nano::active_transactions::block_cemented_callback (std::shared_ptr<nano::block> const & block_a)
void nano::active_transactions::block_cemented_callback (std::shared_ptr<nano::block> 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::election_status_type> nano::active_transactions::election_status (std::shared_ptr<nano::block> const & block)
{
boost::optional<nano::election_status_type> 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<nano::vote_with_weight_info> 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;
}
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;

return status_type;
}

void nano::active_transactions::process_inactive_confirmation (nano::store::read_transaction const & transaction, std::shared_ptr<nano::block> const & block)
{
nano::election_status status{ block, 0, 0, std::chrono::duration_cast<std::chrono::milliseconds> (std::chrono::system_clock::now ().time_since_epoch ()), std::chrono::duration_values<std::chrono::milliseconds>::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<nano::block> const & block, nano::election_status_type status_type)
{
auto hash (block->hash ());
nano::unique_lock<nano::mutex> election_winners_lk{ election_winner_details_mutex };
auto existing = election_winner_details.find (hash);
if (existing != election_winner_details.end ())
// Next-block activations are only done for blocks with previously active elections
if (cemented_bootstrap_count_reached && was_active)
{
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);
}
activate_successors (transaction, block);
}
}

void nano::active_transactions::handle_confirmation (nano::store::read_transaction const & transaction, std::shared_ptr<nano::block> const & block, std::shared_ptr<nano::election> 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 ();
notify_observers (transaction, status, votes);
}

void nano::active_transactions::notify_observers (nano::store::read_transaction const & transaction, nano::election_status const & status, std::vector<nano::vote_with_weight_info> const & votes)
{
auto block = status.winner;
Expand All @@ -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<nano::block> 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<nano::block> const & block)
{
node.scheduler.priority.activate (block->account (), transaction);
Expand All @@ -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::election> nano::active_transactions::remove_election_winner_details (nano::block_hash const & hash_a)
{
nano::lock_guard<nano::mutex> guard{ election_winner_details_mutex };
election_winner_details.erase (hash_a);
std::shared_ptr<nano::election> 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)
Expand Down Expand Up @@ -653,33 +616,6 @@ bool nano::active_transactions::publish (std::shared_ptr<nano::block> const & bl
return result;
}

// Returns the type of election status requiring callbacks calling later
boost::optional<nano::election_status_type> nano::active_transactions::confirm_block (std::shared_ptr<nano::block> const & block_a)
{
auto const hash = block_a->hash ();
std::shared_ptr<nano::election> election = nullptr;
{
nano::lock_guard<nano::mutex> guard{ mutex };
auto existing = blocks.find (hash);
if (existing != blocks.end ())
{
election = existing->second;
}
}

boost::optional<nano::election_status_type> 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<nano::vote> const vote)
{
if (node.ledger.weight (vote->account) > node.minimum_principal_weight ())
Expand Down
8 changes: 1 addition & 7 deletions nano/node/active_transactions.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,6 @@ class active_transactions final
bool empty () const;
std::size_t size () const;
bool publish (std::shared_ptr<nano::block> const &);
boost::optional<nano::election_status_type> confirm_block (std::shared_ptr<nano::block> const &);
void block_cemented_callback (std::shared_ptr<nano::block> const &);
void block_already_cemented_callback (nano::block_hash const &);

Expand All @@ -177,7 +176,7 @@ class active_transactions final

std::size_t election_winner_details_size ();
void add_election_winner_details (nano::block_hash const &, std::shared_ptr<nano::election> const &);
void remove_election_winner_details (nano::block_hash const &);
std::shared_ptr<nano::election> remove_election_winner_details (nano::block_hash const &);

private:
// Erase elections if we're over capacity
Expand All @@ -195,11 +194,6 @@ class active_transactions final
* TODO: Should be moved to `vote_cache` class
*/
void add_vote_cache (nano::block_hash const & hash, std::shared_ptr<nano::vote> vote);
boost::optional<nano::election_status_type> election_status (std::shared_ptr<nano::block> const & block);
void process_inactive_confirmation (nano::store::read_transaction const & transaction, std::shared_ptr<nano::block> const & block);
void process_active_confirmation (nano::store::read_transaction const & transaction, std::shared_ptr<nano::block> const & block, nano::election_status_type status);
void handle_final_votes_confirmation (std::shared_ptr<nano::block> 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<nano::block> const & block, std::shared_ptr<nano::election> election, nano::election_status_type status);
void activate_successors (nano::store::read_transaction const & transaction, std::shared_ptr<nano::block> const & block);
void notify_observers (nano::store::read_transaction const & transaction, nano::election_status const & status, std::vector<nano::vote_with_weight_info> const & votes);

Expand Down
35 changes: 6 additions & 29 deletions nano/node/election.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ nano::election::election (nano::node & node_a, std::shared_ptr<nano::block> cons
last_blocks.emplace (block_a->hash (), block_a);
}

void nano::election::confirm_once (nano::unique_lock<nano::mutex> & lock_a, nano::election_status_type type_a)
void nano::election::confirm_once (nano::unique_lock<nano::mutex> & lock_a)
{
debug_assert (lock_a.owns_lock ());

Expand All @@ -51,7 +51,6 @@ void nano::election::confirm_once (nano::unique_lock<nano::mutex> & lock_a, nano
status.confirmation_request_count = confirmation_request_count;
status.block_count = nano::narrow_cast<decltype (status.block_count)> (last_blocks.size ());
status.voter_count = nano::narrow_cast<decltype (status.voter_count)> (last_votes.size ());
status.type = type_a;
auto const status_l = status;

node.active.recently_confirmed.put (qualified_root, status_l.winner->hash ());
Expand Down Expand Up @@ -403,44 +402,22 @@ void nano::election::confirm_if_quorum (nano::unique_lock<nano::mutex> & 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_status_type> nano::election::try_confirm (nano::block_hash const & hash)
void nano::election::try_confirm (nano::block_hash const & hash)
{
boost::optional<nano::election_status_type> status_type;
nano::unique_lock<nano::mutex> 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<nano::election_status_type>{};
}
return status_type;
}

nano::election_status nano::election::set_status_type (nano::election_status_type status_type)
{
nano::unique_lock<nano::mutex> 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::block> nano::election::find (nano::block_hash const & hash_a) const
Expand Down Expand Up @@ -709,11 +686,11 @@ bool nano::election::replace_by_weight (nano::unique_lock<nano::mutex> & 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<nano::mutex> lock{ mutex };
confirm_once (lock, type_a);
confirm_once (lock);
}

std::unordered_map<nano::block_hash, std::shared_ptr<nano::block>> nano::election::blocks () const
Expand Down
7 changes: 3 additions & 4 deletions nano/node/election.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,7 @@ class election final : public std::enable_shared_from_this<nano::election>
bool publish (std::shared_ptr<nano::block> const & block_a);
// Confirm this block if quorum is met
void confirm_if_quorum (nano::unique_lock<nano::mutex> &);
boost::optional<nano::election_status_type> 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
Expand All @@ -173,7 +172,7 @@ class election final : public std::enable_shared_from_this<nano::election>
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<nano::mutex> & lock_a, nano::election_status_type = nano::election_status_type::active_confirmed_quorum);
void confirm_once (nano::unique_lock<nano::mutex> & lock_a);
bool broadcast_block_predicate () const;
void broadcast_block (nano::confirmation_solicitor &);
void send_confirm_req (nano::confirmation_solicitor &);
Expand Down Expand Up @@ -217,7 +216,7 @@ class election final : public std::enable_shared_from_this<nano::election>
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<nano::account, nano::vote_info> votes () const;
std::unordered_map<nano::block_hash, std::shared_ptr<nano::block>> blocks () const;

Expand Down
16 changes: 8 additions & 8 deletions nano/secure/common.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,14 +360,14 @@ class election_status final
{
public:
std::shared_ptr<nano::block> 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::milliseconds> (std::chrono::system_clock::now ().time_since_epoch ()) };
std::chrono::milliseconds election_duration{ std::chrono::duration_values<std::chrono::milliseconds>::zero () };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't this be just { 0 } or {}?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't check, I can't quite remember the rules for default initialization versus being uninitialized, this just seems more explicit.

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 ();
Expand Down
Loading