diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index 217914d2b3b09..838a6fb90219a 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -109,8 +109,11 @@ pub fn new_partial(config: &Configuration) -> Result { - /// Return finality proofs for the given authorities set id, if it is provided, otherwise the - /// current one will be used. + /// Prove finality for the given block number by returning a Justification for the last block of + /// the authority set. fn rpc_prove_finality( &self, - begin: Block::Hash, - end: Block::Hash, - authorities_set_id: u64, - ) -> Result, sp_blockchain::Error>; + block: NumberFor, + ) -> Result, sc_finality_grandpa::FinalityProofError>; } impl RpcFinalityProofProvider for FinalityProofProvider @@ -44,11 +42,9 @@ where { fn rpc_prove_finality( &self, - begin: Block::Hash, - end: Block::Hash, - authorities_set_id: u64, - ) -> Result, sp_blockchain::Error> { - self.prove_finality(begin, end, authorities_set_id) - .map(|x| x.map(|y| EncodedFinalityProofs(y.into()))) + block: NumberFor, + ) -> Result, sc_finality_grandpa::FinalityProofError> { + self.prove_finality(block) + .map(|x| x.map(|y| EncodedFinalityProof(y.into()))) } } diff --git a/client/finality-grandpa/rpc/src/lib.rs b/client/finality-grandpa/rpc/src/lib.rs index c6e4613c4f515..204bea4c18e2c 100644 --- a/client/finality-grandpa/rpc/src/lib.rs +++ b/client/finality-grandpa/rpc/src/lib.rs @@ -37,9 +37,9 @@ mod notification; mod report; use sc_finality_grandpa::GrandpaJustificationStream; -use sp_runtime::traits::Block as BlockT; +use sp_runtime::traits::{Block as BlockT, NumberFor}; -use finality::{EncodedFinalityProofs, RpcFinalityProofProvider}; +use finality::{EncodedFinalityProof, RpcFinalityProofProvider}; use report::{ReportAuthoritySet, ReportVoterState, ReportedRoundStates}; use notification::JustificationNotification; @@ -48,7 +48,7 @@ type FutureResult = /// Provides RPC methods for interacting with GRANDPA. #[rpc] -pub trait GrandpaApi { +pub trait GrandpaApi { /// RPC Metadata type Metadata; @@ -82,15 +82,13 @@ pub trait GrandpaApi { id: SubscriptionId ) -> jsonrpc_core::Result; - /// Prove finality for the range (begin; end] hash. Returns None if there are no finalized blocks - /// unknown in the range. If no authorities set is provided, the current one will be attempted. + /// Prove finality for the given block number by returning the Justification for the last block + /// in the set and all the intermediary headers to link them together. #[rpc(name = "grandpa_proveFinality")] fn prove_finality( &self, - begin: Hash, - end: Hash, - authorities_set_id: Option, - ) -> FutureResult>; + block: Number, + ) -> FutureResult>; } /// Implements the GrandpaApi RPC trait for interacting with GRANDPA. @@ -127,7 +125,8 @@ impl } } -impl GrandpaApi +impl + GrandpaApi> for GrandpaRpcHandler where VoterState: ReportVoterState + Send + Sync + 'static, @@ -171,16 +170,9 @@ where fn prove_finality( &self, - begin: Block::Hash, - end: Block::Hash, - authorities_set_id: Option, - ) -> FutureResult> { - // If we are not provided a set_id, try with the current one. - let authorities_set_id = authorities_set_id - .unwrap_or_else(|| self.authority_set.get().0); - let result = self - .finality_proof_provider - .rpc_prove_finality(begin, end, authorities_set_id); + block: NumberFor, + ) -> FutureResult> { + let result = self.finality_proof_provider.rpc_prove_finality(block); let future = async move { result }.boxed(); Box::new( future @@ -204,7 +196,7 @@ mod tests { use sc_block_builder::BlockBuilder; use sc_finality_grandpa::{ report, AuthorityId, GrandpaJustificationSender, GrandpaJustification, - FinalityProofFragment, + FinalityProof, }; use sp_blockchain::HeaderBackend; use sp_consensus::RecordProof; @@ -223,7 +215,7 @@ mod tests { struct EmptyVoterState; struct TestFinalityProofProvider { - finality_proofs: Vec>, + finality_proof: Option>, } fn voters() -> HashSet { @@ -262,11 +254,15 @@ mod tests { impl RpcFinalityProofProvider for TestFinalityProofProvider { fn rpc_prove_finality( &self, - _begin: Block::Hash, - _end: Block::Hash, - _authoritites_set_id: u64, - ) -> Result, sp_blockchain::Error> { - Ok(Some(EncodedFinalityProofs(self.finality_proofs.encode().into()))) + _block: NumberFor + ) -> Result, sc_finality_grandpa::FinalityProofError> { + Ok(Some(EncodedFinalityProof( + self.finality_proof + .as_ref() + .expect("Don't call rpc_prove_finality without setting the FinalityProof") + .encode() + .into() + ))) } } @@ -308,12 +304,12 @@ mod tests { ) where VoterState: ReportVoterState + Send + Sync + 'static, { - setup_io_handler_with_finality_proofs(voter_state, Default::default()) + setup_io_handler_with_finality_proofs(voter_state, None) } fn setup_io_handler_with_finality_proofs( voter_state: VoterState, - finality_proofs: Vec>, + finality_proof: Option>, ) -> ( jsonrpc_core::MetaIoHandler, GrandpaJustificationSender, @@ -321,7 +317,7 @@ mod tests { VoterState: ReportVoterState + Send + Sync + 'static, { let (justification_sender, justification_stream) = GrandpaJustificationStream::channel(); - let finality_proof_provider = Arc::new(TestFinalityProofProvider { finality_proofs }); + let finality_proof_provider = Arc::new(TestFinalityProofProvider { finality_proof }); let handler = GrandpaRpcHandler::new( TestAuthoritySet, @@ -520,29 +516,24 @@ mod tests { #[test] fn prove_finality_with_test_finality_proof_provider() { - let finality_proofs = vec![FinalityProofFragment { + let finality_proof = FinalityProof { block: header(42).hash(), justification: create_justification().encode(), unknown_headers: vec![header(2)], - authorities_proof: None, - }]; + }; let (io, _) = setup_io_handler_with_finality_proofs( TestVoterState, - finality_proofs.clone(), + Some(finality_proof.clone()), ); - let request = "{\"jsonrpc\":\"2.0\",\"method\":\"grandpa_proveFinality\",\"params\":[\ - \"0x0000000000000000000000000000000000000000000000000000000000000000\",\ - \"0x0000000000000000000000000000000000000000000000000000000000000001\",\ - 42\ - ],\"id\":1}"; + let request = + "{\"jsonrpc\":\"2.0\",\"method\":\"grandpa_proveFinality\",\"params\":[42],\"id\":1}"; let meta = sc_rpc::Metadata::default(); let resp = io.handle_request_sync(request, meta); let mut resp: serde_json::Value = serde_json::from_str(&resp.unwrap()).unwrap(); let result: sp_core::Bytes = serde_json::from_value(resp["result"].take()).unwrap(); - let fragments: Vec> = - Decode::decode(&mut &result[..]).unwrap(); - assert_eq!(fragments, finality_proofs); + let finality_proof_rpc: FinalityProof
= Decode::decode(&mut &result[..]).unwrap(); + assert_eq!(finality_proof_rpc, finality_proof); } } diff --git a/client/finality-grandpa/src/authorities.rs b/client/finality-grandpa/src/authorities.rs index 62a23a7ceab84..067f6dfc1ae65 100644 --- a/client/finality-grandpa/src/authorities.rs +++ b/client/finality-grandpa/src/authorities.rs @@ -114,6 +114,11 @@ where N: Add + Ord + Clone + Debug, pub fn clone_inner(&self) -> AuthoritySet { self.inner.read().clone() } + + /// Clone the inner `AuthoritySetChanges`. + pub fn authority_set_changes(&self) -> AuthoritySetChanges { + self.inner.read().authority_set_changes.clone() + } } impl From> for SharedAuthoritySet { @@ -152,12 +157,16 @@ pub struct AuthoritySet { /// is lower than the last finalized block (as signaled in the forced /// change) must be applied beforehand. pending_forced_changes: Vec>, + /// Track at which blocks the set id changed. This is useful when we need to prove finality for a + /// given block since we can figure out what set the block belongs to and when the set + /// started/ended. + authority_set_changes: AuthoritySetChanges, } impl AuthoritySet where H: PartialEq, - N: Ord, + N: Ord + Clone, { // authority sets must be non-empty and all weights must be greater than 0 fn invalid_authority_list(authorities: &AuthorityList) -> bool { @@ -175,6 +184,7 @@ where set_id: 0, pending_standard_changes: ForkTree::new(), pending_forced_changes: Vec::new(), + authority_set_changes: AuthoritySetChanges::empty(), }) } @@ -184,6 +194,7 @@ where set_id: u64, pending_standard_changes: ForkTree>, pending_forced_changes: Vec>, + authority_set_changes: AuthoritySetChanges, ) -> Option { if Self::invalid_authority_list(&authorities) { return None; @@ -194,6 +205,7 @@ where set_id, pending_standard_changes, pending_forced_changes, + authority_set_changes, }) } @@ -454,6 +466,9 @@ where "block" => ?change.canon_height ); + let mut authority_set_changes = self.authority_set_changes.clone(); + authority_set_changes.append(self.set_id, median_last_finalized.clone()); + new_set = Some(( median_last_finalized, AuthoritySet { @@ -461,6 +476,7 @@ where set_id: self.set_id + 1, pending_standard_changes: ForkTree::new(), // new set, new changes. pending_forced_changes: Vec::new(), + authority_set_changes, }, )); @@ -532,6 +548,9 @@ where "block" => ?change.canon_height ); + // Store the set_id together with the last block_number for the set + self.authority_set_changes.append(self.set_id, finalized_number.clone()); + self.current_authorities = change.next_authorities; self.set_id += 1; @@ -631,6 +650,45 @@ impl + Clone> PendingChange { } } +// Tracks historical authority set changes. We store the block numbers for the first block of each +// authority set, once they have been finalized. +#[derive(Debug, Encode, Decode, Clone, PartialEq)] +pub struct AuthoritySetChanges(pub Vec<(u64, N)>); + +impl AuthoritySetChanges { + pub(crate) fn empty() -> Self { + Self(Default::default()) + } + + pub(crate) fn append(&mut self, set_id: u64, block_number: N) { + self.0.push((set_id, block_number)); + } + + pub(crate) fn get_set_id(&self, block_number: N) -> Option<(u64, N)> { + let idx = self.0 + .binary_search_by_key(&block_number, |(_, n)| n.clone()) + .unwrap_or_else(|b| b); + if idx < self.0.len() { + let (set_id, block_number) = self.0[idx].clone(); + // To make sure we have the right set we need to check that the one before it also exists. + if idx > 0 { + let (prev_set_id, _) = self.0[idx - 1usize]; + if set_id != prev_set_id + 1u64 { + // Without the preceding set_id we don't have a well-defined start. + return None; + } + } else if set_id != 0 { + // If this is the first index, yet not the first set id then it's not well-defined + // that we are in the right set id. + return None; + } + Some((set_id, block_number)) + } else { + None + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -657,6 +715,7 @@ mod tests { set_id: 0, pending_standard_changes: ForkTree::new(), pending_forced_changes: Vec::new(), + authority_set_changes: AuthoritySetChanges::empty(), }; let change = |height| { @@ -704,6 +763,7 @@ mod tests { set_id: 0, pending_standard_changes: ForkTree::new(), pending_forced_changes: Vec::new(), + authority_set_changes: AuthoritySetChanges::empty(), }; let change_a = PendingChange { @@ -772,6 +832,7 @@ mod tests { set_id: 0, pending_standard_changes: ForkTree::new(), pending_forced_changes: Vec::new(), + authority_set_changes: AuthoritySetChanges::empty(), }; let set_a = vec![(AuthorityId::from_slice(&[1; 32]), 5)]; @@ -820,6 +881,7 @@ mod tests { authorities.pending_changes().collect::>(), vec![&change_a], ); + assert_eq!(authorities.authority_set_changes, AuthoritySetChanges::empty()); // finalizing "hash_d" will enact the change signaled at "hash_a" let status = authorities.apply_standard_changes( @@ -838,6 +900,7 @@ mod tests { assert_eq!(authorities.current_authorities, set_a); assert_eq!(authorities.set_id, 1); assert_eq!(authorities.pending_changes().count(), 0); + assert_eq!(authorities.authority_set_changes, AuthoritySetChanges(vec![(0, 15)])); } #[test] @@ -847,6 +910,7 @@ mod tests { set_id: 0, pending_standard_changes: ForkTree::new(), pending_forced_changes: Vec::new(), + authority_set_changes: AuthoritySetChanges::empty(), }; let set_a = vec![(AuthorityId::from_slice(&[1; 32]), 5)]; @@ -889,6 +953,7 @@ mod tests { authorities.apply_standard_changes("hash_d", 40, &is_descendent_of, false), Err(Error::ForkTree(fork_tree::Error::UnfinalizedAncestor)) )); + assert_eq!(authorities.authority_set_changes, AuthoritySetChanges::empty()); let status = authorities.apply_standard_changes( "hash_b", @@ -902,6 +967,7 @@ mod tests { assert_eq!(authorities.current_authorities, set_a); assert_eq!(authorities.set_id, 1); + assert_eq!(authorities.authority_set_changes, AuthoritySetChanges(vec![(0, 15)])); // after finalizing `change_a` it should be possible to finalize `change_c` let status = authorities.apply_standard_changes( @@ -916,6 +982,7 @@ mod tests { assert_eq!(authorities.current_authorities, set_c); assert_eq!(authorities.set_id, 2); + assert_eq!(authorities.authority_set_changes, AuthoritySetChanges(vec![(0, 15), (1, 40)])); } #[test] @@ -925,6 +992,7 @@ mod tests { set_id: 0, pending_standard_changes: ForkTree::new(), pending_forced_changes: Vec::new(), + authority_set_changes: AuthoritySetChanges::empty(), }; let set_a = vec![(AuthorityId::from_slice(&[1; 32]), 5)]; @@ -991,6 +1059,7 @@ mod tests { set_id: 0, pending_standard_changes: ForkTree::new(), pending_forced_changes: Vec::new(), + authority_set_changes: AuthoritySetChanges::empty(), }; let set_a = vec![(AuthorityId::from_slice(&[1; 32]), 5)]; @@ -1074,6 +1143,7 @@ mod tests { set_id: 1, pending_standard_changes: ForkTree::new(), pending_forced_changes: Vec::new(), + authority_set_changes: AuthoritySetChanges(vec![(0, 42)]), }, ) ); @@ -1087,6 +1157,7 @@ mod tests { set_id: 0, pending_standard_changes: ForkTree::new(), pending_forced_changes: Vec::new(), + authority_set_changes: AuthoritySetChanges::empty(), }; let set_a = vec![(AuthorityId::from_slice(&[1; 32]), 5)]; @@ -1125,6 +1196,7 @@ mod tests { set_id: 0, pending_standard_changes: ForkTree::new(), pending_forced_changes: Vec::new(), + authority_set_changes: AuthoritySetChanges::empty(), }; // effective at #15 @@ -1179,22 +1251,26 @@ mod tests { authorities.apply_forced_changes("hash_d45", 45, &static_is_descendent_of(true), false), Err(Error::ForcedAuthoritySetChangeDependencyUnsatisfied(15)) )); + assert_eq!(authorities.authority_set_changes, AuthoritySetChanges::empty()); // we apply the first pending standard change at #15 authorities .apply_standard_changes("hash_a15", 15, &static_is_descendent_of(true), false) .unwrap(); + assert_eq!(authorities.authority_set_changes, AuthoritySetChanges(vec![(0, 15)])); // but the forced change still depends on the next standard change assert!(matches!( authorities.apply_forced_changes("hash_d", 45, &static_is_descendent_of(true), false), Err(Error::ForcedAuthoritySetChangeDependencyUnsatisfied(20)) )); + assert_eq!(authorities.authority_set_changes, AuthoritySetChanges(vec![(0, 15)])); // we apply the pending standard change at #20 authorities .apply_standard_changes("hash_b", 20, &static_is_descendent_of(true), false) .unwrap(); + assert_eq!(authorities.authority_set_changes, AuthoritySetChanges(vec![(0, 15), (1, 20)])); // afterwards the forced change at #45 can already be applied since it signals // that finality stalled at #31, and the next pending standard change is effective @@ -1211,9 +1287,11 @@ mod tests { set_id: 3, pending_standard_changes: ForkTree::new(), pending_forced_changes: Vec::new(), + authority_set_changes: AuthoritySetChanges(vec![(0, 15), (1, 20), (2, 31)]), } ), ); + assert_eq!(authorities.authority_set_changes, AuthoritySetChanges(vec![(0, 15), (1, 20)])); } #[test] @@ -1225,6 +1303,7 @@ mod tests { set_id: 0, pending_standard_changes: ForkTree::new(), pending_forced_changes: Vec::new(), + authority_set_changes: AuthoritySetChanges::empty(), }; let new_set = current_authorities.clone(); @@ -1343,7 +1422,13 @@ mod tests { // empty authority lists are invalid assert_eq!(AuthoritySet::<(), ()>::genesis(vec![]), None); assert_eq!( - AuthoritySet::<(), ()>::new(vec![], 0, ForkTree::new(), Vec::new()), + AuthoritySet::<(), ()>::new( + vec![], + 0, + ForkTree::new(), + Vec::new(), + AuthoritySetChanges::empty(), + ), None, ); @@ -1362,7 +1447,8 @@ mod tests { invalid_authorities_weight.clone(), 0, ForkTree::new(), - Vec::new() + Vec::new(), + AuthoritySetChanges::empty(), ), None, ); @@ -1417,6 +1503,7 @@ mod tests { set_id: 0, pending_standard_changes: ForkTree::new(), pending_forced_changes: Vec::new(), + authority_set_changes: AuthoritySetChanges::empty(), }; let new_set = current_authorities.clone(); @@ -1512,4 +1599,32 @@ mod tests { "D" ); } + + #[test] + fn authority_set_changes_for_complete_data() { + let mut authority_set_changes = AuthoritySetChanges::empty(); + authority_set_changes.append(0, 41); + authority_set_changes.append(1, 81); + authority_set_changes.append(2, 121); + + assert_eq!(authority_set_changes.get_set_id(20), Some((0, 41))); + assert_eq!(authority_set_changes.get_set_id(40), Some((0, 41))); + assert_eq!(authority_set_changes.get_set_id(41), Some((0, 41))); + assert_eq!(authority_set_changes.get_set_id(42), Some((1, 81))); + assert_eq!(authority_set_changes.get_set_id(141), None); + } + + #[test] + fn authority_set_changes_for_incomplete_data() { + let mut authority_set_changes = AuthoritySetChanges::empty(); + authority_set_changes.append(2, 41); + authority_set_changes.append(3, 81); + authority_set_changes.append(4, 121); + + assert_eq!(authority_set_changes.get_set_id(20), None); + assert_eq!(authority_set_changes.get_set_id(40), None); + assert_eq!(authority_set_changes.get_set_id(41), None); + assert_eq!(authority_set_changes.get_set_id(42), Some((3, 81))); + assert_eq!(authority_set_changes.get_set_id(141), None); + } } diff --git a/client/finality-grandpa/src/aux_schema.rs b/client/finality-grandpa/src/aux_schema.rs index 0146269c8f71a..a5092334b99f5 100644 --- a/client/finality-grandpa/src/aux_schema.rs +++ b/client/finality-grandpa/src/aux_schema.rs @@ -28,7 +28,9 @@ use sp_runtime::traits::{Block as BlockT, NumberFor}; use log::{info, warn}; use sp_finality_grandpa::{AuthorityList, SetId, RoundNumber}; -use crate::authorities::{AuthoritySet, SharedAuthoritySet, PendingChange, DelayKind}; +use crate::authorities::{ + AuthoritySet, AuthoritySetChanges, SharedAuthoritySet, PendingChange, DelayKind, +}; use crate::environment::{ CompletedRound, CompletedRounds, CurrentRounds, HasVoted, SharedVoterSetState, VoterSetState, }; @@ -39,7 +41,7 @@ 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 CURRENT_VERSION: u32 = 2; +const CURRENT_VERSION: u32 = 3; /// The voter set state. #[derive(Debug, Clone, Encode, Decode)] @@ -69,8 +71,9 @@ struct V0AuthoritySet { } impl Into> for V0AuthoritySet -where H: Clone + Debug + PartialEq, - N: Clone + Debug + Ord, +where + H: Clone + Debug + PartialEq, + N: Clone + Debug + Ord, { fn into(self) -> AuthoritySet { let mut pending_standard_changes = ForkTree::new(); @@ -101,19 +104,46 @@ where H: Clone + Debug + PartialEq, self.set_id, pending_standard_changes, Vec::new(), + AuthoritySetChanges::empty(), ); authority_set.expect("current_authorities is non-empty and weights are non-zero; qed.") } } -pub(crate) fn load_decode(backend: &B, key: &[u8]) -> ClientResult> { +impl Into> for V2AuthoritySet +where + H: Clone + Debug + PartialEq, + N: Clone + Debug + Ord, +{ + fn into(self) -> AuthoritySet { + AuthoritySet::new( + self.current_authorities, + self.set_id, + self.pending_standard_changes, + self.pending_forced_changes, + AuthoritySetChanges::empty(), + ) + .expect("current_authorities is non-empty and weights are non-zero; qed.") + } +} + +#[derive(Debug, Clone, Encode, Decode, PartialEq)] +struct V2AuthoritySet { + current_authorities: AuthorityList, + set_id: u64, + pending_standard_changes: ForkTree>, + pending_forced_changes: Vec>, +} + +pub(crate) fn load_decode( + backend: &B, + key: &[u8] +) -> ClientResult> { match backend.get_aux(key)? { None => Ok(None), Some(t) => T::decode(&mut &t[..]) - .map_err( - |e| ClientError::Backend(format!("GRANDPA DB is corrupted: {}", e.what())), - ) + .map_err(|e| ClientError::Backend(format!("GRANDPA DB is corrupted: {}", e.what()))) .map(Some) } } @@ -127,11 +157,15 @@ pub(crate) struct PersistentData { fn migrate_from_version0( backend: &B, genesis_round: &G, -) -> ClientResult>, - VoterSetState, -)>> where B: AuxStore, - G: Fn() -> RoundState>, +) -> ClientResult< + Option<( + AuthoritySet>, + VoterSetState, + )>, +> +where + B: AuxStore, + G: Fn() -> RoundState>, { CURRENT_VERSION.using_encoded(|s| backend.insert_aux(&[(VERSION_KEY, s)], &[]) @@ -144,18 +178,20 @@ fn migrate_from_version0( let new_set: AuthoritySet> = old_set.into(); backend.insert_aux(&[(AUTHORITY_SET_KEY, new_set.encode().as_slice())], &[])?; - let (last_round_number, last_round_state) = match load_decode::<_, V0VoterSetState>>( - backend, - SET_STATE_KEY, - )? { + let (last_round_number, last_round_state) = match load_decode::< + _, + V0VoterSetState>, + >(backend, SET_STATE_KEY)? + { Some((number, state)) => (number, state), None => (0, genesis_round()), }; let set_id = new_set.set_id; - let base = last_round_state.prevote_ghost - .expect("state is for completed round; completed rounds must have a prevote ghost; qed."); + let base = last_round_state.prevote_ghost.expect( + "state is for completed round; completed rounds must have a prevote ghost; qed." + ); let mut current_rounds = CurrentRounds::new(); current_rounds.insert(last_round_number + 1, HasVoted::No); @@ -185,11 +221,15 @@ fn migrate_from_version0( fn migrate_from_version1( backend: &B, genesis_round: &G, -) -> ClientResult>, - VoterSetState, -)>> where B: AuxStore, - G: Fn() -> RoundState>, +) -> ClientResult< + Option<( + AuthoritySet>, + VoterSetState, + )>, +> +where + B: AuxStore, + G: Fn() -> RoundState>, { CURRENT_VERSION.using_encoded(|s| backend.insert_aux(&[(VERSION_KEY, s)], &[]) @@ -257,17 +297,64 @@ fn migrate_from_version1( Ok(None) } +fn migrate_from_version2( + backend: &B, + genesis_round: &G, +) -> ClientResult< + Option<( + AuthoritySet>, + VoterSetState, + )>, +> +where + B: AuxStore, + G: Fn() -> RoundState>, +{ + CURRENT_VERSION.using_encoded(|s| + backend.insert_aux(&[(VERSION_KEY, s)], &[]) + )?; + + if let Some(old_set) = load_decode::<_, V2AuthoritySet>>( + backend, + AUTHORITY_SET_KEY, + )? { + let new_set: AuthoritySet> = old_set.into(); + backend.insert_aux(&[(AUTHORITY_SET_KEY, new_set.encode().as_slice())], &[])?; + + let set_state = match load_decode::<_, VoterSetState>( + backend, + SET_STATE_KEY, + )? { + Some(state) => state, + None => { + let state = genesis_round(); + let base = state.prevote_ghost + .expect("state is for completed round; completed rounds must have a prevote ghost; qed."); + + VoterSetState::live( + new_set.set_id, + &new_set, + base, + ) + } + }; + + return Ok(Some((new_set, set_state))); + } + + Ok(None) +} + /// Load or initialize persistent data from backend. pub(crate) fn load_persistent( backend: &B, genesis_hash: Block::Hash, genesis_number: NumberFor, genesis_authorities: G, -) - -> ClientResult> - where - B: AuxStore, - G: FnOnce() -> ClientResult, +) -> ClientResult> +where + B: AuxStore, + G: FnOnce() -> ClientResult, { let version: Option = load_decode(backend, VERSION_KEY)?; @@ -275,7 +362,9 @@ pub(crate) fn load_persistent( match version { None => { - if let Some((new_set, set_state)) = migrate_from_version0::(backend, &make_genesis_round)? { + if let Some((new_set, set_state)) = + migrate_from_version0::(backend, &make_genesis_round)? + { return Ok(PersistentData { authority_set: new_set.into(), set_state: set_state.into(), @@ -283,7 +372,9 @@ pub(crate) fn load_persistent( } }, Some(1) => { - if let Some((new_set, set_state)) = migrate_from_version1::(backend, &make_genesis_round)? { + if let Some((new_set, set_state)) = + migrate_from_version1::(backend, &make_genesis_round)? + { return Ok(PersistentData { authority_set: new_set.into(), set_state: set_state.into(), @@ -291,6 +382,16 @@ pub(crate) fn load_persistent( } }, Some(2) => { + if let Some((new_set, set_state)) = + migrate_from_version2::(backend, &make_genesis_round)? + { + return Ok(PersistentData { + authority_set: new_set.into(), + set_state: set_state.into(), + }); + } + } + Some(3) => { if let Some(set) = load_decode::<_, AuthoritySet>>( backend, AUTHORITY_SET_KEY, @@ -364,7 +465,8 @@ pub(crate) fn update_authority_set( set: &AuthoritySet>, new_set: Option<&NewAuthoritySet>>, write_aux: F -) -> R where +) -> R +where F: FnOnce(&[(&'static [u8], &[u8])]) -> R, { // write new authority set state to disk. @@ -414,8 +516,9 @@ pub(crate) fn write_concluded_round( } #[cfg(test)] -pub(crate) fn load_authorities(backend: &B) - -> Option> { +pub(crate) fn load_authorities( + backend: &B +) -> Option> { load_decode::<_, AuthoritySet>(backend, AUTHORITY_SET_KEY) .expect("backend error") } @@ -474,10 +577,14 @@ mod test { assert_eq!( load_decode::<_, u32>(&client, VERSION_KEY).unwrap(), - Some(2), + Some(3), ); - let PersistentData { authority_set, set_state, .. } = load_persistent::( + let PersistentData { + authority_set, + set_state, + .. + } = load_persistent::( &client, H256::random(), 0, @@ -491,6 +598,7 @@ mod test { set_id, ForkTree::new(), Vec::new(), + AuthoritySetChanges::empty(), ).unwrap(), ); @@ -535,6 +643,7 @@ mod test { set_id, ForkTree::new(), Vec::new(), + AuthoritySetChanges::empty(), ).unwrap(); let voter_set_state = V1VoterSetState::Live(round_number, round_state.clone()); @@ -564,10 +673,14 @@ mod test { assert_eq!( load_decode::<_, u32>(&client, VERSION_KEY).unwrap(), - Some(2), + Some(3), ); - let PersistentData { authority_set, set_state, .. } = load_persistent::( + let PersistentData { + authority_set, + set_state, + .. + } = load_persistent::( &client, H256::random(), 0, @@ -581,6 +694,7 @@ mod test { set_id, ForkTree::new(), Vec::new(), + AuthoritySetChanges::empty(), ).unwrap(), ); @@ -605,6 +719,79 @@ mod test { ); } + #[test] + fn load_decode_from_v2_migrates_data_format() { + let client = substrate_test_runtime_client::new(); + + let authorities = vec![(AuthorityId::default(), 100)]; + let set_id = 3; + + { + let authority_set = V2AuthoritySet:: { + current_authorities: authorities.clone(), + set_id, + pending_standard_changes: ForkTree::new(), + pending_forced_changes: Vec::new(), + }; + + let genesis_state = (H256::random(), 32); + let voter_set_state: VoterSetState = + VoterSetState::live( + set_id, + &authority_set.clone().into(), // Note the conversion! + genesis_state + ); + + client.insert_aux( + &[ + (AUTHORITY_SET_KEY, authority_set.encode().as_slice()), + (SET_STATE_KEY, voter_set_state.encode().as_slice()), + (VERSION_KEY, 2u32.encode().as_slice()), + ], + &[], + ).unwrap(); + } + + assert_eq!( + load_decode::<_, u32>(&client, VERSION_KEY).unwrap(), + Some(2), + ); + + // should perform the migration + load_persistent::( + &client, + H256::random(), + 0, + || unreachable!(), + ).unwrap(); + + assert_eq!( + load_decode::<_, u32>(&client, VERSION_KEY).unwrap(), + Some(3), + ); + + let PersistentData { + authority_set, + .. + } = load_persistent::( + &client, + H256::random(), + 0, + || unreachable!(), + ).unwrap(); + + assert_eq!( + *authority_set.inner().read(), + AuthoritySet::new( + authorities.clone(), + set_id, + ForkTree::new(), + Vec::new(), + AuthoritySetChanges::empty(), + ).unwrap(), + ); + } + #[test] fn write_read_concluded_rounds() { let client = substrate_test_runtime_client::new(); @@ -625,7 +812,9 @@ mod test { round_number.using_encoded(|n| key.extend(n)); assert_eq!( - load_decode::<_, CompletedRound::>(&client, &key).unwrap(), + load_decode::<_, CompletedRound::>( + &client, &key + ).unwrap(), Some(completed_round), ); } diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index a0fad2f92c881..73726146af4e0 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -16,9 +16,6 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -// NOTE: should be removed with: https://github.com/paritytech/substrate/pull/7339 -#![allow(dead_code)] - //! GRANDPA block finality proof generation and check. //! //! Finality of block B is proved by providing: @@ -42,12 +39,10 @@ use std::sync::Arc; use log::trace; -use sp_blockchain::{Backend as BlockchainBackend, Error as ClientError, Result as ClientResult}; -use sc_client_api::{ - backend::Backend, StorageProof, - light::{FetchChecker, RemoteReadRequest}, - StorageProvider, ProofProvider, +use sp_blockchain::{ + Backend as BlockchainBackend, Error as ClientError, Result as ClientResult, }; +use sc_client_api::{backend::Backend, StorageProvider}; use parity_scale_codec::{Encode, Decode}; use finality_grandpa::BlockNumberOps; use sp_runtime::{ @@ -55,42 +50,25 @@ use sp_runtime::{ traits::{NumberFor, Block as BlockT, Header as HeaderT, Zero, One}, }; use sp_core::storage::StorageKey; -use sc_telemetry::{telemetry, CONSENSUS_INFO}; use sp_finality_grandpa::{AuthorityId, AuthorityList, VersionedAuthorityList, GRANDPA_AUTHORITIES_KEY}; use crate::justification::GrandpaJustification; use crate::VoterSet; +use crate::SharedAuthoritySet; +use crate::authorities::AuthoritySetChanges; -/// Maximum number of fragments that we want to return in a single prove_finality call. -const MAX_FRAGMENTS_IN_PROOF: usize = 8; +const MAX_UNKNOWN_HEADERS: usize = 100_000; -/// GRANDPA authority set related methods for the finality proof provider. pub trait AuthoritySetForFinalityProver: Send + Sync { /// Read GRANDPA_AUTHORITIES_KEY from storage at given block. fn authorities(&self, block: &BlockId) -> ClientResult; - /// Prove storage read of GRANDPA_AUTHORITIES_KEY at given block. - fn prove_authorities(&self, block: &BlockId) -> ClientResult; } -/// Trait that combines `StorageProvider` and `ProofProvider` -pub trait StorageAndProofProvider: StorageProvider + ProofProvider + Send + Sync - where - Block: BlockT, - BE: Backend + Send + Sync, -{} - -/// Blanket implementation. -impl StorageAndProofProvider for P - where - Block: BlockT, - BE: Backend + Send + Sync, - P: StorageProvider + ProofProvider + Send + Sync, -{} - /// Implementation of AuthoritySetForFinalityProver. -impl AuthoritySetForFinalityProver for Arc> - where - BE: Backend + Send + Sync + 'static, +impl AuthoritySetForFinalityProver + for Arc + Send + Sync> +where + BE: Backend + Send + Sync + 'static, { fn authorities(&self, block: &BlockId) -> ClientResult { let storage_key = StorageKey(GRANDPA_AUTHORITIES_KEY.to_vec()); @@ -99,153 +77,113 @@ impl AuthoritySetForFinalityProver for Arc) -> ClientResult { - self.read_proof(block, &mut std::iter::once(GRANDPA_AUTHORITIES_KEY)) - } -} - -/// GRANDPA authority set related methods for the finality proof checker. -pub trait AuthoritySetForFinalityChecker: Send + Sync { - /// Check storage read proof of GRANDPA_AUTHORITIES_KEY at given block. - fn check_authorities_proof( - &self, - hash: Block::Hash, - header: Block::Header, - proof: StorageProof, - ) -> ClientResult; -} - -/// FetchChecker-based implementation of AuthoritySetForFinalityChecker. -impl AuthoritySetForFinalityChecker for Arc> { - fn check_authorities_proof( - &self, - hash: Block::Hash, - header: Block::Header, - proof: StorageProof, - ) -> ClientResult { - let storage_key = GRANDPA_AUTHORITIES_KEY.to_vec(); - let request = RemoteReadRequest { - block: hash, - header, - keys: vec![storage_key.clone()], - retry_count: None, - }; - - self.check_read_proof(&request, proof) - .and_then(|results| { - let maybe_encoded = results.get(&storage_key) - .expect( - "storage_key is listed in the request keys; \ - check_read_proof must return a value for each requested key; - qed" - ); - maybe_encoded - .as_ref() - .and_then(|encoded| { - VersionedAuthorityList::decode(&mut encoded.as_slice()).ok() - }) - .map(|versioned| versioned.into()) - .ok_or(ClientError::InvalidAuthoritiesSet) - }) - } } /// Finality proof provider for serving network requests. -pub struct FinalityProofProvider { - backend: Arc, +pub struct FinalityProofProvider { + backend: Arc, authority_provider: Arc>, + shared_authority_set: Option>>, } impl FinalityProofProvider - where B: Backend + Send + Sync + 'static +where + B: Backend + Send + Sync + 'static, { /// Create new finality proof provider using: /// /// - backend for accessing blockchain data; /// - authority_provider for calling and proving runtime methods. + /// - shared_authority_set for accessing authority set data pub fn new

