From 4a2fcdbf6b9bea9aa46c693826c7a54c700e2f25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Mon, 15 Feb 2021 13:04:59 +0000 Subject: [PATCH 01/13] grandpa: use AuthoritySetChanges to generate warp sync proof --- client/finality-grandpa-warp-sync/src/lib.rs | 34 ++++- client/finality-grandpa/src/finality_proof.rs | 124 +++--------------- 2 files changed, 44 insertions(+), 114 deletions(-) diff --git a/client/finality-grandpa-warp-sync/src/lib.rs b/client/finality-grandpa-warp-sync/src/lib.rs index f7ce59b1c168f..48af5700da8ad 100644 --- a/client/finality-grandpa-warp-sync/src/lib.rs +++ b/client/finality-grandpa-warp-sync/src/lib.rs @@ -28,13 +28,14 @@ use sp_runtime::traits::Block as BlockT; use std::time::Duration; use std::sync::Arc; use sc_service::{SpawnTaskHandle, config::{Configuration, Role}}; -use sc_finality_grandpa::WarpSyncFragmentCache; +use sc_finality_grandpa::{SharedAuthoritySet, WarpSyncFragmentCache}; /// Generates the appropriate [`RequestResponseConfig`] for a given chain configuration. pub fn request_response_config_for_chain + 'static>( config: &Configuration, spawn_handle: SpawnTaskHandle, backend: Arc, + authority_set: SharedAuthoritySet>, ) -> RequestResponseConfig where NumberFor: sc_finality_grandpa::BlockNumberOps, { @@ -48,6 +49,7 @@ pub fn request_response_config_for_chain { backend: Arc, + authority_set: SharedAuthoritySet>, cache: Arc>>, request_receiver: mpsc::Receiver, - _phantom: std::marker::PhantomData + _phantom: std::marker::PhantomData, } impl> GrandpaWarpSyncRequestHandler { /// Create a new [`GrandpaWarpSyncRequestHandler`]. - pub fn new(protocol_id: ProtocolId, backend: Arc) -> (Self, RequestResponseConfig) { + pub fn new( + protocol_id: ProtocolId, + backend: Arc, + authority_set: SharedAuthoritySet>, + ) -> (Self, RequestResponseConfig) { let (tx, request_receiver) = mpsc::channel(20); let mut request_response_config = generate_request_response_config(protocol_id); request_response_config.inbound_queue = Some(tx); - let cache = Arc::new(parking_lot::RwLock::new(WarpSyncFragmentCache::new(WARP_SYNC_CACHE_SIZE))); - - (Self { backend, request_receiver, cache, _phantom: std::marker::PhantomData }, request_response_config) + let cache = Arc::new(parking_lot::RwLock::new(WarpSyncFragmentCache::new( + WARP_SYNC_CACHE_SIZE, + ))); + + ( + Self { + backend, + request_receiver, + cache, + _phantom: std::marker::PhantomData, + authority_set, + }, + request_response_config, + ) } fn handle_request( @@ -121,7 +139,9 @@ impl> GrandpaWarpSyncRequestHandler>( - blockchain: &B, + backend: &B, begin: Block::Hash, - max_fragment_limit: Option, - mut cache: Option<&mut WarpSyncFragmentCache>, + set_changes: &AuthoritySetChanges>, ) -> ::sp_blockchain::Result> { - let begin = BlockId::Hash(begin); - let begin_number = blockchain.block_number_from_id(&begin)? + let begin_number = backend + .block_number_from_id(&begin)? .ok_or_else(|| ClientError::Backend("Missing start block".to_string()))?; - let end = BlockId::Hash(blockchain.last_finalized()?); - let end_number = blockchain.block_number_from_id(&end)? - // This error should not happen, we could also panic. - .ok_or_else(|| ClientError::Backend("Missing last finalized block".to_string()))?; - - if begin_number > end_number { - return Err(ClientError::Backend("Unfinalized start for authority proof".to_string())); - } let mut result = Vec::new(); - let mut last_apply = None; - - let header = blockchain.expect_header(begin)?; - let mut index = *header.number(); - - // Find previous change in case there is a delay. - // This operation is a costy and only for the delay corner case. - while index > Zero::zero() { - index = index - One::one(); - if let Some((fragment, apply_block)) = get_warp_sync_proof_fragment(blockchain, index, &mut cache)? { - if last_apply.map(|next| &next > header.number()).unwrap_or(false) { - result.push(fragment); - last_apply = Some(apply_block); - } else { - break; - } - } - } - let mut index = *header.number(); - while index <= end_number { - if max_fragment_limit.map(|limit| result.len() >= limit).unwrap_or(false) { - break; + for (_, last_block) in &set_changes.0 { + if *last_block < begin_number { + continue; } - if let Some((fragement, apply_block)) = get_warp_sync_proof_fragment(blockchain, index, &mut cache)? { - if last_apply.map(|next| apply_block < next).unwrap_or(false) { - // Previous delayed will not apply, do not include it. - result.pop(); - } - result.push(fragement); - last_apply = Some(apply_block); - } - - index = index + One::one(); - } + let header = backend.header(BlockId::Number(*last_block))?.expect( + "header number comes from previously applied set changes; must exist in db; qed.", + ); - let at_limit = max_fragment_limit.map(|limit| result.len() >= limit).unwrap_or(false); + let justification = backend + .justification(BlockId::Number(*last_block))? + .expect("header is last in set; must have justification; qed."); - // add last finalized block if reached and not already included. - if !at_limit && result.last().as_ref().map(|head| head.header.number()) != Some(&end_number) { - let header = blockchain.expect_header(end)?; - if let Some(justification) = blockchain.justification(BlockId::Number(end_number.clone()))? { - result.push(AuthoritySetProofFragment { - header: header.clone(), - justification, - }); - } else { - // no justification, don't include it. - } + result.push(AuthoritySetProofFragment { + header: header.clone(), + justification, + }); } Ok(result.encode()) } -/// Try get a warp sync proof fragment a a given finalized block. -fn get_warp_sync_proof_fragment>( - blockchain: &B, - index: NumberFor, - cache: &mut Option<&mut WarpSyncFragmentCache>, -) -> sp_blockchain::Result, NumberFor)>> { - if let Some(cache) = cache.as_mut() { - if let Some(result) = cache.get_item(index) { - return Ok(result); - } - } - - let mut result = None; - let header = blockchain.expect_header(BlockId::number(index))?; - - if let Some((block_number, sp_finality_grandpa::ScheduledChange { - next_authorities: _, - delay, - })) = crate::import::find_forced_change::(&header) { - let dest = block_number + delay; - if let Some(justification) = blockchain.justification(BlockId::Number(index.clone()))? { - result = Some((AuthoritySetProofFragment { - header: header.clone(), - justification, - }, dest)); - } else { - return Err(ClientError::Backend("Unjustified block with authority set change".to_string())); - } - } - - if let Some(sp_finality_grandpa::ScheduledChange { - next_authorities: _, - delay, - }) = crate::import::find_scheduled_change::(&header) { - let dest = index + delay; - if let Some(justification) = blockchain.justification(BlockId::Number(index.clone()))? { - result = Some((AuthoritySetProofFragment { - header: header.clone(), - justification, - }, dest)); - } else { - return Err(ClientError::Backend("Unjustified block with authority set change".to_string())); - } - } - - cache.as_mut().map(|cache| cache.new_item(index, result.clone())); - Ok(result) -} - /// Check GRANDPA authority change sequence to assert finality of a target block. /// /// Returns the header of the target block. From ab1d59321d855d17937a2a1d00e1e851c80b13fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Mon, 15 Feb 2021 15:07:12 +0000 Subject: [PATCH 02/13] node: init grandpa warp sync protocol --- bin/node/cli/src/service.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index ca647c5834469..166c2a71e68f3 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -197,9 +197,14 @@ pub fn new_full_base( config.network.extra_sets.push(grandpa::grandpa_peers_set_config()); #[cfg(feature = "cli")] - config.network.request_response_protocols.push(sc_finality_grandpa_warp_sync::request_response_config_for_chain( - &config, task_manager.spawn_handle(), backend.clone(), - )); + config.network.request_response_protocols.push( + sc_finality_grandpa_warp_sync::request_response_config_for_chain( + &config, + task_manager.spawn_handle(), + backend.clone(), + import_setup.1.shared_authority_set().clone(), + ) + ); let (network, network_status_sinks, system_rpc_tx, network_starter) = sc_service::build_network(sc_service::BuildNetworkParams { From c17665540dcffaeca152bb20f4512befeddbaefb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Thu, 18 Feb 2021 09:46:21 +0000 Subject: [PATCH 03/13] grandpa: iterator for AuthoritySetChanges --- client/finality-grandpa/src/authorities.rs | 13 ++++++++++--- client/finality-grandpa/src/finality_proof.rs | 2 +- client/finality-grandpa/src/lib.rs | 2 +- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/client/finality-grandpa/src/authorities.rs b/client/finality-grandpa/src/authorities.rs index 067f6dfc1ae65..07ade2e32cf7c 100644 --- a/client/finality-grandpa/src/authorities.rs +++ b/client/finality-grandpa/src/authorities.rs @@ -650,10 +650,11 @@ 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. +/// Tracks historical authority set changes. We store the block numbers for the last block +/// of each authority set, once they have been finalized. These blocks are guaranteed to +/// have a justification unless they were triggered by a forced change. #[derive(Debug, Encode, Decode, Clone, PartialEq)] -pub struct AuthoritySetChanges(pub Vec<(u64, N)>); +pub struct AuthoritySetChanges(Vec<(u64, N)>); impl AuthoritySetChanges { pub(crate) fn empty() -> Self { @@ -687,6 +688,12 @@ impl AuthoritySetChanges { None } } + + /// Returns an iterator over all historical authority set changes, the iterator yields + /// a tuple representing the set id and the block number of last block in that set. + pub fn iter(&self) -> impl Iterator { + self.0.iter() + } } #[cfg(test)] diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index 9c02191bdaacb..82fc7174d4fbb 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -260,7 +260,7 @@ pub fn prove_warp_sync>( let mut result = Vec::new(); - for (_, last_block) in &set_changes.0 { + for (_, last_block) in set_changes.iter() { if *last_block < begin_number { continue; } diff --git a/client/finality-grandpa/src/lib.rs b/client/finality-grandpa/src/lib.rs index 75500a894d745..ad591407f5b29 100644 --- a/client/finality-grandpa/src/lib.rs +++ b/client/finality-grandpa/src/lib.rs @@ -121,7 +121,7 @@ mod observer; mod until_imported; mod voting_rule; -pub use authorities::{SharedAuthoritySet, AuthoritySet}; +pub use authorities::{AuthoritySet, AuthoritySetChanges, SharedAuthoritySet}; pub use finality_proof::{FinalityProof, FinalityProofProvider, FinalityProofError}; pub use notification::{GrandpaJustificationSender, GrandpaJustificationStream}; pub use import::GrandpaBlockImport; From 0521961b15a0cab2d6ff77a1bccee8bc4cf2425b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Thu, 18 Feb 2021 10:38:20 +0000 Subject: [PATCH 04/13] grandpa: rewrite warp sync proof generation --- client/finality-grandpa-warp-sync/src/lib.rs | 18 +-- .../finality-grandpa-warp-sync/src/proof.rs | 118 ++++++++++++++++++ client/finality-grandpa/src/import.rs | 16 ++- client/finality-grandpa/src/lib.rs | 2 +- 4 files changed, 140 insertions(+), 14 deletions(-) create mode 100644 client/finality-grandpa-warp-sync/src/proof.rs diff --git a/client/finality-grandpa-warp-sync/src/lib.rs b/client/finality-grandpa-warp-sync/src/lib.rs index 48af5700da8ad..c6eb698c6a070 100644 --- a/client/finality-grandpa-warp-sync/src/lib.rs +++ b/client/finality-grandpa-warp-sync/src/lib.rs @@ -17,7 +17,7 @@ //! Helper for handling (i.e. answering) grandpa warp sync requests from a remote peer via the //! [`crate::request_responses::RequestResponsesBehaviour`]. -use codec::Decode; +use codec::{Decode, Encode}; use sc_network::config::{IncomingRequest, OutgoingResponse, ProtocolId, RequestResponseConfig}; use sc_client_api::Backend; use sp_runtime::traits::NumberFor; @@ -30,6 +30,10 @@ use std::sync::Arc; use sc_service::{SpawnTaskHandle, config::{Configuration, Role}}; use sc_finality_grandpa::{SharedAuthoritySet, WarpSyncFragmentCache}; +mod proof; + +pub use proof::{AuthoritySetChangeProof, WarpSyncProof}; + /// Generates the appropriate [`RequestResponseConfig`] for a given chain configuration. pub fn request_response_config_for_chain + 'static>( config: &Configuration, @@ -78,9 +82,9 @@ fn generate_protocol_name(protocol_id: ProtocolId) -> String { s } -#[derive(codec::Decode)] +#[derive(Decode)] struct Request { - begin: B::Hash + begin: B::Hash, } /// Setting a large fragment limit, allowing client @@ -137,15 +141,14 @@ impl> GrandpaWarpSyncRequestHandler::decode(&mut &payload[..])?; - let mut cache = self.cache.write(); - let response = sc_finality_grandpa::prove_warp_sync( + let proof = WarpSyncProof::generate( self.backend.blockchain(), request.begin, &self.authority_set.authority_set_changes(), )?; pending_response.send(OutgoingResponse { - result: Ok(response), + result: Ok(proof.encode()), reputation_changes: Vec::new(), }).map_err(|_| HandleRequestError::SendResponse) } @@ -170,7 +173,7 @@ impl> GrandpaWarpSyncRequestHandler. + +use codec::{Decode, Encode}; + +use sc_finality_grandpa::{AuthoritySetChanges, GrandpaJustification}; +use sp_blockchain::Backend as BlockchainBackend; +use sp_runtime::{ + generic::BlockId, + traits::{Block as BlockT, NumberFor}, +}; + +use crate::HandleRequestError; + +/// The maximum number of authority set change proofs to include in a single warp sync proof. +const MAX_CHANGES_PER_WARP_SYNC_PROOF: usize = 256; + +#[derive(Decode, Encode)] +pub struct AuthoritySetChangeProof { + /// must contain the authorities for the following set + pub header: Block::Header, + /// this justification proves finality of the header above + /// it must be validated against the authorities of the previous set. + pub justification: GrandpaJustification, +} + +#[derive(Decode, Encode)] +pub struct WarpSyncProof { + proofs: Vec>, +} + +impl WarpSyncProof { + pub fn generate( + backend: &Backend, + begin: Block::Hash, + set_changes: &AuthoritySetChanges>, + ) -> Result, HandleRequestError> + where + Backend: BlockchainBackend, + { + // TODO: cache best response (i.e. the one with lowest begin_number) + + let begin_number = backend + .block_number_from_id(&BlockId::Hash(begin))? + .ok_or_else(|| HandleRequestError::InvalidRequest("Missing start block".to_string()))?; + + if begin_number > backend.info().finalized_number { + return Err(HandleRequestError::InvalidRequest( + "Start block is not finalized".to_string(), + )); + } + + let canon_hash = backend.hash(begin_number)?.expect( + "begin number is lower than finalized number; \ + all blocks below finalized number must have been imported; \ + qed.", + ); + + if canon_hash != begin { + return Err(HandleRequestError::InvalidRequest( + "Start block is not in the finalized chain".to_string(), + )); + } + + let mut proofs = Vec::new(); + + for (_, last_block) in set_changes.iter() { + if proofs.len() >= MAX_CHANGES_PER_WARP_SYNC_PROOF { + break; + } + + if *last_block < begin_number { + continue; + } + + let header = backend.header(BlockId::Number(*last_block))?.expect( + "header number comes from previously applied set changes; must exist in db; qed.", + ); + + // the last block in a set is the one that triggers a change to the next set, + // therefore the block must have a digest that signals the authority set change + if sc_finality_grandpa::find_scheduled_change::(&header).is_none() { + // if it doesn't contain a signal for standard change then the set must have changed + // through a forced changed, in which case we stop collecting proofs as the chain of + // trust in authority handoffs was broken. + break; + } + + let justification = backend.justification(BlockId::Number(*last_block))?.expect( + "header is last in set and contains standard change signal; \ + must have justification; \ + qed.", + ); + + let justification = GrandpaJustification::::decode(&mut &justification[..])?; + + proofs.push(AuthoritySetChangeProof { + header: header.clone(), + justification, + }); + } + + Ok(WarpSyncProof { proofs }) + } +} diff --git a/client/finality-grandpa/src/import.rs b/client/finality-grandpa/src/import.rs index d7b83b8032909..2c86319dd54af 100644 --- a/client/finality-grandpa/src/import.rs +++ b/client/finality-grandpa/src/import.rs @@ -182,9 +182,11 @@ impl<'a, Block: 'a + BlockT> Drop for PendingSetChanges<'a, Block> { } } -pub(crate) fn find_scheduled_change(header: &B::Header) - -> Option>> -{ +/// Checks the given header for a consensus digest signalling a **standard** scheduled change and +/// extracts it. +pub fn find_scheduled_change( + header: &B::Header, +) -> Option>> { let id = OpaqueDigestItemId::Consensus(&GRANDPA_ENGINE_ID); let filter_log = |log: ConsensusLog>| match log { @@ -197,9 +199,11 @@ pub(crate) fn find_scheduled_change(header: &B::Header) header.digest().convert_first(|l| l.try_to(id).and_then(filter_log)) } -pub(crate) fn find_forced_change(header: &B::Header) - -> Option<(NumberFor, ScheduledChange>)> -{ +/// Checks the given header for a consensus digest signalling a **forced** scheduled change and +/// extracts it. +pub fn find_forced_change( + header: &B::Header, +) -> Option<(NumberFor, ScheduledChange>)> { let id = OpaqueDigestItemId::Consensus(&GRANDPA_ENGINE_ID); let filter_log = |log: ConsensusLog>| match log { diff --git a/client/finality-grandpa/src/lib.rs b/client/finality-grandpa/src/lib.rs index ad591407f5b29..51df586e6f599 100644 --- a/client/finality-grandpa/src/lib.rs +++ b/client/finality-grandpa/src/lib.rs @@ -124,7 +124,7 @@ mod voting_rule; pub use authorities::{AuthoritySet, AuthoritySetChanges, SharedAuthoritySet}; pub use finality_proof::{FinalityProof, FinalityProofProvider, FinalityProofError}; pub use notification::{GrandpaJustificationSender, GrandpaJustificationStream}; -pub use import::GrandpaBlockImport; +pub use import::{find_scheduled_change, find_forced_change, GrandpaBlockImport}; pub use justification::GrandpaJustification; pub use voting_rule::{ BeforeBestBlockBy, ThreeQuartersOfTheUnfinalizedChain, VotingRule, VotingRulesBuilder From 4637d8414f0a4bffa3bd87044e91d5d69439781a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Thu, 18 Feb 2021 10:44:29 +0000 Subject: [PATCH 05/13] grandpa: remove old code for warp sync generation --- client/finality-grandpa-warp-sync/src/lib.rs | 16 +- client/finality-grandpa/src/finality_proof.rs | 394 +----------------- client/finality-grandpa/src/lib.rs | 1 - 3 files changed, 4 insertions(+), 407 deletions(-) diff --git a/client/finality-grandpa-warp-sync/src/lib.rs b/client/finality-grandpa-warp-sync/src/lib.rs index c6eb698c6a070..3e9b4e8964c9a 100644 --- a/client/finality-grandpa-warp-sync/src/lib.rs +++ b/client/finality-grandpa-warp-sync/src/lib.rs @@ -28,7 +28,7 @@ use sp_runtime::traits::Block as BlockT; use std::time::Duration; use std::sync::Arc; use sc_service::{SpawnTaskHandle, config::{Configuration, Role}}; -use sc_finality_grandpa::{SharedAuthoritySet, WarpSyncFragmentCache}; +use sc_finality_grandpa::SharedAuthoritySet; mod proof; @@ -87,20 +87,10 @@ struct Request { begin: B::Hash, } -/// Setting a large fragment limit, allowing client -/// to define it is possible. -const WARP_SYNC_FRAGMENTS_LIMIT: usize = 100; - -/// Number of item with justification in warp sync cache. -/// This should be customizable, but setting it to the max number of fragments -/// we return seems like a good idea until then. -const WARP_SYNC_CACHE_SIZE: usize = WARP_SYNC_FRAGMENTS_LIMIT; - /// Handler for incoming grandpa warp sync requests from a remote peer. pub struct GrandpaWarpSyncRequestHandler { backend: Arc, authority_set: SharedAuthoritySet>, - cache: Arc>>, request_receiver: mpsc::Receiver, _phantom: std::marker::PhantomData, } @@ -116,15 +106,11 @@ impl> GrandpaWarpSyncRequestHandler { - /// The header of the given block. - pub header: Header, - /// Justification of the block F. - pub justification: Vec, -} - -/// Proof of authority set is the ordered set of authority set fragments, where: -/// - last fragment match target block. -type AuthoritySetProof
= Vec>; - fn prove_finality( blockchain: &B, authority_set_changes: AuthoritySetChanges>, @@ -242,148 +225,6 @@ where )) } -/// Prepare authority proof for the best possible block starting at a given trusted block. -/// -/// Started block should be in range of bonding duration. -/// We only return proof for finalized blocks (with justification). -/// -/// It is assumed that the caller already have a proof-of-finality for the block 'begin'. -pub fn prove_warp_sync>( - backend: &B, - begin: Block::Hash, - set_changes: &AuthoritySetChanges>, -) -> ::sp_blockchain::Result> { - let begin = BlockId::Hash(begin); - let begin_number = backend - .block_number_from_id(&begin)? - .ok_or_else(|| ClientError::Backend("Missing start block".to_string()))?; - - let mut result = Vec::new(); - - for (_, last_block) in set_changes.iter() { - if *last_block < begin_number { - continue; - } - - let header = backend.header(BlockId::Number(*last_block))?.expect( - "header number comes from previously applied set changes; must exist in db; qed.", - ); - - let justification = backend - .justification(BlockId::Number(*last_block))? - .expect("header is last in set; must have justification; qed."); - - result.push(AuthoritySetProofFragment { - header: header.clone(), - justification, - }); - } - - Ok(result.encode()) -} - -/// 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, - remote_proof: Vec, -) -> ClientResult<(Block::Header, u64, AuthorityList)> -where - NumberFor: BlockNumberOps, - J: Decode + ProvableJustification + BlockJustification, -{ - // decode finality proof - let proof = AuthoritySetProof::::decode(&mut &remote_proof[..]) - .map_err(|_| ClientError::BadJustification("failed to decode authority proof".into()))?; - - let last = proof.len() - 1; - - let mut result = (current_set_id, current_authorities, NumberFor::::zero()); - - for (ix, fragment) in proof.into_iter().enumerate() { - let is_last = ix == last; - result = check_warp_sync_proof_fragment::( - result.0, - &result.1, - &result.2, - is_last, - &fragment, - )?; - - if is_last { - return Ok((fragment.header, result.0, result.1)) - } - } - - // empty proof can't prove anything - return Err(ClientError::BadJustification("empty proof of authority".into())); -} - -/// Check finality authority set sequence. -fn check_warp_sync_proof_fragment( - current_set_id: u64, - current_authorities: &AuthorityList, - previous_checked_block: &NumberFor, - is_last: bool, - authorities_proof: &AuthoritySetProofFragment, -) -> ClientResult<(u64, AuthorityList, NumberFor)> -where - NumberFor: BlockNumberOps, - J: Decode + ProvableJustification + BlockJustification, -{ - let justification: J = Decode::decode(&mut authorities_proof.justification.as_slice()) - .map_err(|_| ClientError::JustificationDecode)?; - justification.verify(current_set_id, ¤t_authorities)?; - - // assert justification is for this header - if &justification.number() != authorities_proof.header.number() - || justification.hash().as_ref() != authorities_proof.header.hash().as_ref() { - return Err(ClientError::Backend("Invalid authority warp proof, justification do not match header".to_string())); - } - - if authorities_proof.header.number() <= previous_checked_block { - return Err(ClientError::Backend("Invalid authority warp proof".to_string())); - } - let current_block = authorities_proof.header.number(); - let mut at_block = None; - if let Some(sp_finality_grandpa::ScheduledChange { - next_authorities, - delay, - }) = crate::import::find_scheduled_change::(&authorities_proof.header) { - let dest = *current_block + delay; - at_block = Some((dest, next_authorities)); - } - if let Some((block_number, sp_finality_grandpa::ScheduledChange { - next_authorities, - delay, - })) = crate::import::find_forced_change::(&authorities_proof.header) { - let dest = block_number + delay; - at_block = Some((dest, next_authorities)); - } - - // Fragment without change only allowed for proof last block. - if at_block.is_none() && !is_last { - return Err(ClientError::Backend("Invalid authority warp proof".to_string())); - } - if let Some((at_block, next_authorities)) = at_block { - Ok((current_set_id + 1, next_authorities, at_block)) - } else { - Ok((current_set_id, current_authorities.clone(), current_block.clone())) - } -} - -/// Block info extracted from the justification. -pub(crate) trait BlockJustification { - /// Block number justified. - fn number(&self) -> Header::Number; - - /// Block hash justified. - fn hash(&self) -> Header::Hash; -} - /// Check GRANDPA proof-of-finality for the given block. /// /// Returns the vector of headers that MUST be validated + imported @@ -393,7 +234,7 @@ pub(crate) trait BlockJustification { #[cfg(test)] fn check_finality_proof( current_set_id: u64, - current_authorities: AuthorityList, + current_authorities: sp_finality_grandpa::AuthorityList, remote_proof: Vec, ) -> ClientResult> where @@ -443,69 +284,6 @@ where } } -impl BlockJustification for GrandpaJustification { - fn number(&self) -> NumberFor { - self.commit.target_number.clone() - } - fn hash(&self) -> Block::Hash { - self.commit.target_hash.clone() - } -} - -/// Simple cache for warp sync queries. -pub struct WarpSyncFragmentCache { - header_has_proof_fragment: std::collections::HashMap, - cache: linked_hash_map::LinkedHashMap< - Header::Number, - (AuthoritySetProofFragment
, Header::Number), - >, - limit: usize, -} - -impl WarpSyncFragmentCache
{ - /// Instantiate a new cache for the warp sync prover. - pub fn new(size: usize) -> Self { - WarpSyncFragmentCache { - header_has_proof_fragment: Default::default(), - cache: Default::default(), - limit: size, - } - } - - fn new_item( - &mut self, - at: Header::Number, - item: Option<(AuthoritySetProofFragment
, Header::Number)>, - ) { - self.header_has_proof_fragment.insert(at, item.is_some()); - - if let Some(item) = item { - if self.cache.len() == self.limit { - self.pop_one(); - } - - self.cache.insert(at, item); - } - } - - fn pop_one(&mut self) { - if let Some((header_number, _)) = self.cache.pop_front() { - self.header_has_proof_fragment.remove(&header_number); - } - } - - fn get_item( - &mut self, - block: Header::Number, - ) -> Option, Header::Number)>> { - match self.header_has_proof_fragment.get(&block) { - Some(true) => Some(self.cache.get_refresh(&block).cloned()), - Some(false) => Some(None), - None => None - } - } -} - #[cfg(test)] pub(crate) mod tests { use super::*; @@ -534,15 +312,6 @@ pub(crate) mod tests { #[derive(Debug, PartialEq, Encode, Decode)] pub struct TestBlockJustification(TestJustification, u64, H256); - impl BlockJustification
for TestBlockJustification { - fn number(&self) ->
::Number { - self.1 - } - fn hash(&self) ->
::Hash { - self.2.clone() - } - } - impl ProvableJustification
for TestBlockJustification { fn verify(&self, set_id: u64, authorities: &[(AuthorityId, u64)]) -> ClientResult<()> { self.0.verify(set_id, authorities) @@ -736,161 +505,4 @@ pub(crate) mod tests { } ); } - - #[test] - fn warp_sync_proof_encoding_decoding() { - fn test_blockchain( - nb_blocks: u64, - mut set_change: &[(u64, Vec)], - mut justifications: &[(u64, Vec)], - ) -> (InMemoryBlockchain, Vec) { - let blockchain = InMemoryBlockchain::::new(); - let mut hashes = Vec::::new(); - let mut set_id = 0; - for i in 0..nb_blocks { - let mut set_id_next = set_id; - let mut header = header(i); - set_change.first() - .map(|j| if i == j.0 { - set_change = &set_change[1..]; - let next_authorities: Vec<_> = j.1.iter().map(|i| (AuthorityId::from_slice(&[*i; 32]), 1u64)).collect(); - set_id_next += 1; - header.digest_mut().logs.push( - sp_runtime::generic::DigestItem::Consensus( - sp_finality_grandpa::GRANDPA_ENGINE_ID, - sp_finality_grandpa::ConsensusLog::ScheduledChange( - sp_finality_grandpa::ScheduledChange { delay: 0u64, next_authorities } - ).encode(), - )); - }); - - if let Some(parent) = hashes.last() { - header.set_parent_hash(parent.clone()); - } - let header_hash = header.hash(); - - let justification = justifications.first() - .and_then(|j| if i == j.0 { - justifications = &justifications[1..]; - - let authority = j.1.iter().map(|j| - (AuthorityId::from_slice(&[*j; 32]), 1u64) - ).collect(); - let justification = TestBlockJustification( - TestJustification((set_id, authority), vec![i as u8]), - i, - header_hash, - ); - Some(justification.encode()) - } else { - None - }); - hashes.push(header_hash.clone()); - set_id = set_id_next; - - blockchain.insert(header_hash, header, justification, None, NewBlockState::Final) - .unwrap(); - } - (blockchain, hashes) - } - - let (blockchain, hashes) = test_blockchain( - 7, - vec![(3, vec![9])].as_slice(), - vec![ - (1, vec![1, 2, 3]), - (2, vec![1, 2, 3]), - (3, vec![1, 2, 3]), - (4, vec![9]), - (6, vec![9]), - ].as_slice(), - ); - - // proof after set change - let mut cache = WarpSyncFragmentCache::new(5); - let proof_no_cache = prove_warp_sync(&blockchain, hashes[6], None, Some(&mut cache)).unwrap(); - let proof = prove_warp_sync(&blockchain, hashes[6], None, Some(&mut cache)).unwrap(); - assert_eq!(proof_no_cache, proof); - - let initial_authorities: Vec<_> = [1u8, 2, 3].iter().map(|i| - (AuthorityId::from_slice(&[*i; 32]), 1u64) - ).collect(); - - let authorities_next: Vec<_> = [9u8].iter().map(|i| - (AuthorityId::from_slice(&[*i; 32]), 1u64) - ).collect(); - - assert!(check_warp_sync_proof::( - 0, - initial_authorities.clone(), - proof.clone(), - ).is_err()); - assert!(check_warp_sync_proof::( - 0, - authorities_next.clone(), - proof.clone(), - ).is_err()); - assert!(check_warp_sync_proof::( - 1, - initial_authorities.clone(), - proof.clone(), - ).is_err()); - let ( - _header, - current_set_id, - current_set, - ) = check_warp_sync_proof::( - 1, - authorities_next.clone(), - proof.clone(), - ).unwrap(); - - assert_eq!(current_set_id, 1); - assert_eq!(current_set, authorities_next); - - // proof before set change - let proof = prove_warp_sync(&blockchain, hashes[1], None, None).unwrap(); - let ( - _header, - current_set_id, - current_set, - ) = check_warp_sync_proof::( - 0, - initial_authorities.clone(), - proof.clone(), - ).unwrap(); - - assert_eq!(current_set_id, 1); - assert_eq!(current_set, authorities_next); - - // two changes - let (blockchain, hashes) = test_blockchain( - 13, - vec![(3, vec![7]), (8, vec![9])].as_slice(), - vec![ - (1, vec![1, 2, 3]), - (2, vec![1, 2, 3]), - (3, vec![1, 2, 3]), - (4, vec![7]), - (6, vec![7]), - (8, vec![7]), // warning, requires a justification on change set - (10, vec![9]), - ].as_slice(), - ); - - // proof before set change - let proof = prove_warp_sync(&blockchain, hashes[1], None, None).unwrap(); - let ( - _header, - current_set_id, - current_set, - ) = check_warp_sync_proof::( - 0, - initial_authorities.clone(), - proof.clone(), - ).unwrap(); - - assert_eq!(current_set_id, 2); - assert_eq!(current_set, authorities_next); - } } diff --git a/client/finality-grandpa/src/lib.rs b/client/finality-grandpa/src/lib.rs index 51df586e6f599..785db89e9ee86 100644 --- a/client/finality-grandpa/src/lib.rs +++ b/client/finality-grandpa/src/lib.rs @@ -130,7 +130,6 @@ pub use voting_rule::{ BeforeBestBlockBy, ThreeQuartersOfTheUnfinalizedChain, VotingRule, VotingRulesBuilder }; pub use finality_grandpa::voter::report; -pub use finality_proof::{prove_warp_sync, WarpSyncFragmentCache}; use aux_schema::PersistentData; use environment::{Environment, VoterSetState}; From 71454db2ecec120e6ea5910f5d6e5f6519252d4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Thu, 18 Feb 2021 10:49:25 +0000 Subject: [PATCH 06/13] grandpa: fix indentation --- client/finality-grandpa-warp-sync/src/proof.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/finality-grandpa-warp-sync/src/proof.rs b/client/finality-grandpa-warp-sync/src/proof.rs index f41577ae36749..e082f366c8a48 100644 --- a/client/finality-grandpa-warp-sync/src/proof.rs +++ b/client/finality-grandpa-warp-sync/src/proof.rs @@ -101,8 +101,8 @@ impl WarpSyncProof { let justification = backend.justification(BlockId::Number(*last_block))?.expect( "header is last in set and contains standard change signal; \ - must have justification; \ - qed.", + must have justification; \ + qed.", ); let justification = GrandpaJustification::::decode(&mut &justification[..])?; From 87b0351bd9aaa4ffcf07b6d3dc6e2e287b42c6d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Thu, 18 Feb 2021 10:51:01 +0000 Subject: [PATCH 07/13] grandpa: fix off by one --- client/finality-grandpa-warp-sync/src/proof.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/finality-grandpa-warp-sync/src/proof.rs b/client/finality-grandpa-warp-sync/src/proof.rs index e082f366c8a48..173f69634ad25 100644 --- a/client/finality-grandpa-warp-sync/src/proof.rs +++ b/client/finality-grandpa-warp-sync/src/proof.rs @@ -82,7 +82,7 @@ impl WarpSyncProof { break; } - if *last_block < begin_number { + if *last_block <= begin_number { continue; } From 9c6bda80e7a4f032a9a93db704085eb8800c3815 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Tue, 23 Feb 2021 18:13:47 +0000 Subject: [PATCH 08/13] grandpa: use binary search to find start idx when generating warp sync proof --- .../finality-grandpa-warp-sync/src/proof.rs | 2 +- client/finality-grandpa/src/authorities.rs | 44 +++++++++++++++++-- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/client/finality-grandpa-warp-sync/src/proof.rs b/client/finality-grandpa-warp-sync/src/proof.rs index 173f69634ad25..82b975a446f87 100644 --- a/client/finality-grandpa-warp-sync/src/proof.rs +++ b/client/finality-grandpa-warp-sync/src/proof.rs @@ -77,7 +77,7 @@ impl WarpSyncProof { let mut proofs = Vec::new(); - for (_, last_block) in set_changes.iter() { + for (_, last_block) in set_changes.iter_from(begin_number) { if proofs.len() >= MAX_CHANGES_PER_WARP_SYNC_PROOF { break; } diff --git a/client/finality-grandpa/src/authorities.rs b/client/finality-grandpa/src/authorities.rs index 07ade2e32cf7c..0089be35d12e0 100644 --- a/client/finality-grandpa/src/authorities.rs +++ b/client/finality-grandpa/src/authorities.rs @@ -669,6 +669,7 @@ impl AuthoritySetChanges { 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. @@ -689,10 +690,17 @@ impl AuthoritySetChanges { } } - /// Returns an iterator over all historical authority set changes, the iterator yields - /// a tuple representing the set id and the block number of last block in that set. - pub fn iter(&self) -> impl Iterator { - self.0.iter() + /// Returns an iterator over all historical authority set changes starting at the given block + /// number (excluded). The iterator yields a tuple representing the set id and the block number + /// of the last block in that set. + pub fn iter_from(&self, block_number: N) -> impl Iterator { + let idx = self.0.binary_search_by_key(&block_number, |(_, n)| n.clone()) + // if there was a change at the given block number then we should start on the next + // index since we want to exclude the current block number + .map(|n| n + 1) + .unwrap_or_else(|b| b); + + self.0[idx..].iter() } } @@ -1634,4 +1642,32 @@ mod tests { assert_eq!(authority_set_changes.get_set_id(42), Some((3, 81))); assert_eq!(authority_set_changes.get_set_id(141), None); } + + #[test] + fn iter_from_works() { + let mut authority_set_changes = AuthoritySetChanges::empty(); + authority_set_changes.append(1, 41); + authority_set_changes.append(2, 81); + authority_set_changes.append(3, 121); + + assert_eq!( + vec![(1, 41), (2, 81), (3, 121)], + authority_set_changes.iter_from(40).cloned().collect::>(), + ); + + assert_eq!( + vec![(2, 81), (3, 121)], + authority_set_changes.iter_from(41).cloned().collect::>(), + ); + + assert_eq!( + 0, + authority_set_changes.iter_from(121).count(), + ); + + assert_eq!( + 0, + authority_set_changes.iter_from(200).count(), + ); + } } From c0e49a02e6084782c4c2c09d0c0953d7cc4cf371 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Wed, 24 Feb 2021 10:19:06 +0000 Subject: [PATCH 09/13] grandpa: add method to verify warp sync proofs --- Cargo.lock | 1 + client/finality-grandpa-warp-sync/Cargo.toml | 19 +++++----- client/finality-grandpa-warp-sync/src/lib.rs | 3 ++ .../finality-grandpa-warp-sync/src/proof.rs | 37 ++++++++++++++++++- client/finality-grandpa/src/finality_proof.rs | 2 +- client/finality-grandpa/src/justification.rs | 35 ++++++++++++++---- 6 files changed, 77 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e3c9131a86e3b..bd093cca99a09 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7189,6 +7189,7 @@ dependencies = [ "sc-network", "sc-service", "sp-blockchain", + "sp-finality-grandpa", "sp-runtime", ] diff --git a/client/finality-grandpa-warp-sync/Cargo.toml b/client/finality-grandpa-warp-sync/Cargo.toml index 740c85940e77d..4e4bf5b19163a 100644 --- a/client/finality-grandpa-warp-sync/Cargo.toml +++ b/client/finality-grandpa-warp-sync/Cargo.toml @@ -12,16 +12,17 @@ repository = "https://github.com/paritytech/substrate/" targets = ["x86_64-unknown-linux-gnu"] [dependencies] -sc-network = { version = "0.9.0", path = "../network" } -sc-finality-grandpa = { version = "0.9.0", path = "../finality-grandpa" } -sp-runtime = { version = "3.0.0", path = "../../primitives/runtime" } -sp-blockchain = { version = "3.0.0", path = "../../primitives/blockchain" } -sc-client-api = { version = "3.0.0", path = "../api" } -sc-service = { version = "0.9.0", path = "../service" } +codec = { package = "parity-scale-codec", version = "2.0.0" } +derive_more = "0.99.11" futures = "0.3.8" log = "0.4.11" -derive_more = "0.99.11" -codec = { package = "parity-scale-codec", version = "2.0.0" } -prost = "0.7" num-traits = "0.2.14" parking_lot = "0.11.1" +prost = "0.7" +sc-client-api = { version = "3.0.0", path = "../api" } +sc-finality-grandpa = { version = "0.9.0", path = "../finality-grandpa" } +sc-network = { version = "0.9.0", path = "../network" } +sc-service = { version = "0.9.0", path = "../service" } +sp-blockchain = { version = "3.0.0", path = "../../primitives/blockchain" } +sp-finality-grandpa = { version = "3.0.0", path = "../../primitives/finality-grandpa" } +sp-runtime = { version = "3.0.0", path = "../../primitives/runtime" } diff --git a/client/finality-grandpa-warp-sync/src/lib.rs b/client/finality-grandpa-warp-sync/src/lib.rs index 3e9b4e8964c9a..df2e4f5957463 100644 --- a/client/finality-grandpa-warp-sync/src/lib.rs +++ b/client/finality-grandpa-warp-sync/src/lib.rs @@ -167,7 +167,10 @@ pub enum HandleRequestError { #[display(fmt = "Failed to decode block hash: {}.", _0)] DecodeScale(codec::Error), Client(sp_blockchain::Error), + #[from(ignore)] InvalidRequest(String), + #[from(ignore)] + InvalidProof(String), #[display(fmt = "Failed to send response.")] SendResponse, } diff --git a/client/finality-grandpa-warp-sync/src/proof.rs b/client/finality-grandpa-warp-sync/src/proof.rs index 82b975a446f87..aa7cb328c2662 100644 --- a/client/finality-grandpa-warp-sync/src/proof.rs +++ b/client/finality-grandpa-warp-sync/src/proof.rs @@ -16,8 +16,11 @@ use codec::{Decode, Encode}; -use sc_finality_grandpa::{AuthoritySetChanges, GrandpaJustification}; +use sc_finality_grandpa::{ + find_scheduled_change, AuthoritySetChanges, BlockNumberOps, GrandpaJustification, +}; use sp_blockchain::Backend as BlockchainBackend; +use sp_finality_grandpa::{AuthorityList, SetId}; use sp_runtime::{ generic::BlockId, traits::{Block as BlockT, NumberFor}, @@ -92,7 +95,7 @@ impl WarpSyncProof { // the last block in a set is the one that triggers a change to the next set, // therefore the block must have a digest that signals the authority set change - if sc_finality_grandpa::find_scheduled_change::(&header).is_none() { + if find_scheduled_change::(&header).is_none() { // if it doesn't contain a signal for standard change then the set must have changed // through a forced changed, in which case we stop collecting proofs as the chain of // trust in authority handoffs was broken. @@ -115,4 +118,34 @@ impl WarpSyncProof { Ok(WarpSyncProof { proofs }) } + + pub fn verify( + &self, + set_id: SetId, + authorities: AuthorityList, + ) -> Result<(SetId, AuthorityList), HandleRequestError> + where + NumberFor: BlockNumberOps, + { + let mut current_set_id = set_id; + let mut current_authorities = authorities; + + for proof in &self.proofs { + proof + .justification + .verify(current_set_id, ¤t_authorities) + .map_err(|err| HandleRequestError::InvalidProof(err.to_string()))?; + + let scheduled_change = find_scheduled_change::(&proof.header).ok_or( + HandleRequestError::InvalidProof( + "Header is missing authority set change digest".to_string(), + ), + )?; + + current_authorities = scheduled_change.next_authorities; + current_set_id += 1; + } + + Ok((current_set_id, current_authorities)) + } } diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index d2c38d1771303..c88faa2498928 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -280,7 +280,7 @@ where ClientError::Consensus(sp_consensus::Error::InvalidAuthoritiesSet), )?; - GrandpaJustification::verify(self, set_id, &authorities) + GrandpaJustification::verify_with_voter_set(self, set_id, &authorities) } } diff --git a/client/finality-grandpa/src/justification.rs b/client/finality-grandpa/src/justification.rs index eba909bad5ef6..69ca70386007d 100644 --- a/client/finality-grandpa/src/justification.rs +++ b/client/finality-grandpa/src/justification.rs @@ -19,15 +19,16 @@ use std::collections::{HashMap, HashSet}; use std::sync::Arc; +use finality_grandpa::{voter_set::VoterSet, Error as GrandpaError}; +use parity_scale_codec::{Decode, Encode}; use sp_blockchain::{Error as ClientError, HeaderBackend}; -use parity_scale_codec::{Encode, Decode}; -use finality_grandpa::voter_set::VoterSet; -use finality_grandpa::{Error as GrandpaError}; -use sp_runtime::generic::BlockId; -use sp_runtime::traits::{NumberFor, Block as BlockT, Header as HeaderT}; use sp_finality_grandpa::AuthorityId; +use sp_runtime::{ + generic::BlockId, + traits::{Block as BlockT, Header as HeaderT, NumberFor}, +}; -use crate::{Commit, Error}; +use crate::{AuthorityList, Commit, Error}; /// A GRANDPA justification for block finality, it includes a commit message and /// an ancestry proof including all headers routing all precommit target blocks @@ -105,12 +106,30 @@ impl GrandpaJustification { let msg = "invalid commit target in grandpa justification".to_string(); Err(ClientError::BadJustification(msg)) } else { - justification.verify(set_id, voters).map(|_| justification) + justification + .verify_with_voter_set(set_id, voters) + .map(|_| justification) } } /// Validate the commit and the votes' ancestry proofs. - pub(crate) fn verify(&self, set_id: u64, voters: &VoterSet) -> Result<(), ClientError> + pub fn verify(&self, set_id: u64, authorities: &AuthorityList) -> Result<(), ClientError> + where + NumberFor: finality_grandpa::BlockNumberOps, + { + let voters = VoterSet::new(authorities.iter().cloned()).ok_or(ClientError::Consensus( + sp_consensus::Error::InvalidAuthoritiesSet, + ))?; + + self.verify_with_voter_set(set_id, &voters) + } + + /// Validate the commit and the votes' ancestry proofs. + pub(crate) fn verify_with_voter_set( + &self, + set_id: u64, + voters: &VoterSet, + ) -> Result<(), ClientError> where NumberFor: finality_grandpa::BlockNumberOps, { From cec03ab176779c259421970033b0ba07cb56bec2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Wed, 24 Feb 2021 10:19:23 +0000 Subject: [PATCH 10/13] grandpa: remove unnecessary code to skip authority set changes --- client/finality-grandpa-warp-sync/src/proof.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/client/finality-grandpa-warp-sync/src/proof.rs b/client/finality-grandpa-warp-sync/src/proof.rs index aa7cb328c2662..922289ad2b031 100644 --- a/client/finality-grandpa-warp-sync/src/proof.rs +++ b/client/finality-grandpa-warp-sync/src/proof.rs @@ -85,10 +85,6 @@ impl WarpSyncProof { break; } - if *last_block <= begin_number { - continue; - } - let header = backend.header(BlockId::Number(*last_block))?.expect( "header number comes from previously applied set changes; must exist in db; qed.", ); From 57b8e5b4a339600642a36a818852c27fdbb6a602 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Wed, 24 Feb 2021 11:48:23 +0000 Subject: [PATCH 11/13] grandpa: add test for warp sync proof generation and verification --- Cargo.lock | 6 + client/finality-grandpa-warp-sync/Cargo.toml | 8 + client/finality-grandpa-warp-sync/src/lib.rs | 2 +- .../finality-grandpa-warp-sync/src/proof.rs | 143 ++++++++++++++++++ client/finality-grandpa/src/authorities.rs | 6 + 5 files changed, 164 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index bd093cca99a09..115f64d719ef2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7178,19 +7178,25 @@ name = "sc-finality-grandpa-warp-sync" version = "0.9.0" dependencies = [ "derive_more", + "finality-grandpa", "futures 0.3.12", "log", "num-traits", "parity-scale-codec", "parking_lot 0.11.1", "prost", + "rand 0.8.3", + "sc-block-builder", "sc-client-api", "sc-finality-grandpa", "sc-network", "sc-service", "sp-blockchain", + "sp-consensus", "sp-finality-grandpa", + "sp-keyring", "sp-runtime", + "substrate-test-runtime-client", ] [[package]] diff --git a/client/finality-grandpa-warp-sync/Cargo.toml b/client/finality-grandpa-warp-sync/Cargo.toml index 4e4bf5b19163a..3557d543c987e 100644 --- a/client/finality-grandpa-warp-sync/Cargo.toml +++ b/client/finality-grandpa-warp-sync/Cargo.toml @@ -26,3 +26,11 @@ sc-service = { version = "0.9.0", path = "../service" } sp-blockchain = { version = "3.0.0", path = "../../primitives/blockchain" } sp-finality-grandpa = { version = "3.0.0", path = "../../primitives/finality-grandpa" } sp-runtime = { version = "3.0.0", path = "../../primitives/runtime" } + +[dev-dependencies] +finality-grandpa = { version = "0.14.0" } +rand = "0.8" +sc-block-builder = { version = "0.9.0", path = "../block-builder" } +sp-consensus = { version = "0.9.0", path = "../../primitives/consensus/common" } +sp-keyring = { version = "3.0.0", path = "../../primitives/keyring" } +substrate-test-runtime-client = { version = "2.0.0", path = "../../test-utils/runtime/client" } diff --git a/client/finality-grandpa-warp-sync/src/lib.rs b/client/finality-grandpa-warp-sync/src/lib.rs index df2e4f5957463..5bd0363878577 100644 --- a/client/finality-grandpa-warp-sync/src/lib.rs +++ b/client/finality-grandpa-warp-sync/src/lib.rs @@ -158,7 +158,7 @@ impl> GrandpaWarpSyncRequestHandler WarpSyncProof { Ok((current_set_id, current_authorities)) } } + +#[cfg(test)] +mod tests { + use crate::WarpSyncProof; + 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; + use sp_keyring::Ed25519Keyring; + use sp_runtime::{generic::BlockId, traits::Header as _}; + use std::sync::Arc; + use substrate_test_runtime_client::{ + ClientBlockImportExt, ClientExt, DefaultTestClientBuilderExt, TestClientBuilder, + TestClientBuilderExt, + }; + + #[test] + fn warp_sync_proof_generate_verify() { + let mut rng = rand::rngs::StdRng::from_seed([0; 32]); + let builder = TestClientBuilder::new(); + let backend = builder.backend(); + let mut client = Arc::new(builder.build()); + + let available_authorities = Ed25519Keyring::iter().collect::>(); + let genesis_authorities = vec![(Ed25519Keyring::Alice.public().into(), 1)]; + + let mut current_authorities = vec![Ed25519Keyring::Alice]; + let mut current_set_id = 0; + let mut authority_set_changes = Vec::new(); + + for n in 1..=100 { + let mut block = client + .new_block(Default::default()) + .unwrap() + .build() + .unwrap() + .block; + + let mut new_authorities = None; + + // we will trigger an authority set change every 10 blocks + if n != 0 && n % 10 == 0 { + // pick next authorities and add digest for the set change + let n_authorities = rng.gen_range(1..available_authorities.len()); + let next_authorities = available_authorities + .choose_multiple(&mut rng, n_authorities) + .cloned() + .collect::>(); + + new_authorities = Some(next_authorities.clone()); + + let next_authorities = next_authorities + .iter() + .map(|keyring| (keyring.public().into(), 1)) + .collect::>(); + + let digest = sp_runtime::generic::DigestItem::Consensus( + sp_finality_grandpa::GRANDPA_ENGINE_ID, + sp_finality_grandpa::ConsensusLog::ScheduledChange( + sp_finality_grandpa::ScheduledChange { + delay: 0u64, + next_authorities, + }, + ) + .encode(), + ); + + block.header.digest_mut().logs.push(digest); + } + + client.import(BlockOrigin::Own, block).unwrap(); + + if let Some(new_authorities) = new_authorities { + // generate a justification for this block, finalize it and note the authority set + // change + let (target_hash, target_number) = { + let info = client.info(); + (info.best_hash, info.best_number) + }; + + let mut precommits = Vec::new(); + for keyring in ¤t_authorities { + let precommit = finality_grandpa::Precommit { + target_hash, + target_number, + }; + + let msg = finality_grandpa::Message::Precommit(precommit.clone()); + let encoded = sp_finality_grandpa::localized_payload(42, current_set_id, &msg); + let signature = keyring.sign(&encoded[..]).into(); + + let precommit = finality_grandpa::SignedPrecommit { + precommit, + signature, + id: keyring.public().into(), + }; + + precommits.push(precommit); + } + + let commit = finality_grandpa::Commit { + target_hash, + target_number, + precommits, + }; + + let justification = GrandpaJustification::from_commit(&client, 42, commit).unwrap(); + + client + .finalize_block(BlockId::Hash(target_hash), Some(justification.encode())) + .unwrap(); + + authority_set_changes.push((current_set_id, n)); + + current_set_id += 1; + current_authorities = new_authorities; + } + } + + let authority_set_changes = AuthoritySetChanges::from(authority_set_changes); + + // generate a warp sync proof + let genesis_hash = client.hash(0).unwrap().unwrap(); + + let warp_sync_proof = + WarpSyncProof::generate(backend.blockchain(), 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(); + + let expected_authorities = current_authorities + .iter() + .map(|keyring| (keyring.public().into(), 1)) + .collect::>(); + + assert_eq!(new_set_id, current_set_id); + assert_eq!(new_authorities, expected_authorities,); + } +} diff --git a/client/finality-grandpa/src/authorities.rs b/client/finality-grandpa/src/authorities.rs index 0089be35d12e0..11d3d4ba691da 100644 --- a/client/finality-grandpa/src/authorities.rs +++ b/client/finality-grandpa/src/authorities.rs @@ -656,6 +656,12 @@ impl + Clone> PendingChange { #[derive(Debug, Encode, Decode, Clone, PartialEq)] pub struct AuthoritySetChanges(Vec<(u64, N)>); +impl From> for AuthoritySetChanges { + fn from(changes: Vec<(u64, N)>) -> AuthoritySetChanges { + AuthoritySetChanges(changes) + } +} + impl AuthoritySetChanges { pub(crate) fn empty() -> Self { Self(Default::default()) From f6e62f7de05e9a5abd09229d05ca007d9479421f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Wed, 24 Feb 2021 11:55:43 +0000 Subject: [PATCH 12/13] grandpa: add missing docs --- client/finality-grandpa-warp-sync/src/proof.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/client/finality-grandpa-warp-sync/src/proof.rs b/client/finality-grandpa-warp-sync/src/proof.rs index d58da4b08e6a2..6e9f862ab6b0a 100644 --- a/client/finality-grandpa-warp-sync/src/proof.rs +++ b/client/finality-grandpa-warp-sync/src/proof.rs @@ -31,21 +31,27 @@ use crate::HandleRequestError; /// The maximum number of authority set change proofs to include in a single warp sync proof. const MAX_CHANGES_PER_WARP_SYNC_PROOF: usize = 256; +/// A proof of an authority set change. #[derive(Decode, Encode)] pub struct AuthoritySetChangeProof { - /// must contain the authorities for the following set + /// The last block that the given authority set finalized. This block should contain a digest + /// signaling an authority set change from which we can fetch the next authority set. pub header: Block::Header, - /// this justification proves finality of the header above - /// it must be validated against the authorities of the previous set. + /// A justification for the header above which proves its finality. In order to validate it the + /// verifier must be aware of the authorities and set id for which the justification refers to. pub justification: GrandpaJustification, } +/// An accumulated proof of multiple authority set changes. #[derive(Decode, Encode)] pub struct WarpSyncProof { proofs: Vec>, } impl WarpSyncProof { + /// Generates a warp sync proof starting at the given block. It will generate authority set + /// change proofs for all changes that happened from `begin` until the current authority set + /// (capped by MAX_CHANGES_PER_WARP_SYNC_PROOF). pub fn generate( backend: &Backend, begin: Block::Hash, @@ -115,6 +121,8 @@ impl WarpSyncProof { Ok(WarpSyncProof { proofs }) } + /// Verifies the warp sync proof starting at the given set id and with the given authorities. + /// If the proof is valid the new set id and authorities is returned. pub fn verify( &self, set_id: SetId, From 14b8161f025a512b55a35151c2bb5434a0251cdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Wed, 24 Feb 2021 21:43:38 +0000 Subject: [PATCH 13/13] grandpa: remove trailing comma --- client/finality-grandpa-warp-sync/src/proof.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/finality-grandpa-warp-sync/src/proof.rs b/client/finality-grandpa-warp-sync/src/proof.rs index 6e9f862ab6b0a..4dd7b4f57f18a 100644 --- a/client/finality-grandpa-warp-sync/src/proof.rs +++ b/client/finality-grandpa-warp-sync/src/proof.rs @@ -293,6 +293,6 @@ mod tests { .collect::>(); assert_eq!(new_set_id, current_set_id); - assert_eq!(new_authorities, expected_authorities,); + assert_eq!(new_authorities, expected_authorities); } }