update light aux storage when GRANDPA set changes (#5861)

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

@@ -219,7 +219,7 @@ pub struct FinalityEffects<Header: HeaderT> {
 /// 2) headers sub-chain (B; F] if B != F;
 /// 3) proof of GRANDPA::authorities() if the set changes at block F.
 #[derive(Debug, PartialEq, Encode, Decode)]
-struct FinalityProofFragment<Header: HeaderT> {
+pub(crate) struct FinalityProofFragment<Header: HeaderT> {
 	/// The hash of block F for which justification is provided.
 	pub block: Header::Hash,
 	/// Justification of the block F.
@@ -425,26 +425,7 @@ pub(crate) fn prove_finality<Block: BlockT, B: BlockchainBackend<Block>, J>(
 ///
 /// Returns the vector of headers that MUST be validated + imported
 /// AND if at least one of those headers is invalid, all other MUST be considered invalid.
-pub(crate) fn check_finality_proof<Block: BlockT, B>(
+pub(crate) fn check_finality_proof<Block: BlockT, B, J>(
-	blockchain: &B,
-	current_set_id: u64,
-	current_authorities: AuthorityList,
-	authorities_provider: &dyn AuthoritySetForFinalityChecker<Block>,
-	remote_proof: Vec<u8>,
-) -> ClientResult<FinalityEffects<Block::Header>>
-	where
-		NumberFor<Block>: BlockNumberOps,
-		B: BlockchainBackend<Block>,
-{
-	do_check_finality_proof::<_, _, GrandpaJustification<Block>>(
-		blockchain,
-		current_set_id,
-		current_authorities,
-		authorities_provider,
-		remote_proof)
-}
-
-fn do_check_finality_proof<Block: BlockT, B, J>(
 	blockchain: &B,
 	current_set_id: u64,
 	current_authorities: AuthorityList,
@@ -605,7 +586,7 @@ pub(crate) mod tests {
 	use super::*;
 	use sp_core::crypto::Public;
 
-	type FinalityProof = super::FinalityProof<Header>;
+	pub(crate) type FinalityProof = super::FinalityProof<Header>;
 
 	impl<GetAuthorities, ProveAuthorities> AuthoritySetForFinalityProver<Block> for (GetAuthorities, ProveAuthorities)
 		where
@@ -621,7 +602,7 @@ pub(crate) mod tests {
 		}
 	}
 
-	struct ClosureAuthoritySetForFinalityChecker<Closure>(pub Closure);
+	pub(crate) struct ClosureAuthoritySetForFinalityChecker<Closure>(pub Closure);
 
 	impl<Closure> AuthoritySetForFinalityChecker<Block> for ClosureAuthoritySetForFinalityChecker<Closure>
 		where
@@ -887,7 +868,7 @@ pub(crate) mod tests {
 		blockchain.insert(header(4).hash(), header(4), None, None, NewBlockState::Final).unwrap();
 		blockchain.insert(header(5).hash(), header(5), None, None, NewBlockState::Final).unwrap();
 		blockchain.insert(header(6).hash(), header(6), None, None, NewBlockState::Final).unwrap();
-		let effects = do_check_finality_proof::<_, _, TestJustification>(
+		let effects = check_finality_proof::<_, _, TestJustification>(
 			&blockchain,
 			0,
 			auth3,
@@ -915,7 +896,7 @@ pub(crate) mod tests {
 		let blockchain = test_blockchain();
 
 		// when we can't decode proof from Vec<u8>
-		do_check_finality_proof::<_, _, TestJustification>(
+		check_finality_proof::<_, _, TestJustification>(
 			&blockchain,
 			1,
 			vec![(AuthorityId::from_slice(&[3u8; 32]), 1u64)],
@@ -929,7 +910,7 @@ pub(crate) mod tests {
 		let blockchain = test_blockchain();
 
 		// when decoded proof has zero length
-		do_check_finality_proof::<_, _, TestJustification>(
+		check_finality_proof::<_, _, TestJustification>(
 			&blockchain,
 			1,
 			vec![(AuthorityId::from_slice(&[3u8; 32]), 1u64)],
@@ -944,7 +925,7 @@ pub(crate) mod tests {
 
 		// when intermediate (#0) fragment has non-empty unknown headers
 		let authorities = vec![(AuthorityId::from_slice(&[3u8; 32]), 1u64)];
-		do_check_finality_proof::<_, _, TestJustification>(
+		check_finality_proof::<_, _, TestJustification>(
 			&blockchain,
 			1,
 			authorities.clone(),
@@ -969,7 +950,7 @@ pub(crate) mod tests {
 
 		// when intermediate (#0) fragment has empty authorities proof
 		let authorities = vec![(AuthorityId::from_slice(&[3u8; 32]), 1u64)];
-		do_check_finality_proof::<_, _, TestJustification>(
+		check_finality_proof::<_, _, TestJustification>(
 			&blockchain,
 			1,
 			authorities.clone(),
@@ -994,7 +975,7 @@ pub(crate) mod tests {
 
 		let initial_authorities = vec![(AuthorityId::from_slice(&[3u8; 32]), 1u64)];
 		let next_authorities = vec![(AuthorityId::from_slice(&[4u8; 32]), 1u64)];
-		let effects = do_check_finality_proof::<_, _, TestJustification>(
+		let effects = check_finality_proof::<_, _, TestJustification>(
 			&blockchain,
 			1,
 			initial_authorities.clone(),

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

@@ -319,7 +319,7 @@ fn do_import_finality_proof<B, C, Block: BlockT, J>(
 {
 	let authority_set_id = data.authority_set.set_id();
 	let authorities = data.authority_set.authorities();
-	let finality_effects = crate::finality_proof::check_finality_proof(
+	let finality_effects = crate::finality_proof::check_finality_proof::<_, _, J>(
 		backend.blockchain(),
 		authority_set_id,
 		authorities,
@@ -359,7 +359,7 @@ fn do_import_finality_proof<B, C, Block: BlockT, J>(
 		.expect_block_number_from_id(&BlockId::Hash(finality_effects.block))
 		.map_err(|e| ConsensusError::ClientImport(e.to_string()))?;
 	do_finalize_block(
-		client,
+		client.clone(),
 		data,
 		finalized_block_hash,
 		finalized_block_number,
@@ -372,6 +372,14 @@ fn do_import_finality_proof<B, C, Block: BlockT, J>(
 		finality_effects.new_authorities,
 	);
 
+	// store new authorities set
+	require_insert_aux(
+		&client,
+		LIGHT_AUTHORITY_SET_KEY,
+		&data.authority_set,
+		"authority set",
+	)?;
+
 	Ok((finalized_block_hash, finalized_block_number))
 }
 
@@ -564,13 +572,30 @@ fn on_post_finalization_error(error: ClientError, value_type: &str) -> Consensus
 #[cfg(test)]
 pub mod tests {
 	use super::*;
-	use sp_consensus::{ForkChoiceStrategy, BlockImport};
+	use sp_consensus::{import_queue::CacheKeyId, ForkChoiceStrategy, BlockImport};
 	use sp_finality_grandpa::AuthorityId;
 	use sp_core::{H256, crypto::Public};
-	use sc_client_api::in_mem::Blockchain as InMemoryAuxStore;
+	use sc_client_api::{in_mem::Blockchain as InMemoryAuxStore, StorageProof};
 	use substrate_test_runtime_client::runtime::{Block, Header};
 	use crate::tests::TestApi;
-	use crate::finality_proof::tests::TestJustification;
+	use crate::finality_proof::{
+		FinalityProofFragment,
+		tests::{TestJustification, ClosureAuthoritySetForFinalityChecker},
+	};
+
+	struct OkVerifier;
+
+	impl Verifier<Block> for OkVerifier {
+		fn verify(
+			&mut self,
+			origin: BlockOrigin,
+			header: Header,
+			_justification: Option<Justification>,
+			_body: Option<Vec<<Block as BlockT>::Extrinsic>>,
+		) -> Result<(BlockImportParams<Block, ()>, Option<Vec<(CacheKeyId, Vec<u8>)>>), String> {
+			Ok((BlockImportParams::new(origin, header), None))
+		}
+	}
 
 	pub struct NoJustificationsImport<BE, Block: BlockT, Client>(
 		pub GrandpaLightBlockImport<BE, Block, Client>
@@ -666,9 +691,12 @@ pub mod tests {
 	fn import_block(
 		new_cache: HashMap<well_known_cache_keys::Id, Vec<u8>>,
 		justification: Option<Justification>,
-	) -> ImportResult {
+	) -> (
-		let (client, _backend) = substrate_test_runtime_client::new_light();
+		ImportResult,
-		let client = Arc::new(client);
+		substrate_test_runtime_client::client::Client<substrate_test_runtime_client::LightBackend, substrate_test_runtime_client::LightExecutor, Block, substrate_test_runtime_client::runtime::RuntimeApi>,
+		Arc<substrate_test_runtime_client::LightBackend>,
+	) {
+		let (client, backend) = substrate_test_runtime_client::new_light();
 		let mut import_data = LightImportData {
 			last_finalized: Default::default(),
 			authority_set: LightAuthoritySet::genesis(vec![(AuthorityId::from_slice(&[1; 32]), 1)]),
@@ -687,17 +715,21 @@ pub mod tests {
 		block.justification = justification;
 		block.fork_choice = Some(ForkChoiceStrategy::LongestChain);
 
-		do_import_block::<_, _, _, TestJustification>(
+		(
-			&*client,
+			do_import_block::<_, _, _, TestJustification>(
-			&mut import_data,
+				&client,
-			block,
+				&mut import_data,
-			new_cache,
+				block,
-		).unwrap()
+				new_cache,
+			).unwrap(),
+			client,
+			backend,
+		)
 	}
 
 	#[test]
 	fn finality_proof_not_required_when_consensus_data_does_not_changes_and_no_justification_provided() {
-		assert_eq!(import_block(HashMap::new(), None), ImportResult::Imported(ImportedAux {
+		assert_eq!(import_block(HashMap::new(), None).0, ImportResult::Imported(ImportedAux {
 			clear_justification_requests: false,
 			needs_justification: false,
 			bad_justification: false,
@@ -710,7 +742,7 @@ pub mod tests {
 	#[test]
 	fn finality_proof_not_required_when_consensus_data_does_not_changes_and_correct_justification_provided() {
 		let justification = TestJustification((0, vec![(AuthorityId::from_slice(&[1; 32]), 1)]), Vec::new()).encode();
-		assert_eq!(import_block(HashMap::new(), Some(justification)), ImportResult::Imported(ImportedAux {
+		assert_eq!(import_block(HashMap::new(), Some(justification)).0, ImportResult::Imported(ImportedAux {
 			clear_justification_requests: false,
 			needs_justification: false,
 			bad_justification: false,
@@ -724,7 +756,7 @@ pub mod tests {
 	fn finality_proof_required_when_consensus_data_changes_and_no_justification_provided() {
 		let mut cache = HashMap::new();
 		cache.insert(well_known_cache_keys::AUTHORITIES, vec![AuthorityId::from_slice(&[2; 32])].encode());
-		assert_eq!(import_block(cache, None), ImportResult::Imported(ImportedAux {
+		assert_eq!(import_block(cache, None).0, ImportResult::Imported(ImportedAux {
 			clear_justification_requests: false,
 			needs_justification: false,
 			bad_justification: false,
@@ -740,7 +772,7 @@ pub mod tests {
 		let mut cache = HashMap::new();
 		cache.insert(well_known_cache_keys::AUTHORITIES, vec![AuthorityId::from_slice(&[2; 32])].encode());
 		assert_eq!(
-			import_block(cache, Some(justification)),
+			import_block(cache, Some(justification)).0,
 			ImportResult::Imported(ImportedAux {
 				clear_justification_requests: false,
 				needs_justification: false,
@@ -797,4 +829,54 @@ pub mod tests {
 		assert_eq!(data.authority_set.authorities(), vec![(AuthorityId::from_slice(&[42; 32]), 2)]);
 		assert_eq!(data.consensus_changes.pending_changes(), &[(42, Default::default())]);
 	}
+
+	#[test]
+	fn authority_set_is_updated_on_finality_proof_import() {
+		let initial_set_id = 0;
+		let initial_set = vec![(AuthorityId::from_slice(&[1; 32]), 1)];
+		let updated_set = vec![(AuthorityId::from_slice(&[2; 32]), 2)];
+		let babe_set_signal = vec![AuthorityId::from_slice(&[42; 32])].encode();
+		
+		// import block #1 without justification
+		let mut cache = HashMap::new();
+		cache.insert(well_known_cache_keys::AUTHORITIES, babe_set_signal);
+		let (_, client, backend) = import_block(cache, None);
+
+		// import finality proof for block #1
+		let hash = client.block_hash(1).unwrap().unwrap();
+		let mut verifier = OkVerifier;
+		let mut import_data = LightImportData {
+			last_finalized: Default::default(),
+			authority_set: LightAuthoritySet::genesis(initial_set.clone()),
+			consensus_changes: ConsensusChanges::empty(),
+		};
+
+		// import finality proof
+		do_import_finality_proof::<_, _, _, TestJustification>(
+			&client,
+			backend,
+			&ClosureAuthoritySetForFinalityChecker(
+				|_, _, _| Ok(updated_set.clone())
+			),
+			&mut import_data,
+			Default::default(),
+			Default::default(),
+			vec![
+				FinalityProofFragment::<Header> {
+					block: hash,
+					justification: TestJustification(
+						(initial_set_id, initial_set.clone()),
+						Vec::new(),
+					).encode(),
+					unknown_headers: Vec::new(),
+					authorities_proof: Some(StorageProof::new(vec![])),
+				},
+			].encode(),
+			&mut verifier,
+		).unwrap();
+
+		// verify that new authorities set has been saved to the aux storage
+		let data = load_aux_import_data(Default::default(), &client, &TestApi::new(initial_set)).unwrap();
+		assert_eq!(data.authority_set.authorities(), updated_set);
+	}
 }