From 11f144ee65b29cce597aa9feec87619ba241ba0d Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 6 May 2020 14:12:11 +0300 Subject: [PATCH] update light aux storage when GRANDPA set changes (#5861) --- .../finality-grandpa/src/finality_proof.rs | 39 ++---- .../finality-grandpa/src/light_import.rs | 118 +++++++++++++++--- 2 files changed, 110 insertions(+), 47 deletions(-) diff --git a/substrate/client/finality-grandpa/src/finality_proof.rs b/substrate/client/finality-grandpa/src/finality_proof.rs index c52cc8fc90..bf3662aba3 100644 --- a/substrate/client/finality-grandpa/src/finality_proof.rs +++ b/substrate/client/finality-grandpa/src/finality_proof.rs @@ -219,7 +219,7 @@ pub struct FinalityEffects { /// 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 { +pub(crate) struct FinalityProofFragment { /// 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, 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( - blockchain: &B, - current_set_id: u64, - current_authorities: AuthorityList, - authorities_provider: &dyn AuthoritySetForFinalityChecker, - remote_proof: Vec, -) -> ClientResult> - where - NumberFor: BlockNumberOps, - B: BlockchainBackend, -{ - do_check_finality_proof::<_, _, GrandpaJustification>( - blockchain, - current_set_id, - current_authorities, - authorities_provider, - remote_proof) -} - -fn do_check_finality_proof( +pub(crate) fn check_finality_proof( 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
; + pub(crate) type FinalityProof = super::FinalityProof
; impl AuthoritySetForFinalityProver for (GetAuthorities, ProveAuthorities) where @@ -621,7 +602,7 @@ pub(crate) mod tests { } } - struct ClosureAuthoritySetForFinalityChecker(pub Closure); + pub(crate) struct ClosureAuthoritySetForFinalityChecker(pub Closure); impl AuthoritySetForFinalityChecker for ClosureAuthoritySetForFinalityChecker 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 - 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(), diff --git a/substrate/client/finality-grandpa/src/light_import.rs b/substrate/client/finality-grandpa/src/light_import.rs index dd80dd8274..7b860bc34d 100644 --- a/substrate/client/finality-grandpa/src/light_import.rs +++ b/substrate/client/finality-grandpa/src/light_import.rs @@ -319,7 +319,7 @@ fn do_import_finality_proof( { 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( .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( 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 for OkVerifier { + fn verify( + &mut self, + origin: BlockOrigin, + header: Header, + _justification: Option, + _body: Option::Extrinsic>>, + ) -> Result<(BlockImportParams, Option)>>), String> { + Ok((BlockImportParams::new(origin, header), None)) + } + } pub struct NoJustificationsImport( pub GrandpaLightBlockImport @@ -666,9 +691,12 @@ pub mod tests { fn import_block( new_cache: HashMap>, justification: Option, - ) -> ImportResult { - let (client, _backend) = substrate_test_runtime_client::new_light(); - let client = Arc::new(client); + ) -> ( + ImportResult, + substrate_test_runtime_client::client::Client, + Arc, + ) { + 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, - &mut import_data, - block, - new_cache, - ).unwrap() + ( + do_import_block::<_, _, _, TestJustification>( + &client, + &mut import_data, + block, + 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::
{ + 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); + } }