Fix unit test rpc history pruning (#4089)

* standardise assert timelys to be 5 seconds

* Bugfix: Stop depending on block processor flush and use nano::test::confirm()

* Simplify the rest of the test case

* Add fixme comment to nano::test::confirm()

nano/rpc_test/rpc.cpp

@@ -1358,7 +1358,11 @@ TEST (rpc, history_pruning)
 	nano::node_flags node_flags;
 	node_flags.enable_pruning = true;
 	auto node0 = add_ipc_enabled_node (system, node_config, node_flags);
+	std::vector<std::shared_ptr<nano::block>> blocks;
+
 	nano::block_builder builder;
+
+	// noop change block
 	auto change = builder
 				  .change ()
 				  .previous (nano::dev::genesis->hash ())
@@ -1366,7 +1370,9 @@ TEST (rpc, history_pruning)
 				  .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
 				  .work (*node0->work.generate (nano::dev::genesis->hash ()))
 				  .build_shared ();
-	node0->process_active (change);
+	blocks.push_back (change);
+
+	// legacy send to itself
 	auto send = builder
 				.send ()
 				.previous (change->hash ())
@@ -1375,7 +1381,9 @@ TEST (rpc, history_pruning)
 				.sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
 				.work (*node0->work.generate (change->hash ()))
 				.build_shared ();
-	node0->process_active (send);
+	blocks.push_back (send);
+
+	// legacy receive the legacy self send
 	auto receive = builder
 				   .receive ()
 				   .previous (send->hash ())
@@ -1383,7 +1391,9 @@ TEST (rpc, history_pruning)
 				   .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
 				   .work (*node0->work.generate (send->hash ()))
 				   .build_shared ();
-	node0->process_active (receive);
+	blocks.push_back (receive);
+
+	// non legacy self send
 	auto usend = builder
 				 .state ()
 				 .account (nano::dev::genesis->account ())
@@ -1394,6 +1404,9 @@ TEST (rpc, history_pruning)
 				 .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
 				 .work (*node0->work_generate_blocking (receive->hash ()))
 				 .build_shared ();
+	blocks.push_back (usend);
+
+	// non legacy receive of the non legacy self send
 	auto ureceive = builder
 					.state ()
 					.account (nano::dev::genesis->account ())
@@ -1404,6 +1417,9 @@ TEST (rpc, history_pruning)
 					.sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
 					.work (*node0->work_generate_blocking (usend->hash ()))
 					.build_shared ();
+	blocks.push_back (ureceive);
+
+	// change genesis to a random rep
 	auto uchange = builder
 				   .state ()
 				   .account (nano::dev::genesis->account ())
@@ -1414,127 +1430,91 @@ TEST (rpc, history_pruning)
 				   .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
 				   .work (*node0->work_generate_blocking (ureceive->hash ()))
 				   .build_shared ();
-	node0->process_active (usend);
+	blocks.push_back (uchange);
-	node0->process_active (ureceive);
+
-	node0->process_active (uchange);
+	nano::test::process_live (*node0, blocks);
-	node0->block_processor.flush ();
+	ASSERT_TIMELY (5s, nano::test::exists (*node0, blocks));
 	system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv);
