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

Encapsulate election mutex #4310

Merged
merged 3 commits into from
Oct 19, 2023
Merged

Encapsulate election mutex #4310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ef1927b
Encapsulate mutex of election class
gr0vity-dev Oct 9, 2023
9abd345
Update unit tests
gr0vity-dev Oct 9, 2023
8f1721a
remove the need to hold the lock during notify_observers
gr0vity-dev Oct 9, 2023
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
3 changes: 1 addition & 2 deletions nano/core_test/confirmation_solicitor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,7 @@ TEST (confirmation_solicitor, bypass_max_requests_cap)
// Add a vote for something else, not the winner
for (auto const & rep : representatives)
{
nano::lock_guard<nano::mutex> guard (election->mutex);
election->last_votes[rep.account] = { std::chrono::steady_clock::now (), 1, 1 };
election->set_last_vote (rep.account, { std::chrono::steady_clock::now (), 1, 1 });
}
ASSERT_FALSE (solicitor.add (*election));
ASSERT_FALSE (solicitor.broadcast (*election));
Expand Down
19 changes: 9 additions & 10 deletions nano/core_test/ledger.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1008,15 +1008,15 @@ TEST (votes, add_existing)
ASSERT_TIMELY (5s, node1.active.active (*send2));
auto vote2 (std::make_shared<nano::vote> (nano::dev::genesis_key.pub, nano::dev::genesis_key.prv, nano::vote::timestamp_min * 2, 0, std::vector<nano::block_hash>{ send2->hash () }));
// Pretend we've waited the timeout
nano::unique_lock<nano::mutex> lock{ election1->mutex };
election1->last_votes[nano::dev::genesis_key.pub].time = std::chrono::steady_clock::now () - std::chrono::seconds (20);
lock.unlock ();
auto vote_info1 = election1->get_last_vote (nano::dev::genesis_key.pub);
vote_info1.time = std::chrono::steady_clock::now () - std::chrono::seconds (20);
election1->set_last_vote (nano::dev::genesis_key.pub, vote_info1);
ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote2));
ASSERT_EQ (nano::vote::timestamp_min * 2, election1->last_votes[nano::dev::genesis_key.pub].timestamp);
// Also resend the old vote, and see if we respect the timestamp
lock.lock ();
election1->last_votes[nano::dev::genesis_key.pub].time = std::chrono::steady_clock::now () - std::chrono::seconds (20);
lock.unlock ();
auto vote_info2 = election1->get_last_vote (nano::dev::genesis_key.pub);
vote_info2.time = std::chrono::steady_clock::now () - std::chrono::seconds (20);
election1->set_last_vote (nano::dev::genesis_key.pub, vote_info2);
ASSERT_EQ (nano::vote_code::replay, node1.active.vote (vote1));
ASSERT_EQ (nano::vote::timestamp_min * 2, election1->votes ()[nano::dev::genesis_key.pub].timestamp);
auto votes (election1->votes ());
Expand Down Expand Up @@ -1061,10 +1061,9 @@ TEST (votes, add_old)
.build_shared ();
node1.work_generate_blocking (*send2);
auto vote2 = std::make_shared<nano::vote> (nano::dev::genesis_key.pub, nano::dev::genesis_key.prv, nano::vote::timestamp_min * 1, 0, std::vector<nano::block_hash>{ send2->hash () });
{
nano::lock_guard<nano::mutex> lock{ election1->mutex };
election1->last_votes[nano::dev::genesis_key.pub].time = std::chrono::steady_clock::now () - std::chrono::seconds (20);
}
auto vote_info = election1->get_last_vote (nano::dev::genesis_key.pub);
vote_info.time = std::chrono::steady_clock::now () - std::chrono::seconds (20);
election1->set_last_vote (nano::dev::genesis_key.pub, vote_info);
node1.vote_processor.vote_blocking (vote2, channel);
ASSERT_EQ (2, election1->votes ().size ());
auto votes (election1->votes ());
Expand Down
47 changes: 6 additions & 41 deletions nano/node/active_transactions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ void nano::active_transactions::process_active_confirmation (nano::store::read_t
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 ();
update_recently_cemented (election);
recently_cemented.put (election->get_status ());

nano::account account;
nano::uint128_t amount (0);
Expand All @@ -144,14 +144,9 @@ void nano::active_transactions::handle_confirmation (nano::store::read_transacti

handle_block_confirmation (transaction, block, hash, account, amount, is_state_send, is_state_epoch, pending_account);

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<nano::election> const & election)
{
nano::unique_lock<nano::mutex> election_lk{ election->mutex };
recently_cemented.put (election->status);
auto status = election->set_status_type (status_type);
auto votes = election->votes_with_weight ();
notify_observers (status, votes, account, amount, is_state_send, is_state_epoch, pending_account);
}

void nano::active_transactions::handle_block_confirmation (nano::store::read_transaction const & transaction, std::shared_ptr<nano::block> 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)
Expand All @@ -161,18 +156,8 @@ void nano::active_transactions::handle_block_confirmation (nano::store::read_tra
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<nano::election> election, nano::election_status_type status_type)
{
nano::unique_lock<nano::mutex> 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<nano::election> const & election, nano::account const & account, nano::uint128_t amount, bool is_state_send, bool is_state_epoch, nano::account const & pending_account)
void nano::active_transactions::notify_observers (nano::election_status const & status, std::vector<nano::vote_with_weight_info> const & votes, 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)
Expand Down Expand Up @@ -696,27 +681,7 @@ boost::optional<nano::election_status_type> nano::active_transactions::confirm_b
boost::optional<nano::election_status_type> status_type;
if (election)
{
nano::unique_lock<nano::mutex> election_lock{ election->mutex };
if (election->status.winner && election->status.winner->hash () == hash)
{
if (!election->confirmed ())
{
election->confirm_once (election_lock, nano::election_status_type::active_confirmation_height);
status_type = nano::election_status_type::active_confirmation_height;
}
else
{
#ifndef NDEBUG
nano::unique_lock<nano::mutex> election_winners_lk{ election_winner_details_mutex };
debug_assert (election_winner_details.find (hash) != election_winner_details.cend ());
#endif
status_type = nano::election_status_type::active_confirmed_quorum;
}
}
else
{
status_type = boost::optional<nano::election_status_type>{};
}
status_type = election->try_confirm (hash);
}
else
{
Expand Down
4 changes: 1 addition & 3 deletions nano/node/active_transactions.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,10 +203,8 @@ class active_transactions final
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 (const nano::account & account, std::shared_ptr<nano::block> const & block, nano::store::read_transaction const & transaction);
void update_recently_cemented (std::shared_ptr<nano::election> const & election);
void handle_block_confirmation (nano::store::read_transaction const & transaction, std::shared_ptr<nano::block> 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<nano::election> election, nano::election_status_type status);
void notify_observers (std::shared_ptr<nano::election> const & election, nano::account const & account, nano::uint128_t amount, bool is_state_send, bool is_state_epoch, nano::account const & pending_account);
void notify_observers (nano::election_status const & status, std::vector<nano::vote_with_weight_info> const & votes, 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;
Expand Down
53 changes: 53 additions & 0 deletions nano/node/election.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,24 @@ void nano::election::broadcast_vote ()
}
}

nano::vote_info nano::election::get_last_vote (nano::account const & account)
{
nano::lock_guard<nano::mutex> guard{ mutex };
return last_votes[account];
}

void nano::election::set_last_vote (nano::account const & account, nano::vote_info vote_info)
{
nano::lock_guard<nano::mutex> guard{ mutex };
last_votes[account] = vote_info;
}

nano::election_status nano::election::get_status () const
{
nano::lock_guard<nano::mutex> guard{ mutex };
return status;
}

bool nano::election::transition_time (nano::confirmation_solicitor & solicitor_a)
{
bool result = false;
Expand Down Expand Up @@ -356,6 +374,41 @@ void nano::election::confirm_if_quorum (nano::unique_lock<nano::mutex> & lock_a)
}
}

boost::optional<nano::election_status_type> 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 ())
{
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;
}
}
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;
}

void nano::election::log_votes (nano::tally_t const & tally_a, std::string const & prefix_a) const
{
std::stringstream tally;
Expand Down
5 changes: 5 additions & 0 deletions nano/node/election.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,12 +141,17 @@ 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);

/**
* Broadcasts vote for the current winner of this election
* Checks if sufficient amount of time (`vote_generation_interval`) passed since the last vote generation
*/
void broadcast_vote ();
nano::vote_info get_last_vote (nano::account const & account);
void set_last_vote (nano::account const & account, nano::vote_info vote_info);
nano::election_status get_status () const;

private: // Dependencies
nano::node & node;
Expand Down
Loading