grandpa: handle error from SelectChain::finality_target (paritytech#5153

)

Fix paritytech/polkadot-sdk#3487.

---------

Co-authored-by: Dmitry Lavrenov <39522748+dmitrylavrenov@users.noreply.github.com>
Co-authored-by: Bastian Köcher <info@kchr.de>
Co-authored-by: Bastian Köcher <git@kchr.de>

client/consensus/grandpa/src/environment.rs

@@ -1209,14 +1209,20 @@ where
 			.header(target_hash)?
 			.expect("Header known to exist after `finality_target` call; qed"),
 		Err(err) => {
-			warn!(
+			debug!(
 				target: LOG_TARGET,
 				"Encountered error finding best chain containing {:?}: couldn't find target block: {}",
 				block,
 				err,
 			);
 
-			return Ok(None)
+			// NOTE: in case the given `SelectChain` doesn't provide any block we fallback to using
+			// the given base block provided by the GRANDPA voter.
+			//
+			// For example, `LongestChain` will error if the given block to use as base isn't part
+			// of the best chain (as defined by `LongestChain`), which could happen if there was a
+			// re-org.
+			base_header.clone()
 		},
 	};
 

client/consensus/grandpa/src/tests.rs

@@ -1738,6 +1738,116 @@ async fn grandpa_environment_checks_if_best_block_is_descendent_of_finality_targ
 	);
 }
 
+// This is a regression test for an issue that was triggered by a reorg
+// - https://github.com/paritytech/polkadot-sdk/issues/3487
+// - https://github.com/humanode-network/humanode/issues/1104
+#[tokio::test]
+async fn grandpa_environment_uses_round_base_block_for_voting_if_finality_target_errors() {
+	use finality_grandpa::voter::Environment;
+	use sp_consensus::SelectChain;
+
+	let peers = &[Ed25519Keyring::Alice];
+	let voters = make_ids(peers);
+
+	let mut net = GrandpaTestNet::new(TestApi::new(voters), 1, 0);
+	let peer = net.peer(0);
+	let network_service = peer.network_service().clone();
+	let sync_service = peer.sync_service().clone();
+	let notification_service =
+		peer.take_notification_service(&grandpa_protocol_name::NAME.into()).unwrap();
+	let link = peer.data.lock().take().unwrap();
+	let client = peer.client().as_client().clone();
+	let select_chain = sc_consensus::LongestChain::new(peer.client().as_backend());
+
+	// create a chain that is 10 blocks long
+	peer.push_blocks(10, false);
+
+	let env = test_environment_with_select_chain(
+		&link,
+		None,
+		network_service.clone(),
+		sync_service,
+		notification_service,
+		select_chain.clone(),
+		VotingRulesBuilder::default().build(),
+	);
+
+	let hashof7 = client.expect_block_hash_from_id(&BlockId::Number(7)).unwrap();
+	let hashof8_a = client.expect_block_hash_from_id(&BlockId::Number(8)).unwrap();
+
+	// finalize the 7th block
+	peer.client().finalize_block(hashof7, None, false).unwrap();
+
+	assert_eq!(peer.client().info().finalized_hash, hashof7);
+
+	// simulate completed grandpa round
+	env.completed(
+		1,
+		finality_grandpa::round::State {
+			prevote_ghost: Some((hashof8_a, 8)),
+			finalized: Some((hashof7, 7)),
+			estimate: Some((hashof8_a, 8)),
+			completable: true,
+		},
+		Default::default(),
+		&finality_grandpa::HistoricalVotes::new(),
+	)
+	.unwrap();
+
+	// check simulated last completed round
+	assert_eq!(
+		env.voter_set_state.read().last_completed_round().state,
+		finality_grandpa::round::State {
+			prevote_ghost: Some((hashof8_a, 8)),
+			finalized: Some((hashof7, 7)),
+			estimate: Some((hashof8_a, 8)),
+			completable: true
+		}
+	);
+
+	// `hashof8_a` should be finalized next, `best_chain_containing` should return `hashof8_a`
+	assert_eq!(env.best_chain_containing(hashof8_a).await.unwrap().unwrap().0, hashof8_a);
+
+	// simulate reorg on block 8 by creating a fork starting at block 7 that is 10 blocks long
+	peer.generate_blocks_at(
+		BlockId::Number(7),
+		10,
+		BlockOrigin::File,
+		|mut builder| {
+			builder.push_deposit_log_digest_item(DigestItem::Other(vec![1])).unwrap();
+			builder.build().unwrap().block
+		},
+		false,
+		false,
+		true,
+		ForkChoiceStrategy::LongestChain,
+	);
+
+	// check that new best chain is on longest chain
+	assert_eq!(env.select_chain.best_chain().await.unwrap().number, 17);
+
+	// verify that last completed round has `prevote_ghost` and `estimate` blocks related to
+	// `hashof8_a`
+	assert_eq!(
+		env.voter_set_state.read().last_completed_round().state,
+		finality_grandpa::round::State {
+			prevote_ghost: Some((hashof8_a, 8)),
+			finalized: Some((hashof7, 7)),
+			estimate: Some((hashof8_a, 8)),
+			completable: true
+		}
+	);
+
+	// `hashof8_a` should be finalized next, `best_chain_containing` should still return `hashof8_a`
+	assert_eq!(env.best_chain_containing(hashof8_a).await.unwrap().unwrap().0, hashof8_a);
+
+	// simulate finalization of the `hashof8_a` block
+	peer.client().finalize_block(hashof8_a, None, false).unwrap();
+
+	// check that best chain is reorged back
+	assert_eq!(env.select_chain.best_chain().await.unwrap().number, 10);
+}
+
 #[tokio::test]
 async fn grandpa_environment_never_overwrites_round_voter_state() {
 	use finality_grandpa::voter::Environment;

prdoc/pr_5153.prdoc

@@ -0,0 +1,12 @@
+title: "Grandpa: Ensure voting doesn't fail after a re-org"
+
+doc:
+  - audience: Node Operator
+    description: |
+      Ensures that a node is still able to vote with Grandpa, when a re-org happened that 
+      changed the best chain. This ultimately prevents that a network may runs into a 
+      potential finality stall.
+
+crates:
+  - name: sc-consensus-grandpa
+    bump: patch