From f89451d02ba34ec95bcd8f2dc2d71e237009d93c Mon Sep 17 00:00:00 2001 From: Guilherme Lawless Date: Thu, 27 Aug 2020 14:14:42 +0100 Subject: [PATCH] Constrained successor and destination block activation (#2895) * Activations only on active_confirmed_quorum or active_confirmation_height * Only activate after cementing hardcoded bootstrap block count * Limit activation of successor/destination elections * Lazily evaluate one of the conditions --- nano/core_test/active_transactions.cpp | 62 ++++++++++++++++++++++++++ nano/core_test/confirmation_height.cpp | 11 ++--- nano/node/active_transactions.cpp | 29 +++++++++--- nano/node/active_transactions.hpp | 1 + nano/node/node.cpp | 2 +- 5 files changed, 90 insertions(+), 15 deletions(-) diff --git a/nano/core_test/active_transactions.cpp b/nano/core_test/active_transactions.cpp index 38d0373c..c559844f 100644 --- a/nano/core_test/active_transactions.cpp +++ b/nano/core_test/active_transactions.cpp @@ -1372,3 +1372,65 @@ TEST (active_transactions, activate_account_chain) ASSERT_TIMELY (3s, node.block_confirmed (send3->hash ())); ASSERT_TIMELY (3s, node.active.active (receive->qualified_root ())); } + +TEST (active_transactions, activate_inactive) +{ + nano::system system; + nano::node_flags flags; + nano::node_config config (nano::get_available_port (), system.logging); + config.frontiers_confirmation = nano::frontiers_confirmation_mode::disabled; + auto & node = *system.add_node (config, flags); + + nano::keypair key; + nano::state_block_builder builder; + auto send = builder.make_block () + .account (nano::dev_genesis_key.pub) + .previous (nano::genesis_hash) + .representative (nano::dev_genesis_key.pub) + .link (key.pub) + .balance (nano::genesis_amount - 1) + .sign (nano::dev_genesis_key.prv, nano::dev_genesis_key.pub) + .work (*system.work.generate (nano::genesis_hash)) + .build_shared (); + auto send2 = builder.make_block () + .account (nano::dev_genesis_key.pub) + .previous (send->hash ()) + .representative (nano::dev_genesis_key.pub) + .link (nano::keypair ().pub) + .balance (nano::genesis_amount - 2) + .sign (nano::dev_genesis_key.prv, nano::dev_genesis_key.pub) + .work (*system.work.generate (send->hash ())) + .build_shared (); + auto open = builder.make_block () + .account (key.pub) + .previous (0) + .representative (key.pub) + .link (send->hash ()) + .balance (1) + .sign (key.prv, key.pub) + .work (*system.work.generate (key.pub)) + .build_shared (); + + ASSERT_EQ (nano::process_result::progress, node.process (*send).code); + ASSERT_EQ (nano::process_result::progress, node.process (*send2).code); + ASSERT_EQ (nano::process_result::progress, node.process (*open).code); + + node.block_confirm (send2); + { + auto election = node.active.election (send2->qualified_root ()); + ASSERT_NE (nullptr, election); + nano::lock_guard guard (node.active.mutex); + election->confirm_once (); + } + + ASSERT_TIMELY (3s, !node.confirmation_height_processor.is_processing_added_block (send2->hash ())); + ASSERT_TRUE (node.block_confirmed (send2->hash ())); + ASSERT_TRUE (node.block_confirmed (send->hash ())); + + ASSERT_EQ (1, node.stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::inactive_conf_height, nano::stat::dir::out)); + ASSERT_EQ (1, node.stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::active_quorum, nano::stat::dir::out)); + ASSERT_EQ (0, node.stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::active_conf_height, nano::stat::dir::out)); + + // The first block was not active so no activation takes place + ASSERT_FALSE (node.active.active (open->qualified_root ()) || node.block_confirmed_or_being_confirmed (node.store.tx_begin_read (), open->hash ())); +} diff --git a/nano/core_test/confirmation_height.cpp b/nano/core_test/confirmation_height.cpp index ca97e2aa..e3618552 100644 --- a/nano/core_test/confirmation_height.cpp +++ b/nano/core_test/confirmation_height.cpp @@ -348,16 +348,13 @@ TEST (confirmation_height, gap_live) ASSERT_EQ (nano::genesis_hash, confirmation_height_info.frontier); } + // Vote and confirm all existing blocks + node->block_confirm (send1); + ASSERT_TIMELY (10s, node->stats.count (nano::stat::type::http_callback, nano::stat::detail::http_callback, nano::stat::dir::out) == 3); + // Now complete the chain where the block comes in on the live network node->process_active (open1); node->block_processor.flush (); - node->block_confirm (open1); - { - auto election = node->active.election (open1->qualified_root ()); - ASSERT_NE (nullptr, election); - nano::lock_guard guard (node->active.mutex); - election->confirm_once (); - } ASSERT_TIMELY (10s, node->stats.count (nano::stat::type::http_callback, nano::stat::detail::http_callback, nano::stat::dir::out) == 6); diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index 69c56b5e..59171c5f 100644 --- a/nano/node/active_transactions.cpp +++ b/nano/node/active_transactions.cpp @@ -14,6 +14,8 @@ using namespace std::chrono; +size_t constexpr nano::active_transactions::max_active_elections_frontier_insertion; + nano::active_transactions::active_transactions (nano::node & node_a, nano::confirmation_height_processor & confirmation_height_processor_a) : recently_dropped (node_a.stats), confirmation_height_processor (confirmation_height_processor_a), @@ -59,7 +61,7 @@ void nano::active_transactions::confirm_prioritized_frontiers (nano::transaction auto is_dev_network = node.network_params.network.is_dev_network (); auto roots_size = size (); auto check_time_exceeded = std::chrono::steady_clock::now () >= next_frontier_check; - auto max_elections = 1000ull; + auto max_elections = max_active_elections_frontier_insertion; auto low_active_elections = roots_size < max_elections; bool wallets_check_required = (!skip_wallets || !priority_wallet_cementable_frontiers.empty ()) && !agressive_mode; // Minimise dropping real-time transactions, set the number of frontiers added to a factor of the maximum number of possible active elections @@ -189,16 +191,29 @@ void nano::active_transactions::block_cemented_callback (std::shared_ptraccount ().is_zero () ? block_a->account () : block_a->sideband ().account); debug_assert (!account.is_zero ()); - activate (account); - // 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) + // 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 }; + + // 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 }; + + // Activations are only done if there is not a large amount of active elections, ensuring frontier confirmation takes place + auto const low_active_elections = [this] { return this->size () < nano::active_transactions::max_active_elections_frontier_insertion / 2; }; + + if (cemented_bootstrap_count_reached && was_active && low_active_elections ()) { - activate (destination); + // Start or vote for the next unconfirmed block + activate (account); + + // 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) + { + activate (destination); + } } } } diff --git a/nano/node/active_transactions.hpp b/nano/node/active_transactions.hpp index fc51916a..ee39ddac 100644 --- a/nano/node/active_transactions.hpp +++ b/nano/node/active_transactions.hpp @@ -226,6 +226,7 @@ private: void frontiers_confirmation (nano::unique_lock &); nano::account next_frontier_account{ 0 }; std::chrono::steady_clock::time_point next_frontier_check{ std::chrono::steady_clock::now () }; + constexpr static size_t max_active_elections_frontier_insertion{ 1000 }; nano::condition_variable condition; bool started{ false }; std::atomic stopped{ false }; diff --git a/nano/node/node.cpp b/nano/node/node.cpp index 9b759ddd..5afd863e 100644 --- a/nano/node/node.cpp +++ b/nano/node/node.cpp @@ -411,13 +411,13 @@ node_seq (seq) bool use_bootstrap_weight = ledger.cache.block_count < bootstrap_weights.first; if (use_bootstrap_weight) { - ledger.bootstrap_weight_max_blocks = bootstrap_weights.first; ledger.bootstrap_weights = bootstrap_weights.second; for (auto const & rep : ledger.bootstrap_weights) { logger.always_log ("Using bootstrap rep weight: ", rep.first.to_account (), " -> ", nano::uint128_union (rep.second).format_balance (Mxrb_ratio, 0, true), " XRB"); } } + ledger.bootstrap_weight_max_blocks = bootstrap_weights.first; // Drop unchecked blocks if initial bootstrap is completed if (!flags.disable_unchecked_drop && !use_bootstrap_weight && !flags.read_only)