Skip to content

Commit

Permalink
Move node::receive_confirmed to wallets::receive_confirmed (nanocurre…
Browse files Browse the repository at this point in the history
…ncy#4557)

This is inherently a wallet operation so it's more appropriate to be on the wallets class.
This also breaks a direct dependency/cycle from block processing to the wallet operations.
This also breaks a lock order inversion where a ledger transaction mutex is held while acquiring the wallet mutexes while most operations do it in the reverse order.
  • Loading branch information
clemahieu authored Apr 14, 2024
1 parent 804dcac commit 6f8e1d3
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 39 deletions.
41 changes: 4 additions & 37 deletions nano/node/node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -468,8 +468,9 @@ nano::node::node (std::shared_ptr<boost::asio::io_context> io_ctx_a, std::filesy
confirming_set.cemented_observers.add ([this] (auto const & block) {
if (block->is_send ())
{
auto transaction = store.tx_begin_read ();
receive_confirmed (transaction, block->hash (), block->destination ());
workers.push_task ([this, hash = block->hash (), destination = block->destination ()] () {
wallets.receive_confirmed (hash, destination);
});
}
});
}
Expand Down Expand Up @@ -1217,40 +1218,6 @@ void nano::node::ongoing_online_weight_calculation ()
ongoing_online_weight_calculation_queue ();
}

void nano::node::receive_confirmed (store::transaction const & block_transaction_a, nano::block_hash const & hash_a, nano::account const & destination_a)
{
nano::unique_lock<nano::mutex> lk{ wallets.mutex };
auto wallets_l = wallets.get_wallets ();
auto wallet_transaction = wallets.tx_begin_read ();
lk.unlock ();
for ([[maybe_unused]] auto const & [id, wallet] : wallets_l)
{
if (wallet->store.exists (wallet_transaction, destination_a))
{
nano::account representative;
representative = wallet->store.representative (wallet_transaction);
auto pending = ledger.pending_info (block_transaction_a, nano::pending_key (destination_a, hash_a));
if (pending)
{
auto amount (pending->amount.number ());
wallet->receive_async (hash_a, representative, amount, destination_a, [] (std::shared_ptr<nano::block> const &) {});
}
else
{
if (!ledger.block_or_pruned_exists (block_transaction_a, hash_a))
{
logger.warn (nano::log::type::node, "Confirmed block is missing: {}", hash_a.to_string ());
debug_assert (false, "Confirmed block is missing");
}
else
{
logger.warn (nano::log::type::node, "Block has already been received: {}", hash_a.to_string ());
}
}
}
}
}

void nano::node::process_confirmed (nano::election_status const & status_a, uint64_t iteration_a)
{
auto hash (status_a.winner->hash ());
Expand Down Expand Up @@ -1376,4 +1343,4 @@ std::string nano::node::make_logger_identifier (const nano::keypair & node_id)
{
// Node identifier consists of first 10 characters of node id
return node_id.pub.to_node_id ().substr (0, 10);
}
}
3 changes: 1 addition & 2 deletions nano/node/node.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ class node final : public std::enable_shared_from_this<node>
void stop ();
std::shared_ptr<nano::node> shared ();
int store_version ();
void receive_confirmed (store::transaction const & block_transaction_a, nano::block_hash const & hash_a, nano::account const & destination_a);
void process_confirmed (nano::election_status const &, uint64_t = 0);
void process_active (std::shared_ptr<nano::block> const &);
std::optional<nano::block_status> process_local (std::shared_ptr<nano::block> const &);
Expand Down Expand Up @@ -236,4 +235,4 @@ std::unique_ptr<container_info_component> collect_container_info (node & node, s

nano::node_flags const & inactive_node_flag_defaults ();

}
}
34 changes: 34 additions & 0 deletions nano/node/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1707,6 +1707,40 @@ void nano::wallets::ongoing_compute_reps ()
});
}

void nano::wallets::receive_confirmed (nano::block_hash const & hash_a, nano::account const & destination_a)
{
nano::unique_lock<nano::mutex> lk{ mutex };
auto wallets_l = get_wallets ();
auto wallet_transaction = tx_begin_read ();
lk.unlock ();
for ([[maybe_unused]] auto const & [id, wallet] : wallets_l)
{
if (wallet->store.exists (wallet_transaction, destination_a))
{
nano::account representative;
representative = wallet->store.representative (wallet_transaction);
auto pending = node.ledger.pending_info (node.ledger.store.tx_begin_read (), nano::pending_key (destination_a, hash_a));
if (pending)
{
auto amount (pending->amount.number ());
wallet->receive_async (hash_a, representative, amount, destination_a, [] (std::shared_ptr<nano::block> const &) {});
}
else
{
if (!node.ledger.block_or_pruned_exists (node.ledger.store.tx_begin_read (), hash_a))
{
node.logger.warn (nano::log::type::wallet, "Confirmed block is missing: {}", hash_a.to_string ());
debug_assert (false, "Confirmed block is missing");
}
else
{
node.logger.warn (nano::log::type::wallet, "Block has already been received: {}", hash_a.to_string ());
}
}
}
}
}

std::unordered_map<nano::wallet_id, std::shared_ptr<nano::wallet>> nano::wallets::get_wallets ()
{
debug_assert (!mutex.try_lock ());
Expand Down
1 change: 1 addition & 0 deletions nano/node/wallet.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ class wallets final
bool check_rep (nano::account const &, nano::uint128_t const &, bool const = true);
void compute_reps ();
void ongoing_compute_reps ();
void receive_confirmed (nano::block_hash const & hash_a, nano::account const & destination_a);
std::unordered_map<nano::wallet_id, std::shared_ptr<nano::wallet>> get_wallets ();
nano::network_params & network_params;
std::function<void (bool)> observer;
Expand Down

0 comments on commit 6f8e1d3

Please sign in to comment.