Remove conf height put during ledger processing on a newly opened account

nano/core_test/block_store.cpp

@@ -1959,17 +1959,11 @@ TEST (block_store, confirmation_height)
 		store->confirmation_height_clear (transaction);
 	}
 	auto transaction (store->tx_begin_read ());
-	ASSERT_EQ (store->confirmation_height_count (transaction), 3);
+	ASSERT_EQ (store->confirmation_height_count (transaction), 0);
 	nano::confirmation_height_info confirmation_height_info;
-	ASSERT_FALSE (store->confirmation_height_get (transaction, account1, confirmation_height_info));
-	ASSERT_EQ (confirmation_height_info.height, 0);
-	ASSERT_EQ (confirmation_height_info.frontier, nano::block_hash (0));
-	ASSERT_FALSE (store->confirmation_height_get (transaction, account2, confirmation_height_info));
-	ASSERT_EQ (confirmation_height_info.height, 0);
-	ASSERT_EQ (confirmation_height_info.frontier, nano::block_hash (0));
-	ASSERT_FALSE (store->confirmation_height_get (transaction, account3, confirmation_height_info));
-	ASSERT_EQ (confirmation_height_info.height, 0);
-	ASSERT_EQ (confirmation_height_info.frontier, nano::block_hash (0));
+	ASSERT_TRUE (store->confirmation_height_get (transaction, account1, confirmation_height_info));
+	ASSERT_TRUE (store->confirmation_height_get (transaction, account2, confirmation_height_info));
+	ASSERT_TRUE (store->confirmation_height_get (transaction, account3, confirmation_height_info));
 }
 
 // Ledger versions are not forward compatible

nano/core_test/confirmation_height.cpp

@@ -129,19 +129,19 @@ TEST (confirmation_height, multiple_accounts)
 			ASSERT_EQ (nano::process_result::progress, node->ledger.process (transaction, send6).code);
 			ASSERT_EQ (nano::process_result::progress, node->ledger.process (transaction, receive2).code);
 
-			// Check confirmation heights of all the accounts are uninitialized (0),
+			// Check confirmation heights of all the accounts (except genesis) are uninitialized (0),
 			// as we have any just added them to the ledger and not processed any live transactions yet.
 			nano::confirmation_height_info confirmation_height_info;
 			ASSERT_FALSE (node->store.confirmation_height_get (transaction, nano::dev_genesis_key.pub, confirmation_height_info));
 			ASSERT_EQ (1, confirmation_height_info.height);
 			ASSERT_EQ (nano::genesis_hash, confirmation_height_info.frontier);
-			ASSERT_FALSE (node->store.confirmation_height_get (transaction, key1.pub, confirmation_height_info));
+			ASSERT_TRUE (node->store.confirmation_height_get (transaction, key1.pub, confirmation_height_info));
 			ASSERT_EQ (0, confirmation_height_info.height);
 			ASSERT_EQ (nano::block_hash (0), confirmation_height_info.frontier);
-			ASSERT_FALSE (node->store.confirmation_height_get (transaction, key2.pub, confirmation_height_info));
+			ASSERT_TRUE (node->store.confirmation_height_get (transaction, key2.pub, confirmation_height_info));
 			ASSERT_EQ (0, confirmation_height_info.height);
 			ASSERT_EQ (nano::block_hash (0), confirmation_height_info.frontier);
-			ASSERT_FALSE (node->store.confirmation_height_get (transaction, key3.pub, confirmation_height_info));
+			ASSERT_TRUE (node->store.confirmation_height_get (transaction, key3.pub, confirmation_height_info));
 			ASSERT_EQ (0, confirmation_height_info.height);
 			ASSERT_EQ (nano::block_hash (0), confirmation_height_info.frontier);
 		}
@@ -278,7 +278,7 @@ TEST (confirmation_height, gap_bootstrap)
 			ASSERT_FALSE (node1.store.confirmation_height_get (transaction, nano::dev_genesis_key.pub, confirmation_height_info));
 			ASSERT_EQ (1, confirmation_height_info.height);
 			ASSERT_EQ (genesis.hash (), confirmation_height_info.frontier);
