Remove coupling of block processor and unchecked_map on unchecked_info::modified. (#3728)

* Currently the modified timestamp is updated by the block processor which is a violation of separations of concerns with the unchecked_map. This moves responsibility of setting modification timestamp in to unchecked_map.

* The block_arrival container tracks blocks that are from realtime traffic instead of bootstrap traffic. This is sufficient to trigger process_live on its own. Checking the modification timestamp from unchecked_map is unrelated.

* Encapsulating nano::unchecked_info::modified since it's only set by the unchecked_map class itself when writing.

* Remove nano::unchecked_info friend class in favor of setting nano::unchecked_info::modified in the constructor.

* Removes the unnecessary origination field from block_processor::add

Co-authored-by: Thiago Silva <thiago@nano.org>

nano/core_test/gap_cache.cpp

@@ -95,13 +95,13 @@ TEST (gap_cache, two_dependencies)
 	auto send2 (std::make_shared<nano::send_block> (send1->hash (), key.pub, 0, nano::dev::genesis_key.prv, nano::dev::genesis_key.pub, *system.work.generate (send1->hash ())));
 	auto open (std::make_shared<nano::open_block> (send1->hash (), key.pub, key.pub, key.prv, key.pub, *system.work.generate (key.pub)));
 	ASSERT_EQ (0, node1.gap_cache.size ());
-	node1.block_processor.add (send2, nano::seconds_since_epoch ());
+	node1.block_processor.add (send2);
 	node1.block_processor.flush ();
 	ASSERT_EQ (1, node1.gap_cache.size ());
-	node1.block_processor.add (open, nano::seconds_since_epoch ());
+	node1.block_processor.add (open);
 	node1.block_processor.flush ();
 	ASSERT_EQ (2, node1.gap_cache.size ());
-	node1.block_processor.add (send1, nano::seconds_since_epoch ());
+	node1.block_processor.add (send1);
 	node1.block_processor.flush ();
 	ASSERT_TIMELY (5s, node1.gap_cache.size () == 0);
 	auto transaction (node1.store.tx_begin_read ());

nano/core_test/node.cpp

@@ -2205,8 +2205,8 @@ TEST (node, block_confirm)
 		auto send1_copy = builder.make_block ()
 						  .from (*send1)
 						  .build_shared ();
-		node1.block_processor.add (send1, nano::seconds_since_epoch ());
+		node1.block_processor.add (send1);
-		node2.block_processor.add (send1_copy, nano::seconds_since_epoch ());
+		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 ()));
 		ASSERT_TRUE (node1.ledger.block_or_pruned_exists (send1->hash ()));
 		ASSERT_TRUE (node2.ledger.block_or_pruned_exists (send1_copy->hash ()));

nano/core_test/unchecked_map.cpp

@@ -49,7 +49,7 @@ TEST (unchecked_map, construction)
 TEST (unchecked_map, put_one)
 {
 	context context;
-	nano::unchecked_info info{ block (), nano::dev::genesis_key.pub, nano::seconds_since_epoch () };
+	nano::unchecked_info info{ block (), nano::dev::genesis_key.pub };
 	context.unchecked.put (info.block->previous (), info);
 }
 

nano/nano_node/entry.cpp

@@ -1824,7 +1824,7 @@ int main (int argc, char * const * argv)
 							{
 								std::cout << boost::str (boost::format ("%1% blocks retrieved") % count) << std::endl;
 							}
-							nano::unchecked_info unchecked_info (block, account, 0, nano::signature_verification::unknown);
+							nano::unchecked_info unchecked_info (block, account, nano::signature_verification::unknown);
 							node.node->block_processor.add (unchecked_info);
 							if (block->type () == nano::block_type::state && block->previous ().is_zero () && source_node->ledger.is_epoch_link (block->link ()))
 							{

nano/node/blockprocessor.cpp

@@ -97,9 +97,9 @@ bool nano::block_processor::half_full ()
 	return size () >= node.flags.block_processor_full_size / 2;
 }
 
-void nano::block_processor::add (std::shared_ptr<nano::block> const & block_a, uint64_t origination)
+void nano::block_processor::add (std::shared_ptr<nano::block> const & block_a)
 {
-	nano::unchecked_info info (block_a, 0, origination, nano::signature_verification::unknown);
+	nano::unchecked_info info (block_a, 0, nano::signature_verification::unknown);
 	add (info);
 }
 
