sc-network-test::Peer: block push methods return hashes vec (#12944)

* sc-network-test::Peer: block push methods return hashes vec

This commit reworks the block generation/push methods in
sc-network-test::Peer.

Now methods are providing the vector of hashes that were built.

This allows to get rid of redundant `block_hash_from_id` call, as all
hashes are known just after being built.

Similar approach was taken in BeefyTestNet::generate_blocks_and_sync
method.

This PR is part of BlockId::Number refactoring analysis (paritytech/substrate#11292)

* fix

* Apply suggestions from code review

Co-authored-by: Bastian Köcher <git@kchr.de>

Co-authored-by: Bastian Köcher <git@kchr.de>

substrate/client/network/test/src/sync.rs

@@ -250,13 +250,12 @@ async fn sync_no_common_longer_chain_fails() {
 async fn sync_justifications() {
 	sp_tracing::try_init_simple();
 	let mut net = JustificationTestNet::new(3);
-	net.peer(0).push_blocks(20, false);
+	let hashes = net.peer(0).push_blocks(20, false);
 	net.run_until_sync().await;
 
-	let backend = net.peer(0).client().as_backend();
-	let hashof10 = backend.blockchain().expect_block_hash_from_id(&BlockId::Number(10)).unwrap();
-	let hashof15 = backend.blockchain().expect_block_hash_from_id(&BlockId::Number(15)).unwrap();
-	let hashof20 = backend.blockchain().expect_block_hash_from_id(&BlockId::Number(20)).unwrap();
+	let hashof10 = hashes[9];
+	let hashof15 = hashes[14];
+	let hashof20 = hashes[19];
 
 	// there's currently no justification for block #10
 	assert_eq!(net.peer(0).client().justifications(hashof10).unwrap(), None);
@@ -280,13 +279,13 @@ async fn sync_justifications() {
 	futures::future::poll_fn::<(), _>(|cx| {
 		net.poll(cx);
 
-		for hash in [hashof10, hashof15, hashof20] {
-			if net.peer(0).client().justifications(hash).unwrap() !=
+		for height in (10..21).step_by(5) {
+			if net.peer(0).client().justifications(hashes[height - 1]).unwrap() !=
 				Some(Justifications::from((*b"FRNK", Vec::new())))
 			{
 				return Poll::Pending
 			}
-			if net.peer(1).client().justifications(hash).unwrap() !=
+			if net.peer(1).client().justifications(hashes[height - 1]).unwrap() !=
 				Some(Justifications::from((*b"FRNK", Vec::new())))
 			{
 				return Poll::Pending
@@ -305,8 +304,8 @@ async fn sync_justifications_across_forks() {
 	// we push 5 blocks
 	net.peer(0).push_blocks(5, false);
 	// and then two forks 5 and 6 blocks long
-	let f1_best = net.peer(0).push_blocks_at(BlockId::Number(5), 5, false);
-	let f2_best = net.peer(0).push_blocks_at(BlockId::Number(5), 6, false);
+	let f1_best = net.peer(0).push_blocks_at(BlockId::Number(5), 5, false).pop().unwrap();
+	let f2_best = net.peer(0).push_blocks_at(BlockId::Number(5), 6, false).pop().unwrap();
 
 	// peer 1 will only see the longer fork. but we'll request justifications
 	// for both and finalize the small fork instead.
@@ -364,8 +363,8 @@ async fn syncs_all_forks() {
 	net.peer(0).push_blocks(2, false);
 	net.peer(1).push_blocks(2, false);
 
-	let b1 = net.peer(0).push_blocks(2, true);
-	let b2 = net.peer(1).push_blocks(4, false);
+	let b1 = net.peer(0).push_blocks(2, true).pop().unwrap();
+	let b2 = net.peer(1).push_blocks(4, false).pop().unwrap();
 
 	net.run_until_sync().await;
 	// Check that all peers have all of the branches.
@@ -446,7 +445,7 @@ async fn can_sync_small_non_best_forks() {
 	.await;
 	net.run_until_sync().await;
 
-	let another_fork = net.peer(0).push_blocks_at(BlockId::Number(35), 2, true);
+	let another_fork = net.peer(0).push_blocks_at(BlockId::Number(35), 2, true).pop().unwrap();
 	net.peer(0).announce_block(another_fork, None);
 	futures::future::poll_fn::<(), _>(|cx| {
 		net.poll(cx);
@@ -467,12 +466,16 @@ async fn can_sync_forks_ahead_of_the_best_chain() {
 
 	net.run_until_connected().await;
 	// Peer 0 is on 2-block fork which is announced with is_best=false
-	let fork_hash = net.peer(0).generate_blocks_with_fork_choice(
-		2,
-		BlockOrigin::Own,
-		|builder| builder.build().unwrap().block,
-		ForkChoiceStrategy::Custom(false),
-	);
+	let fork_hash = net
+		.peer(0)
+		.generate_blocks_with_fork_choice(
+			2,
+			BlockOrigin::Own,
+			|builder| builder.build().unwrap().block,
+			ForkChoiceStrategy::Custom(false),
+		)
+		.pop()
+		.unwrap();
 	// Peer 1 is on 1-block fork
 	net.peer(1).push_blocks(1, false);
 	assert!(net.peer(0).client().header(&BlockId::Hash(fork_hash)).unwrap().is_some());
@@ -575,8 +578,8 @@ async fn does_not_sync_announced_old_best_block() {
 	sp_tracing::try_init_simple();
 	let mut net = TestNet::new(3);
 
-	let old_hash = net.peer(0).push_blocks(1, false);
-	let old_hash_with_parent = net.peer(0).push_blocks(1, false);
+	let old_hash = net.peer(0).push_blocks(1, false).pop().unwrap();
+	let old_hash_with_parent = net.peer(0).push_blocks(1, false).pop().unwrap();
 	net.peer(0).push_blocks(18, true);
 	net.peer(1).push_blocks(20, true);
 
@@ -647,12 +650,12 @@ async fn imports_stale_once() {
 	net.run_until_sync().await;
 
 	// check that NEW block is imported from announce message
-	let new_hash = net.peer(0).push_blocks(1, false);
+	let new_hash = net.peer(0).push_blocks(1, false).pop().unwrap();
 	import_with_announce(&mut net, new_hash).await;
 	assert_eq!(net.peer(1).num_downloaded_blocks(), 1);
 
 	// check that KNOWN STALE block is imported from announce message
-	let known_stale_hash = net.peer(0).push_blocks_at(BlockId::Number(0), 1, true);
+	let known_stale_hash = net.peer(0).push_blocks_at(BlockId::Number(0), 1, true).pop().unwrap();
 	import_with_announce(&mut net, known_stale_hash).await;
 	assert_eq!(net.peer(1).num_downloaded_blocks(), 2);
 }
@@ -664,7 +667,7 @@ async fn can_sync_to_peers_with_wrong_common_block() {
 
 	net.peer(0).push_blocks(2, true);
 	net.peer(1).push_blocks(2, true);
-	let fork_hash = net.peer(0).push_blocks_at(BlockId::Number(0), 2, false);
+	let fork_hash = net.peer(0).push_blocks_at(BlockId::Number(0), 2, false).pop().unwrap();
 	net.peer(1).push_blocks_at(BlockId::Number(0), 2, false);
 	// wait for connection
 	net.run_until_connected().await;
@@ -673,7 +676,7 @@ async fn can_sync_to_peers_with_wrong_common_block() {
 	let just = Some((*b"FRNK", Vec::new()));
 	net.peer(0).client().finalize_block(fork_hash, just.clone(), true).unwrap();
 	net.peer(1).client().finalize_block(fork_hash, just, true).unwrap();
-	let final_hash = net.peer(0).push_blocks(1, false);
+	let final_hash = net.peer(0).push_blocks(1, false).pop().unwrap();
 
 	net.run_until_sync().await;
 
@@ -731,12 +734,16 @@ async fn sync_blocks_when_block_announce_validator_says_it_is_new_best() {
 	net.run_until_connected().await;
 
 	// Add blocks but don't set them as best
-	let block_hash = net.peer(0).generate_blocks_with_fork_choice(
-		1,
-		BlockOrigin::Own,
-		|builder| builder.build().unwrap().block,
-		ForkChoiceStrategy::Custom(false),
-	);
+	let block_hash = net
+		.peer(0)
+		.generate_blocks_with_fork_choice(
+			1,
+			BlockOrigin::Own,
+			|builder| builder.build().unwrap().block,
+			ForkChoiceStrategy::Custom(false),
+		)
+		.pop()
+		.unwrap();
 
 	while !net.peer(2).has_block(block_hash) {
 		net.run_until_idle().await;
@@ -774,12 +781,16 @@ async fn wait_until_deferred_block_announce_validation_is_ready() {
 	net.run_until_connected().await;
 
 	// Add blocks but don't set them as best
-	let block_hash = net.peer(0).generate_blocks_with_fork_choice(
-		1,
-		BlockOrigin::Own,
-		|builder| builder.build().unwrap().block,
-		ForkChoiceStrategy::Custom(false),
-	);
+	let block_hash = net
+		.peer(0)
+		.generate_blocks_with_fork_choice(
+			1,
+			BlockOrigin::Own,
+			|builder| builder.build().unwrap().block,
+			ForkChoiceStrategy::Custom(false),
+		)
+		.pop()
+		.unwrap();
 
 	while !net.peer(1).has_block(block_hash) {
 		net.run_until_idle().await;
@@ -794,9 +805,11 @@ async fn sync_to_tip_requires_that_sync_protocol_is_informed_about_best_block()
 	let mut net = TestNet::new(1);
 
 	// Produce some blocks
-	let block_hash =
-		net.peer(0)
-			.push_blocks_at_without_informing_sync(BlockId::Number(0), 3, true, true);
+	let block_hash = net
+		.peer(0)
+		.push_blocks_at_without_informing_sync(BlockId::Number(0), 3, true, true)
+		.pop()
+		.unwrap();
 
 	// Add a node and wait until they are connected
 	net.add_full_peer_with_config(Default::default());
@@ -838,9 +851,11 @@ async fn sync_to_tip_when_we_sync_together_with_multiple_peers() {
 
 	let mut net = TestNet::new(3);
 
-	let block_hash =
-		net.peer(0)
-			.push_blocks_at_without_informing_sync(BlockId::Number(0), 10_000, false, false);
+	let block_hash = net
+		.peer(0)
+		.push_blocks_at_without_informing_sync(BlockId::Number(0), 10_000, false, false)
+		.pop()
+		.unwrap();
 
 	net.peer(1)
 		.push_blocks_at_without_informing_sync(BlockId::Number(0), 5_000, false, false);
@@ -912,7 +927,11 @@ async fn block_announce_data_is_propagated() {
 	})
 	.await;
 
-	let block_hash = net.peer(0).push_blocks_at_without_announcing(BlockId::Number(0), 1, true);
+	let block_hash = net
+		.peer(0)
+		.push_blocks_at_without_announcing(BlockId::Number(0), 1, true)
+		.pop()
+		.unwrap();
 	net.peer(0).announce_block(block_hash, Some(vec![137]));
 
 	while !net.peer(1).has_block(block_hash) || !net.peer(2).has_block(block_hash) {
@@ -956,7 +975,7 @@ async fn continue_to_sync_after_some_block_announcement_verifications_failed() {
 	net.run_until_connected().await;
 	net.run_until_idle().await;
 
-	let block_hash = net.peer(0).push_blocks(500, true);
+	let block_hash = net.peer(0).push_blocks(500, true).pop().unwrap();
 
 	net.run_until_sync().await;
 	assert!(net.peer(1).has_block(block_hash));
@@ -970,10 +989,10 @@ async fn continue_to_sync_after_some_block_announcement_verifications_failed() {
 async fn multiple_requests_are_accepted_as_long_as_they_are_not_fulfilled() {
 	sp_tracing::try_init_simple();
 	let mut net = JustificationTestNet::new(2);
-	net.peer(0).push_blocks(10, false);
+	let hashes = net.peer(0).push_blocks(10, false);
 	net.run_until_sync().await;
 
-	let hashof10 = net.peer(1).client().header(&BlockId::Number(10)).unwrap().unwrap().hash();
+	let hashof10 = hashes[9];
 
 	// there's currently no justification for block #10
 	assert_eq!(net.peer(0).client().justifications(hashof10).unwrap(), None);
@@ -992,14 +1011,8 @@ async fn multiple_requests_are_accepted_as_long_as_they_are_not_fulfilled() {
 		assert_eq!(1, net.peer(0).num_peers());
 	}
 
-	let hashof10 = net
-		.peer(0)
-		.client()
-		.as_backend()
-		.blockchain()
-		.expect_block_hash_from_id(&BlockId::Number(10))
-		.unwrap();
-	// Finalize the block and make the justification available.
+	let hashof10 = hashes[9];
+	// Finalize the 10th block and make the justification available.
 	net.peer(0)
 		.client()
 		.finalize_block(hashof10, Some((*b"FRNK", Vec::new())), true)
@@ -1030,7 +1043,7 @@ async fn syncs_all_forks_from_single_peer() {
 	net.run_until_connected().await;
 
 	// Peer 0 produces new blocks and announces.
-	let branch1 = net.peer(0).push_blocks_at(BlockId::Number(10), 2, true);
+	let branch1 = net.peer(0).push_blocks_at(BlockId::Number(10), 2, true).pop().unwrap();
 
 	// Wait till peer 1 starts downloading
 	futures::future::poll_fn::<(), _>(|cx| {
@@ -1043,7 +1056,7 @@ async fn syncs_all_forks_from_single_peer() {
 	.await;
 
 	// Peer 0 produces and announces another fork
-	let branch2 = net.peer(0).push_blocks_at(BlockId::Number(10), 2, false);
+	let branch2 = net.peer(0).push_blocks_at(BlockId::Number(10), 2, false).pop().unwrap();
 
 	net.run_until_sync().await;
 
@@ -1070,7 +1083,7 @@ async fn syncs_after_missing_announcement() {
 	// Peer 0 produces a new block and announces. Peer 1 ignores announcement.
 	net.peer(0).push_blocks_at(BlockId::Number(10), 1, false);
 	// Peer 0 produces another block and announces.
-	let final_block = net.peer(0).push_blocks_at(BlockId::Number(11), 1, false);
+	let final_block = net.peer(0).push_blocks_at(BlockId::Number(11), 1, false).pop().unwrap();
 	net.peer(1).push_blocks_at(BlockId::Number(10), 1, true);
 	net.run_until_sync().await;
 	assert!(net.peer(1).client().header(&BlockId::Hash(final_block)).unwrap().is_some());
@@ -1119,19 +1132,13 @@ async fn syncs_state() {
 		config_two.sync_mode =
 			SyncMode::Fast { skip_proofs: *skip_proofs, storage_chain_mode: false };
 		net.add_full_peer_with_config(config_two);
-		net.peer(0).push_blocks(64, false);
+		let hashes = net.peer(0).push_blocks(64, false);
 		// Wait for peer 1 to sync header chain.
 		net.run_until_sync().await;
 		assert!(!net.peer(1).client().has_state_at(&BlockId::Number(64)));
 
 		let just = (*b"FRNK", Vec::new());
-		let hashof60 = net
-			.peer(0)
-			.client()
-			.as_backend()
-			.blockchain()
-			.expect_block_hash_from_id(&BlockId::Number(60))
-			.unwrap();
+		let hashof60 = hashes[59];
 		net.peer(1).client().finalize_block(hashof60, Some(just), true).unwrap();
 		// Wait for state sync.
 		futures::future::poll_fn::<(), _>(|cx| {
@@ -1222,8 +1229,8 @@ async fn warp_sync() {
 		sync_mode: SyncMode::Warp,
 		..Default::default()
 	});
-	let gap_end = net.peer(0).push_blocks(63, false);
-	let target = net.peer(0).push_blocks(1, false);
+	let gap_end = net.peer(0).push_blocks(63, false).pop().unwrap();
+	let target = net.peer(0).push_blocks(1, false).pop().unwrap();
 	net.peer(1).push_blocks(64, false);
 	net.peer(2).push_blocks(64, false);
 	// Wait for peer 1 to sync state.