-	// Confirm last block to prune previous
+
-	{
+	// WORKAROUND: this is called repeatedly inside an assert timely because nano::test::confirm()
-		auto election = node0->active.election (change->qualified_root ());
+	// uses block_processor.flush internally which can fail to flush
-		ASSERT_NE (nullptr, election);
+	ASSERT_TIMELY (5s, nano::test::confirm (*node0, blocks));
-		election->force_confirm ();
+
-	}
+	ASSERT_TIMELY (5s, node0->block_confirmed (uchange->hash ()));
-	ASSERT_TIMELY (2s, node0->block_confirmed (change->hash ()) && node0->active.active (send->qualified_root ()));
+	nano::confirmation_height_info confirmation_height_info;
-	{
+	node0->store.confirmation_height.get (node0->store.tx_begin_read (), nano::dev::genesis_key.pub, confirmation_height_info);
-		auto election = node0->active.election (send->qualified_root ());
+	ASSERT_EQ (7, confirmation_height_info.height);
-		ASSERT_NE (nullptr, election);
+
-		election->force_confirm ();
+	// Prune block "change"
-	}
-	ASSERT_TIMELY (2s, node0->block_confirmed (send->hash ()) && node0->active.active (receive->qualified_root ()));
-	{
-		auto election = node0->active.election (receive->qualified_root ());
-		ASSERT_NE (nullptr, election);
-		election->force_confirm ();
-	}
-	ASSERT_TIMELY (2s, node0->block_confirmed (receive->hash ()) && node0->active.active (usend->qualified_root ()));
-	{
-		auto election = node0->active.election (usend->qualified_root ());
-		ASSERT_NE (nullptr, election);
-		election->force_confirm ();
-	}
-	ASSERT_TIMELY (2s, node0->block_confirmed (usend->hash ()) && node0->active.active (ureceive->qualified_root ()));
-	{
-		auto election = node0->active.election (ureceive->qualified_root ());
-		ASSERT_NE (nullptr, election);
-		election->force_confirm ();
-	}
-	ASSERT_TIMELY (2s, node0->block_confirmed (ureceive->hash ()) && node0->active.active (uchange->qualified_root ()));
-	{
-		auto election = node0->active.election (uchange->qualified_root ());
-		ASSERT_NE (nullptr, election);
-		election->force_confirm ();
-	}
-	ASSERT_TIMELY (2s, node0->active.empty () && node0->block_confirmed (uchange->hash ()));
-	ASSERT_TIMELY (2s, node0->ledger.cache.cemented_count == 7 && node0->confirmation_height_processor.current ().is_zero () && node0->confirmation_height_processor.awaiting_processing_size () == 0);
-	// Pruning action
 	{
 		auto transaction (node0->store.tx_begin_write ());
 		ASSERT_EQ (1, node0->ledger.pruning_action (transaction, change->hash (), 1));
 	}
+
 	auto const rpc_ctx = add_rpc (system, node0);
 	boost::property_tree::ptree request;
 	request.put ("action", "history");
 	request.put ("hash", send->hash ().to_string ());
 	request.put ("count", 100);
-	auto response (wait_response (system, rpc_ctx, request));
+	auto response = wait_response (system, rpc_ctx, request);
-	std::vector<std::tuple<std::string, std::string, std::string, std::string>> history_l;
+	auto history_node = response.get_child ("history");
-	auto & history_node (response.get_child ("history"));
+	ASSERT_EQ (history_node.size (), 1);
-	for (auto i (history_node.begin ()), n (history_node.end ()); i != n; ++i)
+	auto entry = (*history_node.begin ()).second;
-	{
+	ASSERT_EQ ("send", entry.get<std::string> ("type"));
-		history_l.push_back (std::make_tuple (i->second.get<std::string> ("type"), i->second.get<std::string> ("account", "-1"), i->second.get<std::string> ("amount", "-1"), i->second.get<std::string> ("hash")));
+	ASSERT_EQ (nano::dev::genesis_key.pub.to_account (), entry.get<std::string> ("account", "N/A"));
-		boost::optional<std::string> amount (i->second.get_optional<std::string> ("amount"));
+	ASSERT_EQ ("N/A", entry.get<std::string> ("amount", "N/A"));
-		ASSERT_FALSE (amount.is_initialized ()); // Cannot calculate amount
+	ASSERT_EQ (send->hash ().to_string (), entry.get<std::string> ("hash"));
-	}
+
-	ASSERT_EQ (1, history_l.size ());
+	// Prune block "send"
-	ASSERT_EQ ("send", std::get<0> (history_l[0]));
-	ASSERT_EQ (nano::dev::genesis_key.pub.to_account (), std::get<1> (history_l[0]));
-	ASSERT_EQ ("-1", std::get<2> (history_l[0]));
-	ASSERT_EQ (send->hash ().to_string (), std::get<3> (history_l[0]));
-	// Pruning action
 	{
 		rpc_ctx.io_scope->reset ();
 		auto transaction (node0->store.tx_begin_write ());
 		ASSERT_EQ (1, node0->ledger.pruning_action (transaction, send->hash (), 1));
 		rpc_ctx.io_scope->renew ();
 	}
+
 	boost::property_tree::ptree request2;
 	request2.put ("action", "history");
 	request2.put ("hash", receive->hash ().to_string ());
 	request2.put ("count", 100);
