From 648ef62389f826ba5d2886bac0470081de16f626 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20W=C3=B3jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Thu, 8 Dec 2022 15:22:59 +0100 Subject: [PATCH] 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 --- nano/core_test/bootstrap_server.cpp | 8 +++++ nano/node/bootstrap/bootstrap_server.cpp | 45 +++++++++++++++--------- nano/node/messages.cpp | 4 +++ nano/node/messages.hpp | 8 +++++ 4 files changed, 48 insertions(+), 17 deletions(-) diff --git a/nano/core_test/bootstrap_server.cpp b/nano/core_test/bootstrap_server.cpp index 3a1032d088..3753c8f008 100644 --- 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 index 2358c51a08..068dee18c4 100644 --- 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 (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 index f6048f0f30..a02f943a1e 100644 --- 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 index 86dd4b3264..3395fc5b8e 100644 --- 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