From c189bd15deee6e6fae04bbc829791024a643a99d Mon Sep 17 00:00:00 2001
From: Davide Galassi <davxy@datawok.net>
Date: Tue, 21 Jun 2022 11:50:51 +0200
Subject: [PATCH] More robust grandpa revert procedure (#11719)

* More robust revert procedure

Return an error if revert is called in a node that is not actively
running grandpa, i.e. grandpa genesis data has not been initialized.
Previous implementation was just firing an `unreachable!` code exception.

Furthermore we skip revert hassle if there is nothing to revert.

* Nit
---
 substrate/client/consensus/babe/src/lib.rs   |  4 ++++
 substrate/client/finality-grandpa/src/lib.rs | 12 ++++++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/substrate/client/consensus/babe/src/lib.rs b/substrate/client/consensus/babe/src/lib.rs
index 84a3c618b3..8478122375 100644
--- a/substrate/client/consensus/babe/src/lib.rs
+++ b/substrate/client/consensus/babe/src/lib.rs
@@ -1873,7 +1873,11 @@ where
 {
 	let best_number = client.info().best_number;
 	let finalized = client.info().finalized_number;
+
 	let revertible = blocks.min(best_number - finalized);
+	if revertible == Zero::zero() {
+		return Ok(())
+	}
 
 	let revert_up_to_number = best_number - revertible;
 	let revert_up_to_hash = client.hash(revert_up_to_number)?.ok_or(ClientError::Backend(
diff --git a/substrate/client/finality-grandpa/src/lib.rs b/substrate/client/finality-grandpa/src/lib.rs
index 17e04affa0..cb32957c0b 100644
--- a/substrate/client/finality-grandpa/src/lib.rs
+++ b/substrate/client/finality-grandpa/src/lib.rs
@@ -1170,11 +1170,15 @@ fn local_authority_id(
 pub fn revert<Block, Client>(client: Arc<Client>, blocks: NumberFor<Block>) -> ClientResult<()>
 where
 	Block: BlockT,
-	Client: AuxStore + HeaderMetadata<Block, Error = sp_blockchain::Error> + HeaderBackend<Block>,
+	Client: AuxStore + HeaderMetadata<Block, Error = ClientError> + HeaderBackend<Block>,
 {
 	let best_number = client.info().best_number;
 	let finalized = client.info().finalized_number;
+
 	let revertible = blocks.min(best_number - finalized);
+	if revertible == Zero::zero() {
+		return Ok(())
+	}
 
 	let number = best_number - revertible;
 	let hash = client
@@ -1185,8 +1189,12 @@ where
 		)))?;
 
 	let info = client.info();
+
 	let persistent_data: PersistentData<Block> =
-		aux_schema::load_persistent(&*client, info.genesis_hash, Zero::zero(), || unreachable!())?;
+		aux_schema::load_persistent(&*client, info.genesis_hash, Zero::zero(), || {
+			const MSG: &str = "Unexpected missing grandpa data during revert";
+			Err(ClientError::Application(Box::from(MSG)))
+		})?;
 
 	let shared_authority_set = persistent_data.authority_set;
 	let mut authority_set = shared_authority_set.inner();