Bootstrap tests improvements (#3103)

* Bootstrap tests improvements
Separated from #3047
- additionally prevent incorrect legacy bootstrap paths
- fix some comments
- add ledger function block_confirmed_or_pruned_exists ()

nano/core_test/node.cpp

@@ -1333,7 +1333,7 @@ TEST (node, fork_bootstrap_flip)
 		auto transaction (node2.store.tx_begin_read ());
 		ASSERT_TRUE (node2.store.block_exists (transaction, send2->hash ()));
 	}
-	node2.bootstrap_initiator.bootstrap (node1.network.endpoint ());
+	node2.bootstrap_initiator.bootstrap (node1.network.endpoint ()); // Additionally add new peer to confirm & replace bootstrap block
 	auto again (true);
 	system1.deadline_set (50s);
 	while (again)
@@ -1606,7 +1606,7 @@ TEST (node, DISABLED_fork_stale)
 	nano::system system2 (1);
 	auto & node1 (*system1.nodes[0]);
 	auto & node2 (*system2.nodes[0]);
-	node2.bootstrap_initiator.bootstrap (node1.network.endpoint ());
+	node2.bootstrap_initiator.bootstrap (node1.network.endpoint (), false);
 	std::shared_ptr<nano::transport::channel> channel (std::make_shared<nano::transport::channel_udp> (node2.network.udp_channels, node1.network.endpoint (), node2.network_params.protocol.protocol_version));
 	auto vote = std::make_shared<nano::vote> (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, 0, std::vector<nano::block_hash> ());
 	node2.rep_crawler.response (channel, vote);
@@ -1661,7 +1661,7 @@ TEST (node, DISABLED_fork_stale)
 	node1.process_active (send2);
 	node2.process_active (send1);
 	node2.process_active (send2);
-	node2.bootstrap_initiator.bootstrap (node1.network.endpoint ());
+	node2.bootstrap_initiator.bootstrap (node1.network.endpoint (), false);
 	while (node2.block (send1->hash ()) == nullptr)
 	{
 		system1.poll ();
@@ -1872,7 +1872,7 @@ TEST (node, DISABLED_bootstrap_no_publish)
 		ASSERT_EQ (nano::process_result::progress, node0->ledger.process (transaction, send0).code);
 	}
 	ASSERT_FALSE (node1->bootstrap_initiator.in_progress ());
-	node1->bootstrap_initiator.bootstrap (node0->network.endpoint ());
+	node1->bootstrap_initiator.bootstrap (node0->network.endpoint (), false);
 	ASSERT_TRUE (node1->active.empty ());
 	system1.deadline_set (10s);
 	while (node1->block (send0.hash ()) == nullptr)
@@ -1899,20 +1899,21 @@ TEST (node, bootstrap_bulk_push)
 	auto node1 (system1.add_node (config1));
 	nano::keypair key0;
 	// node0 knows about send0 but node1 doesn't.
-	auto send0 = *nano::send_block_builder ()
-	              .previous (nano::genesis_hash)
-	              .destination (key0.pub)
-	              .balance (500)
-	              .sign (nano::dev_genesis_key.prv, nano::dev_genesis_key.pub)
-	              .work (*node0->work_generate_blocking (nano::genesis_hash))
-	              .build ();
-	ASSERT_EQ (nano::process_result::progress, node0->process (send0).code);
+	auto send0 = nano::send_block_builder ()
+	             .previous (nano::genesis_hash)
+	             .destination (key0.pub)
+	             .balance (500)
+	             .sign (nano::dev_genesis_key.prv, nano::dev_genesis_key.pub)
+	             .work (*node0->work_generate_blocking (nano::genesis_hash))
+	             .build_shared ();
+	ASSERT_EQ (nano::process_result::progress, node0->process (*send0).code);
+
 	ASSERT_FALSE (node0->bootstrap_initiator.in_progress ());
 	ASSERT_FALSE (node1->bootstrap_initiator.in_progress ());
 	ASSERT_TRUE (node1->active.empty ());
 	node0->bootstrap_initiator.bootstrap (node1->network.endpoint (), false);
 	system1.deadline_set (10s);