-			ASSERT_FALSE (node1.store.confirmation_height_get (transaction, destination.pub, confirmation_height_info));
+			ASSERT_TRUE (node1.store.confirmation_height_get (transaction, destination.pub, confirmation_height_info));
 			ASSERT_EQ (0, confirmation_height_info.height);
 			ASSERT_EQ (nano::block_hash (0), confirmation_height_info.frontier);
 		}

nano/core_test/ledger.cpp

@@ -2984,7 +2984,7 @@ TEST (ledger, confirmation_height_not_updated)
 	ASSERT_EQ (genesis.hash (), confirmation_height_info.frontier);
 	nano::open_block open1 (send1.hash (), nano::genesis_account, key.pub, key.prv, key.pub, *pool.generate (key.pub));
 	ASSERT_EQ (nano::process_result::progress, ledger.process (transaction, open1).code);
-	ASSERT_FALSE (store->confirmation_height_get (transaction, key.pub, confirmation_height_info));
+	ASSERT_TRUE (store->confirmation_height_get (transaction, key.pub, confirmation_height_info));
 	ASSERT_EQ (0, confirmation_height_info.height);
 	ASSERT_EQ (nano::block_hash (0), confirmation_height_info.frontier);
 }
@@ -3229,7 +3229,7 @@ TEST (ledger, dependents_confirmed)
 	                .build_shared ();
 	ASSERT_EQ (nano::process_result::progress, ledger.process (transaction, *receive2).code);
 	ASSERT_FALSE (ledger.dependents_confirmed (transaction, *receive2));
-	ASSERT_FALSE (ledger.store.confirmation_height_get (transaction, key1.pub, height));
+	ASSERT_TRUE (ledger.store.confirmation_height_get (transaction, key1.pub, height));
 	height.height += 1;
 	ledger.store.confirmation_height_put (transaction, key1.pub, height);
 	ASSERT_FALSE (ledger.dependents_confirmed (transaction, *receive2));
@@ -3771,3 +3771,42 @@ TEST (ledger, hash_root_random)
 		ASSERT_LE (iteration, 1000);
 	}
 }
+
+TEST (ledger, unconfirmed_frontiers)
+{
+	nano::logger_mt logger;
+	auto store = nano::make_store (logger, nano::unique_path ());
+	ASSERT_TRUE (!store->init_error ());
+	nano::stat stats;
+	nano::ledger ledger (*store, stats);
+	nano::genesis genesis;
+	store->initialize (store->tx_begin_write (), genesis, ledger.cache);
+	nano::work_pool pool (std::numeric_limits<unsigned>::max ());
+
+	auto unconfirmed_frontiers = ledger.unconfirmed_frontiers ();
+	ASSERT_TRUE (unconfirmed_frontiers.empty ());
+
+	nano::state_block_builder builder;
+	nano::keypair key;
+	auto const latest = ledger.latest (store->tx_begin_read (), nano::genesis_account);
+	auto send = builder.make_block ()
+	            .account (nano::genesis_account)
+	            .previous (latest)
+	            .representative (nano::genesis_account)
+	            .balance (nano::genesis_amount - 100)
+	            .link (key.pub)
+	            .sign (nano::dev_genesis_key.prv, nano::dev_genesis_key.pub)
+	            .work (*pool.generate (latest))
+	            .build ();
+
+	ASSERT_EQ (nano::process_result::progress, ledger.process (store->tx_begin_write (), *send).code);
+
+	unconfirmed_frontiers = ledger.unconfirmed_frontiers ();
+	ASSERT_EQ (unconfirmed_frontiers.size (), 1);
+	ASSERT_EQ (unconfirmed_frontiers.begin ()->first, 1);
+	nano::uncemented_info uncemented_info1{ latest, send->hash (), nano::genesis_account };
+	auto uncemented_info2 = unconfirmed_frontiers.begin ()->second;
+	ASSERT_EQ (uncemented_info1.account, uncemented_info2.account);
+	ASSERT_EQ (uncemented_info1.cemented_frontier, uncemented_info2.cemented_frontier);
+	ASSERT_EQ (uncemented_info1.frontier, uncemented_info2.frontier);
+}