@@ -256,7 +256,7 @@ void nano::block_processor::process_batch (nano::unique_lock<nano::mutex> & lock
 		}
 		else
 		{
-			info = nano::unchecked_info (forced.front (), 0, nano::seconds_since_epoch (), nano::signature_verification::unknown);
+			info = nano::unchecked_info (forced.front (), 0, nano::signature_verification::unknown);
 			forced.pop_front ();
 			hash = info.block->hash ();
 			force = true;
@@ -353,7 +353,7 @@ nano::process_return nano::block_processor::process_one (nano::write_transaction
 				block->serialize_json (block_string, node.config.logging.single_line_record ());
 				node.logger.try_log (boost::str (boost::format ("Processing block %1%: %2%") % hash.to_string () % block_string));
 			}
-			if ((info_a.modified > nano::seconds_since_epoch () - 300 && node.block_arrival.recent (hash)) || forced_a)
+			if (node.block_arrival.recent (hash) || forced_a)
 			{
 				events_a.events.emplace_back ([this, hash, block = info_a.block, result, origin_a] (nano::transaction const & post_event_transaction_a) { process_live (post_event_transaction_a, hash, block, result, origin_a); });
 			}
@@ -376,10 +376,6 @@ nano::process_return nano::block_processor::process_one (nano::write_transaction
 				node.logger.try_log (boost::str (boost::format ("Gap previous for: %1%") % hash.to_string ()));
 			}
 			info_a.verified = result.verified;
-			if (info_a.modified == 0)
-			{
-				info_a.modified = nano::seconds_since_epoch ();
-			}
 			node.unchecked.put (block->previous (), info_a);
 			events_a.events.emplace_back ([this, hash] (nano::transaction const & /* unused */) { this->node.gap_cache.add (hash); });
 			node.stats.inc (nano::stat::type::ledger, nano::stat::detail::gap_previous);
@@ -392,10 +388,6 @@ nano::process_return nano::block_processor::process_one (nano::write_transaction
 				node.logger.try_log (boost::str (boost::format ("Gap source for: %1%") % hash.to_string ()));
 			}
 			info_a.verified = result.verified;
-			if (info_a.modified == 0)
-			{
-				info_a.modified = nano::seconds_since_epoch ();
-			}
 			node.unchecked.put (node.ledger.block_source (transaction_a, *(block)), info_a);
 			events_a.events.emplace_back ([this, hash] (nano::transaction const & /* unused */) { this->node.gap_cache.add (hash); });
 			node.stats.inc (nano::stat::type::ledger, nano::stat::detail::gap_source);
@@ -408,10 +400,6 @@ nano::process_return nano::block_processor::process_one (nano::write_transaction
 				node.logger.try_log (boost::str (boost::format ("Gap pending entries for epoch open: %1%") % hash.to_string ()));
 			}
 			info_a.verified = result.verified;
-			if (info_a.modified == 0)
-			{
-				info_a.modified = nano::seconds_since_epoch ();
-			}
 			node.unchecked.put (block->account (), info_a); // Specific unchecked key starting with epoch open block account public key
 			node.stats.inc (nano::stat::type::ledger, nano::stat::detail::gap_source);
 			break;
@@ -503,7 +491,7 @@ nano::process_return nano::block_processor::process_one (nano::write_transaction
 
 nano::process_return nano::block_processor::process_one (nano::write_transaction const & transaction_a, block_post_events & events_a, std::shared_ptr<nano::block> const & block_a)
 {
-	nano::unchecked_info info (block_a, block_a->account (), 0, nano::signature_verification::unknown);
+	nano::unchecked_info info (block_a, block_a->account (), nano::signature_verification::unknown);
 	auto result (process_one (transaction_a, events_a, info));
 	return result;
 }

nano/node/blockprocessor.hpp

@@ -56,7 +56,7 @@ public:
 	bool half_full ();
 	void add_local (nano::unchecked_info const & info_a);
 	void add (nano::unchecked_info const &);
-	void add (std::shared_ptr<nano::block> const &, uint64_t = 0);
+	void add (std::shared_ptr<nano::block> const &);
 	void force (std::shared_ptr<nano::block> const &);
 	void wait_write ();
 	bool should_log ();

nano/node/bootstrap/bootstrap_attempt.cpp

@@ -144,7 +144,7 @@ bool nano::bootstrap_attempt::process_block (std::shared_ptr<nano::block> const
 	}
 	else
 	{
-		nano::unchecked_info info (block_a, known_account_a, 0, nano::signature_verification::unknown);
+		nano::unchecked_info info (block_a, known_account_a, nano::signature_verification::unknown);
 		node->block_processor.add (info);
 	}
 	return stop_pull;

nano/node/bootstrap/bootstrap_lazy.cpp

@@ -268,7 +268,7 @@ bool nano::bootstrap_attempt_lazy::process_block_lazy (std::shared_ptr<nano::blo
 		}
 		lazy_block_state_backlog_check (block_a, hash);
 		lock.unlock ();
-		nano::unchecked_info info (block_a, known_account_a, 0, nano::signature_verification::unknown);
+		nano::unchecked_info info (block_a, known_account_a, nano::signature_verification::unknown);
 		node->block_processor.add (info);
 	}
 	// Force drop lazy bootstrap connection for long bulk_pull

nano/node/json_handler.cpp

@@ -4078,7 +4078,7 @@ void nano::json_handler::unchecked_get ()
 			if (key.hash == hash)
 			{
 				nano::unchecked_info const & info (i->second);
-				response_l.put ("modified_timestamp", std::to_string (info.modified));
+				response_l.put ("modified_timestamp", std::to_string (info.modified ()));
 
 				if (json_block_l)
 				{
@@ -4126,7 +4126,7 @@ void nano::json_handler::unchecked_keys ()
 			nano::unchecked_info const & info (i->second);
 			entry.put ("key", i->first.key ().to_string ());
 			entry.put ("hash", info.block->hash ().to_string ());
-			entry.put ("modified_timestamp", std::to_string (info.modified));
+			entry.put ("modified_timestamp", std::to_string (info.modified ()));
 			if (json_block_l)
 			{
 				boost::property_tree::ptree block_node_l;

nano/node/node.cpp

@@ -566,7 +566,7 @@ std::unique_ptr<nano::container_info_component> nano::collect_container_info (no
 void nano::node::process_active (std::shared_ptr<nano::block> const & incoming)
 {
 	block_arrival.add (incoming->hash ());
-	block_processor.add (incoming, nano::seconds_since_epoch ());
+	block_processor.add (incoming);
 }
 
 nano::process_return nano::node::process (nano::block & block_a)
@@ -581,7 +581,7 @@ nano::process_return nano::node::process_local (std::shared_ptr<nano::block> con
 	// Add block hash as recently arrived to trigger automatic rebroadcast and election
 	block_arrival.add (block_a->hash ());
 	// Set current time to trigger automatic rebroadcast and election
-	nano::unchecked_info info (block_a, block_a->account (), nano::seconds_since_epoch (), nano::signature_verification::unknown);
+	nano::unchecked_info info (block_a, block_a->account (), nano::signature_verification::unknown);
 	// Notify block processor to release write lock
 	block_processor.wait_write ();
 	// Process block
@@ -595,7 +595,7 @@ void nano::node::process_local_async (std::shared_ptr<nano::block> const & block
 	// Add block hash as recently arrived to trigger automatic rebroadcast and election
 	block_arrival.add (block_a->hash ());
 	// Set current time to trigger automatic rebroadcast and election
-	nano::unchecked_info info (block_a, block_a->account (), nano::seconds_since_epoch (), nano::signature_verification::unknown);
+	nano::unchecked_info info (block_a, block_a->account (), nano::signature_verification::unknown);
 	block_processor.add_local (info);
 }
 
@@ -950,7 +950,7 @@ void nano::node::unchecked_cleanup ()
 		{
 			nano::unchecked_key const & key (i->first);
 			nano::unchecked_info const & info (i->second);
-			if ((now - info.modified) > static_cast<uint64_t> (config.unchecked_cutoff_time.count ()))
+			if ((now - info.modified ()) > static_cast<uint64_t> (config.unchecked_cutoff_time.count ()))
 			{
 				digests.push_back (network.publish_filter.hash (info.block));
 				cleaning_list.push_back (key);

nano/node/unchecked_map.cpp

@@ -1,5 +1,6 @@
 #include <nano/lib/locks.hpp>
 #include <nano/lib/threading.hpp>
+#include <nano/lib/timer.hpp>
 #include <nano/node/unchecked_map.hpp>
 #include <nano/secure/store.hpp>
 
@@ -96,7 +97,7 @@ nano::unchecked_map::item_visitor::item_visitor (unchecked_map & unchecked, nano
 void nano::unchecked_map::item_visitor::operator() (insert const & item)
 {
 	auto const & [dependency, info] = item;
-	unchecked.store.unchecked.put (transaction, dependency, info);
+	unchecked.store.unchecked.put (transaction, dependency, { info.block, info.account, info.verified });
 }
 
 void nano::unchecked_map::item_visitor::operator() (query const & item)

nano/secure/common.cpp

@@ -354,16 +354,16 @@ nano::account const & nano::pending_key::key () const
 	return account;
 }
 
-nano::unchecked_info::unchecked_info (std::shared_ptr<nano::block> const & block_a, nano::account const & account_a, uint64_t modified_a, nano::signature_verification verified_a) :
+nano::unchecked_info::unchecked_info (std::shared_ptr<nano::block> const & block_a, nano::account const & account_a, nano::signature_verification verified_a) :
 	block (block_a),
 	account (account_a),
-	modified (modified_a),
+	modified_m (nano::seconds_since_epoch ()),
 	verified (verified_a)
 {
 }
 
 nano::unchecked_info::unchecked_info (std::shared_ptr<nano::block> const & block) :
-	unchecked_info{ block, block->account (), nano::seconds_since_epoch (), nano::signature_verification::unknown }
+	unchecked_info{ block, block->account (), nano::signature_verification::unknown }
 {
 }
 
@@ -372,7 +372,7 @@ void nano::unchecked_info::serialize (nano::stream & stream_a) const
 	debug_assert (block != nullptr);
 	nano::serialize_block (stream_a, *block);
 	nano::write (stream_a, account.bytes);
-	nano::write (stream_a, modified);
+	nano::write (stream_a, modified_m);
 	nano::write (stream_a, verified);
 }
 
@@ -385,7 +385,7 @@ bool nano::unchecked_info::deserialize (nano::stream & stream_a)
 		try
 		{
 			nano::read (stream_a, account.bytes);
-			nano::read (stream_a, modified);
+			nano::read (stream_a, modified_m);
 			nano::read (stream_a, verified);
 		}
 		catch (std::runtime_error const &)
@@ -396,6 +396,11 @@ bool nano::unchecked_info::deserialize (nano::stream & stream_a)
 	return error;
 }
 
+uint64_t nano::unchecked_info::modified () const
+{
+	return modified_m;
+}
+
 nano::endpoint_key::endpoint_key (std::array<uint8_t, 16> const & address_a, uint16_t port_a) :
 	address (address_a), network_port (boost::endian::native_to_big (port_a))
 {

nano/secure/common.hpp

@@ -199,14 +199,19 @@ class unchecked_info final
 {
 public:
 	unchecked_info () = default;
-	unchecked_info (std::shared_ptr<nano::block> const &, nano::account const &, uint64_t, nano::signature_verification = nano::signature_verification::unknown);
+	unchecked_info (std::shared_ptr<nano::block> const &, nano::account const &, nano::signature_verification = nano::signature_verification::unknown);
 	unchecked_info (std::shared_ptr<nano::block> const &);
 	void serialize (nano::stream &) const;
 	bool deserialize (nano::stream &);
+	uint64_t modified () const;
 	std::shared_ptr<nano::block> block;
 	nano::account account{};
+
+private:
 	/** Seconds since posix epoch */
-	uint64_t modified{ 0 };
+	uint64_t modified_m{ 0 };
+
+public:
 	nano::signature_verification verified{ nano::signature_verification::unknown };
 };
 

nano/slow_test/node.cpp

@@ -531,7 +531,7 @@ TEST (node, mass_vote_by_hash)
 	}
 	for (auto i (blocks.begin ()), n (blocks.end ()); i != n; ++i)
 	{
-		system.nodes[0]->block_processor.add (*i, nano::seconds_since_epoch ());
+		system.nodes[0]->block_processor.add (*i);
 	}
 }