Specify whether bootstrap pull should start at block hash or account (#…

…4018) * Specify whether pull should start at block hash or account * Specify account info request target type
nanocurrency · Dec 9, 2022 · 648ef62 · 648ef62
1 parent dc2447b
commit 648ef62
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 17 deletions.
diff --git a/nano/core_test/bootstrap_server.cpp b/nano/core_test/bootstrap_server.cpp
@@ -171,6 +171,7 @@ TEST (bootstrap_server, serve_account_blocks)
 	nano::asc_pull_req::blocks_payload request_payload;
 	request_payload.start = first_account;
 	request_payload.count = nano::bootstrap_server::max_blocks;
+	request_payload.start_type = nano::asc_pull_req::hash_type::account;
 
 	request.payload = request_payload;
 	request.update_header ();
@@ -217,6 +218,7 @@ TEST (bootstrap_server, serve_hash)
 	nano::asc_pull_req::blocks_payload request_payload;
 	request_payload.start = blocks.front ()->hash ();
 	request_payload.count = nano::bootstrap_server::max_blocks;
+	request_payload.start_type = nano::asc_pull_req::hash_type::block;
 
 	request.payload = request_payload;
 	request.update_header ();
@@ -263,6 +265,7 @@ TEST (bootstrap_server, serve_hash_one)
 	nano::asc_pull_req::blocks_payload request_payload;
 	request_payload.start = blocks.front ()->hash ();
 	request_payload.count = 1;
+	request_payload.start_type = nano::asc_pull_req::hash_type::block;
 
 	request.payload = request_payload;
 	request.update_header ();
@@ -303,6 +306,7 @@ TEST (bootstrap_server, serve_end_of_chain)
 	nano::asc_pull_req::blocks_payload request_payload;
 	request_payload.start = blocks.back ()->hash ();
 	request_payload.count = nano::bootstrap_server::max_blocks;
+	request_payload.start_type = nano::asc_pull_req::hash_type::block;
 
 	request.payload = request_payload;
 	request.update_header ();
@@ -343,6 +347,7 @@ TEST (bootstrap_server, serve_missing)
 	nano::asc_pull_req::blocks_payload request_payload;
 	request_payload.start = nano::test::random_hash ();
 	request_payload.count = nano::bootstrap_server::max_blocks;
+	request_payload.start_type = nano::asc_pull_req::hash_type::block;
 
 	request.payload = request_payload;
 	request.update_header ();
@@ -387,6 +392,7 @@ TEST (bootstrap_server, serve_multiple)
 			nano::asc_pull_req::blocks_payload request_payload;
 			request_payload.start = account;
 			request_payload.count = nano::bootstrap_server::max_blocks;
+			request_payload.start_type = nano::asc_pull_req::hash_type::account;
 
 			request.payload = request_payload;
 			request.update_header ();
@@ -444,6 +450,7 @@ TEST (bootstrap_server, serve_account_info)
 
 	nano::asc_pull_req::account_info_payload request_payload;
 	request_payload.target = account;
+	request_payload.target_type = nano::asc_pull_req::hash_type::account;
 
 	request.payload = request_payload;
 	request.update_header ();
@@ -491,6 +498,7 @@ TEST (bootstrap_server, serve_account_info_missing)
 
 	nano::asc_pull_req::account_info_payload request_payload;
 	request_payload.target = nano::test::random_account ();
+	request_payload.target_type = nano::asc_pull_req::hash_type::account;
 
 	request.payload = request_payload;
 	request.update_header ();

