Minecon724/dncurrency
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions 1

Improve confirmation consistency (#2625)

Browse source 
* Test for confirmation consistency

* Remove unnecessary transaction (Gui review comment)

* Protect access to root container (Gui found during TSAN run)

* Gui review comments

* Remove checking for inserted in add_recently_confirmed
This commit is contained in:
Wesley Shillingford 2020-03-13 13:12:53 +00:00 • committed by GitHub
commit f9ef72a56b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 83 additions and 41 deletions
Download patch file Download diff file Expand all files Collapse all files

28
nano/core_test/active_transactions.cpp
View file

@ -823,3 +823,31 @@ TEST (active_transactions, dropped_cleanup)
ASSERT_FALSE (node.network.publish_filter.apply (block_bytes.data (), block_bytes.size ()));
}
}
namespace nano
{
// Blocks that won an election must always be seen as confirming or cemented
TEST (active_transactions, confirmation_consistency)
{
nano::system system;
nano::node_config node_config (nano::get_available_port (), system.logging);
node_config.frontiers_confirmation = nano::frontiers_confirmation_mode::disabled;
auto & node = *system.add_node (node_config);
system.wallet (0)->insert_adhoc (nano::test_genesis_key.prv);
for (unsigned i = 0; i < 10; ++i)
{
auto block (system.wallet (0)->send_action (nano::test_genesis_key.pub, nano::public_key (), node.config.receive_minimum.number ()));
ASSERT_NE (nullptr, block);
system.deadline_set (5s);
while (!node.ledger.block_confirmed (node.store.tx_begin_read (), block->hash ()))
{
ASSERT_FALSE (node.active.insert (block).second);
ASSERT_NO_ERROR (system.poll (5ms));
}
nano::lock_guard<std::mutex> guard (node.active.mutex);
ASSERT_EQ (i + 1, node.active.recently_confirmed.size ());
ASSERT_EQ (block->qualified_root (), node.active.recently_confirmed.back ());
ASSERT_EQ (i + 1, node.active.recently_cemented.size ());
}
}
}

5
nano/core_test/confirmation_height.cpp
View file

@ -1012,6 +1012,7 @@ TEST (confirmation_height, prioritize_frontiers)
std::array<nano::qualified_root, num_accounts> frontiers{ send17.qualified_root (), send6.qualified_root (), send7.qualified_root (), open2.qualified_root (), send11.qualified_root () };
for (auto & frontier : frontiers)
{
nano::lock_guard<std::mutex> guard (node->active.mutex);
ASSERT_NE (node->active.roots.find (frontier), node->active.roots.end ());
}
};
@ -1122,7 +1123,7 @@ TEST (confirmation_height, callback_confirmed_history)
ASSERT_NO_ERROR (system.poll ());
}
ASSERT_EQ (0, node->active.list_confirmed ().size ());
ASSERT_EQ (0, node->active.list_recently_cemented ().size ());
{
nano::lock_guard<std::mutex> guard (node->active.mutex);
ASSERT_EQ (0, node->active.blocks.size ());
@ -1162,7 +1163,7 @@ TEST (confirmation_height, callback_confirmed_history)
ASSERT_NO_ERROR (system.poll ());
}
ASSERT_EQ (1, node->active.list_confirmed ().size ());
ASSERT_EQ (1, node->active.list_recently_cemented ().size ());
ASSERT_EQ (0, node->active.blocks.size ());
// Confirm the callback is not called under this circumstance

4
nano/core_test/node.cpp
View file

@ -2506,9 +2506,9 @@ TEST (node, block_confirm)
ASSERT_EQ (nano::process_result::progress, node2.ledger.process (transaction, *send2).code);
}
node1.block_confirm (send2);
ASSERT_TRUE (node1.active.list_confirmed ().empty ());
ASSERT_TRUE (node1.active.list_recently_cemented ().empty ());
system.deadline_set (10s);
while (node1.active.list_confirmed ().empty ())
while (node1.active.list_recently_cemented ().empty ())
{
ASSERT_NO_ERROR (system.poll ());
}