-	auto response2 (wait_response (system, rpc_ctx, request2));
+	response = wait_response (system, rpc_ctx, request2);
-	history_l.clear ();
+	history_node = response.get_child ("history");
-	auto & history_node2 (response2.get_child ("history"));
+	ASSERT_EQ (history_node.size (), 1);
-	for (auto i (history_node2.begin ()), n (history_node2.end ()); i != n; ++i)
+	entry = (*history_node.begin ()).second;
-	{
+	ASSERT_EQ ("receive", entry.get<std::string> ("type"));
-		history_l.push_back (std::make_tuple (i->second.get<std::string> ("type"), i->second.get<std::string> ("account", "-1"), i->second.get<std::string> ("amount", "-1"), i->second.get<std::string> ("hash")));
+	ASSERT_EQ ("N/A", entry.get<std::string> ("account", "N/A"));
-		boost::optional<std::string> amount (i->second.get_optional<std::string> ("amount"));
+	ASSERT_EQ ("N/A", entry.get<std::string> ("amount", "N/A"));
-		ASSERT_FALSE (amount.is_initialized ()); // Cannot calculate amount
+	ASSERT_EQ (receive->hash ().to_string (), entry.get<std::string> ("hash"));
-		boost::optional<std::string> account (i->second.get_optional<std::string> ("account"));
+
-		ASSERT_FALSE (account.is_initialized ()); // Cannot find source account
+	// Prune block "receive"
-	}
-	ASSERT_EQ (1, history_l.size ());
-	ASSERT_EQ ("receive", std::get<0> (history_l[0]));
-	ASSERT_EQ ("-1", std::get<1> (history_l[0]));
-	ASSERT_EQ ("-1", std::get<2> (history_l[0]));
-	ASSERT_EQ (receive->hash ().to_string (), std::get<3> (history_l[0]));
-	// Pruning action
 	{
 		rpc_ctx.io_scope->reset ();
 		auto transaction (node0->store.tx_begin_write ());
 		ASSERT_EQ (1, node0->ledger.pruning_action (transaction, receive->hash (), 1));
 		rpc_ctx.io_scope->renew ();
 	}
+
 	boost::property_tree::ptree request3;
 	request3.put ("action", "history");
 	request3.put ("hash", uchange->hash ().to_string ());
 	request3.put ("count", 100);
-	auto response3 (wait_response (system, rpc_ctx, request3));
+	response = wait_response (system, rpc_ctx, request3);
-	history_l.clear ();
+	history_node = response.get_child ("history");
-	auto & history_node3 (response3.get_child ("history"));
+	ASSERT_EQ (history_node.size (), 2);
-	for (auto i (history_node3.begin ()), n (history_node3.end ()); i != n; ++i)
+
-	{
+	// first array element
-		history_l.push_back (std::make_tuple (i->second.get<std::string> ("type"), i->second.get<std::string> ("account", "-1"), i->second.get<std::string> ("amount", "-1"), i->second.get<std::string> ("hash")));
+	entry = (*history_node.begin ()).second;
-	}
+	ASSERT_EQ ("receive", entry.get<std::string> ("type"));
-	ASSERT_EQ (2, history_l.size ());
+	ASSERT_EQ (ureceive->hash ().to_string (), entry.get<std::string> ("hash"));
-	ASSERT_EQ ("receive", std::get<0> (history_l[0]));
+	ASSERT_EQ (nano::dev::genesis_key.pub.to_account (), entry.get<std::string> ("account", "N/A"));
-	ASSERT_EQ (ureceive->hash ().to_string (), std::get<3> (history_l[0]));
+	ASSERT_EQ (nano::Gxrb_ratio.convert_to<std::string> (), entry.get<std::string> ("amount", "N/A"));
-	ASSERT_EQ (nano::dev::genesis_key.pub.to_account (), std::get<1> (history_l[0]));
+
-	ASSERT_EQ (nano::Gxrb_ratio.convert_to<std::string> (), std::get<2> (history_l[0]));
+	// second array element
-	ASSERT_EQ ("unknown", std::get<0> (history_l[1]));
+	entry = (*(++history_node.begin ())).second;
-	ASSERT_EQ ("-1", std::get<1> (history_l[1]));
+	ASSERT_EQ ("unknown", entry.get<std::string> ("type"));
-	ASSERT_EQ ("-1", std::get<2> (history_l[1]));
+	ASSERT_EQ ("N/A", entry.get<std::string> ("account", "N/A"));
-	ASSERT_EQ (usend->hash ().to_string (), std::get<3> (history_l[1]));
+	ASSERT_EQ ("N/A", entry.get<std::string> ("amount", "N/A"));
+	ASSERT_EQ (usend->hash ().to_string (), entry.get<std::string> ("hash"));
 }
 
 TEST (rpc, process_block)

nano/test_common/testutil.cpp

@@ -80,7 +80,7 @@ bool nano::test::process_live (nano::node & node, std::vector<std::shared_ptr<na
 
 bool nano::test::confirm (nano::node & node, std::vector<nano::block_hash> hashes)
 {
-	// Finish processing all blocks
+	// Finish processing all blocks - FIXME: block processor flush is broken and should be removed
 	node.block_processor.flush ();
 	for (auto & hash : hashes)
 	{