diff --git a/nano/node/bootstrap/bootstrap_server.cpp b/nano/node/bootstrap/bootstrap_server.cpp
@@ -174,23 +174,26 @@ nano::asc_pull_ack nano::bootstrap_server::process (nano::transaction const & tr
 {
 	const std::size_t count = std::min (static_cast<std::size_t> (request.count), max_blocks);
 
-	// `start` can represent either account or block hash
-	if (store.block.exists (transaction, request.start.as_block_hash ()))
+	switch (request.start_type)
 	{
-		return prepare_response (transaction, id, request.start.as_block_hash (), count);
-	}
-	if (store.account.exists (transaction, request.start.as_account ()))
-	{
-		auto info = store.account.get (transaction, request.start.as_account ());
-		if (info)
+		case asc_pull_req::hash_type::block:
 		{
-			// Start from open block if pulling by account
-			return prepare_response (transaction, id, info->open_block, count);
+			if (store.block.exists (transaction, request.start.as_block_hash ()))
+			{
+				return prepare_response (transaction, id, request.start.as_block_hash (), count);
+			}
 		}
-		else
+		break;
+		case asc_pull_req::hash_type::account:
 		{
-			debug_assert (false, "account exists but cannot be retrieved");
+			auto info = store.account.get (transaction, request.start.as_account ());
+			if (info)
+			{
+				// Start from open block if pulling by account
+				return prepare_response (transaction, id, info->open_block, count);
+			}
 		}
+		break;
 	}
 
 	// Neither block nor account found, send empty response to indicate that
@@ -258,13 +261,21 @@ nano::asc_pull_ack nano::bootstrap_server::process (const nano::transaction & tr
 	response.id = id;
 	response.type = nano::asc_pull_type::account_info;
 
-	auto target = request.target.as_account ();
-	// Try to lookup account assuming target is block hash
-	if (auto account_from_hash = ledger.account_safe (transaction, request.target.as_block_hash ()); !account_from_hash.is_zero ())
+	nano::account target{ 0 };
+	switch (request.target_type)
 	{
-		target = account_from_hash;
+		case asc_pull_req::hash_type::account:
+		{
+			target = request.target.as_account ();
+		}
+		break;
+		case asc_pull_req::hash_type::block:
+		{
+			// Try to lookup account assuming target is block hash
+			target = ledger.account_safe (transaction, request.target.as_block_hash ());
+		}
+		break;
 	}
-	// Otherwise assume target is an actual account
 
 	nano::asc_pull_ack::account_info_payload response_payload{};
 	response_payload.account = target;

diff --git a/nano/node/messages.cpp b/nano/node/messages.cpp
@@ -1747,12 +1747,14 @@ void nano::asc_pull_req::blocks_payload::serialize (nano::stream & stream) const
 {
 	nano::write (stream, start);
 	nano::write (stream, count);
+	nano::write (stream, start_type);
 }
 
 void nano::asc_pull_req::blocks_payload::deserialize (nano::stream & stream)
 {
 	nano::read (stream, start);
 	nano::read (stream, count);
+	nano::read (stream, start_type);
 }
 
 /*
@@ -1762,11 +1764,13 @@ void nano::asc_pull_req::blocks_payload::deserialize (nano::stream & stream)
 void nano::asc_pull_req::account_info_payload::serialize (stream & stream) const
 {
 	nano::write (stream, target);
+	nano::write (stream, target_type);
 }
 
 void nano::asc_pull_req::account_info_payload::deserialize (stream & stream)
 {
 	nano::read (stream, target);
+	nano::read (stream, target_type);
 }
 
 /*

diff --git a/nano/node/messages.hpp b/nano/node/messages.hpp
@@ -414,6 +414,12 @@ class asc_pull_req final : public message
 	bool verify_consistency () const;
 
 public: // Payload definitions
+	enum class hash_type : uint8_t
+	{
+		account = 0,
+		block = 1,
+	};
+
 	class blocks_payload
 	{
 	public:
@@ -423,6 +429,7 @@ class asc_pull_req final : public message
 	public:
 		nano::hash_or_account start{ 0 };
 		uint8_t count{ 0 };
+		asc_pull_req::hash_type start_type{ 0 };
 	};
 
 	class account_info_payload
@@ -433,6 +440,7 @@ class asc_pull_req final : public message
 
 	public:
 		nano::hash_or_account target{ 0 };
+		asc_pull_req::hash_type target_type{ 0 };
 	};
 
 public: // Payload