( backend: Arc, authority_provider: P, + shared_authority_set: Option>>, ) -> Self - where P: AuthoritySetForFinalityProver + 'static, + where + P: AuthoritySetForFinalityProver + 'static, { - FinalityProofProvider { backend, authority_provider: Arc::new(authority_provider) } + FinalityProofProvider { + backend, + authority_provider: Arc::new(authority_provider), + shared_authority_set, + } } /// Create new finality proof provider for the service using: /// /// - backend for accessing blockchain data; - /// - storage_and_proof_provider, which is generally a client. + /// - storage_provider, which is generally a client. + /// - shared_authority_set for accessing authority set data pub fn new_for_service( backend: Arc, - storage_and_proof_provider: Arc>, + storage_provider: Arc + Send + Sync>, + shared_authority_set: Option>>, ) -> Arc { - Arc::new(Self::new(backend, storage_and_proof_provider)) + Arc::new(Self::new( + backend, + storage_provider, + shared_authority_set, + )) } } impl FinalityProofProvider - where - Block: BlockT, - NumberFor: BlockNumberOps, - B: Backend + Send + Sync + 'static, +where + Block: BlockT, + NumberFor: BlockNumberOps, + B: Backend + Send + Sync + 'static, { - /// Prove finality for the range (begin; end] hash. Returns None if there are no finalized blocks - /// unknown in the range. - pub fn prove_finality( - &self, - begin: Block::Hash, - end: Block::Hash, - authorities_set_id: u64, - ) -> Result>, ClientError> { + /// Prove finality for the given block number by returning a Justification for the last block of + /// the authority set. + pub fn prove_finality(&self, block: NumberFor) + -> Result>, FinalityProofError> + { + let authority_set_changes = if let Some(changes) = self + .shared_authority_set + .as_ref() + .map(SharedAuthoritySet::authority_set_changes) + { + changes + } else { + return Ok(None); + }; + prove_finality::<_, _, GrandpaJustification>( &*self.backend.blockchain(), &*self.authority_provider, - authorities_set_id, - begin, - end, + authority_set_changes, + block, ) } } -/// The effects of block finality. -#[derive(Debug, PartialEq)] -pub struct FinalityEffects { - /// The (ordered) set of headers that could be imported. - pub headers_to_import: Vec

