diff --git a/nano/core_test/signing.cpp b/nano/core_test/signing.cpp index 6ac902d3..30704623 100644 --- a/nano/core_test/signing.cpp +++ b/nano/core_test/signing.cpp @@ -147,3 +147,55 @@ TEST (signature_checker, one) block.signature.bytes[31] ^= 0x1; verify_block (block, 1); } + +TEST (signature_checker, boundary_checks) +{ + // sizes container must be in incrementing order + std::vector sizes{ 0, 1 }; + auto add_boundary = [&sizes](size_t boundary) { + sizes.insert (sizes.end (), { boundary - 1, boundary, boundary + 1 }); + }; + + for (auto i = 1; i <= 5; ++i) + { + add_boundary (nano::signature_checker::batch_size * i); + } + + nano::signature_checker checker (1); + auto max_size = *(sizes.end () - 1); + std::vector hashes; + hashes.reserve (max_size); + std::vector messages; + messages.reserve (max_size); + std::vector lengths; + lengths.reserve (max_size); + std::vector pub_keys; + pub_keys.reserve (max_size); + std::vector signatures; + signatures.reserve (max_size); + nano::keypair key; + nano::state_block block (key.pub, 0, key.pub, 0, 0, key.prv, key.pub, 0); + + auto last_size = 0; + for (auto size : sizes) + { + // The size needed to append to existing containers, saves re-initializing from scratch each iteration + auto extra_size = size - last_size; + + std::vector verifications; + verifications.resize (size); + for (auto i (0); i < extra_size; ++i) + { + hashes.push_back (block.hash ()); + messages.push_back (hashes.back ().bytes.data ()); + lengths.push_back (sizeof (decltype (hashes)::value_type)); + pub_keys.push_back (block.hashables.account.bytes.data ()); + signatures.push_back (block.signature.bytes.data ()); + } + nano::signature_check_set check = { size, messages.data (), lengths.data (), pub_keys.data (), signatures.data (), verifications.data () }; + checker.verify (check); + bool all_valid = std::all_of (verifications.cbegin (), verifications.cend (), [](auto verification) { return verification == 1; }); + ASSERT_TRUE (all_valid); + last_size = size; + } +} diff --git a/nano/node/signatures.cpp b/nano/node/signatures.cpp index ff4d2dea..69d347c3 100644 --- a/nano/node/signatures.cpp +++ b/nano/node/signatures.cpp @@ -50,9 +50,16 @@ void nano::signature_checker::verify (nano::signature_check_set & check_a) auto num_full_batches_thread = (num_base_batches_each * num_threads); if (num_full_overflow_batches > 0) { - size_calling_thread += batch_size; - auto remaining = num_full_overflow_batches - 1; - num_full_batches_thread += remaining; + if (overflow_size == 0) + { + // Give the calling thread priority over any batches when there is no excess remainder. + size_calling_thread += batch_size; + num_full_batches_thread += num_full_overflow_batches - 1; + } + else + { + num_full_batches_thread += num_full_overflow_batches; + } } release_assert (check_a.size == (num_full_batches_thread * batch_size + size_calling_thread)); diff --git a/nano/node/signatures.hpp b/nano/node/signatures.hpp index 72f26b9f..b1d65d7a 100644 --- a/nano/node/signatures.hpp +++ b/nano/node/signatures.hpp @@ -35,6 +35,8 @@ public: void stop (); void flush (); + static size_t constexpr batch_size = 256; + private: struct Task final { @@ -55,7 +57,6 @@ private: void set_thread_names (unsigned num_threads); boost::asio::thread_pool thread_pool; std::atomic tasks_remaining{ 0 }; - static constexpr size_t batch_size = 256; const bool single_threaded; unsigned num_threads; std::atomic stopped{ false }; diff --git a/nano/node/state_block_signature_verification.cpp b/nano/node/state_block_signature_verification.cpp index 4402ef22..5683a0b5 100644 --- a/nano/node/state_block_signature_verification.cpp +++ b/nano/node/state_block_signature_verification.cpp @@ -46,7 +46,7 @@ void nano::state_block_signature_verification::run (uint64_t state_block_signatu { if (!state_blocks.empty ()) { - size_t const max_verification_batch (state_block_signature_verification_size != 0 ? state_block_signature_verification_size : 256 * (node_config.signature_checker_threads + 1)); + size_t const max_verification_batch (state_block_signature_verification_size != 0 ? state_block_signature_verification_size : nano::signature_checker::batch_size * (node_config.signature_checker_threads + 1)); active = true; while (!state_blocks.empty () && !stopped) { diff --git a/nano/slow_test/node.cpp b/nano/slow_test/node.cpp index d794c126..42a9cf87 100644 --- a/nano/slow_test/node.cpp +++ b/nano/slow_test/node.cpp @@ -986,3 +986,59 @@ namespace transport } } } + +// Similar to signature_checker.boundary_checks but more exhaustive. Can take up to 1 minute +TEST (signature_checker, mass_boundary_checks) +{ + // sizes container must be in incrementing order + std::vector sizes{ 0, 1 }; + auto add_boundary = [&sizes](size_t boundary) { + sizes.insert (sizes.end (), { boundary - 1, boundary, boundary + 1 }); + }; + + for (auto i = 1; i <= 10; ++i) + { + add_boundary (nano::signature_checker::batch_size * i); + } + + for (auto num_threads = 0; num_threads < 5; ++num_threads) + { + nano::signature_checker checker (num_threads); + auto max_size = *(sizes.end () - 1); + std::vector hashes; + hashes.reserve (max_size); + std::vector messages; + messages.reserve (max_size); + std::vector lengths; + lengths.reserve (max_size); + std::vector pub_keys; + pub_keys.reserve (max_size); + std::vector signatures; + signatures.reserve (max_size); + nano::keypair key; + nano::state_block block (key.pub, 0, key.pub, 0, 0, key.prv, key.pub, 0); + + auto last_size = 0; + for (auto size : sizes) + { + // The size needed to append to existing containers, saves re-initializing from scratch each iteration + auto extra_size = size - last_size; + + std::vector verifications; + verifications.resize (size); + for (auto i (0); i < extra_size; ++i) + { + hashes.push_back (block.hash ()); + messages.push_back (hashes.back ().bytes.data ()); + lengths.push_back (sizeof (decltype (hashes)::value_type)); + pub_keys.push_back (block.hashables.account.bytes.data ()); + signatures.push_back (block.signature.bytes.data ()); + } + nano::signature_check_set check = { size, messages.data (), lengths.data (), pub_keys.data (), signatures.data (), verifications.data () }; + checker.verify (check); + bool all_valid = std::all_of (verifications.cbegin (), verifications.cend (), [](auto verification) { return verification == 1; }); + ASSERT_TRUE (all_valid); + last_size = size; + } + } +}