-	while (node1->block (send0.hash ()) == nullptr)
+	while (node1->block (send0->hash ()) == nullptr)
 	{
 		ASSERT_NO_ERROR (system0.poll ());
 		ASSERT_NO_ERROR (system1.poll ());
@@ -1977,7 +1978,7 @@ TEST (node, bootstrap_fork_open)
 	ASSERT_EQ (nano::process_result::progress, node1->process (open1).code);
 	ASSERT_FALSE (node1->ledger.block_exists (open0.hash ()));
 	ASSERT_FALSE (node1->bootstrap_initiator.in_progress ());
-	node1->bootstrap_initiator.bootstrap (node0->network.endpoint ());
+	node1->bootstrap_initiator.bootstrap (node0->network.endpoint (), false);
 	ASSERT_TRUE (node1->active.empty ());
 	ASSERT_TIMELY (10s, !node1->ledger.block_exists (open1.hash ()) && node1->ledger.block_exists (open0.hash ()));
 }
@@ -1988,24 +1989,25 @@ TEST (node, bootstrap_confirm_frontiers)
 	nano::system system0 (1);
 	nano::system system1 (1);
 	auto node0 (system0.nodes[0]);
-	auto node1 (system0.nodes[0]);
+	auto node1 (system1.nodes[0]);
 	system0.wallet (0)->insert_adhoc (nano::dev_genesis_key.prv);
 	nano::keypair key0;
 	// node0 knows about send0 but node1 doesn't.
-	auto send0 = *nano::send_block_builder ()
-	              .previous (nano::genesis_hash)
-	              .destination (key0.pub)
-	              .balance (nano::genesis_amount - 500)
-	              .sign (nano::dev_genesis_key.prv, nano::dev_genesis_key.pub)
-	              .work (*node0->work_generate_blocking (nano::genesis_hash))
-	              .build ();
-	ASSERT_EQ (nano::process_result::progress, node0->process (send0).code);
+	auto send0 = nano::send_block_builder ()
+	             .previous (nano::genesis_hash)
+	             .destination (key0.pub)
+	             .balance (nano::genesis_amount - 500)
+	             .sign (nano::dev_genesis_key.prv, nano::dev_genesis_key.pub)
+	             .work (*node0->work_generate_blocking (nano::genesis_hash))
+	             .build_shared ();
+	ASSERT_EQ (nano::process_result::progress, node0->process (*send0).code);
+
 	ASSERT_FALSE (node0->bootstrap_initiator.in_progress ());
 	ASSERT_FALSE (node1->bootstrap_initiator.in_progress ());
 	ASSERT_TRUE (node1->active.empty ());
-	node1->bootstrap_initiator.bootstrap (node0->network.endpoint ());
+	node1->bootstrap_initiator.bootstrap (node0->network.endpoint ()); // Additionally add new peer to confirm bootstrap frontier
 	system1.deadline_set (10s);
-	while (node1->block (send0.hash ()) == nullptr)
+	while (node1->block (send0->hash ()) == nullptr)
 	{
 		ASSERT_NO_ERROR (system0.poll ());
 		ASSERT_NO_ERROR (system1.poll ());
@@ -2019,7 +2021,7 @@ TEST (node, bootstrap_confirm_frontiers)
 	}
 	{
 		nano::lock_guard<nano::mutex> guard (node1->active.mutex);
-		auto existing1 (node1->active.blocks.find (send0.hash ()));
+		auto existing1 (node1->active.blocks.find (send0->hash ()));
 		ASSERT_NE (node1->active.blocks.end (), existing1);
 	}
 	// Wait for confirmation height update