, - /// The hash of the block that could be finalized. - pub block: Header::Hash, - /// The justification for the block. - pub justification: Vec, - /// New authorities set id that should be applied starting from block. - pub new_set_id: u64, - /// New authorities set that should be applied starting from block. - pub new_authorities: AuthorityList, -} - -/// Single fragment of proof-of-finality. -/// /// Finality for block B is proved by providing: /// 1) the justification for the descendant block F; /// 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, Clone)] -pub struct FinalityProofFragment { +pub struct FinalityProof { /// The hash of block F for which justification is provided. pub block: Header::Hash, /// Justification of the block F. pub justification: Vec, - /// The set of headers in the range (U; F] that we believe are unknown to the caller. Ordered. + /// The set of headers in the range (B; F] that we believe are unknown to the caller. Ordered. pub unknown_headers: Vec
, - /// Optional proof of execution of GRANDPA::authorities() at the `block`. - pub authorities_proof: Option, } -/// Proof of finality is the ordered set of finality fragments, where: -/// - last fragment provides justification for the best possible block from the requested range; -/// - all other fragments provide justifications for GRANDPA authorities set changes within requested range. -type FinalityProof
= Vec>; +/// Errors occurring when trying to prove finality +#[derive(Debug, derive_more::Display, derive_more::From)] +pub enum FinalityProofError { + /// The requested block has not yet been finalized. + #[display(fmt = "Block not yet finalized")] + BlockNotYetFinalized, + /// The requested block is not covered by authority set changes. Likely this means the block is + /// in the latest authority set, and the subscription API is more appropriate. + #[display(fmt = "Block not covered by authority set changes")] + BlockNotInAuthoritySetChanges, + /// Errors originating from the client. + Client(sp_blockchain::Error), +} /// Single fragment of authority set proof. /// @@ -264,182 +202,102 @@ pub(crate) struct AuthoritySetProofFragment { /// - last fragment match target block. type AuthoritySetProof
= Vec>; -/// Finality proof request data. -#[derive(Debug, Encode, Decode)] -enum FinalityProofRequest { - /// Original version of the request. - Original(OriginalFinalityProofRequest), -} - -/// Original version of finality proof request. -#[derive(Debug, Encode, Decode)] -struct OriginalFinalityProofRequest { - /// The authorities set id we are waiting proof from. - /// - /// The first justification in the proof must be signed by this authority set. - pub authorities_set_id: u64, - /// Hash of the last known finalized block. - pub last_finalized: H, -} - -/// Prepare proof-of-finality for the best possible block in the range: (begin; end]. -/// -/// It is assumed that the caller already have a proof-of-finality for the block 'begin'. -/// It is assumed that the caller already knows all blocks in the range (begin; end]. -/// -/// Returns None if there are no finalized blocks unknown to the caller. -pub(crate) fn prove_finality, J>( +fn prove_finality( blockchain: &B, authorities_provider: &dyn AuthoritySetForFinalityProver, - authorities_set_id: u64, - begin: Block::Hash, - end: Block::Hash, -) -> ::sp_blockchain::Result>> - where - J: ProvableJustification, + authority_set_changes: AuthoritySetChanges>, + block: NumberFor, +) -> Result>, FinalityProofError> +where + Block: BlockT, + B: BlockchainBackend, + J: ProvableJustification, { - let begin_id = BlockId::Hash(begin); - let begin_number = blockchain.expect_block_number_from_id(&begin_id)?; - - // early-return if we sure that there are no blocks finalized AFTER begin block + // Early-return if we sure that there are no blocks finalized AFTER begin block let info = blockchain.info(); - if info.finalized_number <= begin_number { - trace!( - target: "afg", - "Requested finality proof for descendant of #{} while we only have finalized #{}. Returning empty proof.", - begin_number, + if info.finalized_number <= block { + let err = format!( + "Requested finality proof for descendant of #{} while we only have finalized #{}.", + block, info.finalized_number, ); - - return Ok(None); + trace!(target: "afg", "{}", &err); + return Err(FinalityProofError::BlockNotYetFinalized); } - // check if blocks range is valid. It is the caller responsibility to ensure - // that it only asks peers that know about whole blocks range - let end_number = blockchain.expect_block_number_from_id(&BlockId::Hash(end))?; - if begin_number + One::one() > end_number { - return Err(ClientError::Backend( - format!("Cannot generate finality proof for invalid range: {}..{}", begin_number, end_number), - )); - } - - // early-return if we sure that the block is NOT a part of canonical chain - let canonical_begin = blockchain.expect_block_hash_from_id(&BlockId::Number(begin_number))?; - if begin != canonical_begin { - return Err(ClientError::Backend( - format!("Cannot generate finality proof for non-canonical block: {}", begin), - )); - } - - // iterate justifications && try to prove finality - let mut fragment_index = 0; - let mut current_authorities = authorities_provider.authorities(&begin_id)?; - let mut current_number = begin_number + One::one(); - let mut finality_proof = Vec::new(); - let mut unknown_headers = Vec::new(); - let mut latest_proof_fragment = None; - let begin_authorities = current_authorities.clone(); - loop { - let current_id = BlockId::Number(current_number); - - // check if header is unknown to the caller - if current_number > end_number { - let unknown_header = blockchain.expect_header(current_id)?; - unknown_headers.push(unknown_header); - } - - if let Some(justification) = blockchain.justification(current_id)? { - // check if the current block enacts new GRANDPA authorities set - let new_authorities = authorities_provider.authorities(¤t_id)?; - let new_authorities_proof = if current_authorities != new_authorities { - current_authorities = new_authorities; - Some(authorities_provider.prove_authorities(¤t_id)?) - } else { - None - }; - - // prepare finality proof for the current block - let current = blockchain.expect_block_hash_from_id(&BlockId::Number(current_number))?; - let proof_fragment = FinalityProofFragment { - block: current, - justification, - unknown_headers: ::std::mem::take(&mut unknown_headers), - authorities_proof: new_authorities_proof, - }; - - // append justification to finality proof if required - let justifies_end_block = current_number >= end_number; - let justifies_authority_set_change = proof_fragment.authorities_proof.is_some(); - if justifies_end_block || justifies_authority_set_change { - // check if the proof is generated by the requested authority set - if finality_proof.is_empty() { - let justification_check_result = J::decode_and_verify( - &proof_fragment.justification, - authorities_set_id, - &begin_authorities, - ); - if justification_check_result.is_err() { - trace!( - target: "afg", - "Can not provide finality proof with requested set id #{}\ - (possible forced change?). Returning empty proof.", - authorities_set_id, - ); - - return Ok(None); - } - } - - finality_proof.push(proof_fragment); - latest_proof_fragment = None; - } else { - latest_proof_fragment = Some(proof_fragment); - } - - // we don't need to provide more justifications - if justifies_end_block { - break; - } - } - - // we can't provide more justifications - if current_number == info.finalized_number { - // append last justification - even if we can't generate finality proof for - // the end block, we try to generate it for the latest possible block - if let Some(latest_proof_fragment) = latest_proof_fragment.take() { - finality_proof.push(latest_proof_fragment); - - fragment_index += 1; - if fragment_index == MAX_FRAGMENTS_IN_PROOF { - break; - } - } - break; - } - - // else search for the next justification - current_number += One::one(); - } - - if finality_proof.is_empty() { + // Get set_id the block belongs to, and the last block of the set which should contain a + // Justification we can use to prove the requested block. + let (set_id, last_block_for_set) = if let Some(id) = authority_set_changes.get_set_id(block) { + id + } else { trace!( target: "afg", - "No justifications found when making finality proof for {}. Returning empty proof.", - end, + "AuthoritySetChanges does not cover the requested block #{}. \ + Maybe the subscription API is more appropriate.", + block, ); + return Err(FinalityProofError::BlockNotInAuthoritySetChanges); + }; + + // Get the Justification stored at the last block of the set + let last_block_for_set_id = BlockId::Number(last_block_for_set); + let justification = + if let Some(justification) = blockchain.justification(last_block_for_set_id)? { + justification + } else { + trace!( + target: "afg", + "No justification found when making finality proof for {}. Returning empty proof.", + block, + ); + return Ok(None); + }; - Ok(None) - } else { + + // Check if the justification is generated by the requested authority set + let block_authorities = authorities_provider.authorities(&BlockId::Number(block))?; + let justification_check_result = + J::decode_and_verify(&justification, set_id, &block_authorities); + if justification_check_result.is_err() { trace!( target: "afg", - "Built finality proof for {} of {} fragments. Last fragment for {}.", - end, - finality_proof.len(), - finality_proof.last().expect("checked that !finality_proof.is_empty(); qed").block, + "Can not provide finality proof with requested set id #{}\ + (possible forced change?). Returning empty proof.", + set_id, ); - - Ok(Some(finality_proof.encode())) + return Ok(None); } + + // Collect all headers from the requested block until the last block of the set + let unknown_headers = { + let mut headers = Vec::new(); + let mut current = block + One::one(); + loop { + if current >= last_block_for_set || headers.len() >= MAX_UNKNOWN_HEADERS { + break; + } + if block_authorities != authorities_provider.authorities(&BlockId::Number(current))? { + trace!( + target: "afg", + "Encountered new authorities when collecting unknown headers. \ + Returning empty proof", + ); + return Ok(None); + } + headers.push(blockchain.expect_header(BlockId::Number(current))?); + current += One::one(); + } + headers + }; + + Ok(Some( + FinalityProof { + block: blockchain.expect_block_hash_from_id(&last_block_for_set_id)?, + justification, + unknown_headers, + } + .encode(), + )) } /// Prepare authority proof for the best possible block starting at a given trusted block. @@ -569,67 +427,10 @@ fn get_warp_sync_proof_fragment>( Ok(result) } -/// Check GRANDPA proof-of-finality for the given block. -/// -/// 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, - J: ProvableJustification, -{ - // decode finality proof - let proof = FinalityProof::::decode(&mut &remote_proof[..]) - .map_err(|_| ClientError::BadJustification("failed to decode finality proof".into()))?; - - // empty proof can't prove anything - if proof.is_empty() { - return Err(ClientError::BadJustification("empty proof of finality".into())); - } - - // iterate and verify proof fragments - let last_fragment_index = proof.len() - 1; - let mut authorities = AuthoritiesOrEffects::Authorities(current_set_id, current_authorities); - for (proof_fragment_index, proof_fragment) in proof.into_iter().enumerate() { - // check that proof is non-redundant. The proof still can be valid, but - // we do not want peer to spam us with redundant data - if proof_fragment_index != last_fragment_index { - let has_unknown_headers = !proof_fragment.unknown_headers.is_empty(); - let has_new_authorities = proof_fragment.authorities_proof.is_some(); - if has_unknown_headers || !has_new_authorities { - return Err(ClientError::BadJustification("redundant proof of finality".into())); - } - } - - authorities = check_finality_proof_fragment::<_, _, J>( - blockchain, - authorities, - authorities_provider, - proof_fragment)?; - } - - let effects = authorities.extract_effects().expect("at least one loop iteration is guaranteed - because proof is not empty;\ - check_finality_proof_fragment is called on every iteration;\ - check_finality_proof_fragment always returns FinalityEffects;\ - qed"); - - telemetry!(CONSENSUS_INFO; "afg.finality_proof_ok"; - "set_id" => ?effects.new_set_id, "finalized_header_hash" => ?effects.block); - - Ok(effects) -} - /// Check GRANDPA authority change sequence to assert finality of a target block. /// /// Returns the header of the target block. +#[allow(unused)] pub(crate) fn check_warp_sync_proof( current_set_id: u64, current_authorities: AuthorityList, @@ -719,74 +520,6 @@ where } } -/// Check finality proof for the single block. -fn check_finality_proof_fragment( - blockchain: &B, - authority_set: AuthoritiesOrEffects, - authorities_provider: &dyn AuthoritySetForFinalityChecker, - proof_fragment: FinalityProofFragment, -) -> ClientResult> - where - NumberFor: BlockNumberOps, - B: BlockchainBackend, - J: Decode + ProvableJustification, -{ - // verify justification using previous authorities set - let (mut current_set_id, mut current_authorities) = authority_set.extract_authorities(); - let justification: J = Decode::decode(&mut &proof_fragment.justification[..]) - .map_err(|_| ClientError::JustificationDecode)?; - justification.verify(current_set_id, ¤t_authorities)?; - - // and now verify new authorities proof (if provided) - if let Some(new_authorities_proof) = proof_fragment.authorities_proof { - // the proof is either generated using known header and it is safe to query header - // here, because its non-finality proves that it can't be pruned - // or it is generated using last unknown header (because it is the one who has - // justification => we only generate proofs for headers with justifications) - let header = match proof_fragment.unknown_headers.iter().rev().next().cloned() { - Some(header) => header, - None => blockchain.expect_header(BlockId::Hash(proof_fragment.block))?, - }; - current_authorities = authorities_provider.check_authorities_proof( - proof_fragment.block, - header, - new_authorities_proof, - )?; - - current_set_id += 1; - } - - Ok(AuthoritiesOrEffects::Effects(FinalityEffects { - headers_to_import: proof_fragment.unknown_headers, - block: proof_fragment.block, - justification: proof_fragment.justification, - new_set_id: current_set_id, - new_authorities: current_authorities, - })) -} - -/// Authorities set from initial authorities set or finality effects. -enum AuthoritiesOrEffects { - Authorities(u64, AuthorityList), - Effects(FinalityEffects
), -} - -impl AuthoritiesOrEffects
{ - pub fn extract_authorities(self) -> (u64, AuthorityList) { - match self { - AuthoritiesOrEffects::Authorities(set_id, authorities) => (set_id, authorities), - AuthoritiesOrEffects::Effects(effects) => (effects.new_set_id, effects.new_authorities), - } - } - - pub fn extract_effects(self) -> Option> { - match self { - AuthoritiesOrEffects::Authorities(_, _) => None, - AuthoritiesOrEffects::Effects(effects) => Some(effects), - } - } -} - /// Block info extracted from the justification. pub(crate) trait BlockJustification { /// Block number justified. @@ -796,8 +529,36 @@ pub(crate) trait BlockJustification { fn hash(&self) -> Header::Hash; } +/// Check GRANDPA proof-of-finality for the given block. +/// +/// 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. +/// +/// This is currently not used, and exists primarily as an example of how to check finality proofs. +#[cfg(test)] +fn check_finality_proof( + current_set_id: u64, + current_authorities: AuthorityList, + remote_proof: Vec, +) -> ClientResult> +where + J: ProvableJustification
, +{ + let proof = FinalityProof::
::decode(&mut &remote_proof[..]) + .map_err(|_| ClientError::BadJustification("failed to decode finality proof".into()))?; + + let justification: J = Decode::decode(&mut &proof.justification[..]) + .map_err(|_| ClientError::JustificationDecode)?; + justification.verify(current_set_id, ¤t_authorities)?; + + use sc_telemetry::{telemetry, CONSENSUS_INFO}; + telemetry!(CONSENSUS_INFO; "afg.finality_proof_ok"; + "finalized_header_hash" => ?proof.block); + Ok(proof) +} + /// Justification used to prove block finality. -pub(crate) trait ProvableJustification: Encode + Decode { +pub trait ProvableJustification: Encode + Decode { /// Verify justification with respect to authorities set and authorities set id. fn verify(&self, set_id: u64, authorities: &[(AuthorityId, u64)]) -> ClientResult<()>; @@ -807,16 +568,16 @@ pub(crate) trait ProvableJustification: Encode + Decode { set_id: u64, authorities: &[(AuthorityId, u64)], ) -> ClientResult { - let justification = Self::decode(&mut &**justification) - .map_err(|_| ClientError::JustificationDecode)?; + let justification = + Self::decode(&mut &**justification).map_err(|_| ClientError::JustificationDecode)?; justification.verify(set_id, authorities)?; Ok(justification) } } impl ProvableJustification for GrandpaJustification - where - NumberFor: BlockNumberOps, +where + NumberFor: BlockNumberOps, { fn verify(&self, set_id: u64, authorities: &[(AuthorityId, u64)]) -> ClientResult<()> { let authorities = VoterSet::new(authorities.iter().cloned()).ok_or( @@ -891,41 +652,21 @@ impl WarpSyncFragmentCache
{ #[cfg(test)] pub(crate) mod tests { + use super::*; use substrate_test_runtime_client::runtime::{Block, Header, H256}; use sc_client_api::NewBlockState; use sc_client_api::in_mem::Blockchain as InMemoryBlockchain; - use super::*; use sp_core::crypto::Public; + use crate::authorities::AuthoritySetChanges; pub(crate) type FinalityProof = super::FinalityProof
; - impl AuthoritySetForFinalityProver for (GetAuthorities, ProveAuthorities) - where - GetAuthorities: Send + Sync + Fn(BlockId) -> ClientResult, - ProveAuthorities: Send + Sync + Fn(BlockId) -> ClientResult, + impl AuthoritySetForFinalityProver for GetAuthorities + where + GetAuthorities: Send + Sync + Fn(BlockId) -> ClientResult, { fn authorities(&self, block: &BlockId) -> ClientResult { - self.0(*block) - } - - fn prove_authorities(&self, block: &BlockId) -> ClientResult { - self.1(*block) - } - } - - pub(crate) struct ClosureAuthoritySetForFinalityChecker(pub Closure); - - impl AuthoritySetForFinalityChecker for ClosureAuthoritySetForFinalityChecker - where - Closure: Send + Sync + Fn(H256, Header, StorageProof) -> ClientResult, - { - fn check_authorities_proof( - &self, - hash: H256, - header: Header, - proof: StorageProof, - ) -> ClientResult { - self.0(hash, header, proof) + self(*block) } } @@ -965,392 +706,329 @@ pub(crate) mod tests { 0 => Default::default(), _ => header(number - 1).hash(), }; - Header::new(number, H256::from_low_u64_be(0), H256::from_low_u64_be(0), parent_hash, Default::default()) - } - - fn side_header(number: u64) -> Header { Header::new( number, H256::from_low_u64_be(0), - H256::from_low_u64_be(1), - header(number - 1).hash(), - Default::default(), - ) - } - - fn second_side_header(number: u64) -> Header { - Header::new( - number, H256::from_low_u64_be(0), - H256::from_low_u64_be(1), - side_header(number - 1).hash(), + parent_hash, Default::default(), ) } fn test_blockchain() -> InMemoryBlockchain { let blockchain = InMemoryBlockchain::::new(); - blockchain.insert(header(0).hash(), header(0), Some(vec![0]), None, NewBlockState::Final).unwrap(); - blockchain.insert(header(1).hash(), header(1), Some(vec![1]), None, NewBlockState::Final).unwrap(); - blockchain.insert(header(2).hash(), header(2), None, None, NewBlockState::Best).unwrap(); - blockchain.insert(header(3).hash(), header(3), Some(vec![3]), None, NewBlockState::Final).unwrap(); blockchain - } - - #[test] - fn finality_prove_fails_with_invalid_range() { - let blockchain = test_blockchain(); - - // their last finalized is: 2 - // they request for proof-of-finality of: 2 - // => range is invalid - prove_finality::<_, _, TestJustification>( - &blockchain, - &( - |_| unreachable!("should return before calling GetAuthorities"), - |_| unreachable!("should return before calling ProveAuthorities"), - ), - 0, - header(2).hash(), - header(2).hash(), - ).unwrap_err(); + .insert(header(0).hash(), header(0), Some(vec![0]), None, NewBlockState::Final) + .unwrap(); + blockchain + .insert(header(1).hash(), header(1), Some(vec![1]), None, NewBlockState::Final) + .unwrap(); + blockchain + .insert(header(2).hash(), header(2), None, None, NewBlockState::Best) + .unwrap(); + blockchain + .insert(header(3).hash(), header(3), Some(vec![3]), None, NewBlockState::Final) + .unwrap(); + blockchain } #[test] fn finality_proof_is_none_if_no_more_last_finalized_blocks() { let blockchain = test_blockchain(); - blockchain.insert(header(4).hash(), header(4), None, None, NewBlockState::Best).unwrap(); + blockchain + .insert(header(4).hash(), header(4), Some(vec![1]), None, NewBlockState::Best) + .unwrap(); + blockchain + .insert(header(5).hash(), header(5), Some(vec![2]), None, NewBlockState::Best) + .unwrap(); - // our last finalized is: 3 - // their last finalized is: 3 - // => we can't provide any additional justifications + let mut authority_set_changes = AuthoritySetChanges::empty(); + authority_set_changes.append(0, 5); + + // The last finalized block is 3, so we cannot provide further justifications. let proof_of_4 = prove_finality::<_, _, TestJustification>( &blockchain, - &( - |_| unreachable!("should return before calling GetAuthorities"), - |_| unreachable!("should return before calling ProveAuthorities"), - ), - 0, - header(3).hash(), - header(4).hash(), - ).unwrap(); - assert_eq!(proof_of_4, None); + &|_| unreachable!("Should return before calling GetAuthorities"), + authority_set_changes, + *header(4).number(), + ); + assert!(matches!(proof_of_4, Err(FinalityProofError::BlockNotYetFinalized))); } #[test] - fn finality_proof_fails_for_non_canonical_block() { + fn finality_proof_is_none_if_no_justification_known() { let blockchain = test_blockchain(); - blockchain.insert(header(4).hash(), header(4), None, None, NewBlockState::Best).unwrap(); - blockchain.insert(side_header(4).hash(), side_header(4), None, None, NewBlockState::Best).unwrap(); - blockchain.insert(second_side_header(5).hash(), second_side_header(5), None, None, NewBlockState::Best) + blockchain + .insert(header(4).hash(), header(4), None, None, NewBlockState::Final) .unwrap(); - blockchain.insert(header(5).hash(), header(5), Some(vec![5]), None, NewBlockState::Final).unwrap(); - // chain is 1 -> 2 -> 3 -> 4 -> 5 - // \> 4' -> 5' - // and the best finalized is 5 - // => when requesting for (4'; 5'], error is returned - prove_finality::<_, _, TestJustification>( + let mut authority_set_changes = AuthoritySetChanges::empty(); + authority_set_changes.append(0, 4); + + // Block 4 is finalized without justification + // => we can't prove finality of 3 + let proof_of_3 = prove_finality::<_, _, TestJustification>( &blockchain, - &( - |_| unreachable!("should return before calling GetAuthorities"), - |_| unreachable!("should return before calling ProveAuthorities"), - ), - 0, - side_header(4).hash(), - second_side_header(5).hash(), - ).unwrap_err(); + &|_| unreachable!("Should return before calling GetAuthorities"), + authority_set_changes, + *header(3).number(), + ) + .unwrap(); + assert_eq!(proof_of_3, None); } #[test] - fn finality_proof_is_none_if_no_justification_known() { + fn finality_proof_is_none_if_justification_is_generated_by_unknown_set() { + // This is the case for forced change: set_id has been forcibly increased, + // or when our stored authority set changes is incomplete let blockchain = test_blockchain(); - blockchain.insert(header(4).hash(), header(4), None, None, NewBlockState::Final).unwrap(); + let auth = vec![(AuthorityId::from_slice(&[42u8; 32]), 1u64)]; + let just4 = TestJustification((0, auth), vec![4]).encode(); + blockchain + .insert(header(4).hash(), header(4), Some(just4), None, NewBlockState::Final) + .unwrap(); - // block 4 is finalized without justification - // => we can't prove finality - let proof_of_4 = prove_finality::<_, _, TestJustification>( - &blockchain, - &( - |_| Ok(vec![(AuthorityId::from_slice(&[1u8; 32]), 1u64)]), - |_| unreachable!("authorities didn't change => ProveAuthorities won't be called"), - ), - 0, - header(3).hash(), - header(4).hash(), - ).unwrap(); - assert_eq!(proof_of_4, None); - } + let mut authority_set_changes = AuthoritySetChanges::empty(); + authority_set_changes.append(0, 4); - #[test] - fn finality_proof_works_without_authorities_change() { - let blockchain = test_blockchain(); - let authorities = vec![(AuthorityId::from_slice(&[1u8; 32]), 1u64)]; - let just4 = TestJustification((0, authorities.clone()), vec![4]).encode(); - let just5 = TestJustification((0, authorities.clone()), vec![5]).encode(); - blockchain.insert(header(4).hash(), header(4), Some(just4), None, NewBlockState::Final).unwrap(); - blockchain.insert(header(5).hash(), header(5), Some(just5.clone()), None, NewBlockState::Final).unwrap(); - - // blocks 4 && 5 are finalized with justification - // => since authorities are the same, we only need justification for 5 - let proof_of_5: FinalityProof = Decode::decode(&mut &prove_finality::<_, _, TestJustification>( + let proof_of_3 = prove_finality::<_, _, TestJustification>( &blockchain, - &( - |_| Ok(authorities.clone()), - |_| unreachable!("should return before calling ProveAuthorities"), - ), - 0, - header(3).hash(), - header(5).hash(), - ).unwrap().unwrap()[..]).unwrap(); - assert_eq!(proof_of_5, vec![FinalityProofFragment { - block: header(5).hash(), - justification: just5, - unknown_headers: Vec::new(), - authorities_proof: None, - }]); + &|_| Ok(vec![(AuthorityId::from_slice(&[1u8; 32]), 1u64)]), + authority_set_changes, + *header(3).number(), + ) + .unwrap(); + assert!(proof_of_3.is_none()); } #[test] - fn finality_proof_finalized_earlier_block_if_no_justification_for_target_is_known() { + fn finality_proof_is_none_if_authority_set_id_is_incorrect() { let blockchain = test_blockchain(); - blockchain.insert(header(4).hash(), header(4), Some(vec![4]), None, NewBlockState::Final).unwrap(); - blockchain.insert(header(5).hash(), header(5), None, None, NewBlockState::Final).unwrap(); + let auth = vec![(AuthorityId::from_slice(&[1u8; 32]), 1u64)]; + let just4 = TestJustification((0, auth.clone()), vec![4]).encode(); + blockchain + .insert(header(4).hash(), header(4), Some(just4), None, NewBlockState::Final) + .unwrap(); - // block 4 is finalized with justification + we request for finality of 5 - // => we can't prove finality of 5, but providing finality for 4 is still useful for requester - let proof_of_5: FinalityProof = Decode::decode(&mut &prove_finality::<_, _, TestJustification>( + let mut authority_set_changes = AuthoritySetChanges::empty(); + authority_set_changes.append(0, 1); + authority_set_changes.append(1, 4); + + // We call `prove_finality` with the wrong `authorities_set_id`, since the Justification for + // block 4 contains set id 0. + let proof_of_3 = prove_finality::<_, _, TestJustification>( &blockchain, - &( - |_| Ok(vec![(AuthorityId::from_slice(&[1u8; 32]), 1u64)]), - |_| unreachable!("should return before calling ProveAuthorities"), - ), - 0, - header(3).hash(), - header(5).hash(), - ).unwrap().unwrap()[..]).unwrap(); - assert_eq!(proof_of_5, vec![FinalityProofFragment { - block: header(4).hash(), - justification: vec![4], - unknown_headers: Vec::new(), - authorities_proof: None, - }]); + &|_| Ok(auth.clone()), + authority_set_changes, + *header(3).number(), + ) + .unwrap(); + assert!(proof_of_3.is_none()); } #[test] - fn finality_proof_works_with_authorities_change() { + fn finality_proof_is_none_for_next_set_id_with_new_the_authority_set() { let blockchain = test_blockchain(); - let auth3 = vec![(AuthorityId::from_slice(&[3u8; 32]), 1u64)]; - let auth5 = vec![(AuthorityId::from_slice(&[5u8; 32]), 1u64)]; - let auth7 = vec![(AuthorityId::from_slice(&[7u8; 32]), 1u64)]; - let just4 = TestJustification((0, auth3.clone()), vec![4]).encode(); - let just5 = TestJustification((0, auth3.clone()), vec![5]).encode(); - let just7 = TestJustification((1, auth5.clone()), vec![7]).encode(); - blockchain.insert(header(4).hash(), header(4), Some(just4), None, NewBlockState::Final).unwrap(); - blockchain.insert(header(5).hash(), header(5), Some(just5.clone()), None, NewBlockState::Final).unwrap(); - blockchain.insert(header(6).hash(), header(6), None, None, NewBlockState::Final).unwrap(); - blockchain.insert(header(7).hash(), header(7), Some(just7.clone()), None, NewBlockState::Final).unwrap(); - - // when querying for finality of 6, we assume that the #3 is the last block known to the requester - // => since we only have justification for #7, we provide #7 - let proof_of_6: FinalityProof = Decode::decode(&mut &prove_finality::<_, _, TestJustification>( + let auth1 = vec![(AuthorityId::from_slice(&[1u8; 32]), 1u64)]; + let auth2 = vec![(AuthorityId::from_slice(&[2u8; 32]), 1u64)]; + let just5 = TestJustification((0, auth1.clone()), vec![5]).encode(); + let just6 = TestJustification((1, auth2.clone()), vec![6]).encode(); + blockchain + .insert(header(4).hash(), header(4), None, None, NewBlockState::Final) + .unwrap(); + blockchain + .insert(header(5).hash(), header(5), Some(just5), None, NewBlockState::Final) + .unwrap(); + blockchain + .insert(header(6).hash(), header(6), Some(just6), None, NewBlockState::Final) + .unwrap(); + + let mut authority_set_changes = AuthoritySetChanges::empty(); + authority_set_changes.append(0, 1); + authority_set_changes.append(1, 6); + + // Trying to prove block 4 using block 6 fails as the authority set has changed + let proof_of_4 = prove_finality::<_, _, TestJustification>( &blockchain, - &( - |block_id| match block_id { - BlockId::Hash(h) if h == header(3).hash() => Ok(auth3.clone()), - BlockId::Number(4) => Ok(auth3.clone()), - BlockId::Number(5) => Ok(auth5.clone()), - BlockId::Number(7) => Ok(auth7.clone()), - _ => unreachable!("no other authorities should be fetched: {:?}", block_id), - }, - |block_id| match block_id { - BlockId::Number(5) => Ok(StorageProof::new(vec![vec![50]])), - BlockId::Number(7) => Ok(StorageProof::new(vec![vec![70]])), - _ => unreachable!("no other authorities should be proved: {:?}", block_id), - }, - ), - 0, - header(3).hash(), - header(6).hash(), - ).unwrap().unwrap()[..]).unwrap(); - // initial authorities set (which start acting from #0) is [3; 32] - assert_eq!(proof_of_6, vec![ - // new authorities set starts acting from #5 => we do not provide fragment for #4 - // first fragment provides justification for #5 && authorities set that starts acting from #5 - FinalityProofFragment { - block: header(5).hash(), - justification: just5, - unknown_headers: Vec::new(), - authorities_proof: Some(StorageProof::new(vec![vec![50]])), + &|block_id| match block_id { + BlockId::Number(4) => Ok(auth1.clone()), + _ => unimplemented!("No other authorities should be proved: {:?}", block_id), }, - // last fragment provides justification for #7 && unknown#7 - FinalityProofFragment { - block: header(7).hash(), - justification: just7.clone(), - unknown_headers: vec![header(7)], - authorities_proof: Some(StorageProof::new(vec![vec![70]])), - }, - ]); + authority_set_changes, + *header(4).number(), + ) + .unwrap(); + assert!(proof_of_4.is_none()); + } - // now let's verify finality proof + #[test] + fn finality_proof_is_none_if_the_authority_set_changes_and_changes_back() { let blockchain = test_blockchain(); - 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 = check_finality_proof::<_, _, TestJustification>( - &blockchain, - 0, - auth3, - &ClosureAuthoritySetForFinalityChecker( - |hash, _header, proof: StorageProof| match proof.clone().iter_nodes().next().map(|x| x[0]) { - Some(50) => Ok(auth5.clone()), - Some(70) => Ok(auth7.clone()), - _ => unreachable!("no other proofs should be checked: {}", hash), - } - ), - proof_of_6.encode(), - ).unwrap(); + let auth1 = vec![(AuthorityId::from_slice(&[1u8; 32]), 1u64)]; + let auth2 = vec![(AuthorityId::from_slice(&[2u8; 32]), 1u64)]; + let just5 = TestJustification((0, auth1.clone()), vec![5]).encode(); + let just6 = TestJustification((1, auth2.clone()), vec![6]).encode(); + let just7 = TestJustification((2, auth1.clone()), vec![7]).encode(); + blockchain + .insert(header(4).hash(), header(4), None, None, NewBlockState::Final) + .unwrap(); + blockchain + .insert(header(5).hash(), header(5), Some(just5), None, NewBlockState::Final) + .unwrap(); + blockchain + .insert(header(6).hash(), header(6), Some(just6), None, NewBlockState::Final) + .unwrap(); + blockchain + .insert(header(7).hash(), header(7), Some(just7), None, NewBlockState::Final) + .unwrap(); - assert_eq!(effects, FinalityEffects { - headers_to_import: vec![header(7)], - block: header(7).hash(), - justification: TestJustification((1, auth5.clone()), vec![7]).encode(), - new_set_id: 2, - new_authorities: auth7, - }); + // Set authority set changes so that they don't contain the switch, and switch back, of the + // authorities. As well as incorrect set_id to avoid the guard against that. + // This should trigger the check for walking through the headers and checking for authority + // set changes that are missed. + let mut authority_set_changes = AuthoritySetChanges::empty(); + authority_set_changes.append(0, 1); + authority_set_changes.append(1, 2); + authority_set_changes.append(2, 7); + + let proof_of_4 = + prove_finality::<_, _, TestJustification>( + &blockchain, + &|block_id| match block_id { + BlockId::Number(4) => Ok(auth1.clone()), + BlockId::Number(5) => Ok(auth1.clone()), + BlockId::Number(6) => Ok(auth2.clone()), + _ => unimplemented!("No other authorities should be proved: {:?}", block_id), + }, + authority_set_changes, + *header(4).number(), + ) + .unwrap(); + assert!(proof_of_4.is_none()); } #[test] fn finality_proof_check_fails_when_proof_decode_fails() { - let blockchain = test_blockchain(); - - // when we can't decode proof from Vec - check_finality_proof::<_, _, TestJustification>( - &blockchain, + // When we can't decode proof from Vec + check_finality_proof::<_, TestJustification>( 1, vec![(AuthorityId::from_slice(&[3u8; 32]), 1u64)], - &ClosureAuthoritySetForFinalityChecker(|_, _, _| unreachable!("returns before CheckAuthoritiesProof")), vec![42], - ).unwrap_err(); + ) + .unwrap_err(); } #[test] fn finality_proof_check_fails_when_proof_is_empty() { - let blockchain = test_blockchain(); - - // when decoded proof has zero length - check_finality_proof::<_, _, TestJustification>( - &blockchain, + // When decoded proof has zero length + check_finality_proof::<_, TestJustification>( 1, vec![(AuthorityId::from_slice(&[3u8; 32]), 1u64)], - &ClosureAuthoritySetForFinalityChecker(|_, _, _| unreachable!("returns before CheckAuthoritiesProof")), Vec::::new().encode(), - ).unwrap_err(); + ) + .unwrap_err(); } #[test] - fn finality_proof_check_fails_when_intermediate_fragment_has_unknown_headers() { - let blockchain = test_blockchain(); - - // when intermediate (#0) fragment has non-empty unknown headers - let authorities = vec![(AuthorityId::from_slice(&[3u8; 32]), 1u64)]; - check_finality_proof::<_, _, TestJustification>( - &blockchain, + fn finality_proof_check_works() { + let auth = vec![(AuthorityId::from_slice(&[3u8; 32]), 1u64)]; + let finality_proof = FinalityProof { + block: header(2).hash(), + justification: TestJustification((1, auth.clone()), vec![7]).encode(), + unknown_headers: Vec::new(), + }; + let proof = check_finality_proof::<_, TestJustification>( 1, - authorities.clone(), - &ClosureAuthoritySetForFinalityChecker(|_, _, _| unreachable!("returns before CheckAuthoritiesProof")), - vec![FinalityProofFragment { - block: header(4).hash(), - justification: TestJustification((0, authorities.clone()), vec![7]).encode(), - unknown_headers: vec![header(4)], - authorities_proof: Some(StorageProof::new(vec![vec![42]])), - }, FinalityProofFragment { - block: header(5).hash(), - justification: TestJustification((0, authorities), vec![8]).encode(), - unknown_headers: vec![header(5)], - authorities_proof: None, - }].encode(), - ).unwrap_err(); + auth.clone(), + finality_proof.encode(), + ) + .unwrap(); + assert_eq!(proof, finality_proof); } #[test] - fn finality_proof_check_fails_when_intermediate_fragment_has_no_authorities_proof() { + fn finality_proof_using_authority_set_changes_is_none_with_undefined_start() { let blockchain = test_blockchain(); + let auth = vec![(AuthorityId::from_slice(&[1u8; 32]), 1u64)]; + let just4 = TestJustification((0, auth.clone()), vec![4]).encode(); + let just7 = TestJustification((1, auth.clone()), vec![7]).encode(); + blockchain + .insert(header(4).hash(), header(4), Some(just4), 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(); + blockchain + .insert(header(7).hash(), header(7), Some(just7.clone()), None, NewBlockState::Final) + .unwrap(); - // when intermediate (#0) fragment has empty authorities proof - let authorities = vec![(AuthorityId::from_slice(&[3u8; 32]), 1u64)]; - check_finality_proof::<_, _, TestJustification>( - &blockchain, - 1, - authorities.clone(), - &ClosureAuthoritySetForFinalityChecker(|_, _, _| unreachable!("returns before CheckAuthoritiesProof")), - vec![FinalityProofFragment { - block: header(4).hash(), - justification: TestJustification((0, authorities.clone()), vec![7]).encode(), - unknown_headers: Vec::new(), - authorities_proof: None, - }, FinalityProofFragment { - block: header(5).hash(), - justification: TestJustification((0, authorities), vec![8]).encode(), - unknown_headers: vec![header(5)], - authorities_proof: None, - }].encode(), - ).unwrap_err(); - } - - #[test] - fn finality_proof_check_works() { - let blockchain = test_blockchain(); + // We have stored the correct block number for the relevant set, but as we are missing the + // block for the preceding set the start is not well-defined. + let mut authority_set_changes = AuthoritySetChanges::empty(); + authority_set_changes.append(1, 7); - let initial_authorities = vec![(AuthorityId::from_slice(&[3u8; 32]), 1u64)]; - let next_authorities = vec![(AuthorityId::from_slice(&[4u8; 32]), 1u64)]; - let effects = check_finality_proof::<_, _, TestJustification>( + let proof_of_5 = prove_finality::<_, _, TestJustification>( &blockchain, - 1, - initial_authorities.clone(), - &ClosureAuthoritySetForFinalityChecker(|_, _, _| Ok(next_authorities.clone())), - vec![FinalityProofFragment { - block: header(2).hash(), - justification: TestJustification((1, initial_authorities.clone()), vec![7]).encode(), - unknown_headers: Vec::new(), - authorities_proof: Some(StorageProof::new(vec![vec![42]])), - }, FinalityProofFragment { - block: header(4).hash(), - justification: TestJustification((2, next_authorities.clone()), vec![8]).encode(), - unknown_headers: vec![header(4)], - authorities_proof: None, - }].encode(), - ).unwrap(); - assert_eq!(effects, FinalityEffects { - headers_to_import: vec![header(4)], - block: header(4).hash(), - justification: TestJustification((2, next_authorities.clone()), vec![8]).encode(), - new_set_id: 2, - new_authorities: vec![(AuthorityId::from_slice(&[4u8; 32]), 1u64)], - }); + &|block_id| match block_id { + BlockId::Number(5) => Ok(auth.clone()), + BlockId::Number(6) => Ok(auth.clone()), + _ => unimplemented!("No other authorities should be proved: {:?}", block_id), + }, + authority_set_changes, + *header(5).number(), + ); + assert!(matches!(proof_of_5, Err(FinalityProofError::BlockNotInAuthoritySetChanges))); } #[test] - fn finality_proof_is_none_if_first_justification_is_generated_by_unknown_set() { - // this is the case for forced change: set_id has been forcibly increased on full node - // and light node missed that - // => justification verification will fail on light node anyways, so we do not return - // finality proof at all + fn finality_proof_using_authority_set_changes_works() { let blockchain = test_blockchain(); - let just4 = TestJustification((0, vec![(AuthorityId::from_slice(&[42u8; 32]), 1u64)]), vec![4]).encode(); - blockchain.insert(header(4).hash(), header(4), Some(just4), None, NewBlockState::Final).unwrap(); + let auth = vec![(AuthorityId::from_slice(&[1u8; 32]), 1u64)]; + let just4 = TestJustification((0, auth.clone()), vec![4]).encode(); + let just7 = TestJustification((1, auth.clone()), vec![7]).encode(); + blockchain + .insert(header(4).hash(), header(4), Some(just4), 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(); + blockchain + .insert(header(7).hash(), header(7), Some(just7.clone()), None, NewBlockState::Final) + .unwrap(); - let proof_of_4 = prove_finality::<_, _, TestJustification>( - &blockchain, - &( - |_| Ok(vec![(AuthorityId::from_slice(&[1u8; 32]), 1u64)]), - |_| unreachable!("should return before calling ProveAuthorities"), - ), - 0, - header(3).hash(), - header(4).hash(), - ).unwrap(); - assert!(proof_of_4.is_none()); + let mut authority_set_changes = AuthoritySetChanges::empty(); + authority_set_changes.append(0, 4); + authority_set_changes.append(1, 7); + + let proof_of_5: FinalityProof = Decode::decode( + &mut &prove_finality::<_, _, TestJustification>( + &blockchain, + &|block_id| match block_id { + BlockId::Number(5) => Ok(auth.clone()), + BlockId::Number(6) => Ok(auth.clone()), + _ => unimplemented!("No other authorities should be proved: {:?}", block_id), + }, + authority_set_changes, + *header(5).number(), + ) + .unwrap() + .unwrap()[..], + ) + .unwrap(); + assert_eq!( + proof_of_5, + FinalityProof { + block: header(7).hash(), + justification: just7, + unknown_headers: vec![header(6)], + } + ); } #[test] diff --git a/client/finality-grandpa/src/lib.rs b/client/finality-grandpa/src/lib.rs index d556ae089b61a..c7f7c8517b957 100644 --- a/client/finality-grandpa/src/lib.rs +++ b/client/finality-grandpa/src/lib.rs @@ -122,7 +122,7 @@ mod until_imported; mod voting_rule; pub use authorities::{SharedAuthoritySet, AuthoritySet}; -pub use finality_proof::{FinalityProofFragment, FinalityProofProvider, StorageAndProofProvider}; +pub use finality_proof::{FinalityProof, FinalityProofProvider, FinalityProofError}; pub use notification::{GrandpaJustificationSender, GrandpaJustificationStream}; pub use import::GrandpaBlockImport; pub use justification::GrandpaJustification; diff --git a/client/finality-grandpa/src/tests.rs b/client/finality-grandpa/src/tests.rs index 1ee71dddc4d4f..b94981838138a 100644 --- a/client/finality-grandpa/src/tests.rs +++ b/client/finality-grandpa/src/tests.rs @@ -32,25 +32,20 @@ use tokio::runtime::{Runtime, Handle}; use sp_keyring::Ed25519Keyring; use sc_client_api::backend::TransactionFor; use sp_blockchain::Result; -use sp_api::{ApiRef, StorageProof, ProvideRuntimeApi}; +use sp_api::{ApiRef, ProvideRuntimeApi}; use substrate_test_runtime_client::runtime::BlockNumber; use sp_consensus::{ BlockOrigin, ForkChoiceStrategy, ImportedAux, BlockImportParams, ImportResult, BlockImport, import_queue::BoxJustificationImport, }; use std::{collections::{HashMap, HashSet}, pin::Pin}; -use parity_scale_codec::Decode; -use sp_runtime::traits::{Block as BlockT, Header as HeaderT, HashFor}; +use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; use sp_runtime::generic::{BlockId, DigestItem}; use sp_core::H256; use sp_keystore::{SyncCryptoStorePtr, SyncCryptoStore}; use sp_finality_grandpa::{GRANDPA_ENGINE_ID, AuthorityList, EquivocationProof, GrandpaApi, OpaqueKeyOwnershipProof}; -use sp_state_machine::{InMemoryBackend, prove_read, read_proof_check}; use authorities::AuthoritySet; -use finality_proof::{ - AuthoritySetForFinalityProver, AuthoritySetForFinalityChecker, -}; use sc_block_builder::BlockBuilderProvider; use sc_consensus::LongestChain; use sc_keystore::LocalKeystore; @@ -207,43 +202,6 @@ impl GenesisAuthoritySetProvider for TestApi { } } -impl AuthoritySetForFinalityProver for TestApi { - fn authorities(&self, _block: &BlockId) -> Result { - Ok(self.genesis_authorities.clone()) - } - - fn prove_authorities(&self, block: &BlockId) -> Result { - let authorities = self.authorities(block)?; - let backend = >>::from(vec![ - (None, vec![(b"authorities".to_vec(), Some(authorities.encode()))]) - ]); - let proof = prove_read(backend, vec![b"authorities"]) - .expect("failure proving read from in-memory storage backend"); - Ok(proof) - } -} - -impl AuthoritySetForFinalityChecker for TestApi { - fn check_authorities_proof( - &self, - _hash: ::Hash, - header: ::Header, - proof: StorageProof, - ) -> Result { - let results = read_proof_check::, _>( - *header.state_root(), proof, vec![b"authorities"] - ) - .expect("failure checking read proof for authorities"); - let encoded = results.get(&b"authorities"[..]) - .expect("returned map must contain all proof keys") - .as_ref() - .expect("authorities in proof is None"); - let authorities = Decode::decode(&mut &encoded[..]) - .expect("failure decoding authorities read from proof"); - Ok(authorities) - } -} - const TEST_GOSSIP_DURATION: Duration = Duration::from_millis(500); fn make_ids(keys: &[Ed25519Keyring]) -> AuthorityList {