nano/lib/config.hpp

@@ -215,9 +215,6 @@ std::string get_rpc_toml_config_path (boost::filesystem::path const & data_path)
 std::string get_access_toml_config_path (boost::filesystem::path const & data_path);
 std::string get_qtwallet_toml_config_path (boost::filesystem::path const & data_path);
 
-/** Called by gtest_main to enforce dev network */
-void force_nano_dev_network ();
-
 /** Checks if we are running inside a valgrind instance */
 bool running_within_valgrind ();
 

nano/node/active_transactions.cpp

@@ -141,8 +141,7 @@ void nano::active_transactions::confirm_prioritized_frontiers (nano::transaction
 					if (!this->confirmation_height_processor.is_processing_block (info.head))
 					{
 						nano::confirmation_height_info confirmation_height_info;
-						error = this->node.store.confirmation_height_get (transaction_a, cementable_account.account, confirmation_height_info);
-						release_assert (!error);
+						this->node.store.confirmation_height_get (transaction_a, cementable_account.account, confirmation_height_info);
 
 						if (info.block_count > confirmation_height_info.height)
 						{
@@ -461,8 +460,9 @@ void nano::active_transactions::confirm_expired_frontiers_pessimistically (nano:
 		auto const & account{ i->account };
 		nano::account_info account_info;
 		bool should_delete{ true };
-		if (!node.store.account_get (transaction_a, account, account_info) && !node.store.confirmation_height_get (transaction_a, account, confirmation_height_info))
+		if (!node.store.account_get (transaction_a, account, account_info))
 		{
+			node.store.confirmation_height_get (transaction_a, account, confirmation_height_info);
 			if (account_info.block_count > confirmation_height_info.height)
 			{
 				should_delete = false;
@@ -666,12 +666,13 @@ void nano::active_transactions::prioritize_frontiers_for_confirmation (nano::tra
 
 					auto i (wallet->store.begin (wallet_transaction, next_wallet_frontier_account));
 					auto n (wallet->store.end ());
-					nano::confirmation_height_info confirmation_height_info;
 					for (; i != n && should_iterate (); ++i)
 					{
 						auto const & account (i->first);
-						if (expired_optimistic_election_infos.get<tag_account> ().count (account) == 0 && !node.store.account_get (transaction_a, account, info) && !node.store.confirmation_height_get (transaction_a, account, confirmation_height_info))
+						if (expired_optimistic_election_infos.get<tag_account> ().count (account) == 0 && !node.store.account_get (transaction_a, account, info))
 						{
+							nano::confirmation_height_info confirmation_height_info;
+							node.store.confirmation_height_get (transaction_a, account, confirmation_height_info);
 							// If it exists in normal priority collection delete from there.
 							auto it = priority_cementable_frontiers.find (account);
 							if (it != priority_cementable_frontiers.end ())
@@ -714,15 +715,16 @@ void nano::active_transactions::prioritize_frontiers_for_confirmation (nano::tra
 		nano::timer<std::chrono::milliseconds> timer (nano::timer_state::started);
 		auto i (node.store.accounts_begin (transaction_a, next_frontier_account));
 		auto n (node.store.accounts_end ());
-		nano::confirmation_height_info confirmation_height_info;
 		for (; i != n && should_iterate (); ++i)
 		{
 			auto const & account (i->first);
 			auto const & info (i->second);
 			if (priority_wallet_cementable_frontiers.find (account) == priority_wallet_cementable_frontiers.end ())
 			{
-				if (expired_optimistic_election_infos.get<tag_account> ().count (account) == 0 && !node.store.confirmation_height_get (transaction_a, account, confirmation_height_info))
+				if (expired_optimistic_election_infos.get<tag_account> ().count (account) == 0)
 				{
+					nano::confirmation_height_info confirmation_height_info;
+					node.store.confirmation_height_get (transaction_a, account, confirmation_height_info);
 					auto insert_newed = prioritize_account_for_confirmation (priority_cementable_frontiers, priority_cementable_frontiers_size, account, info, confirmation_height_info.height);
 					if (insert_newed)
 					{
@@ -942,9 +944,8 @@ nano::election_insertion_result nano::active_transactions::activate (nano::accou
 	if (!node.store.account_get (transaction, account_a, account_info))
 	{
 		nano::confirmation_height_info conf_info;
-		auto error = node.store.confirmation_height_get (transaction, account_a, conf_info);
-		debug_assert (!error);
-		if (!error && conf_info.height < account_info.block_count)
+		node.store.confirmation_height_get (transaction, account_a, conf_info);
+		if (conf_info.height < account_info.block_count)
 		{
 			debug_assert (conf_info.frontier != account_info.head);
 			auto hash = conf_info.height == 0 ? account_info.open_block : node.store.block_successor (transaction, conf_info.frontier);

nano/node/blockprocessor.cpp

@@ -214,7 +214,7 @@ void nano::block_processor::process_batch (nano::unique_lock<std::mutex> & lock_
 {
 	auto scoped_write_guard = write_database_queue.wait (nano::writer::process_batch);
 	block_post_events post_events;
-	auto transaction (node.store.tx_begin_write ({ tables::accounts, tables::blocks, tables::frontiers, tables::pending, tables::unchecked }, { tables::confirmation_height }));
+	auto transaction (node.store.tx_begin_write ({ tables::accounts, tables::blocks, tables::frontiers, tables::pending, tables::unchecked }));
 	nano::timer<std::chrono::milliseconds> timer_l;
 	lock_a.lock ();
 	timer_l.start ();

nano/node/confirmation_height_bounded.cpp

@@ -112,9 +112,7 @@ void nano::confirmation_height_bounded::process ()
 		}
 		else
 		{
-			auto error = ledger.store.confirmation_height_get (transaction, account, confirmation_height_info);
-			(void)error;
-			debug_assert (!error);
+			ledger.store.confirmation_height_get (transaction, account, confirmation_height_info);
 			// This block was added to the confirmation height processor but is already confirmed
 			if (first_iter && confirmation_height_info.height >= block->sideband ().height && current == original_hash)
 			{
@@ -378,7 +376,7 @@ void nano::confirmation_height_bounded::cement_blocks (nano::write_guard & scope
 #ifndef NDEBUG
 				// Extra debug checks
 				nano::confirmation_height_info confirmation_height_info;
-				debug_assert (!ledger.store.confirmation_height_get (transaction, account, confirmation_height_info));
+				ledger.store.confirmation_height_get (transaction, account, confirmation_height_info);
 				auto block (ledger.store.block_get (transaction, confirmed_frontier));
 				debug_assert (block != nullptr);
 				debug_assert (block->sideband ().height == confirmation_height_info.height + num_blocks_cemented);
@@ -390,16 +388,10 @@ void nano::confirmation_height_bounded::cement_blocks (nano::write_guard & scope
 			};
 
 			nano::confirmation_height_info confirmation_height_info;
-			error = ledger.store.confirmation_height_get (transaction, pending.account, confirmation_height_info);
-			if (error)
-			{
-				auto error_str = (boost::format ("Failed to read confirmation height for account %1% (bounded processor)") % pending.account.to_account ()).str ();
-				logger.always_log (error_str);
-				std::cerr << error_str << std::endl;
-			}
+			ledger.store.confirmation_height_get (transaction, pending.account, confirmation_height_info);
 
 			// Some blocks need to be cemented at least
-			if (!error && pending.top_height > confirmation_height_info.height)
+			if (pending.top_height > confirmation_height_info.height)
 			{
 				// The highest hash which will be cemented
 				nano::block_hash new_cemented_frontier;

nano/node/confirmation_height_unbounded.cpp

@@ -84,7 +84,7 @@ void nano::confirmation_height_unbounded::process ()
 		else
 		{
 			nano::confirmation_height_info confirmation_height_info;
-			release_assert (!ledger.store.confirmation_height_get (read_transaction, account, confirmation_height_info));
+			ledger.store.confirmation_height_get (read_transaction, account, confirmation_height_info);
 			confirmation_height = confirmation_height_info.height;
 
 			// This block was added to the confirmation height processor but is already confirmed
@@ -349,15 +349,9 @@ void nano::confirmation_height_unbounded::cement_blocks (nano::write_guard & sco
 		{
 			auto & pending = pending_writes.front ();
 			nano::confirmation_height_info confirmation_height_info;
-			error = ledger.store.confirmation_height_get (transaction, pending.account, confirmation_height_info);
-			if (error)
-			{
-				auto error_str = (boost::format ("Failed to read confirmation height for account %1% when writing block %2% (unbounded processor)") % pending.account.to_account () % pending.hash.to_string ()).str ();
-				logger.always_log (error_str);
-				std::cerr << error_str << std::endl;
-			}
+			ledger.store.confirmation_height_get (transaction, pending.account, confirmation_height_info);
 			auto confirmation_height = confirmation_height_info.height;
-			if (!error && pending.height > confirmation_height)
+			if (pending.height > confirmation_height)
 			{
 				auto block = ledger.store.block_get (transaction, pending.hash);
 				debug_assert (network_params.network.is_dev_network () || ledger.pruning || block != nullptr);

nano/node/json_handler.cpp

@@ -605,10 +605,7 @@ void nano::json_handler::account_info ()
 		auto transaction (node.store.tx_begin_read ());
 		auto info (account_info_impl (transaction, account));
 		nano::confirmation_height_info confirmation_height_info;
-		if (node.store.confirmation_height_get (transaction, account, confirmation_height_info))
-		{
-			ec = nano::error_common::account_not_found;
-		}
+		node.store.confirmation_height_get (transaction, account, confirmation_height_info);
 		if (!ec)
 		{
 			response_l.put ("frontier", info.head.to_string ());

nano/node/rocksdb/rocksdb.cpp

@@ -497,7 +497,8 @@ uint64_t nano::rocksdb_store::count (nano::transaction const & transaction_a, ta
 	{
 		db->GetIntProperty (table_to_column_family (table_a), "rocksdb.estimate-num-keys", &sum);
 	}
-	// These should only be used in tests to check database consistency
+	// Accounts and blocks should only be used in tests and CLI commands to check database consistency
+	// otherwise there can be performance issues.
 	else if (table_a == tables::accounts)
 	{
 		debug_assert (network_constants ().is_dev_network ());
@@ -508,6 +509,7 @@ uint64_t nano::rocksdb_store::count (nano::transaction const & transaction_a, ta
 	}
 	else if (table_a == tables::blocks)
 	{
+		// This is also used in some CLI commands
 		for (auto i (blocks_begin (transaction_a)), n (blocks_end ()); i != n; ++i)
 		{
 			++sum;

nano/secure/blockstore_partial.hpp

@@ -76,10 +76,7 @@ class block_store_partial : public block_store
 
 	void confirmation_height_clear (nano::write_transaction const & transaction_a) override
 	{
-		for (auto i (confirmation_height_begin (transaction_a)), n (confirmation_height_end ()); i != n; ++i)
-		{
-			confirmation_height_clear (transaction_a, i->first, i->second.height);
-		}
+		drop (transaction_a, nano::tables::confirmation_height);
 	}
 
 	bool pending_exists (nano::transaction const & transaction_a, nano::pending_key const & key_a) override
@@ -509,7 +506,6 @@ class block_store_partial : public block_store
 	void account_put (nano::write_transaction const & transaction_a, nano::account const & account_a, nano::account_info const & info_a) override
 	{
 		// Check we are still in sync with other tables
-		debug_assert (confirmation_height_exists (transaction_a, account_a));
 		nano::db_val<Val> info (info_a);
 		auto status = put (transaction_a, tables::accounts, account_a, info);
 		release_assert (success (status));
@@ -679,6 +675,12 @@ class block_store_partial : public block_store
 			nano::bufferstream stream (reinterpret_cast<uint8_t const *> (value.data ()), value.size ());
 			result = confirmation_height_info_a.deserialize (stream);
 		}
+		if (result)
+		{
+			confirmation_height_info_a.height = 0;
+			confirmation_height_info_a.frontier = 0;
+		}
+
 		return result;
 	}