Apply Colin review

nano/node/bootstrap/bootstrap_attempt.cpp

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

nano/node/bootstrap/bootstrap_bulk_pull.cpp

@@ -419,7 +419,7 @@ void nano::bulk_pull_server::set_current_end ()
 	include_start = false;
 	debug_assert (request != nullptr);
 	auto transaction (connection->node->store.tx_begin_read ());
-	bool bulk_pull_end_exists = send_unconfirmed_blocks ? connection->node->store.block_exists (transaction, request->end) : connection->node->block_confirmed_or_being_confirmed (transaction, request->end);
+	bool bulk_pull_end_exists = send_unconfirmed_blocks () ? connection->node->store.block_exists (transaction, request->end) : connection->node->block_confirmed_or_being_confirmed (transaction, request->end);
 	if (!bulk_pull_end_exists)
 	{
 		if (connection->node->config.logging.bulk_pull_logging ())
@@ -436,13 +436,13 @@ void nano::bulk_pull_server::set_current_end ()
 			connection->node->logger.try_log (boost::str (boost::format ("Bulk pull request for block hash: %1%") % request->start.to_string ()));
 		}
 
-		bool bulk_pull_start_exists = send_unconfirmed_blocks ? connection->node->store.block_exists (transaction, request->start.as_block_hash ()) : connection->node->block_confirmed_or_being_confirmed (transaction, request->start.as_block_hash ());
+		bool bulk_pull_start_exists = send_unconfirmed_blocks () ? connection->node->store.block_exists (transaction, request->start.as_block_hash ()) : connection->node->block_confirmed_or_being_confirmed (transaction, request->start.as_block_hash ());
 		if (bulk_pull_start_exists)
 		{
 			current = request->start.as_block_hash ();
 			include_start = true;
 		}
-		else if (!request->end.is_zero () && !send_unconfirmed_blocks)
+		else if (!request->end.is_zero () && !send_unconfirmed_blocks ())
 		{
 			// Else start from confirmed frontier for given account
 			auto account (connection->node->ledger.account (transaction, request->start.as_block_hash ()));
@@ -459,7 +459,7 @@ void nano::bulk_pull_server::set_current_end ()
 	{
 		bool no_confirmed_or_no_address;
 		nano::block_hash frontier;
-		if (!send_unconfirmed_blocks)
+		if (!send_unconfirmed_blocks ())
 		{
 			nano::confirmation_height_info info;
 			no_confirmed_or_no_address = connection->node->store.confirmation_height_get (transaction, request->start.as_account (), info);
@@ -648,11 +648,15 @@ void nano::bulk_pull_server::no_block_sent (boost::system::error_code const & ec
 	}
 }
 
+bool nano::bulk_pull_server::send_unconfirmed_blocks ()
+{
+	return request->header.bootstrap_are_unconfirmed_blocks_present ();
+}
+
 nano::bulk_pull_server::bulk_pull_server (std::shared_ptr<nano::bootstrap_server> const & connection_a, std::unique_ptr<nano::bulk_pull> request_a) :
 connection (connection_a),
 request (std::move (request_a))
 {
-	send_unconfirmed_blocks = request->header.bootstrap_are_unconfirmed_blocks_present ();
 	set_current_end ();
 }
 

nano/node/bootstrap/bootstrap_bulk_pull.hpp

@@ -69,8 +69,8 @@ class bulk_pull_server final : public std::enable_shared_from_this<nano::bulk_pu
 	void sent_action (boost::system::error_code const &, size_t);
 	void send_finished ();
 	void no_block_sent (boost::system::error_code const &, size_t);
+	bool send_unconfirmed_blocks ();
 	std::shared_ptr<nano::bootstrap_server> connection;
-	bool send_unconfirmed_blocks;
 	std::unique_ptr<nano::bulk_pull> request;
 	nano::block_hash current;
 	bool include_start;

nano/node/bootstrap/bootstrap_frontier.cpp

@@ -237,7 +237,6 @@ frontier (0),
 request (std::move (request_a)),
 count (0)
 {
-	send_unconfirmed_blocks = request->header.bootstrap_are_unconfirmed_blocks_present ();
 	next ();
 }
 
@@ -329,7 +328,7 @@ void nano::frontier_req_server::next ()
 		size_t max_size (128);
 		auto transaction (connection->node->store.tx_begin_read ());
 		// Send only confirmed blocks
-		if (!send_unconfirmed_blocks)
+		if (!send_unconfirmed_blocks ())
 		{
 			for (auto i (connection->node->store.confirmation_height_begin (transaction, current.number () + 1)), n (connection->node->store.confirmation_height_end ()); i != n && accounts.size () != max_size; ++i)
 			{
@@ -379,3 +378,8 @@ void nano::frontier_req_server::next ()
 	frontier = account_pair.second;
 	accounts.pop_front ();
 }
+
+bool nano::frontier_req_server::send_unconfirmed_blocks ()
+{
+	return request->header.bootstrap_are_unconfirmed_blocks_present ();
+}

nano/node/bootstrap/bootstrap_frontier.hpp

@@ -44,11 +44,11 @@ class frontier_req_server final : public std::enable_shared_from_this<nano::fron
 	void send_finished ();
 	void no_block_sent (boost::system::error_code const &, size_t);
 	void next ();
+	bool send_unconfirmed_blocks ();
 	std::shared_ptr<nano::bootstrap_server> connection;
 	nano::account current;
 	nano::block_hash frontier;
 	std::unique_ptr<nano::frontier_req> request;
-	bool send_unconfirmed_blocks;
 	size_t count;
 	std::deque<std::pair<nano::account, nano::block_hash>> accounts;
 };

nano/node/bootstrap/bootstrap_server.cpp

@@ -305,14 +305,7 @@ void nano::bootstrap_server::receive_bulk_pull_action (boost::system::error_code
 		{
 			if (node->config.logging.bulk_pull_logging ())
 			{
-				if (!request->start.is_zero ())
-				{
-					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 ())));
-				}
-				else
-				{
-					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));
-				}
+				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)
 			{