@@ -2029,7 +2031,7 @@ TEST (node, bootstrap_confirm_frontiers)
 	{
 		{
 			auto transaction (node1->store.tx_begin_read ());
-			done = node1->ledger.block_confirmed (transaction, send0.hash ());
+			done = node1->ledger.block_confirmed (transaction, send0->hash ());
 		}
 		ASSERT_NO_ERROR (system0.poll ());
 		ASSERT_NO_ERROR (system1.poll ());

nano/node/bootstrap/bootstrap_bulk_pull.cpp

@@ -31,8 +31,9 @@ unexpected_count (0)
 
 nano::bulk_pull_client::~bulk_pull_client ()
 {
-	// If received end block is not expected end block
-	if (expected != pull.end)
+	/* If received end block is not expected end block
+	Or if given start and end blocks are from different chains (i.e. forked node or malicious node) */
+	if (expected != pull.end && !expected.is_zero ())
 	{
 		pull.head = expected;
 		if (attempt->mode != nano::bootstrap_mode::legacy)

nano/node/bootstrap/bootstrap_frontier.cpp

@@ -208,7 +208,7 @@ void nano::frontier_req_client::next ()
 			accounts.emplace_back (account, info.head);
 		}
 		/* If loop breaks before max_size, then accounts_end () is reached
-		Add empty record to finish frontier_req_server */
+		Add empty record */
 		if (accounts.size () != max_size)
 		{
 			accounts.emplace_back (nano::account (0), nano::block_hash (0));
@@ -240,6 +240,8 @@ void nano::frontier_req_server::send_next ()
 			nano::vectorstream stream (send_buffer);
 			write (stream, current.bytes);
 			write (stream, frontier.bytes);
+			debug_assert (!current.is_zero ());
+			debug_assert (!frontier.is_zero ());
 		}
 		auto this_l (shared_from_this ());
 		if (connection->node->config.logging.bulk_pull_logging ())
@@ -313,13 +315,13 @@ void nano::frontier_req_server::next ()
 	if (accounts.empty ())
 	{
 		auto now (nano::seconds_since_epoch ());
-		bool skip_old (request->age != std::numeric_limits<decltype (request->age)>::max ());
+		bool disable_age_filter (request->age == std::numeric_limits<decltype (request->age)>::max ());
 		size_t max_size (128);
 		auto transaction (connection->node->store.tx_begin_read ());
 		for (auto i (connection->node->store.accounts_begin (transaction, current.number () + 1)), n (connection->node->store.accounts_end ()); i != n && accounts.size () != max_size; ++i)
 		{
 			nano::account_info const & info (i->second);
-			if (!skip_old || (now - info.modified) <= request->age)
+			if (disable_age_filter || (now - info.modified) <= request->age)
 			{
 				nano::account const & account (i->first);
 				accounts.emplace_back (account, info.head);

nano/node/bootstrap/bootstrap_legacy.cpp

@@ -88,9 +88,12 @@ void nano::bootstrap_attempt_legacy::request_push (nano::unique_lock<nano::mutex
 
 void nano::bootstrap_attempt_legacy::add_frontier (nano::pull_info const & pull_a)
 {
-	nano::pull_info pull (pull_a);
-	nano::lock_guard<nano::mutex> lock (mutex);
-	frontier_pulls.push_back (pull);
+	// Prevent incorrect or malicious pulls with frontier 0 insertion
+	if (!pull_a.head.is_zero ())
+	{
+		nano::lock_guard<nano::mutex> lock (mutex);
+		frontier_pulls.push_back (pull_a);
+	}
 }
 
 void nano::bootstrap_attempt_legacy::add_bulk_push_target (nano::block_hash const & head, nano::block_hash const & end)

nano/node/bootstrap/bootstrap_server.cpp

@@ -305,7 +305,7 @@ void nano::bootstrap_server::receive_bulk_pull_action (boost::system::error_code
 		{
 			if (node->config.logging.bulk_pull_logging ())
 			{
-				node->logger.try_log (boost::str (boost::format ("Received bulk pull for %1% down to %2%, maximum of %3%") % request->start.to_string () % request->end.to_string () % (request->count ? request->count : std::numeric_limits<double>::infinity ())));
+				node->logger.try_log (boost::str (boost::format ("Received bulk pull for %1% down to %2%, maximum of %3% from %4%") % request->start.to_string () % request->end.to_string () % (request->count ? request->count : std::numeric_limits<double>::infinity ()) % remote_endpoint));
 			}
 			if (is_bootstrap_connection () && !node->flags.disable_bootstrap_bulk_pull_server)
 			{

nano/node/testing.cpp

@@ -202,7 +202,7 @@ std::unique_ptr<nano::state_block> nano::upgrade_epoch (nano::work_pool & pool_a
 	return !error ? std::move (epoch) : nullptr;
 }
 
-void nano::blocks_confirm (nano::node & node_a, std::vector<std::shared_ptr<nano::block>> const & blocks_a)
+void nano::blocks_confirm (nano::node & node_a, std::vector<std::shared_ptr<nano::block>> const & blocks_a, bool const forced_a)
 {
 	// Finish processing all blocks
 	node_a.block_processor.flush ();
@@ -213,6 +213,12 @@ void nano::blocks_confirm (nano::node & node_a, std::vector<std::shared_ptr<nano
 		debug_assert (disk_block != nullptr);
 		debug_assert (disk_block->has_sideband ());
 		node_a.block_confirm (disk_block);
+		if (forced_a)
+		{
+			auto election = node_a.active.election (disk_block->qualified_root ());
+			debug_assert (election != nullptr);
+			election->force_confirm ();
+		}
 	}
 }
 

nano/node/testing.hpp

@@ -55,7 +55,7 @@ class system final
 	unsigned node_sequence{ 0 };
 };
 std::unique_ptr<nano::state_block> upgrade_epoch (nano::work_pool &, nano::ledger &, nano::epoch);
-void blocks_confirm (nano::node &, std::vector<std::shared_ptr<nano::block>> const &);
+void blocks_confirm (nano::node &, std::vector<std::shared_ptr<nano::block>> const &, bool const = false);
 uint16_t get_available_port ();
 void cleanup_dev_directories_on_exit ();
 /** To use RocksDB in tests make sure the environment variable TEST_USE_ROCKSDB=1 is set */

nano/secure/ledger.cpp

@@ -840,7 +840,7 @@ nano::uint128_t nano::ledger::account_pending (nano::transaction const & transac
 		nano::pending_info const & info (i->second);
 		if (only_confirmed_a)
 		{
-			if (block_confirmed (transaction_a, i->first.hash) || (pruning && store.pruned_exists (transaction_a, i->first.hash)))
+			if (block_confirmed_or_pruned_exists (transaction_a, i->first.hash))
 			{
 				result += info.amount.number ();
 			}
@@ -894,6 +894,11 @@ bool nano::ledger::block_or_pruned_exists (nano::block_hash const & hash_a) cons
 	return block_or_pruned_exists (store.tx_begin_read (), hash_a);
 }
 
+bool nano::ledger::block_confirmed_or_pruned_exists (nano::transaction const & transaction_a, nano::block_hash const & hash_a) const
+{
+	return block_confirmed (transaction_a, hash_a) || (pruning && store.pruned_exists (transaction_a, hash_a));
+}
+
 std::string nano::ledger::block_text (char const * hash_a)
 {
 	return block_text (nano::block_hash (hash_a));

nano/secure/ledger.hpp

@@ -48,6 +48,7 @@ class ledger final
 	bool block_exists (nano::block_hash const &) const;
 	bool block_or_pruned_exists (nano::transaction const &, nano::block_hash const &) const;
 	bool block_or_pruned_exists (nano::block_hash const &) const;
+	bool block_confirmed_or_pruned_exists (nano::transaction const &, nano::block_hash const &) const;
 	std::string block_text (char const *);
 	std::string block_text (nano::block_hash const &);
 	bool is_send (nano::transaction const &, nano::state_block const &) const;