From ab4e51de8fdd85ae8b88f06eb21c2a34f508ff97 Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Tue, 19 Mar 2024 10:24:02 +0000 Subject: [PATCH 1/7] Add block::is_epoch query --- nano/lib/blocks.cpp | 12 ++++++++++++ nano/lib/blocks.hpp | 1 + 2 files changed, 13 insertions(+) diff --git a/nano/lib/blocks.cpp b/nano/lib/blocks.cpp index 060681ac12..9d38cd404f 100644 --- a/nano/lib/blocks.cpp +++ b/nano/lib/blocks.cpp @@ -162,6 +162,18 @@ bool nano::block::is_change () const noexcept } } +bool nano::block::is_epoch () const noexcept +{ + release_assert (has_sideband ()); + switch (type ()) + { + case nano::block_type::state: + return sideband ().details.is_epoch; + default: + return false; + } +} + nano::block_hash const & nano::block::hash () const { if (!cached_hash.is_zero ()) diff --git a/nano/lib/blocks.hpp b/nano/lib/blocks.hpp index d6cca74374..99abb9743d 100644 --- a/nano/lib/blocks.hpp +++ b/nano/lib/blocks.hpp @@ -55,6 +55,7 @@ class block bool is_send () const noexcept; bool is_receive () const noexcept; bool is_change () const noexcept; + bool is_epoch () const noexcept; public: // Direct access to the block fields or nullopt if the block type does not have the specified field // Returns account field or account from sideband From 328c340529aee3ea65f3b464b767a740a21e416a Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Tue, 19 Mar 2024 10:45:16 +0000 Subject: [PATCH 2/7] Use active_transactions::notify_observers for both active/inactive confirmation. Previously the account_balance observers would not be notified if a block was indirectly confirmed without an election --- nano/node/active_transactions.cpp | 3 ++- nano/node/node_observers.hpp | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index f2b9b61eb0..7620cb2f59 100644 --- a/nano/node/active_transactions.cpp +++ b/nano/node/active_transactions.cpp @@ -126,7 +126,8 @@ void nano::active_transactions::process_inactive_confirmation (nano::store::read 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); + 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 (status, {}, account, amount, is_state_send, is_state_epoch, pending_account); } void nano::active_transactions::process_active_confirmation (nano::store::read_transaction const & transaction, std::shared_ptr const & block, nano::election_status_type status_type) diff --git a/nano/node/node_observers.hpp b/nano/node/node_observers.hpp index 5526c957c4..8825655c5d 100644 --- a/nano/node/node_observers.hpp +++ b/nano/node/node_observers.hpp @@ -12,7 +12,7 @@ class node_observers final { public: using blocks_t = nano::observer_set const &, nano::account const &, nano::uint128_t const &, bool, bool>; - blocks_t blocks; + blocks_t blocks; // Notification upon election completion or cancellation nano::observer_set wallet; nano::observer_set, std::shared_ptr, nano::vote_code> vote; nano::observer_set active_started; From c3b3614223baa1f738806d0fec61b53581940628 Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Tue, 19 Mar 2024 11:08:25 +0000 Subject: [PATCH 3/7] Inlining single usage of handle_block_confirmation in to caller. --- nano/node/active_transactions.cpp | 13 ++++--------- nano/node/active_transactions.hpp | 1 - 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index 7620cb2f59..8ec3815d71 100644 --- a/nano/node/active_transactions.cpp +++ b/nano/node/active_transactions.cpp @@ -158,20 +158,15 @@ void nano::active_transactions::handle_confirmation (nano::store::read_transacti 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 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 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) -{ if (block->is_send ()) { node.receive_confirmed (transaction, hash, block->destination ()); } node.process_confirmed_data (transaction, block, hash, account, amount, is_state_send, is_state_epoch, pending_account); + + 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::notify_observers (nano::election_status const & status, std::vector const & votes, nano::account const & account, nano::uint128_t amount, bool is_state_send, bool is_state_epoch, nano::account const & pending_account) diff --git a/nano/node/active_transactions.hpp b/nano/node/active_transactions.hpp index c2b945a9a9..791054879a 100644 --- a/nano/node/active_transactions.hpp +++ b/nano/node/active_transactions.hpp @@ -201,7 +201,6 @@ class active_transactions final 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 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 notify_observers (nano::election_status const & status, std::vector 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 From 37a27400f1d40ddb9806cd9f02f057a37bf760d6 Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Tue, 19 Mar 2024 11:18:49 +0000 Subject: [PATCH 4/7] Remove node::process_confirmed_data which only served as a helper for active_transactions. This data can now be easily obtained directly from block objects. --- nano/node/active_transactions.cpp | 29 ++++++++-------------- nano/node/active_transactions.hpp | 2 +- nano/node/node.cpp | 41 ------------------------------- nano/node/node.hpp | 1 - 4 files changed, 11 insertions(+), 62 deletions(-) diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index 8ec3815d71..6b001637f6 100644 --- a/nano/node/active_transactions.cpp +++ b/nano/node/active_transactions.cpp @@ -120,14 +120,8 @@ boost::optional nano::active_transactions::election_ 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); 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 (status, {}, account, amount, is_state_send, is_state_epoch, pending_account); + 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) @@ -152,33 +146,30 @@ void nano::active_transactions::handle_confirmation (nano::store::read_transacti nano::block_hash hash = block->hash (); recently_cemented.put (election->get_status ()); - nano::account account; - nano::uint128_t amount (0); - bool is_state_send = false; - bool is_state_epoch = false; - nano::account pending_account; - if (block->is_send ()) { node.receive_confirmed (transaction, hash, block->destination ()); } - node.process_confirmed_data (transaction, block, hash, account, amount, is_state_send, is_state_epoch, pending_account); - 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); + notify_observers (transaction, status, votes); } -void nano::active_transactions::notify_observers (nano::election_status const & status, std::vector const & votes, 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::store::read_transaction const & transaction, nano::election_status const & status, std::vector const & votes) { + auto block = status.winner; + auto account = block->account (); + auto amount = node.ledger.amount (transaction, block->hash ()).value_or (0); + auto is_state_send = block->type () == block_type::state && block->is_send (); + auto is_state_epoch = block->type () == block_type::state && block->is_epoch (); 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 ()) + if (block->is_send ()) { - node.observers.account_balance.notify (pending_account, true); + node.observers.account_balance.notify (block->destination (), true); } } } diff --git a/nano/node/active_transactions.hpp b/nano/node/active_transactions.hpp index 791054879a..e1c4a5f8e2 100644 --- a/nano/node/active_transactions.hpp +++ b/nano/node/active_transactions.hpp @@ -201,7 +201,7 @@ class active_transactions final 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 notify_observers (nano::election_status const & status, std::vector const & votes, 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::store::read_transaction const & transaction, nano::election_status const & status, std::vector const & votes); private: // Dependencies nano::node & node; diff --git a/nano/node/node.cpp b/nano/node/node.cpp index 054e3669f7..c169e8aef7 100644 --- a/nano/node/node.cpp +++ b/nano/node/node.cpp @@ -1212,47 +1212,6 @@ void nano::node::receive_confirmed (store::transaction const & block_transaction } } -void nano::node::process_confirmed_data (store::transaction const & transaction_a, std::shared_ptr const & block_a, nano::block_hash const & hash_a, nano::account & account_a, nano::uint128_t & amount_a, bool & is_state_send_a, bool & is_state_epoch_a, nano::account & pending_account_a) -{ - // Faster account calculation - account_a = block_a->account (); - // Faster amount calculation - auto previous (block_a->previous ()); - auto previous_balance = ledger.balance (transaction_a, previous); - auto block_balance = block_a->balance (); - if (hash_a != ledger.constants.genesis->account ()) - { - if (previous_balance) - { - amount_a = block_balance > previous_balance.value () ? block_balance.number () - previous_balance.value () : previous_balance.value () - block_balance.number (); - } - else - { - amount_a = 0; - } - } - else - { - amount_a = nano::dev::constants.genesis_amount; - } - if (auto state = dynamic_cast (block_a.get ())) - { - if (state->hashables.balance < previous_balance) - { - is_state_send_a = true; - } - if (amount_a == 0 && network_params.ledger.epochs.is_epoch_link (state->link_field ().value ())) - { - is_state_epoch_a = true; - } - pending_account_a = state->hashables.link.as_account (); - } - if (auto send = dynamic_cast (block_a.get ())) - { - pending_account_a = send->hashables.destination; - } -} - void nano::node::process_confirmed (nano::election_status const & status_a, uint64_t iteration_a) { auto hash (status_a.winner->hash ()); diff --git a/nano/node/node.hpp b/nano/node/node.hpp index 65ad016ab1..0b42593166 100644 --- a/nano/node/node.hpp +++ b/nano/node/node.hpp @@ -83,7 +83,6 @@ class node final : public std::enable_shared_from_this std::shared_ptr 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_data (store::transaction const &, std::shared_ptr const &, nano::block_hash const &, nano::account &, nano::uint128_t &, bool &, bool &, nano::account &); void process_confirmed (nano::election_status const &, uint64_t = 0); void process_active (std::shared_ptr const &); std::optional process_local (std::shared_ptr const &); From 144c602b66f217d7deae4897130bfa610fe6ae91 Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Tue, 19 Mar 2024 11:37:16 +0000 Subject: [PATCH 5/7] Move wallet receiving in to its own observer. This is an interaction between the ledger when a block is confirmed and the wallets that might need to generate receive blocks. Previously this functionality was unnecessarily being handled by the active_transactions class. It could also miss notifications if the block was indirectly confirmed without an active election, or if the election had already been removed from active_transactions. --- nano/node/active_transactions.cpp | 4 ---- nano/node/node.cpp | 7 +++++++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index 6b001637f6..d2fbb6f319 100644 --- a/nano/node/active_transactions.cpp +++ b/nano/node/active_transactions.cpp @@ -146,10 +146,6 @@ void nano::active_transactions::handle_confirmation (nano::store::read_transacti nano::block_hash hash = block->hash (); recently_cemented.put (election->get_status ()); - if (block->is_send ()) - { - node.receive_confirmed (transaction, hash, block->destination ()); - } auto status = election->set_status_type (status_type); auto votes = election->votes_with_weight (); notify_observers (transaction, status, votes); diff --git a/nano/node/node.cpp b/nano/node/node.cpp index c169e8aef7..338bab5fcf 100644 --- a/nano/node/node.cpp +++ b/nano/node/node.cpp @@ -435,6 +435,13 @@ nano::node::node (boost::asio::io_context & io_ctx_a, std::filesystem::path cons std::exit (1); } } + confirmation_height_processor.add_cemented_observer ([this] (auto const & block) { + if (block->is_send ()) + { + auto transaction = store.tx_begin_read (); + receive_confirmed (transaction, block->hash (), block->destination ()); + } + }); } node_initialized_latch.count_down (); } From 315b77742c9493c6950ee062b04b001fa6c576f0 Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Tue, 19 Mar 2024 12:05:10 +0000 Subject: [PATCH 6/7] Remove unused transaction parameters. --- nano/node/active_transactions.cpp | 10 +++++----- nano/node/active_transactions.hpp | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index d2fbb6f319..1b9c4af78b 100644 --- a/nano/node/active_transactions.cpp +++ b/nano/node/active_transactions.cpp @@ -82,12 +82,12 @@ 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); + auto status_type = election_status (block_a); if (!status_type) return; + auto transaction = node.store.tx_begin_read (); switch (*status_type) { case nano::election_status_type::inactive_confirmation_height: @@ -102,13 +102,13 @@ void nano::active_transactions::block_cemented_callback (std::shared_ptr nano::active_transactions::election_status (nano::store::read_transaction const & transaction, std::shared_ptr const & block) +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 ())) { - status_type = confirm_block (transaction, block); + status_type = confirm_block (block); } else { @@ -654,7 +654,7 @@ bool nano::active_transactions::publish (std::shared_ptr const & bl } // Returns the type of election status requiring callbacks calling later -boost::optional nano::active_transactions::confirm_block (store::transaction const & transaction_a, std::shared_ptr const & block_a) +boost::optional nano::active_transactions::confirm_block (std::shared_ptr const & block_a) { auto const hash = block_a->hash (); std::shared_ptr election = nullptr; diff --git a/nano/node/active_transactions.hpp b/nano/node/active_transactions.hpp index e1c4a5f8e2..3ce2772f7a 100644 --- a/nano/node/active_transactions.hpp +++ b/nano/node/active_transactions.hpp @@ -160,7 +160,7 @@ class active_transactions final bool empty () const; std::size_t size () const; bool publish (std::shared_ptr const &); - boost::optional confirm_block (store::transaction const &, 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 &); @@ -195,7 +195,7 @@ class active_transactions final * 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 (nano::store::read_transaction const & transaction, std::shared_ptr const & block); + 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); From b2607dbabfbb9c9f55a919d9ade5dedebaf2f8aa Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Tue, 19 Mar 2024 12:48:49 +0000 Subject: [PATCH 7/7] Clean active_transactions::activate_successors signature. --- nano/node/active_transactions.cpp | 8 ++++---- nano/node/active_transactions.hpp | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index 1b9c4af78b..d9d8891848 100644 --- a/nano/node/active_transactions.cpp +++ b/nano/node/active_transactions.cpp @@ -179,16 +179,16 @@ void nano::active_transactions::handle_final_votes_confirmation (std::shared_ptr // Next-block activations are only done for blocks with previously active elections if (cemented_bootstrap_count_reached && was_active) { - activate_successors (account, block, transaction); + activate_successors (transaction, block); } } -void nano::active_transactions::activate_successors (const nano::account & account, std::shared_ptr const & block, nano::store::read_transaction const & transaction) +void nano::active_transactions::activate_successors (nano::store::read_transaction const & transaction, std::shared_ptr const & block) { - node.scheduler.priority.activate (account, transaction); + node.scheduler.priority.activate (block->account (), transaction); // Start or vote for the next unconfirmed block in the destination account - if (block->is_send () && !block->destination ().is_zero () && block->destination () != account) + if (block->is_send () && !block->destination ().is_zero () && block->destination () != block->account ()) { node.scheduler.priority.activate (block->destination (), transaction); } diff --git a/nano/node/active_transactions.hpp b/nano/node/active_transactions.hpp index 3ce2772f7a..9f56d5cad6 100644 --- a/nano/node/active_transactions.hpp +++ b/nano/node/active_transactions.hpp @@ -200,7 +200,7 @@ class active_transactions final 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 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); private: // Dependencies