diff --git a/client/finality-grandpa-warp-sync/src/lib.rs b/client/finality-grandpa-warp-sync/src/lib.rs index 54d06650bc376..52e18e38909c4 100644 --- a/client/finality-grandpa-warp-sync/src/lib.rs +++ b/client/finality-grandpa-warp-sync/src/lib.rs @@ -120,14 +120,14 @@ impl> GrandpaWarpSyncRequestHandler, - pending_response: oneshot::Sender + pending_response: oneshot::Sender, ) -> Result<(), HandleRequestError> where NumberFor: sc_finality_grandpa::BlockNumberOps, { let request = Request::::decode(&mut &payload[..])?; let proof = WarpSyncProof::generate( - self.backend.blockchain(), + &*self.backend, request.begin, &self.authority_set.authority_set_changes(), )?; diff --git a/client/finality-grandpa-warp-sync/src/proof.rs b/client/finality-grandpa-warp-sync/src/proof.rs index 4677d2401e835..6b7002555d39a 100644 --- a/client/finality-grandpa-warp-sync/src/proof.rs +++ b/client/finality-grandpa-warp-sync/src/proof.rs @@ -16,14 +16,15 @@ use codec::{Decode, Encode}; +use sc_client_api::Backend as ClientBackend; use sc_finality_grandpa::{ find_scheduled_change, AuthoritySetChanges, BlockNumberOps, GrandpaJustification, }; -use sp_blockchain::Backend as BlockchainBackend; +use sp_blockchain::{Backend as BlockchainBackend, HeaderBackend}; use sp_finality_grandpa::{AuthorityList, SetId, GRANDPA_ENGINE_ID}; use sp_runtime::{ generic::BlockId, - traits::{Block as BlockT, NumberFor}, + traits::{Block as BlockT, NumberFor, One}, }; use crate::HandleRequestError; @@ -42,11 +43,21 @@ pub struct AuthoritySetChangeProof { pub justification: GrandpaJustification, } +/// Represents the current state of the warp sync, namely whether it is considered +/// finished, i.e. we have proved everything up until the latest authority set, or not. +/// When the warp sync is finished we might optionally provide a justification for the +/// latest finalized block, which should be checked against the latest authority set. +#[derive(Debug, Decode, Encode)] +pub enum WarpSyncFinished { + No, + Yes(Option>), +} + /// An accumulated proof of multiple authority set changes. #[derive(Decode, Encode)] pub struct WarpSyncProof { proofs: Vec>, - is_finished: bool, + is_finished: WarpSyncFinished, } impl WarpSyncProof { @@ -59,21 +70,22 @@ impl WarpSyncProof { set_changes: &AuthoritySetChanges>, ) -> Result, HandleRequestError> where - Backend: BlockchainBackend, + Backend: ClientBackend, { // TODO: cache best response (i.e. the one with lowest begin_number) + let blockchain = backend.blockchain(); - let begin_number = backend + let begin_number = blockchain .block_number_from_id(&BlockId::Hash(begin))? .ok_or_else(|| HandleRequestError::InvalidRequest("Missing start block".to_string()))?; - if begin_number > backend.info().finalized_number { + if begin_number > blockchain.info().finalized_number { return Err(HandleRequestError::InvalidRequest( "Start block is not finalized".to_string(), )); } - let canon_hash = backend.hash(begin_number)?.expect( + let canon_hash = blockchain.hash(begin_number)?.expect( "begin number is lower than finalized number; \ all blocks below finalized number must have been imported; \ qed.", @@ -86,7 +98,6 @@ impl WarpSyncProof { } let mut proofs = Vec::new(); - let mut proof_limit_reached = false; for (_, last_block) in set_changes.iter_from(begin_number) { @@ -95,7 +106,7 @@ impl WarpSyncProof { break; } - let header = backend.header(BlockId::Number(*last_block))?.expect( + let header = blockchain.header(BlockId::Number(*last_block))?.expect( "header number comes from previously applied set changes; must exist in db; qed.", ); @@ -108,7 +119,7 @@ impl WarpSyncProof { break; } - let justification = backend + let justification = blockchain .justifications(BlockId::Number(*last_block))? .and_then(|just| just.into_justification(GRANDPA_ENGINE_ID)) .expect( @@ -125,9 +136,29 @@ impl WarpSyncProof { }); } + let is_finished = if proof_limit_reached { + WarpSyncFinished::No + } else { + let latest = + sc_finality_grandpa::best_justification(backend)?.filter(|justification| { + // the existing best justification must be for a block higher than the + // last authority set change. if we didn't prove any authority set + // change then we fallback to make sure it's higher or equal to the + // initial warp sync block. + let limit = proofs + .last() + .map(|proof| proof.justification.target().0 + One::one()) + .unwrap_or(begin_number); + + justification.target().0 >= limit + }); + + WarpSyncFinished::Yes(latest) + }; + Ok(WarpSyncProof { proofs, - is_finished: !proof_limit_reached, + is_finished, }) } @@ -160,6 +191,12 @@ impl WarpSyncProof { current_set_id += 1; } + if let WarpSyncFinished::Yes(Some(ref justification)) = self.is_finished { + justification + .verify(current_set_id, ¤t_authorities) + .map_err(|err| HandleRequestError::InvalidProof(err.to_string()))?; + } + Ok((current_set_id, current_authorities)) } } @@ -170,7 +207,6 @@ mod tests { use codec::Encode; use rand::prelude::*; use sc_block_builder::BlockBuilderProvider; - use sc_client_api::Backend; use sc_finality_grandpa::{AuthoritySetChanges, GrandpaJustification}; use sp_blockchain::HeaderBackend; use sp_consensus::BlockOrigin; @@ -295,8 +331,7 @@ mod tests { let genesis_hash = client.hash(0).unwrap().unwrap(); let warp_sync_proof = - WarpSyncProof::generate(backend.blockchain(), genesis_hash, &authority_set_changes) - .unwrap(); + WarpSyncProof::generate(&*backend, genesis_hash, &authority_set_changes).unwrap(); // verifying the proof should yield the last set id and authorities let (new_set_id, new_authorities) = warp_sync_proof.verify(0, genesis_authorities).unwrap(); diff --git a/client/finality-grandpa/src/aux_schema.rs b/client/finality-grandpa/src/aux_schema.rs index 8ecfae40f68c7..296f7c13c5244 100644 --- a/client/finality-grandpa/src/aux_schema.rs +++ b/client/finality-grandpa/src/aux_schema.rs @@ -19,27 +19,30 @@ //! Schema for stuff in the aux-db. use std::fmt::Debug; -use parity_scale_codec::{Encode, Decode}; -use sc_client_api::backend::AuxStore; -use sp_blockchain::{Result as ClientResult, Error as ClientError}; -use fork_tree::ForkTree; + use finality_grandpa::round::State as RoundState; -use sp_runtime::traits::{Block as BlockT, NumberFor}; use log::{info, warn}; -use sp_finality_grandpa::{AuthorityList, SetId, RoundNumber}; +use parity_scale_codec::{Decode, Encode}; + +use fork_tree::ForkTree; +use sc_client_api::backend::AuxStore; +use sp_blockchain::{Error as ClientError, Result as ClientResult}; +use sp_finality_grandpa::{AuthorityList, RoundNumber, SetId}; +use sp_runtime::traits::{Block as BlockT, NumberFor}; use crate::authorities::{ - AuthoritySet, AuthoritySetChanges, SharedAuthoritySet, PendingChange, DelayKind, + AuthoritySet, AuthoritySetChanges, DelayKind, PendingChange, SharedAuthoritySet, }; use crate::environment::{ CompletedRound, CompletedRounds, CurrentRounds, HasVoted, SharedVoterSetState, VoterSetState, }; -use crate::NewAuthoritySet; +use crate::{GrandpaJustification, NewAuthoritySet}; const VERSION_KEY: &[u8] = b"grandpa_schema_version"; const SET_STATE_KEY: &[u8] = b"grandpa_completed_round"; const CONCLUDED_ROUNDS: &[u8] = b"grandpa_concluded_rounds"; const AUTHORITY_SET_KEY: &[u8] = b"grandpa_voters"; +const BEST_JUSTIFICATION: &[u8] = b"grandpa_best_justification"; const CURRENT_VERSION: u32 = 3; @@ -464,7 +467,7 @@ where pub(crate) fn update_authority_set( set: &AuthoritySet>, new_set: Option<&NewAuthoritySet>>, - write_aux: F + write_aux: F, ) -> R where F: FnOnce(&[(&'static [u8], &[u8])]) -> R, @@ -492,6 +495,33 @@ where } } +/// Update the justification for the latest finalized block on-disk. +/// +/// We always keep around the justification for the best finalized block and overwrite it +/// as we finalize new blocks, this makes sure that we don't store useless justifications +/// but can always prove finality of the latest block. +pub(crate) fn update_best_justification( + justification: &GrandpaJustification, + write_aux: F, +) -> R +where + F: FnOnce(&[(&'static [u8], &[u8])]) -> R, +{ + let encoded_justification = justification.encode(); + write_aux(&[(BEST_JUSTIFICATION, &encoded_justification[..])]) +} + +/// Fetch the justification for the latest block finalized by GRANDPA, if any. +pub fn best_justification( + backend: &B, +) -> ClientResult>> +where + B: AuxStore, + Block: BlockT, +{ + load_decode::<_, GrandpaJustification>(backend, BEST_JUSTIFICATION) +} + /// Write voter set state. pub(crate) fn write_voter_set_state( backend: &B, @@ -517,10 +547,9 @@ pub(crate) fn write_concluded_round( #[cfg(test)] pub(crate) fn load_authorities( - backend: &B + backend: &B, ) -> Option> { - load_decode::<_, AuthoritySet>(backend, AUTHORITY_SET_KEY) - .expect("backend error") + load_decode::<_, AuthoritySet>(backend, AUTHORITY_SET_KEY).expect("backend error") } #[cfg(test)] diff --git a/client/finality-grandpa/src/environment.rs b/client/finality-grandpa/src/environment.rs index 3786355d2db4e..d3a5b49b50726 100644 --- a/client/finality-grandpa/src/environment.rs +++ b/client/finality-grandpa/src/environment.rs @@ -1275,11 +1275,8 @@ where // `N+1`. this assumption is required to make sure we store // justifications for transition blocks which will be requested by // syncing clients. - let justification = match justification_or_commit { - JustificationOrCommit::Justification(justification) => { - notify_justification(justification_sender, || Ok(justification.clone())); - Some(justification.encode()) - }, + let (justification_required, justification) = match justification_or_commit { + JustificationOrCommit::Justification(justification) => (true, justification), JustificationOrCommit::Commit((round_number, commit)) => { let mut justification_required = // justification is always required when block that enacts new authorities @@ -1297,42 +1294,35 @@ where } } - // NOTE: the code below is a bit more verbose because we - // really want to avoid creating a justification if it isn't - // needed (e.g. if there's no subscribers), and also to avoid - // creating it twice. depending on the vote tree for the round, - // creating a justification might require multiple fetches of - // headers from the database. - let justification = || GrandpaJustification::from_commit( + let justification = GrandpaJustification::from_commit( &client, round_number, commit, - ); - - if justification_required { - let justification = justification()?; - notify_justification(justification_sender, || Ok(justification.clone())); - - Some(justification.encode()) - } else { - notify_justification(justification_sender, justification); + )?; - None - } + (justification_required, justification) }, }; - debug!(target: "afg", "Finalizing blocks up to ({:?}, {})", number, hash); + notify_justification(justification_sender, || Ok(justification.clone())); + + let persisted_justification = if justification_required { + Some((GRANDPA_ENGINE_ID, justification.encode())) + } else { + None + }; // ideally some handle to a synchronization oracle would be used // to avoid unconditionally notifying. - let justification = justification.map(|j| (GRANDPA_ENGINE_ID, j.clone())); client - .apply_finality(import_op, BlockId::Hash(hash), justification, true) + .apply_finality(import_op, BlockId::Hash(hash), persisted_justification, true) .map_err(|e| { warn!(target: "afg", "Error applying finality to block {:?}: {:?}", (hash, number), e); e })?; + + debug!(target: "afg", "Finalizing blocks up to ({:?}, {})", number, hash); + telemetry!( telemetry; CONSENSUS_INFO; @@ -1340,6 +1330,11 @@ where "number" => ?number, "hash" => ?hash, ); + crate::aux_schema::update_best_justification( + &justification, + |insert| apply_aux(import_op, insert, &[]), + )?; + let new_authorities = if let Some((canon_hash, canon_number)) = status.new_set_block { // the authority set has changed. let (new_id, set_ref) = authority_set.current(); diff --git a/client/finality-grandpa/src/justification.rs b/client/finality-grandpa/src/justification.rs index 69ca70386007d..7805161f06c62 100644 --- a/client/finality-grandpa/src/justification.rs +++ b/client/finality-grandpa/src/justification.rs @@ -195,6 +195,11 @@ impl GrandpaJustification { Ok(()) } + + /// The target block number and hash that this justifications proves finality for. + pub fn target(&self) -> (NumberFor, Block::Hash) { + (self.commit.target_number, self.commit.target_hash) + } } /// A utility trait implementing `finality_grandpa::Chain` using a given set of headers. diff --git a/client/finality-grandpa/src/lib.rs b/client/finality-grandpa/src/lib.rs index 9a8939660473b..fb9ecaa2c1370 100644 --- a/client/finality-grandpa/src/lib.rs +++ b/client/finality-grandpa/src/lib.rs @@ -122,6 +122,7 @@ mod until_imported; mod voting_rule; pub use authorities::{AuthoritySet, AuthoritySetChanges, SharedAuthoritySet}; +pub use aux_schema::best_justification; pub use finality_proof::{FinalityProof, FinalityProofProvider, FinalityProofError}; pub use notification::{GrandpaJustificationSender, GrandpaJustificationStream}; pub use import::{find_scheduled_change, find_forced_change, GrandpaBlockImport}; diff --git a/client/finality-grandpa/src/tests.rs b/client/finality-grandpa/src/tests.rs index b87bbefc11375..fa4bd028bfe25 100644 --- a/client/finality-grandpa/src/tests.rs +++ b/client/finality-grandpa/src/tests.rs @@ -451,10 +451,19 @@ fn finalize_3_voters_1_full_observer() { } // wait for all finalized on each. - let wait_for = futures::future::join_all(finality_notifications) - .map(|_| ()); + let wait_for = futures::future::join_all(finality_notifications).map(|_| ()); block_until_complete(wait_for, &net, &mut runtime); + + // all peers should have stored the justification for the best finalized block #20 + for peer_id in 0..4 { + let client = net.lock().peers[peer_id].client().as_full().unwrap(); + let justification = crate::aux_schema::best_justification::<_, Block>(&*client) + .unwrap() + .unwrap(); + + assert_eq!(justification.commit.target_number, 20); + } } #[test]