grandpa: always store justification for the latest finalized block (#8392)

* grandpa: always store justification for best finalized block

* grandpa-warp-sync: add latest justification when finished proving

* grandpa-warp-sync: change logic for sending best justification when finished

* grandpa: test storing best justification

* grandpa: reorder variants in WarpSyncFinished

substrate/client/finality-grandpa/src/aux_schema.rs

@@ -19,27 +19,30 @@
 //! Schema for stuff in the aux-db.
 
 use std::fmt::Debug;
-use parity_scale_codec::{Encode, Decode};
-use sc_client_api::backend::AuxStore;
-use sp_blockchain::{Result as ClientResult, Error as ClientError};
-use fork_tree::ForkTree;
+
 use finality_grandpa::round::State as RoundState;
-use sp_runtime::traits::{Block as BlockT, NumberFor};
 use log::{info, warn};
-use sp_finality_grandpa::{AuthorityList, SetId, RoundNumber};
+use parity_scale_codec::{Decode, Encode};
+
+use fork_tree::ForkTree;
+use sc_client_api::backend::AuxStore;
+use sp_blockchain::{Error as ClientError, Result as ClientResult};
+use sp_finality_grandpa::{AuthorityList, RoundNumber, SetId};
+use sp_runtime::traits::{Block as BlockT, NumberFor};
 
 use crate::authorities::{
-	AuthoritySet, AuthoritySetChanges, SharedAuthoritySet, PendingChange, DelayKind,
+	AuthoritySet, AuthoritySetChanges, DelayKind, PendingChange, SharedAuthoritySet,
 };
 use crate::environment::{
 	CompletedRound, CompletedRounds, CurrentRounds, HasVoted, SharedVoterSetState, VoterSetState,
 };
-use crate::NewAuthoritySet;
+use crate::{GrandpaJustification, NewAuthoritySet};
 
 const VERSION_KEY: &[u8] = b"grandpa_schema_version";
 const SET_STATE_KEY: &[u8] = b"grandpa_completed_round";
 const CONCLUDED_ROUNDS: &[u8] = b"grandpa_concluded_rounds";
 const AUTHORITY_SET_KEY: &[u8] = b"grandpa_voters";
+const BEST_JUSTIFICATION: &[u8] = b"grandpa_best_justification";
 
 const CURRENT_VERSION: u32 = 3;
 
@@ -464,7 +467,7 @@ where
 pub(crate) fn update_authority_set<Block: BlockT, F, R>(
 	set: &AuthoritySet<Block::Hash, NumberFor<Block>>,
 	new_set: Option<&NewAuthoritySet<Block::Hash, NumberFor<Block>>>,
-	write_aux: F
+	write_aux: F,
 ) -> R
 where
 	F: FnOnce(&[(&'static [u8], &[u8])]) -> R,
@@ -492,6 +495,33 @@ where
 	}
 }
 
+/// Update the justification for the latest finalized block on-disk.
+///
+/// We always keep around the justification for the best finalized block and overwrite it
+/// as we finalize new blocks, this makes sure that we don't store useless justifications
+/// but can always prove finality of the latest block.
+pub(crate) fn update_best_justification<Block: BlockT, F, R>(
+	justification: &GrandpaJustification<Block>,
+	write_aux: F,
+) -> R
+where
+	F: FnOnce(&[(&'static [u8], &[u8])]) -> R,
+{
+	let encoded_justification = justification.encode();
+	write_aux(&[(BEST_JUSTIFICATION, &encoded_justification[..])])
+}
+
+/// Fetch the justification for the latest block finalized by GRANDPA, if any.
+pub fn best_justification<B, Block>(
+	backend: &B,
+) -> ClientResult<Option<GrandpaJustification<Block>>>
+where
+	B: AuxStore,
+	Block: BlockT,
+{
+	load_decode::<_, GrandpaJustification<Block>>(backend, BEST_JUSTIFICATION)
+}
+
 /// Write voter set state.
 pub(crate) fn write_voter_set_state<Block: BlockT, B: AuxStore>(
 	backend: &B,
@@ -517,10 +547,9 @@ pub(crate) fn write_concluded_round<Block: BlockT, B: AuxStore>(
 
 #[cfg(test)]
 pub(crate) fn load_authorities<B: AuxStore, H: Decode, N: Decode + Clone + Ord>(
-	backend: &B
+	backend: &B,
 ) -> Option<AuthoritySet<H, N>> {
-	load_decode::<_, AuthoritySet<H, N>>(backend, AUTHORITY_SET_KEY)
-		.expect("backend error")
+	load_decode::<_, AuthoritySet<H, N>>(backend, AUTHORITY_SET_KEY).expect("backend error")
 }
 
 #[cfg(test)]

substrate/client/finality-grandpa/src/environment.rs

@@ -1275,11 +1275,8 @@ where
 		// `N+1`. this assumption is required to make sure we store
 		// justifications for transition blocks which will be requested by
 		// syncing clients.
-		let justification = match justification_or_commit {
-			JustificationOrCommit::Justification(justification) => {
-				notify_justification(justification_sender, || Ok(justification.clone()));
-				Some(justification.encode())
-			},
+		let (justification_required, justification) = match justification_or_commit {
+			JustificationOrCommit::Justification(justification) => (true, justification),
 			JustificationOrCommit::Commit((round_number, commit)) => {
 				let mut justification_required =
 					// justification is always required when block that enacts new authorities
@@ -1297,42 +1294,35 @@ where
 					}
 				}
 
-				// NOTE: the code below is a bit more verbose because we
-				// really want to avoid creating a justification if it isn't
-				// needed (e.g. if there's no subscribers), and also to avoid
-				// creating it twice. depending on the vote tree for the round,
-				// creating a justification might require multiple fetches of
-				// headers from the database.
-				let justification = || GrandpaJustification::from_commit(
+				let justification = GrandpaJustification::from_commit(
 					&client,
 					round_number,
 					commit,
-				);
+				)?;
 
-				if justification_required {
-					let justification = justification()?;
-					notify_justification(justification_sender, || Ok(justification.clone()));
-
-					Some(justification.encode())
-				} else {
-					notify_justification(justification_sender, justification);
-
-					None
-				}
+				(justification_required, justification)
 			},
 		};
 
-		debug!(target: "afg", "Finalizing blocks up to ({:?}, {})", number, hash);
+		notify_justification(justification_sender, || Ok(justification.clone()));
+
+		let persisted_justification = if justification_required {
+			Some((GRANDPA_ENGINE_ID, justification.encode()))
+		} else {
+			None
+		};
 
 		// ideally some handle to a synchronization oracle would be used
 		// to avoid unconditionally notifying.
-		let justification = justification.map(|j| (GRANDPA_ENGINE_ID, j.clone()));
 		client
-			.apply_finality(import_op, BlockId::Hash(hash), justification, true)
+			.apply_finality(import_op, BlockId::Hash(hash), persisted_justification, true)
 			.map_err(|e| {
 				warn!(target: "afg", "Error applying finality to block {:?}: {:?}", (hash, number), e);
 				e
 			})?;
+
+		debug!(target: "afg", "Finalizing blocks up to ({:?}, {})", number, hash);
+
 		telemetry!(
 			telemetry;
 			CONSENSUS_INFO;
@@ -1340,6 +1330,11 @@ where
 			"number" => ?number, "hash" => ?hash,
 		);
 
+		crate::aux_schema::update_best_justification(
+			&justification,
+			|insert| apply_aux(import_op, insert, &[]),
+		)?;
+
 		let new_authorities = if let Some((canon_hash, canon_number)) = status.new_set_block {
 			// the authority set has changed.
 			let (new_id, set_ref) = authority_set.current();

substrate/client/finality-grandpa/src/justification.rs

@@ -195,6 +195,11 @@ impl<Block: BlockT> GrandpaJustification<Block> {
 
 		Ok(())
 	}
+
+	/// The target block number and hash that this justifications proves finality for.
+	pub fn target(&self) -> (NumberFor<Block>, Block::Hash) {
+		(self.commit.target_number, self.commit.target_hash)
+	}
 }
 
 /// A utility trait implementing `finality_grandpa::Chain` using a given set of headers.

substrate/client/finality-grandpa/src/lib.rs

@@ -122,6 +122,7 @@ mod until_imported;
 mod voting_rule;
 
 pub use authorities::{AuthoritySet, AuthoritySetChanges, SharedAuthoritySet};
+pub use aux_schema::best_justification;
 pub use finality_proof::{FinalityProof, FinalityProofProvider, FinalityProofError};
 pub use notification::{GrandpaJustificationSender, GrandpaJustificationStream};
 pub use import::{find_scheduled_change, find_forced_change, GrandpaBlockImport};

substrate/client/finality-grandpa/src/tests.rs

@@ -451,10 +451,19 @@ fn finalize_3_voters_1_full_observer() {
 	}
 
 	// wait for all finalized on each.
-	let wait_for = futures::future::join_all(finality_notifications)
-		.map(|_| ());
+	let wait_for = futures::future::join_all(finality_notifications).map(|_| ());
 
 	block_until_complete(wait_for, &net, &mut runtime);
+
+	// all peers should have stored the justification for the best finalized block #20
+	for peer_id in 0..4 {
+		let client = net.lock().peers[peer_id].client().as_full().unwrap();
+		let justification = crate::aux_schema::best_justification::<_, Block>(&*client)
+			.unwrap()
+			.unwrap();
+
+		assert_eq!(justification.commit.target_number, 20);
+	}
 }
 
 #[test]