43
nano/node/active_transactions.cpp
View file

@ -158,12 +158,10 @@ void nano::active_transactions::block_cemented_callback (std::shared_ptr<nano::b
auto election = existing->second;
election_winner_details.erase (hash);
election_winners_lk.unlock ();
// Make sure mutex is held before election usage so we know that confirm_once has
// finished removing the root from active to avoid any data race.
nano::unique_lock<std::mutex> lk (mutex);
if (election->confirmed () && election->status.winner->hash () == hash)
{
add_confirmed (election->status, block_a->qualified_root ());
add_recently_cemented (election->status);
lk.unlock ();
node.receive_confirmed (transaction, block_a, hash);
nano::account account (0);
@ -171,9 +169,11 @@ void nano::active_transactions::block_cemented_callback (std::shared_ptr<nano::b
bool is_state_send (false);
nano::account pending_account (0);
node.process_confirmed_data (transaction, block_a, hash, account, amount, is_state_send, pending_account);
lk.lock ();
election->status.type = *election_status_type;
election->status.confirmation_request_count = election->confirmation_request_count;
node.observers.blocks.notify (election->status, account, amount, is_state_send);
lk.unlock ();
if (amount > 0)
{
node.observers.account_balance.notify (account, false);
@ -488,7 +488,7 @@ std::pair<std::shared_ptr<nano::election>, bool> nano::active_transactions::inse
auto existing (roots.get<tag_root> ().find (root));
if (existing == roots.get<tag_root> ().end ())
{
if (confirmed_set.get<tag_root> ().find (root) == confirmed_set.get<tag_root> ().end ())
if (recently_confirmed.get<tag_root> ().find (root) == recently_confirmed.get<tag_root> ().end ())
{
result.second = true;
auto hash (block_a->hash ());
@ -777,23 +777,27 @@ std::deque<std::shared_ptr<nano::block>> nano::active_transactions::list_blocks
return result;
}
std::deque<nano::election_status> nano::active_transactions::list_confirmed ()
std::deque<nano::election_status> nano::active_transactions::list_recently_cemented ()
{
nano::lock_guard<std::mutex> lock (mutex);
return confirmed;
return recently_cemented;
}
void nano::active_transactions::add_confirmed (nano::election_status const & status_a, nano::qualified_root const & root_a)
void nano::active_transactions::add_recently_cemented (nano::election_status const & status_a)
{
confirmed.push_back (status_a);
auto inserted (confirmed_set.get<tag_sequence> ().push_back (root_a));
if (confirmed.size () > node.config.confirmation_history_size)
recently_cemented.push_back (status_a);
if (recently_cemented.size () > node.config.confirmation_history_size)
{
confirmed.pop_front ();
if (inserted.second)
{
confirmed_set.get<tag_sequence> ().pop_front ();
}
recently_cemented.pop_front ();
}
}
void nano::active_transactions::add_recently_confirmed (nano::qualified_root const & root_a)
{
recently_confirmed.get<tag_sequence> ().push_back (root_a);
if (recently_confirmed.size () > node.config.confirmation_history_size)
{
recently_confirmed.get<tag_sequence> ().pop_front ();
}
}
@ -1015,20 +1019,23 @@ std::unique_ptr<nano::container_info_component> nano::collect_container_info (ac
{
size_t roots_count;
size_t blocks_count;
size_t confirmed_count;
size_t recently_confirmed_count;
size_t recently_cemented_count;
{
nano::lock_guard<std::mutex> guard (active_transactions.mutex);
roots_count = active_transactions.roots.size ();
blocks_count = active_transactions.blocks.size ();
confirmed_count = active_transactions.confirmed.size ();
recently_confirmed_count = active_transactions.recently_confirmed.size ();
recently_cemented_count = active_transactions.recently_cemented.size ();
}
auto composite = std::make_unique<container_info_composite> (name);
composite->add_component (std::make_unique<container_info_leaf> (container_info{ "roots", roots_count, sizeof (decltype (active_transactions.roots)::value_type) }));
composite->add_component (std::make_unique<container_info_leaf> (container_info{ "blocks", blocks_count, sizeof (decltype (active_transactions.blocks)::value_type) }));
composite->add_component (std::make_unique<container_info_leaf> (container_info{ "election_winner_details", active_transactions.election_winner_details_size (), sizeof (decltype (active_transactions.election_winner_details)::value_type) }));
composite->add_component (std::make_unique<container_info_leaf> (container_info{ "confirmed", confirmed_count, sizeof (decltype (active_transactions.confirmed)::value_type) }));
composite->add_component (std::make_unique<container_info_leaf> (container_info{ "recently_confirmed", recently_confirmed_count, sizeof (decltype (active_transactions.recently_confirmed)::value_type) }));
composite->add_component (std::make_unique<container_info_leaf> (container_info{ "recently_cemented", recently_cemented_count, sizeof (decltype (active_transactions.recently_cemented)::value_type) }));
composite->add_component (std::make_unique<container_info_leaf> (container_info{ "priority_wallet_cementable_frontiers_count", active_transactions.priority_wallet_cementable_frontiers_size (), sizeof (nano::cementable_account) }));
composite->add_component (std::make_unique<container_info_leaf> (container_info{ "priority_cementable_frontiers_count", active_transactions.priority_cementable_frontiers_size (), sizeof (nano::cementable_account) }));
composite->add_component (std::make_unique<container_info_leaf> (container_info{ "inactive_votes_cache_count", active_transactions.inactive_votes_cache_size (), sizeof (nano::gap_information) }));

11
nano/node/active_transactions.hpp
View file

@ -120,9 +120,11 @@ public:
roots;
// clang-format on
std::unordered_map<nano::block_hash, std::shared_ptr<nano::election>> blocks;
std::deque<nano::election_status> list_confirmed ();
std::deque<nano::election_status> confirmed;
void add_confirmed (nano::election_status const &, nano::qualified_root const &);
std::deque<nano::election_status> list_recently_cemented ();
std::deque<nano::election_status> recently_cemented;
void add_recently_cemented (nano::election_status const &);
void add_recently_confirmed (nano::qualified_root const &);
void add_inactive_votes_cache (nano::block_hash const &, nano::account const &);
nano::inactive_cache_information find_inactive_votes_cache (nano::block_hash const &);
void erase_inactive_votes_cache (nano::block_hash const &);
@ -168,7 +170,7 @@ private:
mi::sequenced<mi::tag<tag_sequence>>,
mi::hashed_unique<mi::tag<tag_root>,
mi::identity<nano::qualified_root>>>>
confirmed_set;
recently_confirmed;
using prioritize_num_uncemented = boost::multi_index_container<nano::cementable_account,
mi::indexed_by<
mi::hashed_unique<mi::tag<tag_account>,
@ -202,6 +204,7 @@ private:
friend class active_transactions_dropped_cleanup_Test;
friend class confirmation_height_prioritize_frontiers_Test;
friend class confirmation_height_prioritize_frontiers_overwrite_Test;
friend class active_transactions_confirmation_consistency_Test;
friend std::unique_ptr<container_info_component> collect_container_info (active_transactions &, const std::string &);
};

1
nano/node/election.cpp
View file

@ -54,6 +54,7 @@ void nano::election::confirm_once (nano::election_status_type type_a)
// election winner details can be cleared consistently sucessfully in active_transactions::block_cemented_callback
node.active.add_election_winner_details (status_l.winner->hash (), this_l);
}
node.active.add_recently_confirmed (status_l.winner->qualified_root ());
node.background ([node_l, status_l, confirmation_action_l, this_l]() {
node_l->process_confirmed (status_l, this_l);
confirmation_action_l (status_l.winner);

15
nano/node/json_handler.cpp
View file

@ -1022,8 +1022,11 @@ void nano::json_handler::block_confirm ()
{
if (!node.ledger.block_confirmed (transaction, hash))
{
// Start new confirmation for unconfirmed block
node.block_confirm (std::move (block_l));
// Start new confirmation for unconfirmed (or not being confirmed) block
if (!node.confirmation_height_processor.is_processing_block (hash))
{
node.block_confirm (std::move (block_l));
}
}
else
{
@ -1031,11 +1034,7 @@ void nano::json_handler::block_confirm ()
nano::election_status status{ block_l, 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::active_confirmation_height };
{
nano::lock_guard<std::mutex> lock (node.active.mutex);
node.active.confirmed.push_back (status);
if (node.active.confirmed.size () > node.config.confirmation_history_size)
{
node.active.confirmed.pop_front ();
}
node.active.add_recently_cemented (status);
}
// Trigger callback for confirmed block
node.block_arrival.add (hash);
@ -1812,7 +1811,7 @@ void nano::json_handler::confirmation_history ()
}
if (!ec)
{
auto confirmed (node.active.list_confirmed ());
auto confirmed (node.active.list_recently_cemented ());
for (auto i (confirmed.begin ()), n (confirmed.end ()); i != n; ++i)
{
if (hash.is_zero () || i->winner->hash () == hash)

7
nano/node/wallet.cpp
View file

@ -1275,8 +1275,11 @@ bool nano::wallet::search_pending ()
}
else
{
// Request confirmation for unconfirmed block
wallets.node.block_confirm (block);
if (!wallets.node.confirmation_height_processor.is_processing_block (hash))
{
// Request confirmation for block which is not being processed yet
wallets.node.block_confirm (block);
}
}
}
}

10
nano/rpc_test/rpc.cpp
View file

@ -6235,11 +6235,11 @@ TEST (rpc, confirmation_history)
auto node = add_ipc_enabled_node (system);
nano::keypair key;
system.wallet (0)->insert_adhoc (nano::test_genesis_key.prv);
ASSERT_TRUE (node->active.list_confirmed ().empty ());
ASSERT_TRUE (node->active.list_recently_cemented ().empty ());
auto block (system.wallet (0)->send_action (nano::test_genesis_key.pub, key.pub, nano::Gxrb_ratio));
scoped_io_thread_name_change scoped_thread_name_io;
system.deadline_set (10s);
while (node->active.list_confirmed ().empty ())
while (node->active.list_recently_cemented ().empty ())
{
ASSERT_NO_ERROR (system.poll ());
}
@ -6282,13 +6282,13 @@ TEST (rpc, confirmation_history_hash)
auto node = add_ipc_enabled_node (system);
nano::keypair key;
system.wallet (0)->insert_adhoc (nano::test_genesis_key.prv);
ASSERT_TRUE (node->active.list_confirmed ().empty ());
ASSERT_TRUE (node->active.list_recently_cemented ().empty ());
auto send1 (system.wallet (0)->send_action (nano::test_genesis_key.pub, key.pub, nano::Gxrb_ratio));
auto send2 (system.wallet (0)->send_action (nano::test_genesis_key.pub, key.pub, nano::Gxrb_ratio));
auto send3 (system.wallet (0)->send_action (nano::test_genesis_key.pub, key.pub, nano::Gxrb_ratio));
scoped_io_thread_name_change scoped_thread_name_io;
system.deadline_set (10s);
while (node->active.list_confirmed ().size () != 3)
while (node->active.list_recently_cemented ().size () != 3)
{
ASSERT_NO_ERROR (system.poll ());
}
@ -6419,7 +6419,7 @@ TEST (rpc, block_confirm_confirmed)
ASSERT_EQ (200, response.status);
ASSERT_EQ ("1", response.json.get<std::string> ("started"));
// Check confirmation history
auto confirmed (node->active.list_confirmed ());
auto confirmed (node->active.list_recently_cemented ());
ASSERT_EQ (1, confirmed.size ());
ASSERT_EQ (nano::genesis_hash, confirmed.begin ()->winner->hash ());
// Check callback