From cf544ef5732dd8b82c262270a5264bd1f6aca625 Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Mon, 7 Dec 2020 10:41:58 +0100 Subject: [PATCH 01/34] Made a start --- client/network/src/behaviour.rs | 4 +- client/network/src/config.rs | 13 ++ .../src/grandpa_warp_sync_request_handler.rs | 220 ++++++++++++++++++ client/network/src/lib.rs | 1 + client/network/src/service.rs | 6 +- client/service/src/builder.rs | 19 +- 6 files changed, 259 insertions(+), 4 deletions(-) create mode 100644 client/network/src/grandpa_warp_sync_request_handler.rs diff --git a/client/network/src/behaviour.rs b/client/network/src/behaviour.rs index b2914a5e0a72b..5a24eb50c0056 100644 --- a/client/network/src/behaviour.rs +++ b/client/network/src/behaviour.rs @@ -181,14 +181,14 @@ impl Behaviour { block_requests: block_requests::BlockRequests, light_client_handler: light_client_handler::LightClientHandler, disco_config: DiscoveryConfig, - request_response_protocols: Vec, + request_response_protocols: impl Iterator, ) -> Result { Ok(Behaviour { substrate, peer_info: peer_info::PeerInfoBehaviour::new(user_agent, local_public_key), discovery: disco_config.finish(), request_responses: - request_responses::RequestResponsesBehaviour::new(request_response_protocols.into_iter())?, + request_responses::RequestResponsesBehaviour::new(request_response_protocols)?, block_requests, light_client_handler, events: VecDeque::new(), diff --git a/client/network/src/config.rs b/client/network/src/config.rs index b7b113dc14692..4c6ec437fcb7b 100644 --- a/client/network/src/config.rs +++ b/client/network/src/config.rs @@ -95,6 +95,19 @@ pub struct Params { /// Registry for recording prometheus metrics to. pub metrics_registry: Option, + + /// Request response configuration for the grandpa warp sync request protocol. + /// + /// [`RequestResponseConfig`] [`name`] is used to tag outgoing grandpa warp sync requests with + /// the correct protocol name. In addition all of [`RequestResponseConfig`] is used to handle + /// incoming grandpa warp sync requests, if enabled. + /// + /// Can be constructed either via + /// [`grandpa_warp_sync_request_handler::generate_protocol_config`] allowing outgoing but not + /// incoming requests, or constructed via + /// [`grandpa_warp_sync_request_handler::GrandpaWarpSyncRequestHandler::new`] allowing both + /// outgoing and incoming requests. + pub grandpa_warp_sync_request_protocol_config: RequestResponseConfig, } /// Role of the local node. diff --git a/client/network/src/grandpa_warp_sync_request_handler.rs b/client/network/src/grandpa_warp_sync_request_handler.rs new file mode 100644 index 0000000000000..a0cc5abcff69f --- /dev/null +++ b/client/network/src/grandpa_warp_sync_request_handler.rs @@ -0,0 +1,220 @@ +// Copyright 2020 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +//! Helper for handling (i.e. answering) grandpa warp sync requests from a remote peer via the +//! [`crate::request_responses::RequestResponsesBehaviour`]. + +use codec::{Encode, Decode}; +use crate::chain::Client; +use crate::config::ProtocolId; +use crate::protocol::{message::BlockAttributes}; +use crate::request_responses::{IncomingRequest, ProtocolConfig}; +use crate::schema::v1::block_request::FromBlock; +use crate::schema::v1::{BlockResponse, Direction}; +use futures::channel::{mpsc, oneshot}; +use futures::stream::StreamExt; +use log::debug; +use prost::Message; +use sp_runtime::generic::BlockId; +use sp_runtime::traits::{Block as BlockT, Header, One, Zero}; +use std::cmp::min; +use std::sync::{Arc}; +use std::time::Duration; + +const LOG_TARGET: &str = "grandpa-warp-sync-request-handler"; +const MAX_BLOCKS_IN_RESPONSE: usize = 128; +const MAX_BODY_BYTES: usize = 8 * 1024 * 1024; + +/// Generates a [`ProtocolConfig`] for the block request protocol, refusing incoming requests. +pub fn generate_protocol_config(protocol_id: ProtocolId) -> ProtocolConfig { + ProtocolConfig { + name: generate_protocol_name(protocol_id).into(), + max_request_size: 1024 * 1024, + max_response_size: 16 * 1024 * 1024, + request_timeout: Duration::from_secs(40), + inbound_queue: None, + } +} + +/// Generate the block protocol name from chain specific protocol identifier. +fn generate_protocol_name(protocol_id: ProtocolId) -> String { + let mut s = String::new(); + s.push_str("/"); + s.push_str(protocol_id.as_ref()); + s.push_str("/sync/warp"); + s +} + +/// Handler for incoming block requests from a remote peer. +pub struct GrandpaWarpSyncRequestHandler { + client: Arc>, + request_receiver: mpsc::Receiver, +} + +impl GrandpaWarpSyncRequestHandler { + /// Create a new [`BlockRequestHandler`]. + pub fn new(protocol_id: ProtocolId, client: Arc>) -> (Self, ProtocolConfig) { + // Rate of arrival multiplied with the waiting time in the queue equals the queue length. + // + // An average Polkadot sentry node serves less than 5 requests per second. The 95th percentile + // serving a request is less than 2 second. Thus one would estimate the queue length to be + // below 10. + // + // Choosing 20 as the queue length to give some additional buffer. + let (tx, request_receiver) = mpsc::channel(20); + + let mut protocol_config = generate_protocol_config(protocol_id); + protocol_config.inbound_queue = Some(tx); + + (Self { client, request_receiver }, protocol_config) + } + + fn handle_request( + &self, + payload: Vec, + pending_response: oneshot::Sender> + ) -> Result<(), HandleRequestError> { + let request = crate::schema::v1::BlockRequest::decode(&payload[..])?; + + let from_block_id = match request.from_block.ok_or(HandleRequestError::MissingFromField)? { + FromBlock::Hash(ref h) => { + let h = Decode::decode(&mut h.as_ref())?; + BlockId::::Hash(h) + } + FromBlock::Number(ref n) => { + let n = Decode::decode(&mut n.as_ref())?; + BlockId::::Number(n) + } + }; + + let max_blocks = if request.max_blocks == 0 { + MAX_BLOCKS_IN_RESPONSE + } else { + min(request.max_blocks as usize, MAX_BLOCKS_IN_RESPONSE) + }; + + let direction = Direction::from_i32(request.direction) + .ok_or(HandleRequestError::ParseDirection)?; + let attributes = BlockAttributes::from_be_u32(request.fields)?; + let get_header = attributes.contains(BlockAttributes::HEADER); + let get_body = attributes.contains(BlockAttributes::BODY); + let get_justification = attributes.contains(BlockAttributes::JUSTIFICATION); + + let mut blocks = Vec::new(); + let mut block_id = from_block_id; + + let mut total_size: usize = 0; + while let Some(header) = self.client.header(block_id).unwrap_or(None) { + let number = *header.number(); + let hash = header.hash(); + let parent_hash = *header.parent_hash(); + let justification = if get_justification { + self.client.justification(&BlockId::Hash(hash))? + } else { + None + }; + let is_empty_justification = justification.as_ref().map(|j| j.is_empty()).unwrap_or(false); + + let body = if get_body { + match self.client.block_body(&BlockId::Hash(hash))? { + Some(mut extrinsics) => extrinsics.iter_mut() + .map(|extrinsic| extrinsic.encode()) + .collect(), + None => { + log::trace!(target: "sync", "Missing data for block request."); + break; + } + } + } else { + Vec::new() + }; + + let block_data = crate::schema::v1::BlockData { + hash: hash.encode(), + header: if get_header { + header.encode() + } else { + Vec::new() + }, + body, + receipt: Vec::new(), + message_queue: Vec::new(), + justification: justification.unwrap_or_default(), + is_empty_justification, + }; + + total_size += block_data.body.len(); + blocks.push(block_data); + + if blocks.len() >= max_blocks as usize || total_size > MAX_BODY_BYTES { + break + } + + match direction { + Direction::Ascending => { + block_id = BlockId::Number(number + One::one()) + } + Direction::Descending => { + if number.is_zero() { + break + } + block_id = BlockId::Hash(parent_hash) + } + } + } + + let res = BlockResponse { blocks }; + + let mut data = Vec::with_capacity(res.encoded_len()); + res.encode(&mut data)?; + + pending_response.send(data) + .map_err(|_| HandleRequestError::SendResponse) + } + + /// Run [`BlockRequestHandler`]. + pub async fn run(mut self) { + while let Some(request) = self.request_receiver.next().await { + let IncomingRequest { peer, payload, pending_response } = request; + + match self.handle_request(payload, pending_response) { + Ok(()) => debug!(target: LOG_TARGET, "Handled block request from {}.", peer), + Err(e) => debug!( + target: LOG_TARGET, + "Failed to handle block request from {}: {}", + peer, e, + ), + } + } + } +} + +#[derive(derive_more::Display, derive_more::From)] +enum HandleRequestError { + #[display(fmt = "Failed to decode request: {}.", _0)] + DecodeProto(prost::DecodeError), + #[display(fmt = "Failed to encode response: {}.", _0)] + EncodeProto(prost::EncodeError), + #[display(fmt = "Failed to decode block hash: {}.", _0)] + DecodeScale(codec::Error), + #[display(fmt = "Missing `BlockRequest::from_block` field.")] + MissingFromField, + #[display(fmt = "Failed to parse BlockRequest::direction.")] + ParseDirection, + Client(sp_blockchain::Error), + #[display(fmt = "Failed to send response.")] + SendResponse, +} diff --git a/client/network/src/lib.rs b/client/network/src/lib.rs index fb65c754d79a2..3d2b809db60cc 100644 --- a/client/network/src/lib.rs +++ b/client/network/src/lib.rs @@ -263,6 +263,7 @@ pub mod config; pub mod error; pub mod gossip; pub mod network_state; +pub mod grandpa_warp_sync_request_handler; #[doc(inline)] pub use libp2p::{multiaddr, Multiaddr, PeerId}; diff --git a/client/network/src/service.rs b/client/network/src/service.rs index b6f162affd671..3244c139c4a8f 100644 --- a/client/network/src/service.rs +++ b/client/network/src/service.rs @@ -323,6 +323,10 @@ impl NetworkWorker { config }; + let request_response_protocols = params.network_config.request_response_protocols + .into_iter() + .chain(std::iter::once(params.grandpa_warp_sync_request_protocol_config)); + let mut behaviour = { let result = Behaviour::new( protocol, @@ -332,7 +336,7 @@ impl NetworkWorker { block_requests, light_client_handler, discovery_config, - params.network_config.request_response_protocols, + request_response_protocols, ); match result { diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index 52c1121d504df..2c93630f1ef1d 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -43,6 +43,7 @@ use sc_keystore::LocalKeystore; use log::{info, warn}; use sc_network::config::{Role, OnDemand}; use sc_network::NetworkService; +use sc_network::grandpa_warp_sync_request_handler::{self, GrandpaWarpSyncRequestHandler}; use sp_runtime::generic::BlockId; use sp_runtime::traits::{ Block as BlockT, SaturatedConversion, HashFor, Zero, BlockIdTo, @@ -882,6 +883,21 @@ pub fn build_network( Box::new(DefaultBlockAnnounceValidator) }; + let grandpa_warp_sync_request_protocol_config = { + if matches!(config.role, Role::Light) { + // Allow outgoing requests but deny incoming requests. + grandpa_warp_sync_request_handler::generate_protocol_config(protocol_id.clone()) + } else { + // Allow both outgoing and incoming requests. + let (handler, protocol_config) = GrandpaWarpSyncRequestHandler::new( + protocol_id.clone(), + client.clone(), + ); + spawn_handle.spawn("grandpa_warp_sync_request_handler", handler.run()); + protocol_config + } + }; + let network_params = sc_network::config::Params { role: config.role.clone(), executor: { @@ -897,7 +913,8 @@ pub fn build_network( import_queue: Box::new(import_queue), protocol_id, block_announce_validator, - metrics_registry: config.prometheus_config.as_ref().map(|config| config.registry.clone()) + metrics_registry: config.prometheus_config.as_ref().map(|config| config.registry.clone()), + grandpa_warp_sync_request_protocol_config, }; let has_bootnodes = !network_params.network_config.boot_nodes.is_empty(); From e7722a6f2c1a22b8390cbdf9df08bd0165da3526 Mon Sep 17 00:00:00 2001 From: cheme Date: Wed, 2 Sep 2020 17:39:16 +0200 Subject: [PATCH 02/34] So the proof between authority set is phragmen one, this is crazy big, or is there some signing of the result : that is the storage key, damn? --- client/finality-grandpa/src/finality_proof.rs | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index bf367ab3f4a55..c07173afcf772 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -483,6 +483,27 @@ pub(crate) fn check_finality_proof( Ok(effects) } +/// Check finality authority set sequence. +fn authority_sequence_next( + current_set_id: &mut u64, + current_authorities: &mut AuthorityList, + authorities_provider: &dyn AuthoritySetForFinalityChecker, + authorities_proof: StorageProof, // TODO not a storage proof +) -> ClientResult<(u64, AuthorityList)> + where + NumberFor: BlockNumberOps, +{ + let header = unimplemented!("We do not have header here"); + let block = unimplemented!("We do not have header here"); + *current_authorities = authorities_provider.check_authorities_proof( + block, + header, + authorities_proof, + )?; + + *current_set_id += 1; +} + /// Check finality proof for the single block. fn check_finality_proof_fragment( blockchain: &B, From fe0556040639a63ae3267be43c953ce6d762b842 Mon Sep 17 00:00:00 2001 From: cheme Date: Wed, 2 Sep 2020 18:20:35 +0200 Subject: [PATCH 03/34] ok getting from header digest seems doable. --- client/finality-grandpa/src/finality_proof.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index c07173afcf772..54fd2c6d872a6 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -493,6 +493,16 @@ fn authority_sequence_next( where NumberFor: BlockNumberOps, { + // TODO we get new authority set from the header log/digest. + // We get Forced Change and Scheduled change. + // + // Then for every log we also get header justification. + // This method check justification for the header against + // current set and update to next set afterward. + // + // TODO debug against value of stored authority set (each + // got a delay and forced got a block start: not sure why + // not using delay there). let header = unimplemented!("We do not have header here"); let block = unimplemented!("We do not have header here"); *current_authorities = authorities_provider.check_authorities_proof( From 43ff21ee0261273712edb071df228c91f5c8b0f8 Mon Sep 17 00:00:00 2001 From: cheme Date: Thu, 3 Sep 2020 19:29:28 +0200 Subject: [PATCH 04/34] for testing --- Cargo.lock | 6 +- bin/node/cli/src/cli.rs | 7 - bin/node/cli/src/command.rs | 5 - bin/node/inspect/src/lib.rs | 329 ------------------ client/db/Cargo.toml | 3 + client/db/src/lib.rs | 53 ++- client/finality-grandpa/src/finality_proof.rs | 211 +++++++++-- client/finality-grandpa/src/import.rs | 4 +- client/finality-grandpa/src/lib.rs | 3 +- client/service/Cargo.toml | 1 + client/service/src/builder.rs | 6 +- test-utils/client/Cargo.toml | 2 +- test-utils/client/src/lib.rs | 12 +- 13 files changed, 263 insertions(+), 379 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 53c42e08774b5..048772d9c6c2b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6484,6 +6484,7 @@ dependencies = [ "kvdb-rocksdb", "linked-hash-map", "log", + "num-traits", "parity-db", "parity-scale-codec", "parity-util-mem", @@ -6491,12 +6492,14 @@ dependencies = [ "quickcheck", "sc-client-api", "sc-executor", + "sc-finality-grandpa", "sc-state-db", "sp-arithmetic", "sp-blockchain", "sp-consensus", "sp-core", "sp-database", + "sp-finality-grandpa", "sp-keyring", "sp-runtime", "sp-state-machine", @@ -7229,6 +7232,7 @@ dependencies = [ "async-std", "directories 3.0.1", "exit-future", + "finality-grandpa", "futures 0.1.30", "futures 0.3.8", "futures-timer 3.0.2", @@ -8857,6 +8861,7 @@ dependencies = [ name = "substrate-test-client" version = "2.0.0" dependencies = [ + "finality-grandpa", "futures 0.1.30", "futures 0.3.8", "hash-db", @@ -8874,7 +8879,6 @@ dependencies = [ "sp-consensus", "sp-core", "sp-keyring", - "sp-keystore", "sp-runtime", "sp-state-machine", ] diff --git a/bin/node/cli/src/cli.rs b/bin/node/cli/src/cli.rs index 2130ff1e4b106..6515a5c343250 100644 --- a/bin/node/cli/src/cli.rs +++ b/bin/node/cli/src/cli.rs @@ -36,13 +36,6 @@ pub enum Subcommand { /// Key management cli utilities Key(KeySubcommand), - /// The custom inspect subcommmand for decoding blocks and extrinsics. - #[structopt( - name = "inspect", - about = "Decode given block or extrinsic using current native runtime." - )] - Inspect(node_inspect::cli::InspectCmd), - /// The custom benchmark subcommmand benchmarking runtime pallets. #[structopt(name = "benchmark", about = "Benchmark runtime pallets.")] Benchmark(frame_benchmarking_cli::BenchmarkCmd), diff --git a/bin/node/cli/src/command.rs b/bin/node/cli/src/command.rs index f8a0f3f9b3a34..ab75729fc46c1 100644 --- a/bin/node/cli/src/command.rs +++ b/bin/node/cli/src/command.rs @@ -79,11 +79,6 @@ pub fn run() -> Result<()> { } }) } - Some(Subcommand::Inspect(cmd)) => { - let runner = cli.create_runner(cmd)?; - - runner.sync_run(|config| cmd.run::(config)) - } Some(Subcommand::Benchmark(cmd)) => { if cfg!(feature = "runtime-benchmarks") { let runner = cli.create_runner(cmd)?; diff --git a/bin/node/inspect/src/lib.rs b/bin/node/inspect/src/lib.rs index 02f5614b81a78..e69de29bb2d1d 100644 --- a/bin/node/inspect/src/lib.rs +++ b/bin/node/inspect/src/lib.rs @@ -1,329 +0,0 @@ -// This file is part of Substrate. -// -// Copyright (C) 2020 Parity Technologies (UK) Ltd. -// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 -// -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with this program. If not, see . - -//! A CLI extension for substrate node, adding sub-command to pretty print debug info -//! about blocks and extrinsics. -//! -//! The blocks and extrinsics can either be retrieved from the database (on-chain), -//! or a raw SCALE-encoding can be provided. - -#![warn(missing_docs)] - -pub mod cli; -pub mod command; - -use std::{ - fmt, - fmt::Debug, - marker::PhantomData, - str::FromStr, -}; -use codec::{Encode, Decode}; -use sc_client_api::BlockBackend; -use sp_blockchain::HeaderBackend; -use sp_core::hexdisplay::HexDisplay; -use sp_runtime::{ - generic::BlockId, - traits::{Block, HashFor, NumberFor, Hash} -}; - -/// A helper type for a generic block input. -pub type BlockAddressFor = BlockAddress< - as Hash>::Output, - NumberFor ->; - -/// A Pretty formatter implementation. -pub trait PrettyPrinter { - /// Nicely format block. - fn fmt_block(&self, fmt: &mut fmt::Formatter, block: &TBlock) -> fmt::Result; - /// Nicely format extrinsic. - fn fmt_extrinsic(&self, fmt: &mut fmt::Formatter, extrinsic: &TBlock::Extrinsic) -> fmt::Result; -} - -/// Default dummy debug printer. -#[derive(Default)] -pub struct DebugPrinter; -impl PrettyPrinter for DebugPrinter { - fn fmt_block(&self, fmt: &mut fmt::Formatter, block: &TBlock) -> fmt::Result { - writeln!(fmt, "Header:")?; - writeln!(fmt, "{:?}", block.header())?; - writeln!(fmt, "Block bytes: {:?}", HexDisplay::from(&block.encode()))?; - writeln!(fmt, "Extrinsics ({})", block.extrinsics().len())?; - for (idx, ex) in block.extrinsics().iter().enumerate() { - writeln!(fmt, "- {}:", idx)?; - >::fmt_extrinsic(self, fmt, ex)?; - } - Ok(()) - } - - fn fmt_extrinsic(&self, fmt: &mut fmt::Formatter, extrinsic: &TBlock::Extrinsic) -> fmt::Result { - writeln!(fmt, " {:?}", extrinsic)?; - writeln!(fmt, " Bytes: {:?}", HexDisplay::from(&extrinsic.encode()))?; - Ok(()) - } -} - -/// Aggregated error for `Inspector` operations. -#[derive(Debug, derive_more::From, derive_more::Display)] -pub enum Error { - /// Could not decode Block or Extrinsic. - Codec(codec::Error), - /// Error accessing blockchain DB. - Blockchain(sp_blockchain::Error), - /// Given block has not been found. - NotFound(String), -} - -impl std::error::Error for Error { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match *self { - Self::Codec(ref e) => Some(e), - Self::Blockchain(ref e) => Some(e), - Self::NotFound(_) => None, - } - } -} - -/// A helper trait to access block headers and bodies. -pub trait ChainAccess: - HeaderBackend + - BlockBackend -{} - -impl ChainAccess for T where - TBlock: Block, - T: sp_blockchain::HeaderBackend + sc_client_api::BlockBackend, -{} - -/// Blockchain inspector. -pub struct Inspector = DebugPrinter> { - printer: TPrinter, - chain: Box>, - _block: PhantomData, -} - -impl> Inspector { - /// Create new instance of the inspector with default printer. - pub fn new( - chain: impl ChainAccess + 'static, - ) -> Self where TPrinter: Default { - Self::with_printer(chain, Default::default()) - } - - /// Customize pretty-printing of the data. - pub fn with_printer( - chain: impl ChainAccess + 'static, - printer: TPrinter, - ) -> Self { - Inspector { - chain: Box::new(chain) as _, - printer, - _block: Default::default(), - } - } - - /// Get a pretty-printed block. - pub fn block(&self, input: BlockAddressFor) -> Result { - struct BlockPrinter<'a, A, B>(A, &'a B); - impl<'a, A: Block, B: PrettyPrinter> fmt::Display for BlockPrinter<'a, A, B> { - fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - self.1.fmt_block(fmt, &self.0) - } - } - - let block = self.get_block(input)?; - Ok(format!("{}", BlockPrinter(block, &self.printer))) - } - - fn get_block(&self, input: BlockAddressFor) -> Result { - Ok(match input { - BlockAddress::Bytes(bytes) => { - TBlock::decode(&mut &*bytes)? - }, - BlockAddress::Number(number) => { - let id = BlockId::number(number); - let not_found = format!("Could not find block {:?}", id); - let body = self.chain.block_body(&id)? - .ok_or_else(|| Error::NotFound(not_found.clone()))?; - let header = self.chain.header(id)? - .ok_or_else(|| Error::NotFound(not_found.clone()))?; - TBlock::new(header, body) - }, - BlockAddress::Hash(hash) => { - let id = BlockId::hash(hash); - let not_found = format!("Could not find block {:?}", id); - let body = self.chain.block_body(&id)? - .ok_or_else(|| Error::NotFound(not_found.clone()))?; - let header = self.chain.header(id)? - .ok_or_else(|| Error::NotFound(not_found.clone()))?; - TBlock::new(header, body) - }, - }) - } - - /// Get a pretty-printed extrinsic. - pub fn extrinsic( - &self, - input: ExtrinsicAddress< as Hash>::Output, NumberFor>, - ) -> Result { - struct ExtrinsicPrinter<'a, A: Block, B>(A::Extrinsic, &'a B); - impl<'a, A: Block, B: PrettyPrinter> fmt::Display for ExtrinsicPrinter<'a, A, B> { - fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - self.1.fmt_extrinsic(fmt, &self.0) - } - } - - let ext = match input { - ExtrinsicAddress::Block(block, index) => { - let block = self.get_block(block)?; - block.extrinsics() - .get(index) - .cloned() - .ok_or_else(|| Error::NotFound(format!( - "Could not find extrinsic {} in block {:?}", index, block - )))? - }, - ExtrinsicAddress::Bytes(bytes) => { - TBlock::Extrinsic::decode(&mut &*bytes)? - } - }; - - Ok(format!("{}", ExtrinsicPrinter(ext, &self.printer))) - } -} - -/// A block to retrieve. -#[derive(Debug, Clone, PartialEq)] -pub enum BlockAddress { - /// Get block by hash. - Hash(Hash), - /// Get block by number. - Number(Number), - /// Raw SCALE-encoded bytes. - Bytes(Vec), -} - -impl FromStr for BlockAddress { - type Err = String; - - fn from_str(s: &str) -> Result { - // try to parse hash first - if let Ok(hash) = s.parse() { - return Ok(Self::Hash(hash)) - } - - // then number - if let Ok(number) = s.parse() { - return Ok(Self::Number(number)) - } - - // then assume it's bytes (hex-encoded) - sp_core::bytes::from_hex(s) - .map(Self::Bytes) - .map_err(|e| format!( - "Given string does not look like hash or number. It could not be parsed as bytes either: {}", - e - )) - } -} - -/// An extrinsic address to decode and print out. -#[derive(Debug, Clone, PartialEq)] -pub enum ExtrinsicAddress { - /// Extrinsic as part of existing block. - Block(BlockAddress, usize), - /// Raw SCALE-encoded extrinsic bytes. - Bytes(Vec), -} - -impl FromStr for ExtrinsicAddress { - type Err = String; - - fn from_str(s: &str) -> Result { - // first try raw bytes - if let Ok(bytes) = sp_core::bytes::from_hex(s).map(Self::Bytes) { - return Ok(bytes) - } - - // split by a bunch of different characters - let mut it = s.split(|c| c == '.' || c == ':' || c == ' '); - let block = it.next() - .expect("First element of split iterator is never empty; qed") - .parse()?; - - let index = it.next() - .ok_or_else(|| format!("Extrinsic index missing: example \"5:0\""))? - .parse() - .map_err(|e| format!("Invalid index format: {}", e))?; - - Ok(Self::Block(block, index)) - } -} - -#[cfg(test)] -mod tests { - use super::*; - use sp_core::hash::H160 as Hash; - - #[test] - fn should_parse_block_strings() { - type BlockAddress = super::BlockAddress; - - let b0 = BlockAddress::from_str("3BfC20f0B9aFcAcE800D73D2191166FF16540258"); - let b1 = BlockAddress::from_str("1234"); - let b2 = BlockAddress::from_str("0"); - let b3 = BlockAddress::from_str("0x0012345f"); - - - assert_eq!(b0, Ok(BlockAddress::Hash( - "3BfC20f0B9aFcAcE800D73D2191166FF16540258".parse().unwrap() - ))); - assert_eq!(b1, Ok(BlockAddress::Number(1234))); - assert_eq!(b2, Ok(BlockAddress::Number(0))); - assert_eq!(b3, Ok(BlockAddress::Bytes(vec![0, 0x12, 0x34, 0x5f]))); - } - - #[test] - fn should_parse_extrinsic_address() { - type BlockAddress = super::BlockAddress; - type ExtrinsicAddress = super::ExtrinsicAddress; - - let e0 = ExtrinsicAddress::from_str("1234"); - let b0 = ExtrinsicAddress::from_str("3BfC20f0B9aFcAcE800D73D2191166FF16540258:5"); - let b1 = ExtrinsicAddress::from_str("1234:0"); - let b2 = ExtrinsicAddress::from_str("0 0"); - let b3 = ExtrinsicAddress::from_str("0x0012345f"); - - - assert_eq!(e0, Err("Extrinsic index missing: example \"5:0\"".into())); - assert_eq!(b0, Ok(ExtrinsicAddress::Block( - BlockAddress::Hash("3BfC20f0B9aFcAcE800D73D2191166FF16540258".parse().unwrap()), - 5 - ))); - assert_eq!(b1, Ok(ExtrinsicAddress::Block( - BlockAddress::Number(1234), - 0 - ))); - assert_eq!(b2, Ok(ExtrinsicAddress::Block( - BlockAddress::Number(0), - 0 - ))); - assert_eq!(b3, Ok(ExtrinsicAddress::Bytes(vec![0, 0x12, 0x34, 0x5f]))); - } -} diff --git a/client/db/Cargo.toml b/client/db/Cargo.toml index 70a0b19532593..f6a64e41b05e0 100644 --- a/client/db/Cargo.toml +++ b/client/db/Cargo.toml @@ -25,6 +25,9 @@ codec = { package = "parity-scale-codec", version = "1.3.4", features = ["derive blake2-rfc = "0.2.18" sc-client-api = { version = "2.0.0", path = "../api" } +sc-finality-grandpa = { version = "0.8.0", path = "../finality-grandpa" } +sp-finality-grandpa = { version = "2.0.0", path = "../../primitives/finality-grandpa" } +num-traits = "0.2.8" sp-arithmetic = { version = "2.0.0", path = "../../primitives/arithmetic" } sp-core = { version = "2.0.0", path = "../../primitives/core" } sp-runtime = { version = "2.0.0", path = "../../primitives/runtime" } diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index 8254e652f68b0..fb23aed7323a5 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -53,6 +53,8 @@ use parking_lot::{Mutex, RwLock}; use linked_hash_map::LinkedHashMap; use log::{trace, debug, warn}; +use sc_finality_grandpa::BlockNumberOps; // TODO remove (for test) +use num_traits::cast::AsPrimitive; // TODO remove (for test) use sc_client_api::{ UsageInfo, MemoryInfo, IoInfo, MemorySize, backend::{NewBlockState, PrunableStateChangesTrieStorage, ProvideChtRoots}, @@ -859,7 +861,9 @@ pub struct Backend { state_usage: Arc, } -impl Backend { +impl Backend + where NumberFor: BlockNumberOps, +{ /// Create a new instance of database backend. /// /// The pruning window is how old a block must be before the state is pruned. @@ -1050,6 +1054,45 @@ impl Backend { justification.encode(), ); } + + + // TODO remove and deps (only to test without writing rpc) + use std::convert::TryInto; + if let Ok(number32) = number.try_into() { + let interval = 100_000; + if number32 % interval == 0 && number32 != 0 { + let begin32 = number32 - interval; + let begin = if let Ok(begin) = begin32.try_into() { + begin + } else { + unreachable!() + }; + let head_begin = self.blockchain.expect_header(BlockId::Number(begin))?; + + let state = { + use sc_client_api::backend::Backend; + self.state_at(BlockId::Number(begin)).unwrap() + }; + + let current_authorities = state.storage(b"grandpa_authorities")? + .and_then(|encoded| sp_finality_grandpa::VersionedAuthorityList::decode(&mut encoded.as_slice()).ok()) + .map(|versioned| versioned.into()) + .ok_or(ClientError::InvalidAuthoritiesSet)?; + + + let current_set_id = unimplemented!(); + sc_finality_grandpa::finality_proof::prove_authority::< + Block, + BlockchainDb, + sc_finality_grandpa::GrandpaJustification, + >( + &self.blockchain, + head_begin.hash(), + current_set_id, + current_authorities, + )?; + } + } Ok((*hash, number, false, true)) } @@ -1440,7 +1483,9 @@ impl sc_client_api::backend::AuxStore for Backend where Block: Blo } } -impl sc_client_api::backend::Backend for Backend { +impl sc_client_api::backend::Backend for Backend + where NumberFor: BlockNumberOps, +{ type BlockImportOperation = BlockImportOperation; type Blockchain = BlockchainDb; type State = SyncingCachingState, Block>; @@ -1764,7 +1809,9 @@ impl sc_client_api::backend::Backend for Backend { } } -impl sc_client_api::backend::LocalBackend for Backend {} +impl sc_client_api::backend::LocalBackend for Backend +where NumberFor: BlockNumberOps, +{} #[cfg(test)] pub(crate) mod tests { diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index 54fd2c6d872a6..68d3eae4b6efb 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -52,7 +52,7 @@ use parity_scale_codec::{Encode, Decode}; use finality_grandpa::BlockNumberOps; use sp_runtime::{ Justification, generic::BlockId, - traits::{NumberFor, Block as BlockT, Header as HeaderT, One}, + traits::{NumberFor, Block as BlockT, Header as HeaderT, Zero, One}, }; use sp_core::storage::StorageKey; use sc_telemetry::{telemetry, CONSENSUS_INFO}; @@ -247,6 +247,23 @@ pub struct FinalityProofFragment { /// - all other fragments provide justifications for GRANDPA authorities set changes within requested range. type FinalityProof
= Vec>; +/// Single fragment of authority set proof. +/// +/// Finality for block B is proved by providing: +/// 1) headers of this block; +/// 2) the justification for the block containing a authority set change digest; +#[derive(Debug, PartialEq, Encode, Decode)] +pub(crate) struct AuthoritySetProofFragment { + /// 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>; + /// Finality proof request data. #[derive(Debug, Encode, Decode)] enum FinalityProofRequest { @@ -425,6 +442,93 @@ pub(crate) fn prove_finality, J>( } } +/// 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'. +/// TODO switch to pub(crate) +pub fn prove_authority, J>( + blockchain: &B, + begin: Block::Hash, + current_set_id: u64, // TODO remove after debugging + current_authorities: AuthorityList, // TODO remove after debugging +) -> ::sp_blockchain::Result> + where + J: ProvableJustification, + + NumberFor: BlockNumberOps, // TODO remove after debugging + B: BlockchainBackend, // TODO remove after debugging +{ + + let begin = BlockId::Hash(begin); + let begin_number = blockchain.block_number_from_id(&begin)? + .ok_or_else(|| ClientError::Msg("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::Msg("Missing last finalized block".to_string()))?; + + if begin_number > end_number { + return Err(ClientError::Msg("Unfinalized start for authority proof".to_string())); + } + + // TODO fetch bonding duration and store it to error when not from it. + + // TODO use a cache here and encode on slice. + let mut result = Vec::new(); + + let mut header = blockchain.expect_header(begin)?; + let mut index = *header.number() + One::one(); + + while index <= end_number { + 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 dest <= end_number { + result.push(header.clone()); + index = dest; + continue; + } + } + + if let Some(sp_finality_grandpa::ScheduledChange { + next_authorities: _, + delay, + }) = crate::import::find_scheduled_change::(&header) { + let dest = index + delay; + if dest <= end_number { + result.push(header.clone()); + index = dest; + continue; + } + } + + index = *header.number() + One::one(); + } + + if result.last().as_ref().map(|head| head.number()) != Some(&end_number) { + let header = blockchain.expect_header(end)?; + result.push(header); + } + + let proof = result.encode(); + + // TODO only for initial testing + check_authority_proof::( + current_set_id, + current_authorities, + proof.clone(), + )?; + + Ok(proof) +} + /// Check GRANDPA proof-of-finality for the given block. /// /// Returns the vector of headers that MUST be validated + imported @@ -483,35 +587,89 @@ pub(crate) fn check_finality_proof( Ok(effects) } +/// Check GRANDPA authority change sequence to assert finality of a target block. +/// +/// Returns the header of the target block. +pub(crate) fn check_authority_proof( + current_set_id: u64, + current_authorities: AuthorityList, + remote_proof: Vec, +) -> ClientResult<(Block::Header, u64, AuthorityList)> + where + NumberFor: BlockNumberOps, + J: ProvableJustification, +{ + // 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_authority_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 authority_sequence_next( - current_set_id: &mut u64, - current_authorities: &mut AuthorityList, - authorities_provider: &dyn AuthoritySetForFinalityChecker, - authorities_proof: StorageProof, // TODO not a storage proof -) -> ClientResult<(u64, AuthorityList)> +fn check_authority_proof_fragment( + current_set_id: u64, + current_authorities: &AuthorityList, + current_block: &NumberFor, + is_last: bool, + authorities_proof: &AuthoritySetProofFragment, +) -> ClientResult<(u64, AuthorityList, NumberFor)> where NumberFor: BlockNumberOps, + J: Decode + ProvableJustification, { - // TODO we get new authority set from the header log/digest. - // We get Forced Change and Scheduled change. - // - // Then for every log we also get header justification. - // This method check justification for the header against - // current set and update to next set afterward. - // - // TODO debug against value of stored authority set (each - // got a delay and forced got a block start: not sure why - // not using delay there). - let header = unimplemented!("We do not have header here"); - let block = unimplemented!("We do not have header here"); - *current_authorities = authorities_provider.check_authorities_proof( - block, - header, - authorities_proof, - )?; + let justification: J = Decode::decode(&mut authorities_proof.justification.as_slice()) + .map_err(|_| ClientError::JustificationDecode)?; + justification.verify(current_set_id, ¤t_authorities)?; + + if authorities_proof.header.number() <= current_block { + return Err(ClientError::Msg("Invalid authority warp proof".to_string())); + } + 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)); + } - *current_set_id += 1; + // only fragment with no change for target + if at_block.is_none() && !is_last { + return Err(ClientError::Msg("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())) + } } /// Check finality proof for the single block. @@ -583,7 +741,8 @@ impl AuthoritiesOrEffects
{ } /// Justification used to prove block finality. -pub(crate) trait ProvableJustification: Encode + Decode { +/// TODO switch back to pub(crate) +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<()>; diff --git a/client/finality-grandpa/src/import.rs b/client/finality-grandpa/src/import.rs index 89f9d0c16ad7c..c196267dc933d 100644 --- a/client/finality-grandpa/src/import.rs +++ b/client/finality-grandpa/src/import.rs @@ -182,7 +182,7 @@ impl<'a, Block: 'a + BlockT> Drop for PendingSetChanges<'a, Block> { } } -fn find_scheduled_change(header: &B::Header) +pub(crate) fn find_scheduled_change(header: &B::Header) -> Option>> { let id = OpaqueDigestItemId::Consensus(&GRANDPA_ENGINE_ID); @@ -197,7 +197,7 @@ fn find_scheduled_change(header: &B::Header) header.digest().convert_first(|l| l.try_to(id).and_then(filter_log)) } -fn find_forced_change(header: &B::Header) +pub(crate) fn find_forced_change(header: &B::Header) -> Option<(NumberFor, ScheduledChange>)> { let id = OpaqueDigestItemId::Consensus(&GRANDPA_ENGINE_ID); diff --git a/client/finality-grandpa/src/lib.rs b/client/finality-grandpa/src/lib.rs index ced101b8c8562..2dff3f43d10bd 100644 --- a/client/finality-grandpa/src/lib.rs +++ b/client/finality-grandpa/src/lib.rs @@ -113,7 +113,8 @@ mod authorities; mod aux_schema; mod communication; mod environment; -mod finality_proof; +// TODO remove pub +pub mod finality_proof; mod import; mod justification; mod notification; diff --git a/client/service/Cargo.toml b/client/service/Cargo.toml index 4350e1a2bf2a9..09ab35ea6d486 100644 --- a/client/service/Cargo.toml +++ b/client/service/Cargo.toml @@ -79,6 +79,7 @@ sp-tracing = { version = "2.0.0", path = "../../primitives/tracing" } tracing = "0.1.22" tracing-futures = { version = "0.2.4" } parity-util-mem = { version = "0.7.0", default-features = false, features = ["primitive-types"] } +finality-grandpa = "0.12.3" [target.'cfg(not(target_os = "unknown"))'.dependencies] tempfile = "3.1.0" diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index 52c1121d504df..b8b954cb98de6 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -45,8 +45,9 @@ use sc_network::config::{Role, OnDemand}; use sc_network::NetworkService; use sp_runtime::generic::BlockId; use sp_runtime::traits::{ - Block as BlockT, SaturatedConversion, HashFor, Zero, BlockIdTo, + Block as BlockT, SaturatedConversion, HashFor, Zero, BlockIdTo, NumberFor, }; +use finality_grandpa::BlockNumberOps; use sp_api::{ProvideRuntimeApi, CallApiAt}; use sc_executor::{NativeExecutor, NativeExecutionDispatch, RuntimeInfo}; use std::sync::Arc; @@ -260,6 +261,7 @@ pub fn new_full_client( config: &Configuration, ) -> Result, Error> where TBl: BlockT, + NumberFor: BlockNumberOps, TExecDisp: NativeExecutionDispatch + 'static, { new_full_parts(config).map(|parts| parts.0) @@ -270,6 +272,7 @@ pub fn new_full_parts( config: &Configuration, ) -> Result, Error> where TBl: BlockT, + NumberFor: BlockNumberOps, TExecDisp: NativeExecutionDispatch + 'static, { let keystore_container = KeystoreContainer::new(&config.keystore)?; @@ -408,6 +411,7 @@ pub fn new_client( where Block: BlockT, E: CodeExecutor + RuntimeInfo, + NumberFor: BlockNumberOps, { const CANONICALIZATION_DELAY: u64 = 4096; diff --git a/test-utils/client/Cargo.toml b/test-utils/client/Cargo.toml index 07d28660f6188..fd11abd478eaf 100644 --- a/test-utils/client/Cargo.toml +++ b/test-utils/client/Cargo.toml @@ -28,7 +28,7 @@ sc-service = { version = "0.8.0", default-features = false, features = ["test-he sp-blockchain = { version = "2.0.0", path = "../../primitives/blockchain" } sp-consensus = { version = "0.8.0", path = "../../primitives/consensus/common" } sp-core = { version = "2.0.0", path = "../../primitives/core" } -sp-keystore = { version = "0.8.0", path = "../../primitives/keystore" } sp-keyring = { version = "2.0.0", path = "../../primitives/keyring" } sp-runtime = { version = "2.0.0", path = "../../primitives/runtime" } sp-state-machine = { version = "0.8.0", path = "../../primitives/state-machine" } +finality-grandpa = "0.12.3" diff --git a/test-utils/client/src/lib.rs b/test-utils/client/src/lib.rs index bde2156e0cc88..e084cd7684c9e 100644 --- a/test-utils/client/src/lib.rs +++ b/test-utils/client/src/lib.rs @@ -34,6 +34,7 @@ pub use sp_keyring::{ sr25519::Keyring as Sr25519Keyring, }; pub use sp_keystore::{SyncCryptoStorePtr, SyncCryptoStore}; +use finality_grandpa::BlockNumberOps; pub use sp_runtime::{Storage, StorageChild}; pub use sp_state_machine::ExecutionStrategy; pub use sc_service::{RpcHandlers, RpcSession, client}; @@ -45,7 +46,7 @@ use std::collections::{HashSet, HashMap}; use futures::{future::{Future, FutureExt}, stream::StreamExt}; use serde::Deserialize; use sp_core::storage::ChildInfo; -use sp_runtime::{OpaqueExtrinsic, codec::Encode, traits::{Block as BlockT, BlakeTwo256}}; +use sp_runtime::{OpaqueExtrinsic, codec::Encode, traits::{Block as BlockT, BlakeTwo256, NumberFor}}; use sc_service::client::{LocalCallExecutor, ClientConfig}; use sc_client_api::BlockchainEvents; @@ -82,13 +83,18 @@ pub struct TestClientBuilder { } impl Default - for TestClientBuilder, G> { + for TestClientBuilder, G> + +where NumberFor: BlockNumberOps, +{ fn default() -> Self { Self::with_default_backend() } } -impl TestClientBuilder, G> { +impl TestClientBuilder, G> +where NumberFor: BlockNumberOps, +{ /// Create new `TestClientBuilder` with default backend. pub fn with_default_backend() -> Self { let backend = Arc::new(Backend::new_test(std::u32::MAX, std::u64::MAX)); From 771f689b4c78fa7a88339ddc8cf58a6c7c2618c8 Mon Sep 17 00:00:00 2001 From: cheme Date: Thu, 3 Sep 2020 21:14:55 +0200 Subject: [PATCH 05/34] get set id from storage directly (should use runtime to handler change). --- client/db/src/lib.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index fb23aed7323a5..0c04d2dbde1f5 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -54,7 +54,6 @@ use linked_hash_map::LinkedHashMap; use log::{trace, debug, warn}; use sc_finality_grandpa::BlockNumberOps; // TODO remove (for test) -use num_traits::cast::AsPrimitive; // TODO remove (for test) use sc_client_api::{ UsageInfo, MemoryInfo, IoInfo, MemorySize, backend::{NewBlockState, PrunableStateChangesTrieStorage, ProvideChtRoots}, @@ -1078,9 +1077,11 @@ impl Backend .and_then(|encoded| sp_finality_grandpa::VersionedAuthorityList::decode(&mut encoded.as_slice()).ok()) .map(|versioned| versioned.into()) .ok_or(ClientError::InvalidAuthoritiesSet)?; +//"0x2371e21684d2fae99bcb4d579242f74a8a2d09463effcc78a22d75b9cb87dffc" - - let current_set_id = unimplemented!(); + let current_set_id = state.storage(&[35u8, 113, 226, 22, 132, 210, 250, 233, 155, 203, 77, 87, 146, 66, 247, 74, 138, 45, 9, 70, 62, 255, 204, 120, 162, 45, 117, 185, 203, 135, 223, 252][..])? + .and_then(|encoded| u64::decode(&mut encoded.as_slice()).ok()) + .ok_or(ClientError::InvalidAuthoritiesSet)?; sc_finality_grandpa::finality_proof::prove_authority::< Block, BlockchainDb, From 9cb49077ab769222b88d4f6984ba957960261b03 Mon Sep 17 00:00:00 2001 From: cheme Date: Fri, 4 Sep 2020 10:15:03 +0200 Subject: [PATCH 06/34] move test to init --- client/db/src/lib.rs | 98 ++++++++++++++++++++++++++------------------ 1 file changed, 57 insertions(+), 41 deletions(-) diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index 0c04d2dbde1f5..1dede011c560b 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -922,7 +922,7 @@ impl Backend }, )?; - Ok(Backend { + let result = Backend { storage: Arc::new(storage_db), offchain_storage, changes_tries_storage, @@ -936,9 +936,64 @@ impl Backend is_archive: is_archive_pruning, io_stats: FrozenForDuration::new(std::time::Duration::from_secs(1)), state_usage: Arc::new(StateUsageStats::new()), - }) + }; + + result.test_it(1); + result.test_it(10_000); + result.test_it(30_000); + result.test_it(50_000); + result.test_it(100_000); + result.test_it(200_000); + result.test_it(300_000); + + Ok(result) } + fn test_it(&self, begin32: u32) + -> ClientResult<()> { + let last_finalized_hash = self.blockchain.meta.read().finalized_hash; + let last_fin_head = self.blockchain.expect_header(BlockId::Hash(last_finalized_hash.clone()))?; + let number = last_fin_head.number(); + + // TODO remove and deps (only to test without writing rpc) + use std::convert::TryInto; + if let Ok(number32) = number.clone().try_into() { + let interval = 10_000; + let begin = if let Ok(begin) = begin32.try_into() { + begin + } else { + unreachable!() + }; + let head_begin = self.blockchain.expect_header(BlockId::Number(begin))?; + + let state = { + use sc_client_api::backend::Backend; + self.state_at(BlockId::Number(begin)).unwrap() + }; + + let current_authorities = state.storage(b"grandpa_authorities")? + .and_then(|encoded| sp_finality_grandpa::VersionedAuthorityList::decode(&mut encoded.as_slice()).ok()) + .map(|versioned| versioned.into()) + .ok_or(ClientError::InvalidAuthoritiesSet)?; +//"0x2371e21684d2fae99bcb4d579242f74a8a2d09463effcc78a22d75b9cb87dffc" + + let current_set_id = state.storage(&[35u8, 113, 226, 22, 132, 210, 250, 233, 155, 203, 77, 87, 146, 66, 247, 74, 138, 45, 9, 70, 62, 255, 204, 120, 162, 45, 117, 185, 203, 135, 223, 252][..])? + .and_then(|encoded| u64::decode(&mut encoded.as_slice()).ok()) + .ok_or(ClientError::InvalidAuthoritiesSet)?; + sc_finality_grandpa::finality_proof::prove_authority::< + Block, + BlockchainDb, + sc_finality_grandpa::GrandpaJustification, + >( + &self.blockchain, + head_begin.hash(), + current_set_id, + current_authorities, + )?; + } + Ok(()) + } + /// Handle setting head within a transaction. `route_to` should be the last /// block that existed in the database. `best_to` should be the best block /// to be set. @@ -1055,45 +1110,6 @@ impl Backend } - // TODO remove and deps (only to test without writing rpc) - use std::convert::TryInto; - if let Ok(number32) = number.try_into() { - let interval = 100_000; - if number32 % interval == 0 && number32 != 0 { - let begin32 = number32 - interval; - let begin = if let Ok(begin) = begin32.try_into() { - begin - } else { - unreachable!() - }; - let head_begin = self.blockchain.expect_header(BlockId::Number(begin))?; - - let state = { - use sc_client_api::backend::Backend; - self.state_at(BlockId::Number(begin)).unwrap() - }; - - let current_authorities = state.storage(b"grandpa_authorities")? - .and_then(|encoded| sp_finality_grandpa::VersionedAuthorityList::decode(&mut encoded.as_slice()).ok()) - .map(|versioned| versioned.into()) - .ok_or(ClientError::InvalidAuthoritiesSet)?; -//"0x2371e21684d2fae99bcb4d579242f74a8a2d09463effcc78a22d75b9cb87dffc" - - let current_set_id = state.storage(&[35u8, 113, 226, 22, 132, 210, 250, 233, 155, 203, 77, 87, 146, 66, 247, 74, 138, 45, 9, 70, 62, 255, 204, 120, 162, 45, 117, 185, 203, 135, 223, 252][..])? - .and_then(|encoded| u64::decode(&mut encoded.as_slice()).ok()) - .ok_or(ClientError::InvalidAuthoritiesSet)?; - sc_finality_grandpa::finality_proof::prove_authority::< - Block, - BlockchainDb, - sc_finality_grandpa::GrandpaJustification, - >( - &self.blockchain, - head_begin.hash(), - current_set_id, - current_authorities, - )?; - } - } Ok((*hash, number, false, true)) } From e395804b0214a1310d4d6ef17e9901ec49b7d3b2 Mon Sep 17 00:00:00 2001 From: cheme Date: Fri, 4 Sep 2020 10:51:15 +0200 Subject: [PATCH 07/34] correct auth key --- client/db/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index 1dede011c560b..b315f1522251e 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -971,7 +971,7 @@ impl Backend self.state_at(BlockId::Number(begin)).unwrap() }; - let current_authorities = state.storage(b"grandpa_authorities")? + let current_authorities = state.storage(b":grandpa_authorities")? .and_then(|encoded| sp_finality_grandpa::VersionedAuthorityList::decode(&mut encoded.as_slice()).ok()) .map(|versioned| versioned.into()) .ok_or(ClientError::InvalidAuthoritiesSet)?; From 792f4c9e109c31d26f2596d3cb12854866cc9e4e Mon Sep 17 00:00:00 2001 From: cheme Date: Fri, 4 Sep 2020 11:15:45 +0200 Subject: [PATCH 08/34] fix iteration --- client/finality-grandpa/src/finality_proof.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index 68d3eae4b6efb..8e91f0bec4be4 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -492,7 +492,11 @@ pub fn prove_authority, J>( let dest = block_number + delay; if dest <= end_number { result.push(header.clone()); + let inc = delay == Zero::zero() && block_number == index; index = dest; + if inc { + index += One::one(); + } continue; } } @@ -505,6 +509,9 @@ pub fn prove_authority, J>( if dest <= end_number { result.push(header.clone()); index = dest; + if delay == Zero::zero() { + index += One::one(); + } continue; } } From ef36090a70b6b82145df6c797963d8d60eebb7da Mon Sep 17 00:00:00 2001 From: cheme Date: Fri, 4 Sep 2020 11:41:59 +0200 Subject: [PATCH 09/34] Correct proof content --- client/finality-grandpa/src/finality_proof.rs | 30 ++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index 8e91f0bec4be4..f43b590cbfabb 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -491,7 +491,15 @@ pub fn prove_authority, J>( })) = crate::import::find_forced_change::(&header) { let dest = block_number + delay; if dest <= end_number { - result.push(header.clone()); + if let Some(justification) = blockchain.justification(BlockId::Number(index.clone()))? { + result.push(AuthoritySetProofFragment { + header: header.clone(), + justification, + }); + } else { + println!("should be unreachable"); + } + let inc = delay == Zero::zero() && block_number == index; index = dest; if inc { @@ -507,7 +515,14 @@ pub fn prove_authority, J>( }) = crate::import::find_scheduled_change::(&header) { let dest = index + delay; if dest <= end_number { - result.push(header.clone()); + if let Some(justification) = blockchain.justification(BlockId::Number(index.clone()))? { + result.push(AuthoritySetProofFragment { + header: header.clone(), + justification, + }); + } else { + println!("should be unreachable"); + } index = dest; if delay == Zero::zero() { index += One::one(); @@ -519,9 +534,16 @@ pub fn prove_authority, J>( index = *header.number() + One::one(); } - if result.last().as_ref().map(|head| head.number()) != Some(&end_number) { + if result.last().as_ref().map(|head| head.header.number()) != Some(&end_number) { let header = blockchain.expect_header(end)?; - result.push(header); + if let Some(justification) = blockchain.justification(BlockId::Number(end_number.clone()))? { + result.push(AuthoritySetProofFragment { + header: header.clone(), + justification, + }); + } else { + println!("should be unreachable"); + } } let proof = result.encode(); From d9a6089140b5019fde109e6cfdb4d13ed36a60ae Mon Sep 17 00:00:00 2001 From: cheme Date: Fri, 4 Sep 2020 12:22:54 +0200 Subject: [PATCH 10/34] actually update block number. --- client/finality-grandpa/src/finality_proof.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index f43b590cbfabb..f723da3dd7d04 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -674,6 +674,7 @@ fn check_authority_proof_fragment( if authorities_proof.header.number() <= current_block { return Err(ClientError::Msg("Invalid authority warp proof".to_string())); } + current_block = authorities_proof.header.number(); let mut at_block = None; if let Some(sp_finality_grandpa::ScheduledChange { next_authorities, From 989bda28718ac76f2782b83226c4a1d2c40861a9 Mon Sep 17 00:00:00 2001 From: cheme Date: Fri, 4 Sep 2020 12:47:27 +0200 Subject: [PATCH 11/34] actually check last justif against its header --- client/db/src/lib.rs | 15 +++++++++- client/finality-grandpa/src/finality_proof.rs | 30 +++++++------------ client/finality-grandpa/src/justification.rs | 2 +- 3 files changed, 25 insertions(+), 22 deletions(-) diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index b315f1522251e..a20d0fd963cf1 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -980,16 +980,29 @@ impl Backend let current_set_id = state.storage(&[35u8, 113, 226, 22, 132, 210, 250, 233, 155, 203, 77, 87, 146, 66, 247, 74, 138, 45, 9, 70, 62, 255, 204, 120, 162, 45, 117, 185, 203, 135, 223, 252][..])? .and_then(|encoded| u64::decode(&mut encoded.as_slice()).ok()) .ok_or(ClientError::InvalidAuthoritiesSet)?; - sc_finality_grandpa::finality_proof::prove_authority::< + let proof = sc_finality_grandpa::finality_proof::prove_authority::< Block, BlockchainDb, sc_finality_grandpa::GrandpaJustification, >( &self.blockchain, head_begin.hash(), + )?; + let (head_target, nb, authorities, justification) = sc_finality_grandpa::finality_proof::check_authority_proof::< + Block, + sc_finality_grandpa::GrandpaJustification, + >( current_set_id, current_authorities, + proof.clone(), )?; + + let is_valid = justification.commit.target_hash == head_target.hash(); + let is_valid = &justification.commit.target_number == head_target.number(); + let is_valid = head_target.hash() == last_fin_head.hash(); + let is_valid = head_target.number() == last_fin_head.number(); + let is_valid = head_target == last_fin_head; + } Ok(()) } diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index f723da3dd7d04..a0a16cff8b22d 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -452,8 +452,6 @@ pub(crate) fn prove_finality, J>( pub fn prove_authority, J>( blockchain: &B, begin: Block::Hash, - current_set_id: u64, // TODO remove after debugging - current_authorities: AuthorityList, // TODO remove after debugging ) -> ::sp_blockchain::Result> where J: ProvableJustification, @@ -546,16 +544,7 @@ pub fn prove_authority, J>( } } - let proof = result.encode(); - - // TODO only for initial testing - check_authority_proof::( - current_set_id, - current_authorities, - proof.clone(), - )?; - - Ok(proof) + Ok(result.encode()) } /// Check GRANDPA proof-of-finality for the given block. @@ -619,11 +608,11 @@ pub(crate) fn check_finality_proof( /// Check GRANDPA authority change sequence to assert finality of a target block. /// /// Returns the header of the target block. -pub(crate) fn check_authority_proof( +pub fn check_authority_proof( current_set_id: u64, current_authorities: AuthorityList, remote_proof: Vec, -) -> ClientResult<(Block::Header, u64, AuthorityList)> +) -> ClientResult<(Block::Header, u64, AuthorityList, J)> where NumberFor: BlockNumberOps, J: ProvableJustification, @@ -638,7 +627,7 @@ pub(crate) fn check_authority_proof( for (ix, fragment) in proof.into_iter().enumerate() { let is_last = ix == last; - result = check_authority_proof_fragment::( + let check_result = check_authority_proof_fragment::( result.0, &result.1, &result.2, @@ -646,8 +635,9 @@ pub(crate) fn check_authority_proof( &fragment, )?; + result = check_result.0; if is_last { - return Ok((fragment.header, result.0, result.1)) + return Ok((fragment.header, result.0, result.1, check_result.1)) } } @@ -662,7 +652,7 @@ fn check_authority_proof_fragment( current_block: &NumberFor, is_last: bool, authorities_proof: &AuthoritySetProofFragment, -) -> ClientResult<(u64, AuthorityList, NumberFor)> +) -> ClientResult<((u64, AuthorityList, NumberFor), J)> where NumberFor: BlockNumberOps, J: Decode + ProvableJustification, @@ -674,7 +664,7 @@ fn check_authority_proof_fragment( if authorities_proof.header.number() <= current_block { return Err(ClientError::Msg("Invalid authority warp proof".to_string())); } - current_block = authorities_proof.header.number(); + let mut current_block = authorities_proof.header.number(); let mut at_block = None; if let Some(sp_finality_grandpa::ScheduledChange { next_authorities, @@ -696,9 +686,9 @@ fn check_authority_proof_fragment( return Err(ClientError::Msg("Invalid authority warp proof".to_string())); } if let Some((at_block, next_authorities)) = at_block { - Ok((current_set_id + 1, next_authorities, at_block)) + Ok(((current_set_id + 1, next_authorities, at_block), justification)) } else { - Ok((current_set_id, current_authorities.clone(), current_block.clone())) + Ok(((current_set_id, current_authorities.clone(), current_block.clone()), justification)) } } diff --git a/client/finality-grandpa/src/justification.rs b/client/finality-grandpa/src/justification.rs index d5ca92d50e937..5234b3d5ee179 100644 --- a/client/finality-grandpa/src/justification.rs +++ b/client/finality-grandpa/src/justification.rs @@ -40,7 +40,7 @@ use crate::{Commit, Error}; #[derive(Clone, Encode, Decode, PartialEq, Eq, Debug)] pub struct GrandpaJustification { round: u64, - pub(crate) commit: Commit, + pub commit: Commit, votes_ancestries: Vec, } From f8fc255f40a763522c18dc286ed9f3d098dc53f2 Mon Sep 17 00:00:00 2001 From: cheme Date: Fri, 4 Sep 2020 15:20:43 +0200 Subject: [PATCH 12/34] justification relation to new authorities through header hash check is needed here. This assumes the hash from header is calculated. --- client/db/src/lib.rs | 4 +- client/finality-grandpa/src/finality_proof.rs | 43 ++++++++++++++----- client/finality-grandpa/src/justification.rs | 2 +- 3 files changed, 35 insertions(+), 14 deletions(-) diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index a20d0fd963cf1..dd3f56e22abe1 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -988,7 +988,7 @@ impl Backend &self.blockchain, head_begin.hash(), )?; - let (head_target, nb, authorities, justification) = sc_finality_grandpa::finality_proof::check_authority_proof::< + let (head_target, nb, authorities) = sc_finality_grandpa::finality_proof::check_authority_proof::< Block, sc_finality_grandpa::GrandpaJustification, >( @@ -997,8 +997,6 @@ impl Backend proof.clone(), )?; - let is_valid = justification.commit.target_hash == head_target.hash(); - let is_valid = &justification.commit.target_number == head_target.number(); let is_valid = head_target.hash() == last_fin_head.hash(); let is_valid = head_target.number() == last_fin_head.number(); let is_valid = head_target == last_fin_head; diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index a0a16cff8b22d..4af131001b054 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -612,10 +612,10 @@ pub fn check_authority_proof( current_set_id: u64, current_authorities: AuthorityList, remote_proof: Vec, -) -> ClientResult<(Block::Header, u64, AuthorityList, J)> +) -> ClientResult<(Block::Header, u64, AuthorityList)> where NumberFor: BlockNumberOps, - J: ProvableJustification, + J: Decode + ProvableJustification + BlockJustification, { // decode finality proof let proof = AuthoritySetProof::::decode(&mut &remote_proof[..]) @@ -627,7 +627,7 @@ pub fn check_authority_proof( for (ix, fragment) in proof.into_iter().enumerate() { let is_last = ix == last; - let check_result = check_authority_proof_fragment::( + result = check_authority_proof_fragment::( result.0, &result.1, &result.2, @@ -635,9 +635,8 @@ pub fn check_authority_proof( &fragment, )?; - result = check_result.0; if is_last { - return Ok((fragment.header, result.0, result.1, check_result.1)) + return Ok((fragment.header, result.0, result.1)) } } @@ -652,19 +651,25 @@ fn check_authority_proof_fragment( current_block: &NumberFor, is_last: bool, authorities_proof: &AuthoritySetProofFragment, -) -> ClientResult<((u64, AuthorityList, NumberFor), J)> +) -> ClientResult<(u64, AuthorityList, NumberFor)> where NumberFor: BlockNumberOps, - J: Decode + ProvableJustification, + 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::Msg("Invalid authority warp proof, justification do not match header".to_string())); + } + if authorities_proof.header.number() <= current_block { return Err(ClientError::Msg("Invalid authority warp proof".to_string())); } - let mut current_block = authorities_proof.header.number(); + let current_block = authorities_proof.header.number(); let mut at_block = None; if let Some(sp_finality_grandpa::ScheduledChange { next_authorities, @@ -686,9 +691,9 @@ fn check_authority_proof_fragment( return Err(ClientError::Msg("Invalid authority warp proof".to_string())); } if let Some((at_block, next_authorities)) = at_block { - Ok(((current_set_id + 1, next_authorities, at_block), justification)) + Ok((current_set_id + 1, next_authorities, at_block)) } else { - Ok(((current_set_id, current_authorities.clone(), current_block.clone()), justification)) + Ok((current_set_id, current_authorities.clone(), current_block.clone())) } } @@ -760,6 +765,15 @@ impl AuthoritiesOrEffects
{ } } +/// Block info extracted from the justification. +pub trait BlockJustification { + /// Block number justified. + fn number(&self) -> Header::Number; + + /// Block hash justified. + fn hash(&self) -> Header::Hash; +} + /// Justification used to prove block finality. /// TODO switch back to pub(crate) pub trait ProvableJustification: Encode + Decode { @@ -792,6 +806,15 @@ impl ProvableJustification for GrandpaJustificatio } } +impl BlockJustification for GrandpaJustification { + fn number(&self) -> NumberFor { + self.commit.target_number.clone() + } + fn hash(&self) -> Block::Hash { + self.commit.target_hash.clone() + } +} + #[cfg(test)] pub(crate) mod tests { use substrate_test_runtime_client::runtime::{Block, Header, H256}; diff --git a/client/finality-grandpa/src/justification.rs b/client/finality-grandpa/src/justification.rs index 5234b3d5ee179..d5ca92d50e937 100644 --- a/client/finality-grandpa/src/justification.rs +++ b/client/finality-grandpa/src/justification.rs @@ -40,7 +40,7 @@ use crate::{Commit, Error}; #[derive(Clone, Encode, Decode, PartialEq, Eq, Debug)] pub struct GrandpaJustification { round: u64, - pub commit: Commit, + pub(crate) commit: Commit, votes_ancestries: Vec, } From 80fac43d0891ab0627d5bf44c24a10e521a0990e Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Mon, 7 Dec 2020 11:22:57 +0100 Subject: [PATCH 13/34] Few changes --- Cargo.lock | 1 + client/db/src/lib.rs | 7 +++---- client/finality-grandpa/src/finality_proof.rs | 12 ++++++------ test-utils/client/Cargo.toml | 1 + 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 048772d9c6c2b..d4c9c9eef44fe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8879,6 +8879,7 @@ dependencies = [ "sp-consensus", "sp-core", "sp-keyring", + "sp-keystore", "sp-runtime", "sp-state-machine", ] diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index dd3f56e22abe1..b786fef3eb831 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -971,13 +971,14 @@ impl Backend self.state_at(BlockId::Number(begin)).unwrap() }; - let current_authorities = state.storage(b":grandpa_authorities")? + let current_authorities = state.storage(b":grandpa_authorities").map_err(ClientError::Storage)? .and_then(|encoded| sp_finality_grandpa::VersionedAuthorityList::decode(&mut encoded.as_slice()).ok()) .map(|versioned| versioned.into()) .ok_or(ClientError::InvalidAuthoritiesSet)?; //"0x2371e21684d2fae99bcb4d579242f74a8a2d09463effcc78a22d75b9cb87dffc" - let current_set_id = state.storage(&[35u8, 113, 226, 22, 132, 210, 250, 233, 155, 203, 77, 87, 146, 66, 247, 74, 138, 45, 9, 70, 62, 255, 204, 120, 162, 45, 117, 185, 203, 135, 223, 252][..])? + let current_set_id = state.storage(&[35u8, 113, 226, 22, 132, 210, 250, 233, 155, 203, 77, 87, 146, 66, 247, 74, 138, 45, 9, 70, 62, 255, 204, 120, 162, 45, 117, 185, 203, 135, 223, 252][..]) + .map_err(ClientError::Storage)? .and_then(|encoded| u64::decode(&mut encoded.as_slice()).ok()) .ok_or(ClientError::InvalidAuthoritiesSet)?; let proof = sc_finality_grandpa::finality_proof::prove_authority::< @@ -1119,8 +1120,6 @@ impl Backend justification.encode(), ); } - - Ok((*hash, number, false, true)) } diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index 4af131001b054..5625297ceb5ac 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -462,14 +462,14 @@ pub fn prove_authority, J>( let begin = BlockId::Hash(begin); let begin_number = blockchain.block_number_from_id(&begin)? - .ok_or_else(|| ClientError::Msg("Missing start block".to_string()))?; + .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::Msg("Missing last finalized block".to_string()))?; + .ok_or_else(|| ClientError::Backend("Missing last finalized block".to_string()))?; if begin_number > end_number { - return Err(ClientError::Msg("Unfinalized start for authority proof".to_string())); + return Err(ClientError::Backend("Unfinalized start for authority proof".to_string())); } // TODO fetch bonding duration and store it to error when not from it. @@ -663,11 +663,11 @@ fn check_authority_proof_fragment( // 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::Msg("Invalid authority warp proof, justification do not match header".to_string())); + return Err(ClientError::Backend("Invalid authority warp proof, justification do not match header".to_string())); } if authorities_proof.header.number() <= current_block { - return Err(ClientError::Msg("Invalid authority warp proof".to_string())); + return Err(ClientError::Backend("Invalid authority warp proof".to_string())); } let current_block = authorities_proof.header.number(); let mut at_block = None; @@ -688,7 +688,7 @@ fn check_authority_proof_fragment( // only fragment with no change for target if at_block.is_none() && !is_last { - return Err(ClientError::Msg("Invalid authority warp proof".to_string())); + 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)) diff --git a/test-utils/client/Cargo.toml b/test-utils/client/Cargo.toml index fd11abd478eaf..454b7bd0594e5 100644 --- a/test-utils/client/Cargo.toml +++ b/test-utils/client/Cargo.toml @@ -28,6 +28,7 @@ sc-service = { version = "0.8.0", default-features = false, features = ["test-he sp-blockchain = { version = "2.0.0", path = "../../primitives/blockchain" } sp-consensus = { version = "0.8.0", path = "../../primitives/consensus/common" } sp-core = { version = "2.0.0", path = "../../primitives/core" } +sp-keystore = { version = "0.8.0", path = "../../primitives/keystore" } sp-keyring = { version = "2.0.0", path = "../../primitives/keyring" } sp-runtime = { version = "2.0.0", path = "../../primitives/runtime" } sp-state-machine = { version = "0.8.0", path = "../../primitives/state-machine" } From fec784e52361f1c787217311df6b5841d0bc02b4 Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Mon, 7 Dec 2020 12:29:06 +0100 Subject: [PATCH 14/34] Connected up cheme's branch --- Cargo.lock | 18 ++ Cargo.toml | 1 + bin/node-template/node/src/service.rs | 2 + bin/node/cli/src/service.rs | 2 + client/grandpa-warp-sync/Cargo.toml | 26 +++ client/grandpa-warp-sync/src/lib.rs | 132 +++++++++++ .../src/grandpa_warp_sync_request_handler.rs | 220 ------------------ client/network/src/lib.rs | 3 +- client/service/Cargo.toml | 1 + client/service/src/builder.rs | 18 +- 10 files changed, 194 insertions(+), 229 deletions(-) create mode 100644 client/grandpa-warp-sync/Cargo.toml create mode 100644 client/grandpa-warp-sync/src/lib.rs delete mode 100644 client/network/src/grandpa_warp_sync_request_handler.rs diff --git a/Cargo.lock b/Cargo.lock index d4c9c9eef44fe..c64b10f884162 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6915,6 +6915,23 @@ dependencies = [ "substrate-test-runtime-client", ] +[[package]] +name = "sc-grandpa-warp-sync" +version = "0.8.0" +dependencies = [ + "derive_more", + "futures 0.3.8", + "log", + "num-traits", + "parity-scale-codec", + "prost", + "sc-client-api", + "sc-finality-grandpa", + "sc-network", + "sp-blockchain", + "sp-runtime", +] + [[package]] name = "sc-informant" version = "0.8.0" @@ -7252,6 +7269,7 @@ dependencies = [ "sc-client-db", "sc-executor", "sc-finality-grandpa", + "sc-grandpa-warp-sync", "sc-informant", "sc-keystore", "sc-light", diff --git a/Cargo.toml b/Cargo.toml index 6a007a209f1fa..b025bc1d73b85 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,6 +38,7 @@ members = [ "client/executor/wasmtime", "client/executor/runtime-test", "client/finality-grandpa", + "client/grandpa-warp-sync", "client/informant", "client/light", "client/tracing", diff --git a/bin/node-template/node/src/service.rs b/bin/node-template/node/src/service.rs index 1fa1a372a05d3..30a5520cc166d 100644 --- a/bin/node-template/node/src/service.rs +++ b/bin/node-template/node/src/service.rs @@ -92,6 +92,7 @@ pub fn new_full(mut config: Configuration) -> Result sc_service::build_network(sc_service::BuildNetworkParams { config: &config, client: client.clone(), + backend: backend.clone(), transaction_pool: transaction_pool.clone(), spawn_handle: task_manager.spawn_handle(), import_queue, @@ -261,6 +262,7 @@ pub fn new_light(mut config: Configuration) -> Result sc_service::build_network(sc_service::BuildNetworkParams { config: &config, client: client.clone(), + backend: backend.clone(), transaction_pool: transaction_pool.clone(), spawn_handle: task_manager.spawn_handle(), import_queue, diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index 5eb8e35e69ec5..f2f08e64ce952 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -184,6 +184,7 @@ pub fn new_full_base( sc_service::build_network(sc_service::BuildNetworkParams { config: &config, client: client.clone(), + backend: backend.clone(), transaction_pool: transaction_pool.clone(), spawn_handle: task_manager.spawn_handle(), import_queue, @@ -389,6 +390,7 @@ pub fn new_light_base(mut config: Configuration) -> Result<( sc_service::build_network(sc_service::BuildNetworkParams { config: &config, client: client.clone(), + backend: backend.clone(), transaction_pool: transaction_pool.clone(), spawn_handle: task_manager.spawn_handle(), import_queue, diff --git a/client/grandpa-warp-sync/Cargo.toml b/client/grandpa-warp-sync/Cargo.toml new file mode 100644 index 0000000000000..770d8081b112e --- /dev/null +++ b/client/grandpa-warp-sync/Cargo.toml @@ -0,0 +1,26 @@ +[package] +description = "..." +name = "sc-grandpa-warp-sync" +version = "0.8.0" +license = "GPL-3.0-or-later WITH Classpath-exception-2.0" +authors = ["Parity Technologies "] +edition = "2018" +publish = false +homepage = "https://substrate.dev" +repository = "https://github.com/paritytech/substrate/" + +[package.metadata.docs.rs] +targets = ["x86_64-unknown-linux-gnu"] + +[dependencies] +sc-network = { version = "0.8.0", path = "../network" } +sc-finality-grandpa = { version = "0.8.0", path = "../finality-grandpa" } +sp-runtime = { version = "2.0.0", path = "../../primitives/runtime" } +sp-blockchain = { version = "2.0.0", path = "../../primitives/blockchain" } +sc-client-api = { version = "2.0.0", path = "../api" } +futures = "0.3.8" +log = "0.4.11" +derive_more = "0.99.11" +codec = { package = "parity-scale-codec", version = "1.3.5" } +prost = "0.6.1" +num-traits = "0.2.14" diff --git a/client/grandpa-warp-sync/src/lib.rs b/client/grandpa-warp-sync/src/lib.rs new file mode 100644 index 0000000000000..9b380fb711e9f --- /dev/null +++ b/client/grandpa-warp-sync/src/lib.rs @@ -0,0 +1,132 @@ +// Copyright 2020 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +//! Helper for handling (i.e. answering) grandpa warp sync requests from a remote peer via the +//! [`crate::request_responses::RequestResponsesBehaviour`]. + +use codec::Decode; +use sc_network::config::ProtocolId; +use sc_network::request_responses::{IncomingRequest, ProtocolConfig}; +use sc_client_api::Backend; +use sc_finality_grandpa::GrandpaJustification; +use sp_runtime::traits::NumberFor; +use futures::channel::{mpsc, oneshot}; +use futures::stream::StreamExt; +use log::debug; +use sp_runtime::traits::Block as BlockT; +use std::time::Duration; +use std::sync::Arc; + +const LOG_TARGET: &str = "grandpa-warp-sync-request-handler"; + +/// Generates a [`ProtocolConfig`] for the block request protocol, refusing incoming requests. +pub fn generate_protocol_config(protocol_id: ProtocolId) -> ProtocolConfig { + ProtocolConfig { + name: generate_protocol_name(protocol_id).into(), + max_request_size: 1024 * 1024, + max_response_size: 16 * 1024 * 1024, + request_timeout: Duration::from_secs(40), + inbound_queue: None, + } +} + +/// Generate the block protocol name from chain specific protocol identifier. +fn generate_protocol_name(protocol_id: ProtocolId) -> String { + let mut s = String::new(); + s.push_str("/"); + s.push_str(protocol_id.as_ref()); + s.push_str("/sync/warp"); + s +} + +#[derive(codec::Decode)] +struct Request { + begin: B::Hash +} + +/// Handler for incoming block requests from a remote peer. +pub struct GrandpaWarpSyncRequestHandler { + backend: Arc, + request_receiver: mpsc::Receiver, + _phantom: std::marker::PhantomData +} + +impl> GrandpaWarpSyncRequestHandler { + /// Create a new [`BlockRequestHandler`]. + pub fn new(protocol_id: ProtocolId, backend: Arc) -> (Self, ProtocolConfig) { + // Rate of arrival multiplied with the waiting time in the queue equals the queue length. + // + // An average Polkadot sentry node serves less than 5 requests per second. The 95th percentile + // serving a request is less than 2 second. Thus one would estimate the queue length to be + // below 10. + // + // Choosing 20 as the queue length to give some additional buffer. + let (tx, request_receiver) = mpsc::channel(20); + + let mut protocol_config = generate_protocol_config(protocol_id); + protocol_config.inbound_queue = Some(tx); + + (Self { backend, request_receiver, _phantom: std::marker::PhantomData }, protocol_config) + } + + fn handle_request( + &self, + payload: Vec, + pending_response: oneshot::Sender> + ) -> Result<(), HandleRequestError> + where NumberFor: sc_finality_grandpa::BlockNumberOps, + { + let request = Request::::decode(&mut &payload[..])?; + + let response = sc_finality_grandpa::finality_proof::prove_authority::<_, _, GrandpaJustification>( + self.backend.blockchain(), request.begin, + )?; + + pending_response.send(response) + .map_err(|_| HandleRequestError::SendResponse) + } + + /// Run [`BlockRequestHandler`]. + pub async fn run(mut self) + where NumberFor: sc_finality_grandpa::BlockNumberOps, + { + while let Some(request) = self.request_receiver.next().await { + let IncomingRequest { peer, payload, pending_response } = request; + + match self.handle_request(payload, pending_response) { + Ok(()) => debug!(target: LOG_TARGET, "Handled block request from {}.", peer), + Err(e) => debug!( + target: LOG_TARGET, + "Failed to handle block request from {}: {}", + peer, e, + ), + } + } + } +} + +#[derive(derive_more::Display, derive_more::From)] +enum HandleRequestError { + #[display(fmt = "Failed to decode request: {}.", _0)] + DecodeProto(prost::DecodeError), + #[display(fmt = "Failed to encode response: {}.", _0)] + EncodeProto(prost::EncodeError), + #[display(fmt = "Failed to decode block hash: {}.", _0)] + DecodeScale(codec::Error), + Client(sp_blockchain::Error), + #[display(fmt = "Failed to send response.")] + SendResponse, +} diff --git a/client/network/src/grandpa_warp_sync_request_handler.rs b/client/network/src/grandpa_warp_sync_request_handler.rs deleted file mode 100644 index a0cc5abcff69f..0000000000000 --- a/client/network/src/grandpa_warp_sync_request_handler.rs +++ /dev/null @@ -1,220 +0,0 @@ -// Copyright 2020 Parity Technologies (UK) Ltd. -// This file is part of Substrate. - -// Substrate is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// Substrate is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with Substrate. If not, see . - -//! Helper for handling (i.e. answering) grandpa warp sync requests from a remote peer via the -//! [`crate::request_responses::RequestResponsesBehaviour`]. - -use codec::{Encode, Decode}; -use crate::chain::Client; -use crate::config::ProtocolId; -use crate::protocol::{message::BlockAttributes}; -use crate::request_responses::{IncomingRequest, ProtocolConfig}; -use crate::schema::v1::block_request::FromBlock; -use crate::schema::v1::{BlockResponse, Direction}; -use futures::channel::{mpsc, oneshot}; -use futures::stream::StreamExt; -use log::debug; -use prost::Message; -use sp_runtime::generic::BlockId; -use sp_runtime::traits::{Block as BlockT, Header, One, Zero}; -use std::cmp::min; -use std::sync::{Arc}; -use std::time::Duration; - -const LOG_TARGET: &str = "grandpa-warp-sync-request-handler"; -const MAX_BLOCKS_IN_RESPONSE: usize = 128; -const MAX_BODY_BYTES: usize = 8 * 1024 * 1024; - -/// Generates a [`ProtocolConfig`] for the block request protocol, refusing incoming requests. -pub fn generate_protocol_config(protocol_id: ProtocolId) -> ProtocolConfig { - ProtocolConfig { - name: generate_protocol_name(protocol_id).into(), - max_request_size: 1024 * 1024, - max_response_size: 16 * 1024 * 1024, - request_timeout: Duration::from_secs(40), - inbound_queue: None, - } -} - -/// Generate the block protocol name from chain specific protocol identifier. -fn generate_protocol_name(protocol_id: ProtocolId) -> String { - let mut s = String::new(); - s.push_str("/"); - s.push_str(protocol_id.as_ref()); - s.push_str("/sync/warp"); - s -} - -/// Handler for incoming block requests from a remote peer. -pub struct GrandpaWarpSyncRequestHandler { - client: Arc>, - request_receiver: mpsc::Receiver, -} - -impl GrandpaWarpSyncRequestHandler { - /// Create a new [`BlockRequestHandler`]. - pub fn new(protocol_id: ProtocolId, client: Arc>) -> (Self, ProtocolConfig) { - // Rate of arrival multiplied with the waiting time in the queue equals the queue length. - // - // An average Polkadot sentry node serves less than 5 requests per second. The 95th percentile - // serving a request is less than 2 second. Thus one would estimate the queue length to be - // below 10. - // - // Choosing 20 as the queue length to give some additional buffer. - let (tx, request_receiver) = mpsc::channel(20); - - let mut protocol_config = generate_protocol_config(protocol_id); - protocol_config.inbound_queue = Some(tx); - - (Self { client, request_receiver }, protocol_config) - } - - fn handle_request( - &self, - payload: Vec, - pending_response: oneshot::Sender> - ) -> Result<(), HandleRequestError> { - let request = crate::schema::v1::BlockRequest::decode(&payload[..])?; - - let from_block_id = match request.from_block.ok_or(HandleRequestError::MissingFromField)? { - FromBlock::Hash(ref h) => { - let h = Decode::decode(&mut h.as_ref())?; - BlockId::::Hash(h) - } - FromBlock::Number(ref n) => { - let n = Decode::decode(&mut n.as_ref())?; - BlockId::::Number(n) - } - }; - - let max_blocks = if request.max_blocks == 0 { - MAX_BLOCKS_IN_RESPONSE - } else { - min(request.max_blocks as usize, MAX_BLOCKS_IN_RESPONSE) - }; - - let direction = Direction::from_i32(request.direction) - .ok_or(HandleRequestError::ParseDirection)?; - let attributes = BlockAttributes::from_be_u32(request.fields)?; - let get_header = attributes.contains(BlockAttributes::HEADER); - let get_body = attributes.contains(BlockAttributes::BODY); - let get_justification = attributes.contains(BlockAttributes::JUSTIFICATION); - - let mut blocks = Vec::new(); - let mut block_id = from_block_id; - - let mut total_size: usize = 0; - while let Some(header) = self.client.header(block_id).unwrap_or(None) { - let number = *header.number(); - let hash = header.hash(); - let parent_hash = *header.parent_hash(); - let justification = if get_justification { - self.client.justification(&BlockId::Hash(hash))? - } else { - None - }; - let is_empty_justification = justification.as_ref().map(|j| j.is_empty()).unwrap_or(false); - - let body = if get_body { - match self.client.block_body(&BlockId::Hash(hash))? { - Some(mut extrinsics) => extrinsics.iter_mut() - .map(|extrinsic| extrinsic.encode()) - .collect(), - None => { - log::trace!(target: "sync", "Missing data for block request."); - break; - } - } - } else { - Vec::new() - }; - - let block_data = crate::schema::v1::BlockData { - hash: hash.encode(), - header: if get_header { - header.encode() - } else { - Vec::new() - }, - body, - receipt: Vec::new(), - message_queue: Vec::new(), - justification: justification.unwrap_or_default(), - is_empty_justification, - }; - - total_size += block_data.body.len(); - blocks.push(block_data); - - if blocks.len() >= max_blocks as usize || total_size > MAX_BODY_BYTES { - break - } - - match direction { - Direction::Ascending => { - block_id = BlockId::Number(number + One::one()) - } - Direction::Descending => { - if number.is_zero() { - break - } - block_id = BlockId::Hash(parent_hash) - } - } - } - - let res = BlockResponse { blocks }; - - let mut data = Vec::with_capacity(res.encoded_len()); - res.encode(&mut data)?; - - pending_response.send(data) - .map_err(|_| HandleRequestError::SendResponse) - } - - /// Run [`BlockRequestHandler`]. - pub async fn run(mut self) { - while let Some(request) = self.request_receiver.next().await { - let IncomingRequest { peer, payload, pending_response } = request; - - match self.handle_request(payload, pending_response) { - Ok(()) => debug!(target: LOG_TARGET, "Handled block request from {}.", peer), - Err(e) => debug!( - target: LOG_TARGET, - "Failed to handle block request from {}: {}", - peer, e, - ), - } - } - } -} - -#[derive(derive_more::Display, derive_more::From)] -enum HandleRequestError { - #[display(fmt = "Failed to decode request: {}.", _0)] - DecodeProto(prost::DecodeError), - #[display(fmt = "Failed to encode response: {}.", _0)] - EncodeProto(prost::EncodeError), - #[display(fmt = "Failed to decode block hash: {}.", _0)] - DecodeScale(codec::Error), - #[display(fmt = "Missing `BlockRequest::from_block` field.")] - MissingFromField, - #[display(fmt = "Failed to parse BlockRequest::direction.")] - ParseDirection, - Client(sp_blockchain::Error), - #[display(fmt = "Failed to send response.")] - SendResponse, -} diff --git a/client/network/src/lib.rs b/client/network/src/lib.rs index 3d2b809db60cc..60c1cf7da5b3d 100644 --- a/client/network/src/lib.rs +++ b/client/network/src/lib.rs @@ -253,7 +253,7 @@ mod discovery; mod light_client_handler; mod on_demand_layer; mod protocol; -mod request_responses; +pub mod request_responses; mod schema; mod service; mod transport; @@ -263,7 +263,6 @@ pub mod config; pub mod error; pub mod gossip; pub mod network_state; -pub mod grandpa_warp_sync_request_handler; #[doc(inline)] pub use libp2p::{multiaddr, Multiaddr, PeerId}; diff --git a/client/service/Cargo.toml b/client/service/Cargo.toml index 09ab35ea6d486..778156d87ee72 100644 --- a/client/service/Cargo.toml +++ b/client/service/Cargo.toml @@ -80,6 +80,7 @@ tracing = "0.1.22" tracing-futures = { version = "0.2.4" } parity-util-mem = { version = "0.7.0", default-features = false, features = ["primitive-types"] } finality-grandpa = "0.12.3" +sc-grandpa-warp-sync = { version = "0.8.0", path = "../grandpa-warp-sync" } [target.'cfg(not(target_os = "unknown"))'.dependencies] tempfile = "3.1.0" diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index 0513ad3c20bde..2be7bbb760dfb 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -43,7 +43,7 @@ use sc_keystore::LocalKeystore; use log::{info, warn}; use sc_network::config::{Role, OnDemand}; use sc_network::NetworkService; -use sc_network::grandpa_warp_sync_request_handler::{self, GrandpaWarpSyncRequestHandler}; +use sc_grandpa_warp_sync::GrandpaWarpSyncRequestHandler; use sp_runtime::generic::BlockId; use sp_runtime::traits::{ Block as BlockT, SaturatedConversion, HashFor, Zero, BlockIdTo, NumberFor, @@ -818,11 +818,13 @@ fn gen_handler( } /// Parameters to pass into `build_network`. -pub struct BuildNetworkParams<'a, TBl: BlockT, TExPool, TImpQu, TCl> { +pub struct BuildNetworkParams<'a, TBl: BlockT, TExPool, TImpQu, TCl, TBackend> { /// The service configuration. pub config: &'a Configuration, /// A shared client returned by `new_full_parts`/`new_light_parts`. pub client: Arc, + /// A shared backend returned by `new_full_parts`/`new_light_parts`. + pub backend: Arc, /// A shared transaction pool. pub transaction_pool: Arc, /// A handle for spawning tasks. @@ -838,8 +840,8 @@ pub struct BuildNetworkParams<'a, TBl: BlockT, TExPool, TImpQu, TCl> { } /// Build the network service, the network status sinks and an RPC sender. -pub fn build_network( - params: BuildNetworkParams +pub fn build_network( + params: BuildNetworkParams ) -> Result< ( Arc::Hash>>, @@ -856,9 +858,11 @@ pub fn build_network( HeaderBackend + BlockchainEvents + 'static, TExPool: MaintainedTransactionPool::Hash> + 'static, TImpQu: ImportQueue + 'static, + TBackend: sc_client_api::backend::Backend + 'static, + NumberFor: BlockNumberOps, { let BuildNetworkParams { - config, client, transaction_pool, spawn_handle, import_queue, on_demand, + config, client, backend, transaction_pool, spawn_handle, import_queue, on_demand, block_announce_validator_builder, } = params; @@ -890,12 +894,12 @@ pub fn build_network( let grandpa_warp_sync_request_protocol_config = { if matches!(config.role, Role::Light) { // Allow outgoing requests but deny incoming requests. - grandpa_warp_sync_request_handler::generate_protocol_config(protocol_id.clone()) + sc_grandpa_warp_sync::generate_protocol_config(protocol_id.clone()) } else { // Allow both outgoing and incoming requests. let (handler, protocol_config) = GrandpaWarpSyncRequestHandler::new( protocol_id.clone(), - client.clone(), + backend.clone(), ); spawn_handle.spawn("grandpa_warp_sync_request_handler", handler.run()); protocol_config From 4c61d2828085527d10113534b557d68ed15c959c Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Mon, 7 Dec 2020 12:32:48 +0100 Subject: [PATCH 15/34] Clean up --- client/grandpa-warp-sync/src/lib.rs | 42 +++++++++++++++-------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/client/grandpa-warp-sync/src/lib.rs b/client/grandpa-warp-sync/src/lib.rs index 9b380fb711e9f..9c10e3b17e556 100644 --- a/client/grandpa-warp-sync/src/lib.rs +++ b/client/grandpa-warp-sync/src/lib.rs @@ -36,7 +36,9 @@ const LOG_TARGET: &str = "grandpa-warp-sync-request-handler"; pub fn generate_protocol_config(protocol_id: ProtocolId) -> ProtocolConfig { ProtocolConfig { name: generate_protocol_name(protocol_id).into(), + // todo max_request_size: 1024 * 1024, + // todo max_response_size: 16 * 1024 * 1024, request_timeout: Duration::from_secs(40), inbound_queue: None, @@ -54,14 +56,14 @@ fn generate_protocol_name(protocol_id: ProtocolId) -> String { #[derive(codec::Decode)] struct Request { - begin: B::Hash + begin: B::Hash } /// Handler for incoming block requests from a remote peer. pub struct GrandpaWarpSyncRequestHandler { - backend: Arc, - request_receiver: mpsc::Receiver, - _phantom: std::marker::PhantomData + backend: Arc, + request_receiver: mpsc::Receiver, + _phantom: std::marker::PhantomData } impl> GrandpaWarpSyncRequestHandler { @@ -86,31 +88,31 @@ impl> GrandpaWarpSyncRequestHandler, pending_response: oneshot::Sender> - ) -> Result<(), HandleRequestError> - where NumberFor: sc_finality_grandpa::BlockNumberOps, - { - let request = Request::::decode(&mut &payload[..])?; - - let response = sc_finality_grandpa::finality_proof::prove_authority::<_, _, GrandpaJustification>( - self.backend.blockchain(), request.begin, - )?; - - pending_response.send(response) - .map_err(|_| HandleRequestError::SendResponse) + ) -> Result<(), HandleRequestError> + where NumberFor: sc_finality_grandpa::BlockNumberOps, + { + let request = Request::::decode(&mut &payload[..])?; + + let response = sc_finality_grandpa::finality_proof::prove_authority::<_, _, GrandpaJustification>( + self.backend.blockchain(), request.begin, + )?; + + pending_response.send(response) + .map_err(|_| HandleRequestError::SendResponse) } /// Run [`BlockRequestHandler`]. - pub async fn run(mut self) - where NumberFor: sc_finality_grandpa::BlockNumberOps, - { + pub async fn run(mut self) + where NumberFor: sc_finality_grandpa::BlockNumberOps, + { while let Some(request) = self.request_receiver.next().await { let IncomingRequest { peer, payload, pending_response } = request; match self.handle_request(payload, pending_response) { - Ok(()) => debug!(target: LOG_TARGET, "Handled block request from {}.", peer), + Ok(()) => debug!(target: LOG_TARGET, "Handled grandpa warp sync request from {}.", peer), Err(e) => debug!( target: LOG_TARGET, - "Failed to handle block request from {}: {}", + "Failed to handle grandpa warp sync request from {}: {}", peer, e, ), } From 504207115b7d82187b74ca8c6131a8aee4d1e1e7 Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Mon, 14 Dec 2020 15:24:31 +0100 Subject: [PATCH 16/34] Move things around a bit so that adding the grandpa warp sync request response protocol happens in the node code --- Cargo.lock | 3 +- bin/node-template/node/src/service.rs | 2 -- bin/node/cli/Cargo.toml | 1 + bin/node/cli/src/service.rs | 6 ++-- client/grandpa-warp-sync/Cargo.toml | 1 + client/grandpa-warp-sync/src/lib.rs | 25 ++++++++++++++ client/network/src/behaviour.rs | 4 +-- client/network/src/config.rs | 13 -------- client/network/src/service.rs | 6 +--- client/service/Cargo.toml | 1 - client/service/src/builder.rs | 48 +++++---------------------- client/service/src/config.rs | 14 ++++++++ 12 files changed, 58 insertions(+), 66 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5a90483790e94..04d2be082d663 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3778,6 +3778,7 @@ dependencies = [ "sc-consensus-epochs", "sc-consensus-slots", "sc-finality-grandpa", + "sc-grandpa-warp-sync", "sc-keystore", "sc-network", "sc-offchain", @@ -6998,6 +6999,7 @@ dependencies = [ "sc-client-api", "sc-finality-grandpa", "sc-network", + "sc-service", "sp-blockchain", "sp-runtime", ] @@ -7339,7 +7341,6 @@ dependencies = [ "sc-client-db", "sc-executor", "sc-finality-grandpa", - "sc-grandpa-warp-sync", "sc-informant", "sc-keystore", "sc-light", diff --git a/bin/node-template/node/src/service.rs b/bin/node-template/node/src/service.rs index db426b873a1a2..7e1939fb023a8 100644 --- a/bin/node-template/node/src/service.rs +++ b/bin/node-template/node/src/service.rs @@ -113,7 +113,6 @@ pub fn new_full(mut config: Configuration) -> Result sc_service::build_network(sc_service::BuildNetworkParams { config: &config, client: client.clone(), - backend: backend.clone(), transaction_pool: transaction_pool.clone(), spawn_handle: task_manager.spawn_handle(), import_queue, @@ -283,7 +282,6 @@ pub fn new_light(mut config: Configuration) -> Result sc_service::build_network(sc_service::BuildNetworkParams { config: &config, client: client.clone(), - backend: backend.clone(), transaction_pool: transaction_pool.clone(), spawn_handle: task_manager.spawn_handle(), import_queue, diff --git a/bin/node/cli/Cargo.toml b/bin/node/cli/Cargo.toml index 6574ccb733b52..de7969b84889a 100644 --- a/bin/node/cli/Cargo.toml +++ b/bin/node/cli/Cargo.toml @@ -75,6 +75,7 @@ sc-service = { version = "0.8.0", default-features = false, path = "../../../cli sc-tracing = { version = "2.0.0", path = "../../../client/tracing" } sc-telemetry = { version = "2.0.0", path = "../../../client/telemetry" } sc-authority-discovery = { version = "0.8.0", path = "../../../client/authority-discovery" } +sc-grandpa-warp-sync = { version = "0.8.0", path = "../../../client/grandpa-warp-sync" } # frame dependencies pallet-indices = { version = "2.0.0", path = "../../../frame/indices" } diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index f2f08e64ce952..78b59897face7 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -180,11 +180,14 @@ pub fn new_full_base( config.network.notifications_protocols.push(grandpa::GRANDPA_PROTOCOL_NAME.into()); + config.network.request_response_protocols.push(sc_grandpa_warp_sync::protocol_config_for_chain( + &config, task_manager.spawn_handle(), backend.clone(), + )); + let (network, network_status_sinks, system_rpc_tx, network_starter) = sc_service::build_network(sc_service::BuildNetworkParams { config: &config, client: client.clone(), - backend: backend.clone(), transaction_pool: transaction_pool.clone(), spawn_handle: task_manager.spawn_handle(), import_queue, @@ -390,7 +393,6 @@ pub fn new_light_base(mut config: Configuration) -> Result<( sc_service::build_network(sc_service::BuildNetworkParams { config: &config, client: client.clone(), - backend: backend.clone(), transaction_pool: transaction_pool.clone(), spawn_handle: task_manager.spawn_handle(), import_queue, diff --git a/client/grandpa-warp-sync/Cargo.toml b/client/grandpa-warp-sync/Cargo.toml index 770d8081b112e..5f14d59eb7f9b 100644 --- a/client/grandpa-warp-sync/Cargo.toml +++ b/client/grandpa-warp-sync/Cargo.toml @@ -18,6 +18,7 @@ sc-finality-grandpa = { version = "0.8.0", path = "../finality-grandpa" } sp-runtime = { version = "2.0.0", path = "../../primitives/runtime" } sp-blockchain = { version = "2.0.0", path = "../../primitives/blockchain" } sc-client-api = { version = "2.0.0", path = "../api" } +sc-service = { version = "0.8.0", path = "../service" } futures = "0.3.8" log = "0.4.11" derive_more = "0.99.11" diff --git a/client/grandpa-warp-sync/src/lib.rs b/client/grandpa-warp-sync/src/lib.rs index 9c10e3b17e556..4ff66ed237be1 100644 --- a/client/grandpa-warp-sync/src/lib.rs +++ b/client/grandpa-warp-sync/src/lib.rs @@ -29,6 +29,31 @@ use log::debug; use sp_runtime::traits::Block as BlockT; use std::time::Duration; use std::sync::Arc; +use sc_service::{SpawnTaskHandle, config::{Configuration, Role}}; + +/// Generates the appropriate [`ProtocolConfig`] for a given chain configuration. +pub fn protocol_config_for_chain + 'static>( + config: &Configuration, + spawn_handle: SpawnTaskHandle, + backend: Arc, +) -> ProtocolConfig + where NumberFor: sc_finality_grandpa::BlockNumberOps, +{ + let protocol_id = config.protocol_id(); + + if matches!(config.role, Role::Light) { + // Allow outgoing requests but deny incoming requests. + generate_protocol_config(protocol_id.clone()) + } else { + // Allow both outgoing and incoming requests. + let (handler, protocol_config) = GrandpaWarpSyncRequestHandler::new( + protocol_id.clone(), + backend.clone(), + ); + spawn_handle.spawn("grandpa_warp_sync_request_handler", handler.run()); + protocol_config + } +} const LOG_TARGET: &str = "grandpa-warp-sync-request-handler"; diff --git a/client/network/src/behaviour.rs b/client/network/src/behaviour.rs index 40cb5860229c3..8b9e321ca599b 100644 --- a/client/network/src/behaviour.rs +++ b/client/network/src/behaviour.rs @@ -181,14 +181,14 @@ impl Behaviour { block_requests: block_requests::BlockRequests, light_client_handler: light_client_handler::LightClientHandler, disco_config: DiscoveryConfig, - request_response_protocols: impl Iterator, + request_response_protocols: Vec, ) -> Result { Ok(Behaviour { substrate, peer_info: peer_info::PeerInfoBehaviour::new(user_agent, local_public_key), discovery: disco_config.finish(), request_responses: - request_responses::RequestResponsesBehaviour::new(request_response_protocols)?, + request_responses::RequestResponsesBehaviour::new(request_response_protocols.into_iter())?, block_requests, light_client_handler, events: VecDeque::new(), diff --git a/client/network/src/config.rs b/client/network/src/config.rs index 4c6ec437fcb7b..b7b113dc14692 100644 --- a/client/network/src/config.rs +++ b/client/network/src/config.rs @@ -95,19 +95,6 @@ pub struct Params { /// Registry for recording prometheus metrics to. pub metrics_registry: Option, - - /// Request response configuration for the grandpa warp sync request protocol. - /// - /// [`RequestResponseConfig`] [`name`] is used to tag outgoing grandpa warp sync requests with - /// the correct protocol name. In addition all of [`RequestResponseConfig`] is used to handle - /// incoming grandpa warp sync requests, if enabled. - /// - /// Can be constructed either via - /// [`grandpa_warp_sync_request_handler::generate_protocol_config`] allowing outgoing but not - /// incoming requests, or constructed via - /// [`grandpa_warp_sync_request_handler::GrandpaWarpSyncRequestHandler::new`] allowing both - /// outgoing and incoming requests. - pub grandpa_warp_sync_request_protocol_config: RequestResponseConfig, } /// Role of the local node. diff --git a/client/network/src/service.rs b/client/network/src/service.rs index 289ccbc365fd5..3a368088e5392 100644 --- a/client/network/src/service.rs +++ b/client/network/src/service.rs @@ -323,10 +323,6 @@ impl NetworkWorker { config }; - let request_response_protocols = params.network_config.request_response_protocols - .into_iter() - .chain(std::iter::once(params.grandpa_warp_sync_request_protocol_config)); - let mut behaviour = { let result = Behaviour::new( protocol, @@ -336,7 +332,7 @@ impl NetworkWorker { block_requests, light_client_handler, discovery_config, - request_response_protocols, + params.network_config.request_response_protocols, ); match result { diff --git a/client/service/Cargo.toml b/client/service/Cargo.toml index 778156d87ee72..09ab35ea6d486 100644 --- a/client/service/Cargo.toml +++ b/client/service/Cargo.toml @@ -80,7 +80,6 @@ tracing = "0.1.22" tracing-futures = { version = "0.2.4" } parity-util-mem = { version = "0.7.0", default-features = false, features = ["primitive-types"] } finality-grandpa = "0.12.3" -sc-grandpa-warp-sync = { version = "0.8.0", path = "../grandpa-warp-sync" } [target.'cfg(not(target_os = "unknown"))'.dependencies] tempfile = "3.1.0" diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index 0fc19d838f464..d3c7c42e18e83 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -17,7 +17,7 @@ // along with this program. If not, see . use crate::{ - error::Error, DEFAULT_PROTOCOL_ID, MallocSizeOfWasm, + error::Error, MallocSizeOfWasm, TelemetryConnectionSinks, RpcHandlers, NetworkStatusSinks, start_rpc_servers, build_network_future, TransactionPoolAdapter, TaskManager, SpawnTaskHandle, metrics::MetricsService, @@ -43,7 +43,6 @@ use sc_keystore::LocalKeystore; use log::{info, warn}; use sc_network::config::{Role, OnDemand}; use sc_network::NetworkService; -use sc_grandpa_warp_sync::GrandpaWarpSyncRequestHandler; use sp_runtime::generic::BlockId; use sp_runtime::traits::{ Block as BlockT, SaturatedConversion, HashFor, Zero, BlockIdTo, NumberFor, @@ -844,13 +843,11 @@ fn gen_handler( } /// Parameters to pass into `build_network`. -pub struct BuildNetworkParams<'a, TBl: BlockT, TExPool, TImpQu, TCl, TBackend> { +pub struct BuildNetworkParams<'a, TBl: BlockT, TExPool, TImpQu, TCl> { /// The service configuration. pub config: &'a Configuration, /// A shared client returned by `new_full_parts`/`new_light_parts`. pub client: Arc, - /// A shared backend returned by `new_full_parts`/`new_light_parts`. - pub backend: Arc, /// A shared transaction pool. pub transaction_pool: Arc, /// A handle for spawning tasks. @@ -866,8 +863,8 @@ pub struct BuildNetworkParams<'a, TBl: BlockT, TExPool, TImpQu, TCl, TBackend> { } /// Build the network service, the network status sinks and an RPC sender. -pub fn build_network( - params: BuildNetworkParams +pub fn build_network( + params: BuildNetworkParams ) -> Result< ( Arc::Hash>>, @@ -884,11 +881,9 @@ pub fn build_network( HeaderBackend + BlockchainEvents + 'static, TExPool: MaintainedTransactionPool::Hash> + 'static, TImpQu: ImportQueue + 'static, - TBackend: sc_client_api::backend::Backend + 'static, - NumberFor: BlockNumberOps, { let BuildNetworkParams { - config, client, backend, transaction_pool, spawn_handle, import_queue, on_demand, + config, client, transaction_pool, spawn_handle, import_queue, on_demand, block_announce_validator_builder, } = params; @@ -898,40 +893,14 @@ pub fn build_network( client: client.clone(), }); - let protocol_id = { - let protocol_id_full = match config.chain_spec.protocol_id() { - Some(pid) => pid, - None => { - warn!("Using default protocol ID {:?} because none is configured in the \ - chain specs", DEFAULT_PROTOCOL_ID - ); - DEFAULT_PROTOCOL_ID - } - }; - sc_network::config::ProtocolId::from(protocol_id_full) - }; + let protocol_id = config.protocol_id(); let block_announce_validator = if let Some(f) = block_announce_validator_builder { f(client.clone()) } else { Box::new(DefaultBlockAnnounceValidator) }; - - let grandpa_warp_sync_request_protocol_config = { - if matches!(config.role, Role::Light) { - // Allow outgoing requests but deny incoming requests. - sc_grandpa_warp_sync::generate_protocol_config(protocol_id.clone()) - } else { - // Allow both outgoing and incoming requests. - let (handler, protocol_config) = GrandpaWarpSyncRequestHandler::new( - protocol_id.clone(), - backend.clone(), - ); - spawn_handle.spawn("grandpa_warp_sync_request_handler", handler.run()); - protocol_config - } - }; - + let network_params = sc_network::config::Params { role: config.role.clone(), executor: { @@ -947,8 +916,7 @@ pub fn build_network( import_queue: Box::new(import_queue), protocol_id, block_announce_validator, - metrics_registry: config.prometheus_config.as_ref().map(|config| config.registry.clone()), - grandpa_warp_sync_request_protocol_config, + metrics_registry: config.prometheus_config.as_ref().map(|config| config.registry.clone()) }; let has_bootnodes = !network_params.network_config.boot_nodes.is_empty(); diff --git a/client/service/src/config.rs b/client/service/src/config.rs index e360e610d490c..d1f06930bd50c 100644 --- a/client/service/src/config.rs +++ b/client/service/src/config.rs @@ -194,6 +194,20 @@ impl Configuration { pub fn prometheus_registry<'a>(&'a self) -> Option<&'a Registry> { self.prometheus_config.as_ref().map(|config| &config.registry) } + + /// Returns the network protocol id from the chain spec, or the default. + pub fn protocol_id(&self) -> sc_network::config::ProtocolId { + let protocol_id_full = match self.chain_spec.protocol_id() { + Some(pid) => pid, + None => { + log::warn!("Using default protocol ID {:?} because none is configured in the \ + chain specs", crate::DEFAULT_PROTOCOL_ID + ); + crate::DEFAULT_PROTOCOL_ID + } + }; + sc_network::config::ProtocolId::from(protocol_id_full) + } } /// Available RPC methods. From ea59807fce85152942dfaa254675d35693a6f406 Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Tue, 5 Jan 2021 16:12:14 +0100 Subject: [PATCH 17/34] Nits --- Cargo.lock | 5 - bin/node/cli/src/cli.rs | 7 + bin/node/cli/src/command.rs | 5 + bin/node/cli/src/service.rs | 2 +- bin/node/inspect/src/lib.rs | 329 ++++++++++++++++++++++++++++ client/db/Cargo.toml | 3 - client/db/src/lib.rs | 13 +- client/finality-grandpa/src/lib.rs | 4 +- client/grandpa-warp-sync/Cargo.toml | 2 +- client/grandpa-warp-sync/src/lib.rs | 44 ++-- client/network/src/lib.rs | 2 +- client/service/Cargo.toml | 1 - client/service/src/builder.rs | 8 +- test-utils/client/Cargo.toml | 1 - test-utils/client/src/lib.rs | 12 +- 15 files changed, 371 insertions(+), 67 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 02ff4b87429ee..65491a6d841f0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6595,7 +6595,6 @@ dependencies = [ "kvdb-rocksdb", "linked-hash-map", "log", - "num-traits", "parity-db", "parity-scale-codec", "parity-util-mem", @@ -6603,14 +6602,12 @@ dependencies = [ "quickcheck", "sc-client-api", "sc-executor", - "sc-finality-grandpa", "sc-state-db", "sp-arithmetic", "sp-blockchain", "sp-consensus", "sp-core", "sp-database", - "sp-finality-grandpa", "sp-keyring", "sp-runtime", "sp-state-machine", @@ -7361,7 +7358,6 @@ dependencies = [ "async-std", "directories 3.0.1", "exit-future", - "finality-grandpa", "futures 0.1.30", "futures 0.3.8", "futures-timer 3.0.2", @@ -8992,7 +8988,6 @@ dependencies = [ name = "substrate-test-client" version = "2.0.0" dependencies = [ - "finality-grandpa", "futures 0.1.30", "futures 0.3.8", "hash-db", diff --git a/bin/node/cli/src/cli.rs b/bin/node/cli/src/cli.rs index 3a329a57570d7..63a07e00e2197 100644 --- a/bin/node/cli/src/cli.rs +++ b/bin/node/cli/src/cli.rs @@ -36,6 +36,13 @@ pub enum Subcommand { /// Key management cli utilities Key(KeySubcommand), + /// The custom inspect subcommmand for decoding blocks and extrinsics. + #[structopt( + name = "inspect", + about = "Decode given block or extrinsic using current native runtime." + )] + Inspect(node_inspect::cli::InspectCmd), + /// The custom benchmark subcommmand benchmarking runtime pallets. #[structopt(name = "benchmark", about = "Benchmark runtime pallets.")] Benchmark(frame_benchmarking_cli::BenchmarkCmd), diff --git a/bin/node/cli/src/command.rs b/bin/node/cli/src/command.rs index ab2936bf1873c..ed3aff88c75de 100644 --- a/bin/node/cli/src/command.rs +++ b/bin/node/cli/src/command.rs @@ -79,6 +79,11 @@ pub fn run() -> Result<()> { } }) } + Some(Subcommand::Inspect(cmd)) => { + let runner = cli.create_runner(cmd)?; + + runner.sync_run(|config| cmd.run::(config)) + } Some(Subcommand::Benchmark(cmd)) => { if cfg!(feature = "runtime-benchmarks") { let runner = cli.create_runner(cmd)?; diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index 70d78a9b8bf9d..99a644ddb9b06 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -180,7 +180,7 @@ pub fn new_full_base( config.network.notifications_protocols.push(grandpa::GRANDPA_PROTOCOL_NAME.into()); - config.network.request_response_protocols.push(sc_grandpa_warp_sync::protocol_config_for_chain( + config.network.request_response_protocols.push(sc_grandpa_warp_sync::request_response_config_for_chain( &config, task_manager.spawn_handle(), backend.clone(), )); diff --git a/bin/node/inspect/src/lib.rs b/bin/node/inspect/src/lib.rs index e69de29bb2d1d..2a55fdcda62ee 100644 --- a/bin/node/inspect/src/lib.rs +++ b/bin/node/inspect/src/lib.rs @@ -0,0 +1,329 @@ +// This file is part of Substrate. +// +// Copyright (C) 2020-2021 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +//! A CLI extension for substrate node, adding sub-command to pretty print debug info +//! about blocks and extrinsics. +//! +//! The blocks and extrinsics can either be retrieved from the database (on-chain), +//! or a raw SCALE-encoding can be provided. + +#![warn(missing_docs)] + +pub mod cli; +pub mod command; + +use std::{ + fmt, + fmt::Debug, + marker::PhantomData, + str::FromStr, +}; +use codec::{Encode, Decode}; +use sc_client_api::BlockBackend; +use sp_blockchain::HeaderBackend; +use sp_core::hexdisplay::HexDisplay; +use sp_runtime::{ + generic::BlockId, + traits::{Block, HashFor, NumberFor, Hash} +}; + +/// A helper type for a generic block input. +pub type BlockAddressFor = BlockAddress< + as Hash>::Output, + NumberFor +>; + +/// A Pretty formatter implementation. +pub trait PrettyPrinter { + /// Nicely format block. + fn fmt_block(&self, fmt: &mut fmt::Formatter, block: &TBlock) -> fmt::Result; + /// Nicely format extrinsic. + fn fmt_extrinsic(&self, fmt: &mut fmt::Formatter, extrinsic: &TBlock::Extrinsic) -> fmt::Result; +} + +/// Default dummy debug printer. +#[derive(Default)] +pub struct DebugPrinter; +impl PrettyPrinter for DebugPrinter { + fn fmt_block(&self, fmt: &mut fmt::Formatter, block: &TBlock) -> fmt::Result { + writeln!(fmt, "Header:")?; + writeln!(fmt, "{:?}", block.header())?; + writeln!(fmt, "Block bytes: {:?}", HexDisplay::from(&block.encode()))?; + writeln!(fmt, "Extrinsics ({})", block.extrinsics().len())?; + for (idx, ex) in block.extrinsics().iter().enumerate() { + writeln!(fmt, "- {}:", idx)?; + >::fmt_extrinsic(self, fmt, ex)?; + } + Ok(()) + } + + fn fmt_extrinsic(&self, fmt: &mut fmt::Formatter, extrinsic: &TBlock::Extrinsic) -> fmt::Result { + writeln!(fmt, " {:?}", extrinsic)?; + writeln!(fmt, " Bytes: {:?}", HexDisplay::from(&extrinsic.encode()))?; + Ok(()) + } +} + +/// Aggregated error for `Inspector` operations. +#[derive(Debug, derive_more::From, derive_more::Display)] +pub enum Error { + /// Could not decode Block or Extrinsic. + Codec(codec::Error), + /// Error accessing blockchain DB. + Blockchain(sp_blockchain::Error), + /// Given block has not been found. + NotFound(String), +} + +impl std::error::Error for Error { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match *self { + Self::Codec(ref e) => Some(e), + Self::Blockchain(ref e) => Some(e), + Self::NotFound(_) => None, + } + } +} + +/// A helper trait to access block headers and bodies. +pub trait ChainAccess: + HeaderBackend + + BlockBackend +{} + +impl ChainAccess for T where + TBlock: Block, + T: sp_blockchain::HeaderBackend + sc_client_api::BlockBackend, +{} + +/// Blockchain inspector. +pub struct Inspector = DebugPrinter> { + printer: TPrinter, + chain: Box>, + _block: PhantomData, +} + +impl> Inspector { + /// Create new instance of the inspector with default printer. + pub fn new( + chain: impl ChainAccess + 'static, + ) -> Self where TPrinter: Default { + Self::with_printer(chain, Default::default()) + } + + /// Customize pretty-printing of the data. + pub fn with_printer( + chain: impl ChainAccess + 'static, + printer: TPrinter, + ) -> Self { + Inspector { + chain: Box::new(chain) as _, + printer, + _block: Default::default(), + } + } + + /// Get a pretty-printed block. + pub fn block(&self, input: BlockAddressFor) -> Result { + struct BlockPrinter<'a, A, B>(A, &'a B); + impl<'a, A: Block, B: PrettyPrinter> fmt::Display for BlockPrinter<'a, A, B> { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + self.1.fmt_block(fmt, &self.0) + } + } + + let block = self.get_block(input)?; + Ok(format!("{}", BlockPrinter(block, &self.printer))) + } + + fn get_block(&self, input: BlockAddressFor) -> Result { + Ok(match input { + BlockAddress::Bytes(bytes) => { + TBlock::decode(&mut &*bytes)? + }, + BlockAddress::Number(number) => { + let id = BlockId::number(number); + let not_found = format!("Could not find block {:?}", id); + let body = self.chain.block_body(&id)? + .ok_or_else(|| Error::NotFound(not_found.clone()))?; + let header = self.chain.header(id)? + .ok_or_else(|| Error::NotFound(not_found.clone()))?; + TBlock::new(header, body) + }, + BlockAddress::Hash(hash) => { + let id = BlockId::hash(hash); + let not_found = format!("Could not find block {:?}", id); + let body = self.chain.block_body(&id)? + .ok_or_else(|| Error::NotFound(not_found.clone()))?; + let header = self.chain.header(id)? + .ok_or_else(|| Error::NotFound(not_found.clone()))?; + TBlock::new(header, body) + }, + }) + } + + /// Get a pretty-printed extrinsic. + pub fn extrinsic( + &self, + input: ExtrinsicAddress< as Hash>::Output, NumberFor>, + ) -> Result { + struct ExtrinsicPrinter<'a, A: Block, B>(A::Extrinsic, &'a B); + impl<'a, A: Block, B: PrettyPrinter> fmt::Display for ExtrinsicPrinter<'a, A, B> { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + self.1.fmt_extrinsic(fmt, &self.0) + } + } + + let ext = match input { + ExtrinsicAddress::Block(block, index) => { + let block = self.get_block(block)?; + block.extrinsics() + .get(index) + .cloned() + .ok_or_else(|| Error::NotFound(format!( + "Could not find extrinsic {} in block {:?}", index, block + )))? + }, + ExtrinsicAddress::Bytes(bytes) => { + TBlock::Extrinsic::decode(&mut &*bytes)? + } + }; + + Ok(format!("{}", ExtrinsicPrinter(ext, &self.printer))) + } +} + +/// A block to retrieve. +#[derive(Debug, Clone, PartialEq)] +pub enum BlockAddress { + /// Get block by hash. + Hash(Hash), + /// Get block by number. + Number(Number), + /// Raw SCALE-encoded bytes. + Bytes(Vec), +} + +impl FromStr for BlockAddress { + type Err = String; + + fn from_str(s: &str) -> Result { + // try to parse hash first + if let Ok(hash) = s.parse() { + return Ok(Self::Hash(hash)) + } + + // then number + if let Ok(number) = s.parse() { + return Ok(Self::Number(number)) + } + + // then assume it's bytes (hex-encoded) + sp_core::bytes::from_hex(s) + .map(Self::Bytes) + .map_err(|e| format!( + "Given string does not look like hash or number. It could not be parsed as bytes either: {}", + e + )) + } +} + +/// An extrinsic address to decode and print out. +#[derive(Debug, Clone, PartialEq)] +pub enum ExtrinsicAddress { + /// Extrinsic as part of existing block. + Block(BlockAddress, usize), + /// Raw SCALE-encoded extrinsic bytes. + Bytes(Vec), +} + +impl FromStr for ExtrinsicAddress { + type Err = String; + + fn from_str(s: &str) -> Result { + // first try raw bytes + if let Ok(bytes) = sp_core::bytes::from_hex(s).map(Self::Bytes) { + return Ok(bytes) + } + + // split by a bunch of different characters + let mut it = s.split(|c| c == '.' || c == ':' || c == ' '); + let block = it.next() + .expect("First element of split iterator is never empty; qed") + .parse()?; + + let index = it.next() + .ok_or_else(|| format!("Extrinsic index missing: example \"5:0\""))? + .parse() + .map_err(|e| format!("Invalid index format: {}", e))?; + + Ok(Self::Block(block, index)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use sp_core::hash::H160 as Hash; + + #[test] + fn should_parse_block_strings() { + type BlockAddress = super::BlockAddress; + + let b0 = BlockAddress::from_str("3BfC20f0B9aFcAcE800D73D2191166FF16540258"); + let b1 = BlockAddress::from_str("1234"); + let b2 = BlockAddress::from_str("0"); + let b3 = BlockAddress::from_str("0x0012345f"); + + + assert_eq!(b0, Ok(BlockAddress::Hash( + "3BfC20f0B9aFcAcE800D73D2191166FF16540258".parse().unwrap() + ))); + assert_eq!(b1, Ok(BlockAddress::Number(1234))); + assert_eq!(b2, Ok(BlockAddress::Number(0))); + assert_eq!(b3, Ok(BlockAddress::Bytes(vec![0, 0x12, 0x34, 0x5f]))); + } + + #[test] + fn should_parse_extrinsic_address() { + type BlockAddress = super::BlockAddress; + type ExtrinsicAddress = super::ExtrinsicAddress; + + let e0 = ExtrinsicAddress::from_str("1234"); + let b0 = ExtrinsicAddress::from_str("3BfC20f0B9aFcAcE800D73D2191166FF16540258:5"); + let b1 = ExtrinsicAddress::from_str("1234:0"); + let b2 = ExtrinsicAddress::from_str("0 0"); + let b3 = ExtrinsicAddress::from_str("0x0012345f"); + + + assert_eq!(e0, Err("Extrinsic index missing: example \"5:0\"".into())); + assert_eq!(b0, Ok(ExtrinsicAddress::Block( + BlockAddress::Hash("3BfC20f0B9aFcAcE800D73D2191166FF16540258".parse().unwrap()), + 5 + ))); + assert_eq!(b1, Ok(ExtrinsicAddress::Block( + BlockAddress::Number(1234), + 0 + ))); + assert_eq!(b2, Ok(ExtrinsicAddress::Block( + BlockAddress::Number(0), + 0 + ))); + assert_eq!(b3, Ok(ExtrinsicAddress::Bytes(vec![0, 0x12, 0x34, 0x5f]))); + } +} diff --git a/client/db/Cargo.toml b/client/db/Cargo.toml index f6a64e41b05e0..70a0b19532593 100644 --- a/client/db/Cargo.toml +++ b/client/db/Cargo.toml @@ -25,9 +25,6 @@ codec = { package = "parity-scale-codec", version = "1.3.4", features = ["derive blake2-rfc = "0.2.18" sc-client-api = { version = "2.0.0", path = "../api" } -sc-finality-grandpa = { version = "0.8.0", path = "../finality-grandpa" } -sp-finality-grandpa = { version = "2.0.0", path = "../../primitives/finality-grandpa" } -num-traits = "0.2.8" sp-arithmetic = { version = "2.0.0", path = "../../primitives/arithmetic" } sp-core = { version = "2.0.0", path = "../../primitives/core" } sp-runtime = { version = "2.0.0", path = "../../primitives/runtime" } diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index 1c0f5135619ee..e3b94b03c87d8 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -53,7 +53,6 @@ use parking_lot::{Mutex, RwLock}; use linked_hash_map::LinkedHashMap; use log::{trace, debug, warn}; -use sc_finality_grandpa::BlockNumberOps; // TODO remove (for test) use sc_client_api::{ UsageInfo, MemoryInfo, IoInfo, MemorySize, backend::{NewBlockState, PrunableStateChangesTrieStorage, ProvideChtRoots}, @@ -860,9 +859,7 @@ pub struct Backend { state_usage: Arc, } -impl Backend - where NumberFor: BlockNumberOps, -{ +impl Backend { /// Create a new instance of database backend. /// /// The pruning window is how old a block must be before the state is pruned. @@ -1443,9 +1440,7 @@ impl sc_client_api::backend::AuxStore for Backend where Block: Blo } } -impl sc_client_api::backend::Backend for Backend - where NumberFor: BlockNumberOps, -{ +impl sc_client_api::backend::Backend for Backend { type BlockImportOperation = BlockImportOperation; type Blockchain = BlockchainDb; type State = SyncingCachingState, Block>; @@ -1769,9 +1764,7 @@ impl sc_client_api::backend::Backend for Backend } } -impl sc_client_api::backend::LocalBackend for Backend -where NumberFor: BlockNumberOps, -{} +impl sc_client_api::backend::LocalBackend for Backend {} #[cfg(test)] pub(crate) mod tests { diff --git a/client/finality-grandpa/src/lib.rs b/client/finality-grandpa/src/lib.rs index 9034a6c62e327..1920973b8b2df 100644 --- a/client/finality-grandpa/src/lib.rs +++ b/client/finality-grandpa/src/lib.rs @@ -113,8 +113,7 @@ mod authorities; mod aux_schema; mod communication; mod environment; -// TODO remove pub -pub mod finality_proof; +mod finality_proof; mod import; mod justification; mod notification; @@ -132,6 +131,7 @@ pub use voting_rule::{ BeforeBestBlockBy, ThreeQuartersOfTheUnfinalizedChain, VotingRule, VotingRulesBuilder }; pub use finality_grandpa::voter::report; +pub use finality_proof::prove_authority; use aux_schema::PersistentData; use environment::{Environment, VoterSetState}; diff --git a/client/grandpa-warp-sync/Cargo.toml b/client/grandpa-warp-sync/Cargo.toml index 5f14d59eb7f9b..8a3053a1eea03 100644 --- a/client/grandpa-warp-sync/Cargo.toml +++ b/client/grandpa-warp-sync/Cargo.toml @@ -1,5 +1,5 @@ [package] -description = "..." +description = "A request-response protocol for handling grandpa warp sync requests" name = "sc-grandpa-warp-sync" version = "0.8.0" license = "GPL-3.0-or-later WITH Classpath-exception-2.0" diff --git a/client/grandpa-warp-sync/src/lib.rs b/client/grandpa-warp-sync/src/lib.rs index 4ff66ed237be1..96b80da367b78 100644 --- a/client/grandpa-warp-sync/src/lib.rs +++ b/client/grandpa-warp-sync/src/lib.rs @@ -18,8 +18,7 @@ //! [`crate::request_responses::RequestResponsesBehaviour`]. use codec::Decode; -use sc_network::config::ProtocolId; -use sc_network::request_responses::{IncomingRequest, ProtocolConfig}; +use sc_network::config::{ProtocolId, IncomingRequest, RequestResponseConfig}; use sc_client_api::Backend; use sc_finality_grandpa::GrandpaJustification; use sp_runtime::traits::NumberFor; @@ -31,41 +30,39 @@ use std::time::Duration; use std::sync::Arc; use sc_service::{SpawnTaskHandle, config::{Configuration, Role}}; -/// Generates the appropriate [`ProtocolConfig`] for a given chain configuration. -pub fn protocol_config_for_chain + 'static>( +/// Generates the appropriate [`RequestResponseConfig`] for a given chain configuration. +pub fn request_response_config_for_chain + 'static>( config: &Configuration, spawn_handle: SpawnTaskHandle, backend: Arc, -) -> ProtocolConfig +) -> RequestResponseConfig where NumberFor: sc_finality_grandpa::BlockNumberOps, { let protocol_id = config.protocol_id(); if matches!(config.role, Role::Light) { // Allow outgoing requests but deny incoming requests. - generate_protocol_config(protocol_id.clone()) + generate_request_response_config(protocol_id.clone()) } else { // Allow both outgoing and incoming requests. - let (handler, protocol_config) = GrandpaWarpSyncRequestHandler::new( + let (handler, request_response_config) = GrandpaWarpSyncRequestHandler::new( protocol_id.clone(), backend.clone(), ); spawn_handle.spawn("grandpa_warp_sync_request_handler", handler.run()); - protocol_config + request_response_config } } const LOG_TARGET: &str = "grandpa-warp-sync-request-handler"; -/// Generates a [`ProtocolConfig`] for the block request protocol, refusing incoming requests. -pub fn generate_protocol_config(protocol_id: ProtocolId) -> ProtocolConfig { - ProtocolConfig { +/// Generates a [`RequestResponseConfig`] for the block request protocol, refusing incoming requests. +pub fn generate_request_response_config(protocol_id: ProtocolId) -> RequestResponseConfig { + RequestResponseConfig { name: generate_protocol_name(protocol_id).into(), - // todo - max_request_size: 1024 * 1024, - // todo + max_request_size: 32, max_response_size: 16 * 1024 * 1024, - request_timeout: Duration::from_secs(40), + request_timeout: Duration::from_secs(10), inbound_queue: None, } } @@ -93,20 +90,13 @@ pub struct GrandpaWarpSyncRequestHandler { impl> GrandpaWarpSyncRequestHandler { /// Create a new [`BlockRequestHandler`]. - pub fn new(protocol_id: ProtocolId, backend: Arc) -> (Self, ProtocolConfig) { - // Rate of arrival multiplied with the waiting time in the queue equals the queue length. - // - // An average Polkadot sentry node serves less than 5 requests per second. The 95th percentile - // serving a request is less than 2 second. Thus one would estimate the queue length to be - // below 10. - // - // Choosing 20 as the queue length to give some additional buffer. + pub fn new(protocol_id: ProtocolId, backend: Arc) -> (Self, RequestResponseConfig) { let (tx, request_receiver) = mpsc::channel(20); - let mut protocol_config = generate_protocol_config(protocol_id); - protocol_config.inbound_queue = Some(tx); + let mut request_response_config = generate_request_response_config(protocol_id); + request_response_config.inbound_queue = Some(tx); - (Self { backend, request_receiver, _phantom: std::marker::PhantomData }, protocol_config) + (Self { backend, request_receiver, _phantom: std::marker::PhantomData }, request_response_config) } fn handle_request( @@ -118,7 +108,7 @@ impl> GrandpaWarpSyncRequestHandler::decode(&mut &payload[..])?; - let response = sc_finality_grandpa::finality_proof::prove_authority::<_, _, GrandpaJustification>( + let response = sc_finality_grandpa::prove_authority::<_, _, GrandpaJustification>( self.backend.blockchain(), request.begin, )?; diff --git a/client/network/src/lib.rs b/client/network/src/lib.rs index f115729325511..533a69dd4d5a5 100644 --- a/client/network/src/lib.rs +++ b/client/network/src/lib.rs @@ -253,7 +253,7 @@ mod discovery; mod light_client_handler; mod on_demand_layer; mod protocol; -pub mod request_responses; +mod request_responses; mod schema; mod service; mod transport; diff --git a/client/service/Cargo.toml b/client/service/Cargo.toml index 09ab35ea6d486..4350e1a2bf2a9 100644 --- a/client/service/Cargo.toml +++ b/client/service/Cargo.toml @@ -79,7 +79,6 @@ sp-tracing = { version = "2.0.0", path = "../../primitives/tracing" } tracing = "0.1.22" tracing-futures = { version = "0.2.4" } parity-util-mem = { version = "0.7.0", default-features = false, features = ["primitive-types"] } -finality-grandpa = "0.12.3" [target.'cfg(not(target_os = "unknown"))'.dependencies] tempfile = "3.1.0" diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index b3e4de6c8fef3..9eb0a02f29939 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -45,9 +45,8 @@ use sc_network::config::{Role, OnDemand}; use sc_network::NetworkService; use sp_runtime::generic::BlockId; use sp_runtime::traits::{ - Block as BlockT, SaturatedConversion, HashFor, Zero, BlockIdTo, NumberFor, + Block as BlockT, SaturatedConversion, HashFor, Zero, BlockIdTo, }; -use finality_grandpa::BlockNumberOps; use sp_api::{ProvideRuntimeApi, CallApiAt}; use sc_executor::{NativeExecutor, NativeExecutionDispatch, RuntimeInfo}; use std::sync::Arc; @@ -287,7 +286,6 @@ pub fn new_full_client( config: &Configuration, ) -> Result, Error> where TBl: BlockT, - NumberFor: BlockNumberOps, TExecDisp: NativeExecutionDispatch + 'static, { new_full_parts(config).map(|parts| parts.0) @@ -298,7 +296,6 @@ pub fn new_full_parts( config: &Configuration, ) -> Result, Error> where TBl: BlockT, - NumberFor: BlockNumberOps, TExecDisp: NativeExecutionDispatch + 'static, { let keystore_container = KeystoreContainer::new(&config.keystore)?; @@ -437,7 +434,6 @@ pub fn new_client( where Block: BlockT, E: CodeExecutor + RuntimeInfo, - NumberFor: BlockNumberOps, { const CANONICALIZATION_DELAY: u64 = 4096; @@ -900,7 +896,7 @@ pub fn build_network( } else { Box::new(DefaultBlockAnnounceValidator) }; - + let network_params = sc_network::config::Params { role: config.role.clone(), executor: { diff --git a/test-utils/client/Cargo.toml b/test-utils/client/Cargo.toml index 454b7bd0594e5..07d28660f6188 100644 --- a/test-utils/client/Cargo.toml +++ b/test-utils/client/Cargo.toml @@ -32,4 +32,3 @@ sp-keystore = { version = "0.8.0", path = "../../primitives/keystore" } sp-keyring = { version = "2.0.0", path = "../../primitives/keyring" } sp-runtime = { version = "2.0.0", path = "../../primitives/runtime" } sp-state-machine = { version = "0.8.0", path = "../../primitives/state-machine" } -finality-grandpa = "0.12.3" diff --git a/test-utils/client/src/lib.rs b/test-utils/client/src/lib.rs index 662f5a7b7b1db..487be14a7896d 100644 --- a/test-utils/client/src/lib.rs +++ b/test-utils/client/src/lib.rs @@ -34,7 +34,6 @@ pub use sp_keyring::{ sr25519::Keyring as Sr25519Keyring, }; pub use sp_keystore::{SyncCryptoStorePtr, SyncCryptoStore}; -use finality_grandpa::BlockNumberOps; pub use sp_runtime::{Storage, StorageChild}; pub use sp_state_machine::ExecutionStrategy; pub use sc_service::{RpcHandlers, RpcSession, client}; @@ -46,7 +45,7 @@ use std::collections::{HashSet, HashMap}; use futures::{future::{Future, FutureExt}, stream::StreamExt}; use serde::Deserialize; use sp_core::storage::ChildInfo; -use sp_runtime::{OpaqueExtrinsic, codec::Encode, traits::{Block as BlockT, BlakeTwo256, NumberFor}}; +use sp_runtime::{OpaqueExtrinsic, codec::Encode, traits::{Block as BlockT, BlakeTwo256}}; use sc_service::client::{LocalCallExecutor, ClientConfig}; use sc_client_api::BlockchainEvents; @@ -83,18 +82,13 @@ pub struct TestClientBuilder { } impl Default - for TestClientBuilder, G> - -where NumberFor: BlockNumberOps, -{ + for TestClientBuilder, G> { fn default() -> Self { Self::with_default_backend() } } -impl TestClientBuilder, G> -where NumberFor: BlockNumberOps, -{ +impl TestClientBuilder, G> { /// Create new `TestClientBuilder` with default backend. pub fn with_default_backend() -> Self { let backend = Arc::new(Backend::new_test(std::u32::MAX, std::u64::MAX)); From 224d2558a0acd41227847b771699bf6fedc4e1d4 Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Tue, 5 Jan 2021 16:17:48 +0100 Subject: [PATCH 18/34] Changes to comments --- client/finality-grandpa/src/finality_proof.rs | 7 +++---- client/grandpa-warp-sync/src/lib.rs | 10 +++++----- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index b2c33cf3e46c4..3416eec09a180 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -497,7 +497,7 @@ pub fn prove_authority, J>( } else { println!("should be unreachable"); } - + let inc = delay == Zero::zero() && block_number == index; index = dest; if inc { @@ -605,7 +605,7 @@ pub(crate) fn check_finality_proof( Ok(effects) } -/// Check GRANDPA authority change sequence to assert finality of a target block. +/// Check GRANDPA authority change sequence to assert finality of a target block. /// /// Returns the header of the target block. pub fn check_authority_proof( @@ -633,7 +633,7 @@ pub fn check_authority_proof( &result.2, is_last, &fragment, - )?; + )?; if is_last { return Ok((fragment.header, result.0, result.1)) @@ -775,7 +775,6 @@ pub trait BlockJustification { } /// Justification used to prove block finality. -/// TODO switch back to pub(crate) 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<()>; diff --git a/client/grandpa-warp-sync/src/lib.rs b/client/grandpa-warp-sync/src/lib.rs index 96b80da367b78..6b8df4a8a07d7 100644 --- a/client/grandpa-warp-sync/src/lib.rs +++ b/client/grandpa-warp-sync/src/lib.rs @@ -56,7 +56,7 @@ pub fn request_response_config_for_chain RequestResponseConfig { RequestResponseConfig { name: generate_protocol_name(protocol_id).into(), @@ -67,7 +67,7 @@ pub fn generate_request_response_config(protocol_id: ProtocolId) -> RequestRespo } } -/// Generate the block protocol name from chain specific protocol identifier. +/// Generate the grandpa warp sync protocol name from chain specific protocol identifier. fn generate_protocol_name(protocol_id: ProtocolId) -> String { let mut s = String::new(); s.push_str("/"); @@ -81,7 +81,7 @@ struct Request { begin: B::Hash } -/// Handler for incoming block requests from a remote peer. +/// Handler for incoming grandpa warp sync requests from a remote peer. pub struct GrandpaWarpSyncRequestHandler { backend: Arc, request_receiver: mpsc::Receiver, @@ -89,7 +89,7 @@ pub struct GrandpaWarpSyncRequestHandler { } impl> GrandpaWarpSyncRequestHandler { - /// Create a new [`BlockRequestHandler`]. + /// Create a new [`GrandpaWarpSyncRequestHandler`]. pub fn new(protocol_id: ProtocolId, backend: Arc) -> (Self, RequestResponseConfig) { let (tx, request_receiver) = mpsc::channel(20); @@ -116,7 +116,7 @@ impl> GrandpaWarpSyncRequestHandler: sc_finality_grandpa::BlockNumberOps, { From d44b2e8605559ba3723aea3e21b060b1ba50b7b5 Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Tue, 5 Jan 2021 16:20:56 +0100 Subject: [PATCH 19/34] Cheme changes --- client/finality-grandpa/src/finality_proof.rs | 6 +++--- client/grandpa-warp-sync/src/lib.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index 3416eec09a180..da3504fd30d38 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -648,7 +648,7 @@ pub fn check_authority_proof( fn check_authority_proof_fragment( current_set_id: u64, current_authorities: &AuthorityList, - current_block: &NumberFor, + previous_checked_block: &NumberFor, is_last: bool, authorities_proof: &AuthoritySetProofFragment, ) -> ClientResult<(u64, AuthorityList, NumberFor)> @@ -666,7 +666,7 @@ fn check_authority_proof_fragment( return Err(ClientError::Backend("Invalid authority warp proof, justification do not match header".to_string())); } - if authorities_proof.header.number() <= current_block { + 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(); @@ -686,7 +686,7 @@ fn check_authority_proof_fragment( at_block = Some((dest, next_authorities)); } - // only fragment with no change for target + // 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())); } diff --git a/client/grandpa-warp-sync/src/lib.rs b/client/grandpa-warp-sync/src/lib.rs index 6b8df4a8a07d7..5233d88f85c2b 100644 --- a/client/grandpa-warp-sync/src/lib.rs +++ b/client/grandpa-warp-sync/src/lib.rs @@ -1,4 +1,4 @@ -// Copyright 2020 Parity Technologies (UK) Ltd. +// Copyright 2021 Parity Technologies (UK) Ltd. // This file is part of Substrate. // Substrate is free software: you can redistribute it and/or modify From 0b5c107195bbbea56c19f91c89ae2c017a11074e Mon Sep 17 00:00:00 2001 From: cheme Date: Tue, 5 Jan 2021 16:39:57 +0100 Subject: [PATCH 20/34] Remove todos and test compile. --- client/finality-grandpa/src/finality_proof.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index b2c33cf3e46c4..e15d4674092f9 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -448,16 +448,12 @@ pub(crate) fn prove_finality, J>( /// 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'. -/// TODO switch to pub(crate) pub fn prove_authority, J>( blockchain: &B, begin: Block::Hash, ) -> ::sp_blockchain::Result> where J: ProvableJustification, - - NumberFor: BlockNumberOps, // TODO remove after debugging - B: BlockchainBackend, // TODO remove after debugging { let begin = BlockId::Hash(begin); @@ -473,8 +469,6 @@ pub fn prove_authority, J>( } // TODO fetch bonding duration and store it to error when not from it. - - // TODO use a cache here and encode on slice. let mut result = Vec::new(); let mut header = blockchain.expect_header(begin)?; @@ -775,7 +769,6 @@ pub trait BlockJustification { } /// Justification used to prove block finality. -/// TODO switch back to pub(crate) 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<()>; From a3a3143d32bef599d90ecec6bdeedfa18b4112cd Mon Sep 17 00:00:00 2001 From: cheme Date: Tue, 5 Jan 2021 16:42:50 +0100 Subject: [PATCH 21/34] Rename _authority_ related proof function to _warp_sync_ . --- client/finality-grandpa/src/finality_proof.rs | 14 ++++++-------- client/grandpa-warp-sync/src/lib.rs | 2 +- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index e15d4674092f9..8db06c166e329 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -448,12 +448,10 @@ pub(crate) fn prove_finality, J>( /// 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_authority, J>( +pub fn prove_warp_sync>( blockchain: &B, begin: Block::Hash, ) -> ::sp_blockchain::Result> - where - J: ProvableJustification, { let begin = BlockId::Hash(begin); @@ -602,7 +600,7 @@ pub(crate) fn check_finality_proof( /// Check GRANDPA authority change sequence to assert finality of a target block. /// /// Returns the header of the target block. -pub fn check_authority_proof( +pub(crate) fn check_warp_sync_proof( current_set_id: u64, current_authorities: AuthorityList, remote_proof: Vec, @@ -621,7 +619,7 @@ pub fn check_authority_proof( for (ix, fragment) in proof.into_iter().enumerate() { let is_last = ix == last; - result = check_authority_proof_fragment::( + result = check_warp_sync_proof_fragment::( result.0, &result.1, &result.2, @@ -639,7 +637,7 @@ pub fn check_authority_proof( } /// Check finality authority set sequence. -fn check_authority_proof_fragment( +fn check_warp_sync_proof_fragment( current_set_id: u64, current_authorities: &AuthorityList, current_block: &NumberFor, @@ -760,7 +758,7 @@ impl AuthoritiesOrEffects
{ } /// Block info extracted from the justification. -pub trait BlockJustification { +pub(crate) trait BlockJustification { /// Block number justified. fn number(&self) -> Header::Number; @@ -769,7 +767,7 @@ pub trait BlockJustification { } /// Justification used to prove block finality. -pub trait ProvableJustification: Encode + Decode { +pub(crate) 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<()>; diff --git a/client/grandpa-warp-sync/src/lib.rs b/client/grandpa-warp-sync/src/lib.rs index 4ff66ed237be1..45eec69643f70 100644 --- a/client/grandpa-warp-sync/src/lib.rs +++ b/client/grandpa-warp-sync/src/lib.rs @@ -118,7 +118,7 @@ impl> GrandpaWarpSyncRequestHandler::decode(&mut &payload[..])?; - let response = sc_finality_grandpa::finality_proof::prove_authority::<_, _, GrandpaJustification>( + let response = sc_finality_grandpa::finality_proof::prove_warp_sync( self.backend.blockchain(), request.begin, )?; From 19448bb749c0bec8f68d0971ff2a3bbc4e1cadcf Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Tue, 5 Jan 2021 17:17:16 +0100 Subject: [PATCH 22/34] Put the warp sync request response protocol behind a feature flag because we dont' need it on a light client. --- bin/node/cli/Cargo.toml | 3 ++- bin/node/cli/src/service.rs | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/bin/node/cli/Cargo.toml b/bin/node/cli/Cargo.toml index de7969b84889a..e4c2771e4f2a4 100644 --- a/bin/node/cli/Cargo.toml +++ b/bin/node/cli/Cargo.toml @@ -75,7 +75,7 @@ sc-service = { version = "0.8.0", default-features = false, path = "../../../cli sc-tracing = { version = "2.0.0", path = "../../../client/tracing" } sc-telemetry = { version = "2.0.0", path = "../../../client/telemetry" } sc-authority-discovery = { version = "0.8.0", path = "../../../client/authority-discovery" } -sc-grandpa-warp-sync = { version = "0.8.0", path = "../../../client/grandpa-warp-sync" } +sc-grandpa-warp-sync = { version = "0.8.0", path = "../../../client/grandpa-warp-sync", optional = true } # frame dependencies pallet-indices = { version = "2.0.0", path = "../../../frame/indices" } @@ -153,6 +153,7 @@ cli = [ "frame-benchmarking-cli", "substrate-frame-cli", "sc-service/db", + "sc-grandpa-warp-sync", "structopt", "substrate-build-script-utils", ] diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index 99a644ddb9b06..b38e4ae931139 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -180,6 +180,7 @@ pub fn new_full_base( config.network.notifications_protocols.push(grandpa::GRANDPA_PROTOCOL_NAME.into()); + #[cfg(feature = "cli")] config.network.request_response_protocols.push(sc_grandpa_warp_sync::request_response_config_for_chain( &config, task_manager.spawn_handle(), backend.clone(), )); From 97736e11bd50b22699910e6aa04243fac612b4df Mon Sep 17 00:00:00 2001 From: cheme Date: Tue, 5 Jan 2021 17:04:28 +0100 Subject: [PATCH 23/34] Update client/grandpa-warp-sync/src/lib.rs quick fix --- client/grandpa-warp-sync/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/grandpa-warp-sync/src/lib.rs b/client/grandpa-warp-sync/src/lib.rs index 87be0af6346aa..9a11af88783ae 100644 --- a/client/grandpa-warp-sync/src/lib.rs +++ b/client/grandpa-warp-sync/src/lib.rs @@ -108,7 +108,7 @@ impl> GrandpaWarpSyncRequestHandler::decode(&mut &payload[..])?; - let response = sc_finality_grandpa::finality_proof::prove_warp_sync( + let response = sc_finality_grandpa::prove_warp_sync( self.backend.blockchain(), request.begin, )?; From 1776180af11983c7cdd037403cb2e4237d421aca Mon Sep 17 00:00:00 2001 From: cheme Date: Tue, 5 Jan 2021 17:54:57 +0100 Subject: [PATCH 24/34] Update client/grandpa-warp-sync/src/lib.rs Quick fix --- client/grandpa-warp-sync/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/client/grandpa-warp-sync/src/lib.rs b/client/grandpa-warp-sync/src/lib.rs index 9a11af88783ae..9b336061c4167 100644 --- a/client/grandpa-warp-sync/src/lib.rs +++ b/client/grandpa-warp-sync/src/lib.rs @@ -20,7 +20,6 @@ use codec::Decode; use sc_network::config::{ProtocolId, IncomingRequest, RequestResponseConfig}; use sc_client_api::Backend; -use sc_finality_grandpa::GrandpaJustification; use sp_runtime::traits::NumberFor; use futures::channel::{mpsc, oneshot}; use futures::stream::StreamExt; From cc6722cd344ba5d539175561245df00637b31443 Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Mon, 11 Jan 2021 12:11:40 +0100 Subject: [PATCH 25/34] Update Cargo.lock --- Cargo.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 7610f3a53ab74..c602658738081 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7095,7 +7095,7 @@ name = "sc-grandpa-warp-sync" version = "0.8.0" dependencies = [ "derive_more", - "futures 0.3.8", + "futures 0.3.9", "log", "num-traits", "parity-scale-codec", From 1cb4290e855fc77cb88312bd3bfb00d1e969756f Mon Sep 17 00:00:00 2001 From: Emeric Chevalier Date: Mon, 11 Jan 2021 18:02:39 +0100 Subject: [PATCH 26/34] Adding test, comment on limitation related to 'delay', this could be implemented but with a cost. --- client/finality-grandpa/src/finality_proof.rs | 188 +++++++++++++++++- client/grandpa-warp-sync/src/lib.rs | 6 +- 2 files changed, 190 insertions(+), 4 deletions(-) diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index 4f9c4869097aa..a41330c736c53 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -448,13 +448,19 @@ pub(crate) fn prove_finality, J>( /// 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'. +/// +/// Note that if block 'begin' is in a scheduled change interval, the proof is very likely +/// to be invalid. One way to prevent that would be to look backward for the previous authority +/// set change and see if a pending set change is in progress. This is not implemented at this +/// point. +/// Similarily changing an authority set during a 'delay' will not work with the given algo. pub fn prove_warp_sync>( blockchain: &B, begin: Block::Hash, + max_fragment_limit: Option, ) -> ::sp_blockchain::Result> { - - let begin = BlockId::Hash(begin); + let begin = BlockId::Hash(begin); let begin_number = blockchain.block_number_from_id(&begin)? .ok_or_else(|| ClientError::Backend("Missing start block".to_string()))?; let end = BlockId::Hash(blockchain.last_finalized()?); @@ -466,13 +472,16 @@ pub fn prove_warp_sync>( return Err(ClientError::Backend("Unfinalized start for authority proof".to_string())); } - // TODO fetch bonding duration and store it to error when not from it. let mut result = Vec::new(); let mut header = blockchain.expect_header(begin)?; let mut index = *header.number() + One::one(); while index <= end_number { + if max_fragment_limit.map(|limit| result.len() <= limit).unwrap_or(false) { + break; + } + header = blockchain.expect_header(BlockId::number(index))?; if let Some((block_number, sp_finality_grandpa::ScheduledChange { @@ -859,6 +868,24 @@ 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) + } + } + fn header(number: u64) -> Header { let parent_hash = match number { 0 => Default::default(), @@ -1251,4 +1278,159 @@ pub(crate) mod tests { ).unwrap(); assert!(proof_of_4.is_none()); } + + #[test] + // TODO better name + fn warp_sync_proof() { + 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 proof = prove_warp_sync(&blockchain, hashes[6], None).unwrap(); + + 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).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).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/grandpa-warp-sync/src/lib.rs b/client/grandpa-warp-sync/src/lib.rs index 9b336061c4167..d5c18ec119622 100644 --- a/client/grandpa-warp-sync/src/lib.rs +++ b/client/grandpa-warp-sync/src/lib.rs @@ -80,6 +80,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; + /// Handler for incoming grandpa warp sync requests from a remote peer. pub struct GrandpaWarpSyncRequestHandler { backend: Arc, @@ -108,7 +112,7 @@ impl> GrandpaWarpSyncRequestHandler::decode(&mut &payload[..])?; let response = sc_finality_grandpa::prove_warp_sync( - self.backend.blockchain(), request.begin, + self.backend.blockchain(), request.begin, Some(WARP_SYNC_FRAGMENTS_LIMIT) )?; pending_response.send(response) From 5deb425886da31b3aba3769573acf6d5d60066a1 Mon Sep 17 00:00:00 2001 From: cheme Date: Tue, 12 Jan 2021 10:27:58 +0100 Subject: [PATCH 27/34] Set between a delay override last fragment. --- client/finality-grandpa/src/finality_proof.rs | 103 +++++++++--------- 1 file changed, 53 insertions(+), 50 deletions(-) diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index a41330c736c53..7682321a6b79b 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -453,7 +453,6 @@ pub(crate) fn prove_finality, J>( /// to be invalid. One way to prevent that would be to look backward for the previous authority /// set change and see if a pending set change is in progress. This is not implemented at this /// point. -/// Similarily changing an authority set during a 'delay' will not work with the given algo. pub fn prove_warp_sync>( blockchain: &B, begin: Block::Hash, @@ -474,63 +473,26 @@ pub fn prove_warp_sync>( let mut result = Vec::new(); - let mut header = blockchain.expect_header(begin)?; + let header = blockchain.expect_header(begin)?; let mut index = *header.number() + One::one(); + let mut last_apply = None; + while index <= end_number { if max_fragment_limit.map(|limit| result.len() <= limit).unwrap_or(false) { break; } - 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 dest <= end_number { - if let Some(justification) = blockchain.justification(BlockId::Number(index.clone()))? { - result.push(AuthoritySetProofFragment { - header: header.clone(), - justification, - }); - } else { - println!("should be unreachable"); - } - - let inc = delay == Zero::zero() && block_number == index; - index = dest; - if inc { - index += One::one(); - } - continue; + if let Some((fragement, apply_block)) = get_warp_sync_proof_fragment(blockchain, index)? { + 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); } - if let Some(sp_finality_grandpa::ScheduledChange { - next_authorities: _, - delay, - }) = crate::import::find_scheduled_change::(&header) { - let dest = index + delay; - if dest <= end_number { - if let Some(justification) = blockchain.justification(BlockId::Number(index.clone()))? { - result.push(AuthoritySetProofFragment { - header: header.clone(), - justification, - }); - } else { - println!("should be unreachable"); - } - index = dest; - if delay == Zero::zero() { - index += One::one(); - } - continue; - } - } - - index = *header.number() + One::one(); + index = index + One::one(); } if result.last().as_ref().map(|head| head.header.number()) != Some(&end_number) { @@ -548,6 +510,48 @@ pub fn prove_warp_sync>( 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, +) -> ::sp_blockchain::Result, NumberFor)>> +{ + + 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()))? { + return Ok(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()))? { + return Ok(Some((AuthoritySetProofFragment { + header: header.clone(), + justification, + }, dest))); + } else { + return Err(ClientError::Backend("Unjustified block with authority set change".to_string())); + } + } + + Ok(None) +} + /// Check GRANDPA proof-of-finality for the given block. /// /// Returns the vector of headers that MUST be validated + imported @@ -1280,8 +1284,7 @@ pub(crate) mod tests { } #[test] - // TODO better name - fn warp_sync_proof() { + fn warp_sync_proof_encoding_decoding() { fn test_blockchain( nb_blocks: u64, mut set_change: &[(u64, Vec)], From b887b7febc8a5a31675aa3cb7eee40b06e1624a8 Mon Sep 17 00:00:00 2001 From: cheme Date: Tue, 12 Jan 2021 10:52:55 +0100 Subject: [PATCH 28/34] Check for pending authority set change at start. --- client/finality-grandpa/src/finality_proof.rs | 27 ++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index 7682321a6b79b..ca9744ffcac97 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -448,11 +448,6 @@ pub(crate) fn prove_finality, J>( /// 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'. -/// -/// Note that if block 'begin' is in a scheduled change interval, the proof is very likely -/// to be invalid. One way to prevent that would be to look backward for the previous authority -/// set change and see if a pending set change is in progress. This is not implemented at this -/// point. pub fn prove_warp_sync>( blockchain: &B, begin: Block::Hash, @@ -472,12 +467,26 @@ pub fn prove_warp_sync>( } let mut result = Vec::new(); + let mut last_apply = None; let header = blockchain.expect_header(begin)?; - let mut index = *header.number() + One::one(); + let mut index = *header.number(); - let mut last_apply = None; + // 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() { + if let Some((fragement, apply_block)) = get_warp_sync_proof_fragment(blockchain, index)? { + if last_apply.map(|next| &next > header.number()).unwrap_or(false) { + result.push(fragement); + last_apply = Some(apply_block); + } else { + break; + } + } + index = index - One::one(); + } + let mut index = *header.number() + One::one(); while index <= end_number { if max_fragment_limit.map(|limit| result.len() <= limit).unwrap_or(false) { break; @@ -485,7 +494,7 @@ pub fn prove_warp_sync>( if let Some((fragement, apply_block)) = get_warp_sync_proof_fragment(blockchain, index)? { if last_apply.map(|next| apply_block < next).unwrap_or(false) { - // previous delayed will not apply, do not include it. + // Previous delayed will not apply, do not include it. result.pop(); } result.push(fragement); @@ -503,7 +512,7 @@ pub fn prove_warp_sync>( justification, }); } else { - println!("should be unreachable"); + // no justification, don't include it. } } From 5c235fab0c3a8565e9aaf9e8b5f183e426ac722d Mon Sep 17 00:00:00 2001 From: cheme Date: Tue, 12 Jan 2021 13:04:08 +0100 Subject: [PATCH 29/34] adjust index --- client/finality-grandpa/src/finality_proof.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index ca9744ffcac97..c841968919324 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -475,6 +475,7 @@ pub fn prove_warp_sync>( // 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((fragement, apply_block)) = get_warp_sync_proof_fragment(blockchain, index)? { if last_apply.map(|next| &next > header.number()).unwrap_or(false) { result.push(fragement); @@ -483,10 +484,9 @@ pub fn prove_warp_sync>( break; } } - index = index - One::one(); } - let mut index = *header.number() + One::one(); + let mut index = *header.number(); while index <= end_number { if max_fragment_limit.map(|limit| result.len() <= limit).unwrap_or(false) { break; From eaa54b2b49ae97ac49c6208efb594873da662b24 Mon Sep 17 00:00:00 2001 From: cheme Date: Tue, 12 Jan 2021 14:57:19 +0100 Subject: [PATCH 30/34] custom cache is not a good idea. --- client/finality-grandpa/src/finality_proof.rs | 165 ++++++++++++++++-- 1 file changed, 152 insertions(+), 13 deletions(-) diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index c841968919324..2e9ecc9baea56 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -252,7 +252,7 @@ type FinalityProof
= Vec>; /// Finality for block B is proved by providing: /// 1) headers of this block; /// 2) the justification for the block containing a authority set change digest; -#[derive(Debug, PartialEq, Encode, Decode)] +#[derive(Debug, PartialEq, Clone, Encode, Decode)] pub(crate) struct AuthoritySetProofFragment { /// The header of the given block. pub header: Header, @@ -452,6 +452,7 @@ pub fn prove_warp_sync>( blockchain: &B, begin: Block::Hash, max_fragment_limit: Option, + cache: Option<&mut WarpSyncFragmentCache>, ) -> ::sp_blockchain::Result> { let begin = BlockId::Hash(begin); @@ -470,34 +471,66 @@ pub fn prove_warp_sync>( let mut last_apply = None; let header = blockchain.expect_header(begin)?; - let mut index = *header.number(); + let begin = *header.number(); + let mut index = begin; // 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((fragement, apply_block)) = get_warp_sync_proof_fragment(blockchain, index)? { - if last_apply.map(|next| &next > header.number()).unwrap_or(false) { - result.push(fragement); - last_apply = Some(apply_block); - } else { - break; + if let Some(cache_item) = cache.and_then(|cache| cache.get_item(begin)) { + if cache_item.apply > begin { + result.push(cache_item.fragment.clone()); + last_apply = Some(cache_item.apply); + } + index = cache_item.end.unwrap_or_else(|| *header.number()); + } else { + while index > Zero::zero() { + index = index - One::one(); + if let Some((fragment, apply_block)) = get_warp_sync_proof_fragment(blockchain, index)? { + if last_apply.map(|next| &next > header.number()).unwrap_or(false) { + result.push(fragment); + last_apply = Some(apply_block); + } else { + break; + } } } + index = *header.number(); } - let mut index = *header.number(); + let mut cache_at = None; + while index <= end_number { + cache.map(|cache| { + if let Some((at, item)) = cache_at.as_ref() { + if at == index { + if let Some((start, at, end, fragment)) = cache_at.take() { + cache.new_item( + start, + at, + end, + fragment, + ); + } + } + } + }); if max_fragment_limit.map(|limit| result.len() <= limit).unwrap_or(false) { break; } - if let Some((fragement, apply_block)) = get_warp_sync_proof_fragment(blockchain, index)? { + if let Some((fragment, apply_block)) = get_warp_sync_proof_fragment(blockchain, index)? { if last_apply.map(|next| apply_block < next).unwrap_or(false) { // Previous delayed will not apply, do not include it. result.pop(); + cache_at = None; } - result.push(fragement); + cache.map(|cache| { + result.last().map(|last| { + let start = *last.header.number(); + cache_at = Some((start, apply_block, index, last.clone())); + }); + }); + result.push(fragment); last_apply = Some(apply_block); } @@ -828,6 +861,112 @@ impl BlockJustification for GrandpaJustification { + // First block to apply this authority set is key, + // last block is added with authority set. + cache: std::collections::BTreeMap>>, + + lru_last: *mut CacheItem
, + lru_first: *mut CacheItem
, +} + +struct CacheItem { + fragment: AuthoritySetProofFragment
, + start: Header::Number, + apply: Header::Number, + end: Option, + lru_prev: *mut CacheItem
, + lru_next: *mut CacheItem
, +} + +impl WarpSyncFragmentCache
{ + /// Instantiate a new cache for the warp sync prover. + pub fn new() -> Self { + WarpSyncFragmentCache { + cache: Default::default(), + lru_last: std::ptr::null_mut(), + lru_first: std::ptr::null_mut(), + } + } + + fn new_item( + &mut self, + start: Header::Number, + apply: Header::Number, + end: Option, + fragment: AuthoritySetProofFragment
, + ) { + if self.cache.len() == WARP_SYNC_CACHE_SIZE { + self.pop_first(); + } + + let mut item = Box::new(CacheItem { + fragment, + start: start.clone(), + apply, + end, + lru_prev: std::ptr::null_mut(), + lru_next: std::ptr::null_mut(), + }); + if self.cache.len() == 0 { + self.lru_first = item.as_mut() as *mut _; + } + if self.cache.len() > 0 { + item.lru_prev = self.lru_last; + let last = unsafe { self.lru_last.as_mut().unwrap() }; + last.lru_next = item.as_mut() as *mut _; + } + self.lru_last = item.as_mut() as *mut _; + self.cache.insert(start.clone(), item); + } + + fn pop_first(&mut self) { + if self.cache.len() == 0 { + return; + } + if self.cache.len() == 1 { + self.cache.clear(); + self.lru_first = std::ptr::null_mut(); + self.lru_last = std::ptr::null_mut(); + return; + } + let first = unsafe { self.lru_first.as_mut().unwrap() }; + self.lru_first = first.lru_next; + let second = unsafe { first.lru_next.as_mut().unwrap() }; + second.lru_prev = std::ptr::null_mut(); + self.cache.remove(&first.start.clone()); + } + + fn get_item( + &mut self, + block: Header::Number, + ) -> Option<&CacheItem
> { + // Not the quickest. + if let Some((_start, mut item)) = self.cache.iter_mut().find(|k| &block >= k.0) { + if item.end.map(|end| block < end).unwrap_or(true) { + if item.lru_prev != std::ptr::null_mut() { + let prev = unsafe { item.lru_prev.as_mut().unwrap() }; + prev.lru_next = item.lru_next; + } + item.lru_prev = self.lru_last; + item.lru_next = std::ptr::null_mut(); + let last = unsafe { self.lru_last.as_mut().unwrap() }; + last.lru_next = item.as_mut() as *mut _; + self.lru_last = item.as_mut() as *mut _; + return Some(&item); + } + } + + None + } +} + #[cfg(test)] pub(crate) mod tests { use substrate_test_runtime_client::runtime::{Block, Header, H256}; From 0bddd32deee56777950860f0147a485a05d44fe1 Mon Sep 17 00:00:00 2001 From: cheme Date: Tue, 12 Jan 2021 16:22:30 +0100 Subject: [PATCH 31/34] Use a simple cache instead. --- Cargo.lock | 2 + client/finality-grandpa/Cargo.toml | 1 + client/finality-grandpa/src/finality_proof.rs | 601 ++++++++---------- client/finality-grandpa/src/lib.rs | 2 +- client/grandpa-warp-sync/Cargo.toml | 1 + client/grandpa-warp-sync/src/lib.rs | 15 +- 6 files changed, 280 insertions(+), 342 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c602658738081..3ad07080609c6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7025,6 +7025,7 @@ dependencies = [ "fork-tree", "futures 0.3.9", "futures-timer 3.0.2", + "linked-hash-map", "log", "parity-scale-codec", "parking_lot 0.11.1", @@ -7099,6 +7100,7 @@ dependencies = [ "log", "num-traits", "parity-scale-codec", + "parking_lot 0.11.1", "prost", "sc-client-api", "sc-finality-grandpa", diff --git a/client/finality-grandpa/Cargo.toml b/client/finality-grandpa/Cargo.toml index 831ac509ff0af..70b31f77a301d 100644 --- a/client/finality-grandpa/Cargo.toml +++ b/client/finality-grandpa/Cargo.toml @@ -45,6 +45,7 @@ prometheus-endpoint = { package = "substrate-prometheus-endpoint", path = "../.. sc-block-builder = { version = "0.8.0", path = "../block-builder" } finality-grandpa = { version = "0.12.3", features = ["derive-codec"] } pin-project = "0.4.6" +linked-hash-map = "0.5.2" [dev-dependencies] assert_matches = "1.3.0" diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index 2e9ecc9baea56..4a25aa2bf762e 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -452,9 +452,9 @@ pub fn prove_warp_sync>( blockchain: &B, begin: Block::Hash, max_fragment_limit: Option, - cache: Option<&mut WarpSyncFragmentCache>, -) -> ::sp_blockchain::Result> -{ + mut cache: Option<&mut WarpSyncFragmentCache>, +) -> ::sp_blockchain::Result> { + let begin = BlockId::Hash(begin); let begin_number = blockchain.block_number_from_id(&begin)? .ok_or_else(|| ClientError::Backend("Missing start block".to_string()))?; @@ -471,66 +471,34 @@ pub fn prove_warp_sync>( let mut last_apply = None; let header = blockchain.expect_header(begin)?; - let begin = *header.number(); - let mut index = 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. - if let Some(cache_item) = cache.and_then(|cache| cache.get_item(begin)) { - if cache_item.apply > begin { - result.push(cache_item.fragment.clone()); - last_apply = Some(cache_item.apply); - } - index = cache_item.end.unwrap_or_else(|| *header.number()); - } else { - while index > Zero::zero() { - index = index - One::one(); - if let Some((fragment, apply_block)) = get_warp_sync_proof_fragment(blockchain, index)? { - if last_apply.map(|next| &next > header.number()).unwrap_or(false) { - result.push(fragment); - last_apply = Some(apply_block); - } else { - break; - } + while index > Zero::zero() { + index = index - One::one(); + if let Some((fragement, apply_block)) = get_warp_sync_proof_fragment(blockchain, index, &mut cache)? { + if last_apply.map(|next| &next > header.number()).unwrap_or(false) { + result.push(fragement); + last_apply = Some(apply_block); + } else { + break; } } - index = *header.number(); } - let mut cache_at = None; - + let mut index = *header.number(); while index <= end_number { - cache.map(|cache| { - if let Some((at, item)) = cache_at.as_ref() { - if at == index { - if let Some((start, at, end, fragment)) = cache_at.take() { - cache.new_item( - start, - at, - end, - fragment, - ); - } - } - } - }); if max_fragment_limit.map(|limit| result.len() <= limit).unwrap_or(false) { break; } - if let Some((fragment, apply_block)) = get_warp_sync_proof_fragment(blockchain, index)? { + 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(); - cache_at = None; } - cache.map(|cache| { - result.last().map(|last| { - let start = *last.header.number(); - cache_at = Some((start, apply_block, index, last.clone())); - }); - }); - result.push(fragment); + result.push(fragement); last_apply = Some(apply_block); } @@ -556,9 +524,15 @@ pub fn prove_warp_sync>( fn get_warp_sync_proof_fragment>( blockchain: &B, index: NumberFor, -) -> ::sp_blockchain::Result, 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.clone()); + } + } + let mut result = None; let header = blockchain.expect_header(BlockId::number(index))?; if let Some((block_number, sp_finality_grandpa::ScheduledChange { @@ -567,10 +541,10 @@ fn get_warp_sync_proof_fragment>( })) = crate::import::find_forced_change::(&header) { let dest = block_number + delay; if let Some(justification) = blockchain.justification(BlockId::Number(index.clone()))? { - return Ok(Some((AuthoritySetProofFragment { + result = Some((AuthoritySetProofFragment { header: header.clone(), justification, - }, dest))); + }, dest)); } else { return Err(ClientError::Backend("Unjustified block with authority set change".to_string())); } @@ -582,16 +556,17 @@ fn get_warp_sync_proof_fragment>( }) = crate::import::find_scheduled_change::(&header) { let dest = index + delay; if let Some(justification) = blockchain.justification(BlockId::Number(index.clone()))? { - return Ok(Some((AuthoritySetProofFragment { + result = Some((AuthoritySetProofFragment { header: header.clone(), justification, - }, dest))); + }, dest)); } else { return Err(ClientError::Backend("Unjustified block with authority set change".to_string())); } } - Ok(None) + cache.as_mut().map(|cache| cache.new_item(index, result.clone())); + Ok(result) } /// Check GRANDPA proof-of-finality for the given block. @@ -605,7 +580,7 @@ pub(crate) fn check_finality_proof( authorities_provider: &dyn AuthoritySetForFinalityChecker, remote_proof: Vec, ) -> ClientResult> - where +where NumberFor: BlockNumberOps, B: BlockchainBackend, J: ProvableJustification, @@ -614,42 +589,42 @@ pub(crate) fn check_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())); - } + // 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())); + // 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)?; - } + 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 + 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); + telemetry!(CONSENSUS_INFO; "afg.finality_proof_ok"; + "set_id" => ?effects.new_set_id, "finalized_header_hash" => ?effects.block); - Ok(effects) + Ok(effects) } /// Check GRANDPA authority change sequence to assert finality of a target block. @@ -660,7 +635,7 @@ pub(crate) fn check_warp_sync_proof( current_authorities: AuthorityList, remote_proof: Vec, ) -> ClientResult<(Block::Header, u64, AuthorityList)> - where +where NumberFor: BlockNumberOps, J: Decode + ProvableJustification + BlockJustification, { @@ -668,27 +643,27 @@ pub(crate) fn check_warp_sync_proof( let proof = AuthoritySetProof::::decode(&mut &remote_proof[..]) .map_err(|_| ClientError::BadJustification("failed to decode authority proof".into()))?; - let last = proof.len() - 1; + let last = proof.len() - 1; - let mut result = (current_set_id, current_authorities, NumberFor::::zero()); + 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, - )?; + 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)) + 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())); + // empty proof can't prove anything + return Err(ClientError::BadJustification("empty proof of authority".into())); } /// Check finality authority set sequence. @@ -699,49 +674,49 @@ fn check_warp_sync_proof_fragment( is_last: bool, authorities_proof: &AuthoritySetProofFragment, ) -> ClientResult<(u64, AuthorityList, NumberFor)> - where +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)?; + 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())); - } + // 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)); - } + 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())) - } + // 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())) + } } /// Check finality proof for the single block. @@ -751,7 +726,7 @@ fn check_finality_proof_fragment( authorities_provider: &dyn AuthoritySetForFinalityChecker, proof_fragment: FinalityProofFragment, ) -> ClientResult> - where +where NumberFor: BlockNumberOps, B: BlockchainBackend, J: Decode + ProvableJustification, @@ -760,34 +735,34 @@ fn check_finality_proof_fragment( 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, - )?; + 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; - } + 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, - })) + 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. @@ -834,13 +809,13 @@ pub(crate) trait ProvableJustification: Encode + Decode { ) -> ClientResult { let justification = Self::decode(&mut &**justification) .map_err(|_| ClientError::JustificationDecode)?; - justification.verify(set_id, authorities)?; - Ok(justification) + justification.verify(set_id, authorities)?; + Ok(justification) } } impl ProvableJustification for GrandpaJustification - where +where NumberFor: BlockNumberOps, { fn verify(&self, set_id: u64, authorities: &[(AuthorityId, u64)]) -> ClientResult<()> { @@ -861,109 +836,56 @@ impl BlockJustification for GrandpaJustification { - // First block to apply this authority set is key, - // last block is added with authority set. - cache: std::collections::BTreeMap>>, - - lru_last: *mut CacheItem
, - lru_first: *mut CacheItem
, -} - -struct CacheItem { - fragment: AuthoritySetProofFragment
, - start: Header::Number, - apply: Header::Number, - end: Option, - lru_prev: *mut CacheItem
, - lru_next: *mut CacheItem
, +pub struct WarpSyncFragmentCache { + cache: linked_hash_map::LinkedHashMap< + Header::Number, + Option<(AuthoritySetProofFragment
, Header::Number)>, + >, + headers_with_justification: usize, + limit: usize, } impl WarpSyncFragmentCache
{ /// Instantiate a new cache for the warp sync prover. - pub fn new() -> Self { + pub fn new(size: usize) -> Self { WarpSyncFragmentCache { cache: Default::default(), - lru_last: std::ptr::null_mut(), - lru_first: std::ptr::null_mut(), + headers_with_justification: 0, + limit: size, } } fn new_item( &mut self, - start: Header::Number, - apply: Header::Number, - end: Option, - fragment: AuthoritySetProofFragment
, + at: Header::Number, + item: Option<(AuthoritySetProofFragment
, Header::Number)>, ) { - if self.cache.len() == WARP_SYNC_CACHE_SIZE { - self.pop_first(); - } - - let mut item = Box::new(CacheItem { - fragment, - start: start.clone(), - apply, - end, - lru_prev: std::ptr::null_mut(), - lru_next: std::ptr::null_mut(), - }); - if self.cache.len() == 0 { - self.lru_first = item.as_mut() as *mut _; + if self.cache.len() == self.limit { + self.pop_one(); } - if self.cache.len() > 0 { - item.lru_prev = self.lru_last; - let last = unsafe { self.lru_last.as_mut().unwrap() }; - last.lru_next = item.as_mut() as *mut _; + if item.is_some() { + // we do not check previous value as cached value is always supposed to + // be queried before calling 'new_item'. + self.headers_with_justification += 1; } - self.lru_last = item.as_mut() as *mut _; - self.cache.insert(start.clone(), item); + self.cache.insert(at, item); } - fn pop_first(&mut self) { - if self.cache.len() == 0 { - return; - } - if self.cache.len() == 1 { - self.cache.clear(); - self.lru_first = std::ptr::null_mut(); - self.lru_last = std::ptr::null_mut(); - return; + fn pop_one(&mut self) { + while let Some(v) = self.cache.pop_front() { + if v.1.is_some() { + self.headers_with_justification -= 1; + break; + } } - let first = unsafe { self.lru_first.as_mut().unwrap() }; - self.lru_first = first.lru_next; - let second = unsafe { first.lru_next.as_mut().unwrap() }; - second.lru_prev = std::ptr::null_mut(); - self.cache.remove(&first.start.clone()); } fn get_item( &mut self, block: Header::Number, - ) -> Option<&CacheItem
> { - // Not the quickest. - if let Some((_start, mut item)) = self.cache.iter_mut().find(|k| &block >= k.0) { - if item.end.map(|end| block < end).unwrap_or(true) { - if item.lru_prev != std::ptr::null_mut() { - let prev = unsafe { item.lru_prev.as_mut().unwrap() }; - prev.lru_next = item.lru_next; - } - item.lru_prev = self.lru_last; - item.lru_next = std::ptr::null_mut(); - let last = unsafe { self.lru_last.as_mut().unwrap() }; - last.lru_next = item.as_mut() as *mut _; - self.lru_last = item.as_mut() as *mut _; - return Some(&item); - } - } - - None + ) -> Option<&mut Option<(AuthoritySetProofFragment
, Header::Number)>> { + self.cache.get_refresh(&block) } } @@ -978,7 +900,7 @@ pub(crate) mod tests { pub(crate) type FinalityProof = super::FinalityProof
; impl AuthoritySetForFinalityProver for (GetAuthorities, ProveAuthorities) - where + where GetAuthorities: Send + Sync + Fn(BlockId) -> ClientResult, ProveAuthorities: Send + Sync + Fn(BlockId) -> ClientResult, { @@ -995,17 +917,17 @@ pub(crate) mod tests { 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) + 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) + } } - } #[derive(Debug, PartialEq, Encode, Decode)] pub struct TestJustification(pub (u64, AuthorityList), pub Vec); @@ -1122,22 +1044,22 @@ pub(crate) mod tests { 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) .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>( - &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(); + 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>( + &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(); } #[test] @@ -1172,14 +1094,14 @@ pub(crate) mod tests { // 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>( - &blockchain, - &( - |_| Ok(authorities.clone()), - |_| unreachable!("should return before calling ProveAuthorities"), - ), - 0, - header(3).hash(), - header(5).hash(), + &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(), @@ -1198,14 +1120,14 @@ pub(crate) mod tests { // 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>( - &blockchain, - &( - |_| Ok(vec![(AuthorityId::from_slice(&[1u8; 32]), 1u64)]), - |_| unreachable!("should return before calling ProveAuthorities"), - ), - 0, - header(3).hash(), - header(5).hash(), + &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(), @@ -1232,24 +1154,24 @@ pub(crate) mod tests { // 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>( - &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(), + &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![ @@ -1484,7 +1406,7 @@ pub(crate) mod tests { blockchain.insert(header_hash, header, justification, None, NewBlockState::Final) .unwrap(); - } + } (blockchain, hashes) } @@ -1492,16 +1414,19 @@ pub(crate) mod tests { 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]), + (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 proof = prove_warp_sync(&blockchain, hashes[6], None).unwrap(); + 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) @@ -1512,43 +1437,43 @@ pub(crate) mod tests { ).collect(); assert!(check_warp_sync_proof::( - 0, - initial_authorities.clone(), - proof.clone(), + 0, + initial_authorities.clone(), + proof.clone(), ).is_err()); assert!(check_warp_sync_proof::( - 0, - authorities_next.clone(), - proof.clone(), + 0, + authorities_next.clone(), + proof.clone(), ).is_err()); assert!(check_warp_sync_proof::( - 1, - initial_authorities.clone(), - proof.clone(), + 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(), + 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).unwrap(); + 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(), + 0, + initial_authorities.clone(), + proof.clone(), ).unwrap(); assert_eq!(current_set_id, 1); @@ -1559,26 +1484,26 @@ pub(crate) mod tests { 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]), + (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).unwrap(); + 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(), + 0, + initial_authorities.clone(), + proof.clone(), ).unwrap(); assert_eq!(current_set_id, 2); diff --git a/client/finality-grandpa/src/lib.rs b/client/finality-grandpa/src/lib.rs index 87d3220e7debc..0f42b5dd1c0be 100644 --- a/client/finality-grandpa/src/lib.rs +++ b/client/finality-grandpa/src/lib.rs @@ -130,7 +130,7 @@ pub use voting_rule::{ BeforeBestBlockBy, ThreeQuartersOfTheUnfinalizedChain, VotingRule, VotingRulesBuilder }; pub use finality_grandpa::voter::report; -pub use finality_proof::prove_warp_sync; +pub use finality_proof::{prove_warp_sync, WarpSyncFragmentCache}; use aux_schema::PersistentData; use environment::{Environment, VoterSetState}; diff --git a/client/grandpa-warp-sync/Cargo.toml b/client/grandpa-warp-sync/Cargo.toml index 8a3053a1eea03..73fb697c3b2e1 100644 --- a/client/grandpa-warp-sync/Cargo.toml +++ b/client/grandpa-warp-sync/Cargo.toml @@ -25,3 +25,4 @@ derive_more = "0.99.11" codec = { package = "parity-scale-codec", version = "1.3.5" } prost = "0.6.1" num-traits = "0.2.14" +parking_lot = "0.11.1" diff --git a/client/grandpa-warp-sync/src/lib.rs b/client/grandpa-warp-sync/src/lib.rs index d5c18ec119622..48deec409db7e 100644 --- a/client/grandpa-warp-sync/src/lib.rs +++ b/client/grandpa-warp-sync/src/lib.rs @@ -28,6 +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::WarpSyncFragmentCache; /// Generates the appropriate [`RequestResponseConfig`] for a given chain configuration. pub fn request_response_config_for_chain + 'static>( @@ -84,9 +85,15 @@ struct Request { /// 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, setting a low number +/// until then. +const WARP_SYNC_CACHE_SIZE: usize = 20; + /// Handler for incoming grandpa warp sync requests from a remote peer. -pub struct GrandpaWarpSyncRequestHandler { +pub struct GrandpaWarpSyncRequestHandler { backend: Arc, + cache: Arc>>, request_receiver: mpsc::Receiver, _phantom: std::marker::PhantomData } @@ -98,8 +105,9 @@ impl> GrandpaWarpSyncRequestHandler> GrandpaWarpSyncRequestHandler::decode(&mut &payload[..])?; + let mut cache = self.cache.write(); let response = sc_finality_grandpa::prove_warp_sync( - self.backend.blockchain(), request.begin, Some(WARP_SYNC_FRAGMENTS_LIMIT) + self.backend.blockchain(), request.begin, Some(WARP_SYNC_FRAGMENTS_LIMIT), Some(&mut cache) )?; pending_response.send(response) From 827ee85f84fb5327ceee591af956a7c011693512 Mon Sep 17 00:00:00 2001 From: cheme Date: Tue, 12 Jan 2021 16:33:49 +0100 Subject: [PATCH 32/34] restore broken indentation --- client/finality-grandpa/src/finality_proof.rs | 236 +++++++++--------- 1 file changed, 118 insertions(+), 118 deletions(-) diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index 4a25aa2bf762e..a0fad2f92c881 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -580,7 +580,7 @@ pub(crate) fn check_finality_proof( authorities_provider: &dyn AuthoritySetForFinalityChecker, remote_proof: Vec, ) -> ClientResult> -where + where NumberFor: BlockNumberOps, B: BlockchainBackend, J: ProvableJustification, @@ -589,42 +589,42 @@ where 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())); - } + // 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())); - } + // 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 + 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); + telemetry!(CONSENSUS_INFO; "afg.finality_proof_ok"; + "set_id" => ?effects.new_set_id, "finalized_header_hash" => ?effects.block); - Ok(effects) + Ok(effects) } /// Check GRANDPA authority change sequence to assert finality of a target block. @@ -726,7 +726,7 @@ fn check_finality_proof_fragment( authorities_provider: &dyn AuthoritySetForFinalityChecker, proof_fragment: FinalityProofFragment, ) -> ClientResult> -where + where NumberFor: BlockNumberOps, B: BlockchainBackend, J: Decode + ProvableJustification, @@ -735,34 +735,34 @@ where 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, - )?; + 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; - } + 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, - })) + 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. @@ -809,13 +809,13 @@ pub(crate) trait ProvableJustification: Encode + Decode { ) -> ClientResult { let justification = Self::decode(&mut &**justification) .map_err(|_| ClientError::JustificationDecode)?; - justification.verify(set_id, authorities)?; - Ok(justification) + justification.verify(set_id, authorities)?; + Ok(justification) } } impl ProvableJustification for GrandpaJustification -where + where NumberFor: BlockNumberOps, { fn verify(&self, set_id: u64, authorities: &[(AuthorityId, u64)]) -> ClientResult<()> { @@ -900,7 +900,7 @@ pub(crate) mod tests { pub(crate) type FinalityProof = super::FinalityProof
; impl AuthoritySetForFinalityProver for (GetAuthorities, ProveAuthorities) - where + where GetAuthorities: Send + Sync + Fn(BlockId) -> ClientResult, ProveAuthorities: Send + Sync + Fn(BlockId) -> ClientResult, { @@ -917,17 +917,17 @@ pub(crate) mod tests { 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) - } + 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) } + } #[derive(Debug, PartialEq, Encode, Decode)] pub struct TestJustification(pub (u64, AuthorityList), pub Vec); @@ -1044,22 +1044,22 @@ pub(crate) mod tests { 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) .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>( - &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(); + 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>( + &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(); } #[test] @@ -1094,14 +1094,14 @@ pub(crate) mod tests { // 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>( - &blockchain, - &( - |_| Ok(authorities.clone()), - |_| unreachable!("should return before calling ProveAuthorities"), - ), - 0, - header(3).hash(), - header(5).hash(), + &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(), @@ -1120,14 +1120,14 @@ pub(crate) mod tests { // 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>( - &blockchain, - &( - |_| Ok(vec![(AuthorityId::from_slice(&[1u8; 32]), 1u64)]), - |_| unreachable!("should return before calling ProveAuthorities"), - ), - 0, - header(3).hash(), - header(5).hash(), + &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(), @@ -1154,24 +1154,24 @@ pub(crate) mod tests { // 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>( - &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(), + &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![ From 9d9476cde765d710250cfc5bc634b9df10b9ef33 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Thu, 21 Jan 2021 17:24:37 +0100 Subject: [PATCH 33/34] Address crate rename --- Cargo.lock | 72 +++++++++++++------ Cargo.toml | 2 +- bin/node/cli/Cargo.toml | 4 +- bin/node/cli/src/service.rs | 2 +- .../Cargo.toml | 2 +- .../src/lib.rs | 2 +- 6 files changed, 58 insertions(+), 26 deletions(-) rename client/{grandpa-warp-sync => finality-grandpa-warp-sync}/Cargo.toml (96%) rename client/{grandpa-warp-sync => finality-grandpa-warp-sync}/src/lib.rs (98%) diff --git a/Cargo.lock b/Cargo.lock index 2fe8f83db62e6..32159c6936666 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -937,7 +937,7 @@ dependencies = [ "clap", "criterion-plot", "csv", - "itertools", + "itertools 0.9.0", "lazy_static", "num-traits", "oorandom", @@ -959,7 +959,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e022feadec601fba1649cfa83586381a4ad31c6bf3a9ab7d408118b05dd9889d" dependencies = [ "cast", - "itertools", + "itertools 0.9.0", ] [[package]] @@ -2530,6 +2530,15 @@ version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "47be2f14c678be2fdcab04ab1171db51b2762ce6f0a8ee87c8dd4a04ed216135" +[[package]] +name = "itertools" +version = "0.8.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f56a2d0bc861f9165be4eb3442afd3c236d8a98afd426f65d92324ae1091a484" +dependencies = [ + "either", +] + [[package]] name = "itertools" version = "0.9.0" @@ -2878,7 +2887,7 @@ dependencies = [ "parity-multiaddr", "parking_lot 0.11.1", "pin-project 1.0.2", - "prost", + "prost 0.7.0", "prost-build", "rand 0.7.3", "ring", @@ -2935,7 +2944,7 @@ dependencies = [ "libp2p-core", "libp2p-swarm", "log", - "prost", + "prost 0.7.0", "prost-build", "rand 0.7.3", "smallvec 1.5.0", @@ -2957,7 +2966,7 @@ dependencies = [ "libp2p-core", "libp2p-swarm", "log", - "prost", + "prost 0.7.0", "prost-build", "rand 0.7.3", "regex", @@ -2977,7 +2986,7 @@ dependencies = [ "libp2p-core", "libp2p-swarm", "log", - "prost", + "prost 0.7.0", "prost-build", "smallvec 1.5.0", "wasm-timer", @@ -2998,7 +3007,7 @@ dependencies = [ "libp2p-core", "libp2p-swarm", "log", - "prost", + "prost 0.7.0", "prost-build", "rand 0.7.3", "sha2 0.9.2", @@ -3060,7 +3069,7 @@ dependencies = [ "lazy_static", "libp2p-core", "log", - "prost", + "prost 0.7.0", "prost-build", "rand 0.7.3", "sha2 0.9.2", @@ -3096,7 +3105,7 @@ dependencies = [ "futures 0.3.9", "libp2p-core", "log", - "prost", + "prost 0.7.0", "prost-build", "unsigned-varint 0.6.0", "void", @@ -3773,7 +3782,7 @@ dependencies = [ "sc-consensus-epochs", "sc-consensus-slots", "sc-finality-grandpa", - "sc-grandpa-warp-sync", + "sc-finality-grandpa-warp-sync", "sc-keystore", "sc-network", "sc-offchain", @@ -5766,6 +5775,16 @@ dependencies = [ "thiserror", ] +[[package]] +name = "prost" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ce49aefe0a6144a45de32927c77bd2859a5f7677b55f220ae5b744e87389c212" +dependencies = [ + "bytes 0.5.6", + "prost-derive 0.6.1", +] + [[package]] name = "prost" version = "0.7.0" @@ -5773,7 +5792,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9e6984d2f1a23009bd270b8bb56d0926810a3d483f59c987d77969e9d8e840b2" dependencies = [ "bytes 1.0.1", - "prost-derive", + "prost-derive 0.7.0", ] [[package]] @@ -5784,16 +5803,29 @@ checksum = "32d3ebd75ac2679c2af3a92246639f9fcc8a442ee420719cc4fe195b98dd5fa3" dependencies = [ "bytes 1.0.1", "heck", - "itertools", + "itertools 0.9.0", "log", "multimap", "petgraph", - "prost", + "prost 0.7.0", "prost-types", "tempfile", "which 4.0.2", ] +[[package]] +name = "prost-derive" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "537aa19b95acde10a12fec4301466386f757403de4cd4e5b4fa78fb5ecb18f72" +dependencies = [ + "anyhow", + "itertools 0.8.2", + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "prost-derive" version = "0.7.0" @@ -5801,7 +5833,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "169a15f3008ecb5160cba7d37bcd690a7601b6d30cfb87a117d45e59d52af5d4" dependencies = [ "anyhow", - "itertools", + "itertools 0.9.0", "proc-macro2", "quote", "syn", @@ -5814,7 +5846,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b518d7cdd93dab1d1122cf07fa9a60771836c668dde9d9e2a139f957f0d9f1bb" dependencies = [ "bytes 1.0.1", - "prost", + "prost 0.7.0", ] [[package]] @@ -6436,7 +6468,7 @@ dependencies = [ "libp2p", "log", "parity-scale-codec", - "prost", + "prost 0.7.0", "prost-build", "quickcheck", "rand 0.7.3", @@ -7045,7 +7077,7 @@ dependencies = [ ] [[package]] -name = "sc-grandpa-warp-sync" +name = "sc-finality-grandpa-warp-sync" version = "0.8.0" dependencies = [ "derive_more", @@ -7054,7 +7086,7 @@ dependencies = [ "num-traits", "parity-scale-codec", "parking_lot 0.11.1", - "prost", + "prost 0.6.1", "sc-client-api", "sc-finality-grandpa", "sc-network", @@ -7147,7 +7179,7 @@ dependencies = [ "parity-scale-codec", "parking_lot 0.11.1", "pin-project 0.4.27", - "prost", + "prost 0.7.0", "prost-build", "quickcheck", "rand 0.7.3", @@ -10501,6 +10533,6 @@ checksum = "b89249644df056b522696b1bb9e7c18c87e8ffa3e2f0dc3b0155875d6498f01b" dependencies = [ "cc", "glob", - "itertools", + "itertools 0.9.0", "libc", ] diff --git a/Cargo.toml b/Cargo.toml index cd1ca6139390e..fc22f440ca7fd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,7 +37,7 @@ members = [ "client/executor/wasmi", "client/executor/wasmtime", "client/finality-grandpa", - "client/grandpa-warp-sync", + "client/finality-grandpa-warp-sync", "client/informant", "client/keystore", "client/light", diff --git a/bin/node/cli/Cargo.toml b/bin/node/cli/Cargo.toml index 51aee2d9c2265..4c245dcf629fb 100644 --- a/bin/node/cli/Cargo.toml +++ b/bin/node/cli/Cargo.toml @@ -74,7 +74,7 @@ sc-service = { version = "0.8.0", default-features = false, path = "../../../cli sc-tracing = { version = "2.0.0", path = "../../../client/tracing" } sc-telemetry = { version = "2.0.0", path = "../../../client/telemetry" } sc-authority-discovery = { version = "0.8.0", path = "../../../client/authority-discovery" } -sc-grandpa-warp-sync = { version = "0.8.0", path = "../../../client/grandpa-warp-sync", optional = true } +sc-finality-grandpa-warp-sync = { version = "0.8.0", path = "../../../client/finality-grandpa-warp-sync", optional = true } # frame dependencies pallet-indices = { version = "2.0.0", path = "../../../frame/indices" } @@ -152,7 +152,7 @@ cli = [ "frame-benchmarking-cli", "substrate-frame-cli", "sc-service/db", - "sc-grandpa-warp-sync", + "sc-finality-grandpa-warp-sync", "structopt", "substrate-build-script-utils", ] diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index 28728014387b6..217914d2b3b09 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -195,7 +195,7 @@ 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_grandpa_warp_sync::request_response_config_for_chain( + config.network.request_response_protocols.push(sc_finality_grandpa_warp_sync::request_response_config_for_chain( &config, task_manager.spawn_handle(), backend.clone(), )); diff --git a/client/grandpa-warp-sync/Cargo.toml b/client/finality-grandpa-warp-sync/Cargo.toml similarity index 96% rename from client/grandpa-warp-sync/Cargo.toml rename to client/finality-grandpa-warp-sync/Cargo.toml index 73fb697c3b2e1..4f7ee0301f417 100644 --- a/client/grandpa-warp-sync/Cargo.toml +++ b/client/finality-grandpa-warp-sync/Cargo.toml @@ -1,6 +1,6 @@ [package] description = "A request-response protocol for handling grandpa warp sync requests" -name = "sc-grandpa-warp-sync" +name = "sc-finality-grandpa-warp-sync" version = "0.8.0" license = "GPL-3.0-or-later WITH Classpath-exception-2.0" authors = ["Parity Technologies "] diff --git a/client/grandpa-warp-sync/src/lib.rs b/client/finality-grandpa-warp-sync/src/lib.rs similarity index 98% rename from client/grandpa-warp-sync/src/lib.rs rename to client/finality-grandpa-warp-sync/src/lib.rs index 48deec409db7e..d22d74c2faeed 100644 --- a/client/grandpa-warp-sync/src/lib.rs +++ b/client/finality-grandpa-warp-sync/src/lib.rs @@ -54,7 +54,7 @@ pub fn request_response_config_for_chain RequestResponseConfig { From a31f1dd15d089038ad99abd4909bb2f081908886 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Thu, 21 Jan 2021 17:35:58 +0100 Subject: [PATCH 34/34] Merge conflict badly resolved, sorry --- client/service/src/builder.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index 04dba3c54b614..3dc716b4e1c9e 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -17,8 +17,7 @@ // along with this program. If not, see . use crate::{ - error::Error, MallocSizeOfWasm, - TelemetryConnectionSinks, RpcHandlers, NetworkStatusSinks, + error::Error, MallocSizeOfWasm, RpcHandlers, NetworkStatusSinks, start_rpc_servers, build_network_future, TransactionPoolAdapter, TaskManager, SpawnTaskHandle, metrics::MetricsService, client::{light, Client, ClientConfig},