Merge pull request #4267 from clemahieu/remove_signature_threads

This removes the use of separate signature-checking threads/classes from the node.
These classes add complexity to critical code. Even where performance optimizations could be made through multi-threading, it would be better implemented using standard C++ instead of a custom class.
It's unclear if these classes are helping performance at all so we're opting to remove them until/if a performance improvement is needed.

This PR contains 4 commits:

Adds explicit negative tests for state and epoch blocks to verify incorrect signatures are rejected
All blocks passed to block_processor::add_impl are enqueued in the block processor, rather than splitting them based on the type, and passed to separate signature checking threads. This remove use of signature checking threads from block_processor.
Remove use of signature checking threads from vote_processor and check each vote directly in the vote_processor thread
Remove unused signature checking classes.

nano/core_test/CMakeLists.txt

@@ -41,7 +41,6 @@ add_executable(
   scheduler_buckets.cpp
   request_aggregator.cpp
   signal_manager.cpp
-  signing.cpp
   socket.cpp
   system.cpp
   telemetry.cpp

nano/core_test/blockprocessor.cpp

@@ -39,32 +39,3 @@ TEST (block_processor, broadcast_block_on_arrival)
 	// Checks whether the block was broadcast.
 	ASSERT_TIMELY (5s, node2->ledger.block_or_pruned_exists (send1->hash ()));
 }
-
-TEST (block_processor, add_blocking_invalid_block)
-{
-	nano::test::system system;
-
-	nano::node_config config = system.default_config ();
-	config.block_process_timeout = std::chrono::seconds{ 1 };
-	auto & node = *system.add_node (config);
-
-	nano::state_block_builder builder;
-	auto send1 = builder.make_block ()
-				 .account (nano::dev::genesis_key.pub)
-				 .previous (nano::dev::genesis->hash ())
-				 .representative (nano::dev::genesis_key.pub)
-				 .balance (nano::dev::constants.genesis_amount - nano::Gxrb_ratio)
-				 .link (nano::dev::genesis_key.pub)
-				 .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
-				 .work (*system.work.generate (nano::dev::genesis->hash ()))
-				 .build_shared ();
-
-	send1->signature.clear ();
-
-	auto background = std::async (std::launch::async, [&] () {
-		return node.process_local (send1);
-	});
-
-	ASSERT_TIMELY (5s, background.wait_for (std::chrono::seconds (0)) == std::future_status::ready);
-	ASSERT_FALSE (background.get ().has_value ());
-}

nano/core_test/ledger.cpp

@@ -1239,6 +1239,54 @@ TEST (ledger, fail_change_gap_previous)
 	ASSERT_EQ (nano::process_result::gap_previous, result1.code);
 }
 
+TEST (ledger, fail_state_bad_signature)
+{
+	auto ctx = nano::test::context::ledger_empty ();
+	auto & ledger = ctx.ledger ();
+	auto & store = ctx.store ();
+	auto transaction = store.tx_begin_write ();
+	nano::work_pool pool{ nano::dev::network_params.network, std::numeric_limits<unsigned>::max () };
+	nano::block_builder builder;
+	auto block = builder
+				 .state ()
+				 .account (nano::dev::genesis_key.pub)
+				 .previous (nano::dev::genesis->hash ())
+				 .representative (nano::dev::genesis_key.pub)
+				 .balance (0)
+				 .link (nano::dev::genesis_key.pub)
+				 .sign (nano::keypair ().prv, 0)
+				 .work (*pool.generate (nano::dev::genesis->hash ()))
+				 .build ();
+	auto result1 = ledger.process (transaction, *block);
+	ASSERT_EQ (nano::process_result::bad_signature, result1.code);
+}
+
+TEST (ledger, fail_epoch_bad_signature)
+{
+	auto ctx = nano::test::context::ledger_empty ();
+	auto & ledger = ctx.ledger ();
+	auto & store = ctx.store ();
+	auto transaction = store.tx_begin_write ();
+	nano::work_pool pool{ nano::dev::network_params.network, std::numeric_limits<unsigned>::max () };
+	nano::block_builder builder;
+	auto block = builder
+				 .state ()
+				 .account (nano::dev::genesis_key.pub)
+				 .previous (nano::dev::genesis->hash ())
+				 .representative (nano::dev::genesis_key.pub)
+				 .balance (nano::dev::constants.genesis_amount)
+				 .link (ledger.epoch_link (nano::epoch::epoch_1))
+				 .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
+				 .work (*pool.generate (nano::dev::genesis->hash ()))
+				 .build_shared ();
+	block->signature.bytes[0] ^= 1;
+	auto result1 = ledger.process (transaction, *block);
+	ASSERT_EQ (nano::process_result::bad_signature, result1.code); // Fails epoch signature
+	block->signature.bytes[0] ^= 1;
+	auto result2 = ledger.process (transaction, *block);
+	ASSERT_EQ (nano::process_result::progress, result2.code); // Succeeds with epoch signature
+}
+
 TEST (ledger, fail_change_bad_signature)
 {
 	auto ctx = nano::test::context::ledger_empty ();

nano/core_test/node.cpp

@@ -2143,6 +2143,8 @@ TEST (node, block_confirm)
 	auto send1_copy = builder.make_block ()
 					  .from (*send1)
 					  .build_shared ();
+	auto hash1 = send1->hash ();
+	auto hash2 = send1_copy->hash ();
 	node1.block_processor.add (send1);
 	node2.block_processor.add (send1_copy);
 	ASSERT_TIMELY (5s, node1.ledger.block_or_pruned_exists (send1->hash ()) && node2.ledger.block_or_pruned_exists (send1_copy->hash ()));

nano/lib/numbers.cpp

@@ -428,15 +428,6 @@ bool nano::validate_message (nano::public_key const & public_key, nano::uint256_
 	return validate_message (public_key, message.bytes.data (), sizeof (message.bytes), signature);
 }
 
-bool nano::validate_message_batch (const unsigned char ** m, size_t * mlen, const unsigned char ** pk, const unsigned char ** RS, size_t num, int * valid)
-{
-	for (size_t i{ 0 }; i < num; ++i)
-	{
-		valid[i] = (0 == ed25519_sign_open (m[i], mlen[i], pk[i], RS[i]));
-	}
-	return true;
-}
-
 nano::uint128_union::uint128_union (std::string const & string_a)
 {
 	auto error (decode_hex (string_a));

nano/lib/numbers.hpp

@@ -257,7 +257,6 @@ nano::signature sign_message (nano::raw_key const &, nano::public_key const &, n
 nano::signature sign_message (nano::raw_key const &, nano::public_key const &, uint8_t const *, size_t);
 bool validate_message (nano::public_key const &, nano::uint256_union const &, nano::signature const &);
 bool validate_message (nano::public_key const &, uint8_t const *, size_t, nano::signature const &);
-bool validate_message_batch (unsigned char const **, size_t *, unsigned char const **, unsigned char const **, size_t, int *);
 nano::raw_key deterministic_key (nano::raw_key const &, uint32_t);
 nano::public_key pub_key (nano::raw_key const &);