From 4eab2a408a07fb098b7e693825256f2e9babe0e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Tue, 26 May 2020 14:33:37 +0200 Subject: [PATCH 01/26] grandpa-rpc: use FinalityProofProvider to check finality for rpc --- Cargo.lock | 5 ++ bin/node/cli/src/service.rs | 26 ++++-- bin/node/rpc/Cargo.toml | 1 + bin/node/rpc/src/lib.rs | 19 ++-- client/finality-grandpa/rpc/Cargo.toml | 5 ++ client/finality-grandpa/rpc/src/error.rs | 5 ++ client/finality-grandpa/rpc/src/lib.rs | 86 +++++++++++++++++-- client/finality-grandpa/src/finality_proof.rs | 2 +- client/finality-grandpa/src/lib.rs | 2 +- 9 files changed, 129 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 23229c365c384..661cb5082598e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3573,6 +3573,7 @@ dependencies = [ "sp-consensus", "sp-consensus-babe", "sp-runtime", + "sp-state-machine", "sp-transaction-pool", "substrate-frame-rpc-system", ] @@ -6424,9 +6425,13 @@ dependencies = [ "jsonrpc-derive", "log", "sc-finality-grandpa", + "sc-network", + "sc-network-test", "serde", "serde_json", + "sp-blockchain", "sp-core", + "sp-runtime", ] [[package]] diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index b738b5cf1f4ec..9c1b42b439950 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -45,6 +45,7 @@ macro_rules! new_full_start { let mut import_setup = None; let mut rpc_setup = None; + let mut finality_provider = None; let inherent_data_providers = sp_inherents::InherentDataProviders::new(); let builder = sc_service::ServiceBuilder::new_full::< @@ -119,6 +120,11 @@ macro_rules! new_full_start { .expect("SelectChain is present for full services or set up failed; qed."); let keystore = builder.keystore().clone(); + let backend = builder.backend().clone(); + let provider = client.clone() as Arc>; + let finality_provider_for_rpc = Arc::new(grandpa::FinalityProofProvider::new(backend, provider)); + finality_provider = Some(finality_provider_for_rpc.clone()); + Ok(move |deny_unsafe| { let deps = node_rpc::FullDeps { client: client.clone(), @@ -133,12 +139,20 @@ macro_rules! new_full_start { grandpa: node_rpc::GrandpaDeps { shared_voter_state: shared_voter_state.clone(), shared_authority_set: shared_authority_set.clone(), + finality_provider: finality_provider_for_rpc.clone(), }, }; node_rpc::create_full(deps) }) - })?; + })? + .with_finality_proof_provider(|client, backend| { + // let provider = client as Arc>; + // finality_provider = Arc::new(grandpa::FinalityProofProvider::new(backend, provider)); + Ok(finality_provider.unwrap().clone()) + // Ok(Arc::new(grandpa::FinalityProofProvider::new(backend, provider)) as _) + })? + ; (builder, import_setup, inherent_data_providers, rpc_setup) }} @@ -170,11 +184,11 @@ macro_rules! new_full { new_full_start!($config); let service = builder - .with_finality_proof_provider(|client, backend| { - // GenesisAuthoritySetProvider is implemented for StorageAndProofProvider - let provider = client as Arc>; - Ok(Arc::new(grandpa::FinalityProofProvider::new(backend, provider)) as _) - })? + //.with_finality_proof_provider(|client, backend| { + // // GenesisAuthoritySetProvider is implemented for StorageAndProofProvider + // let provider = client as Arc>; + // Ok(Arc::new(grandpa::FinalityProofProvider::new(backend, provider)) as _) + //})? .build()?; let (block_import, grandpa_link, babe_link) = import_setup.take() diff --git a/bin/node/rpc/Cargo.toml b/bin/node/rpc/Cargo.toml index 5eb0d271b99ca..4454812ab121a 100644 --- a/bin/node/rpc/Cargo.toml +++ b/bin/node/rpc/Cargo.toml @@ -31,3 +31,4 @@ sp-blockchain = { version = "2.0.0-dev", path = "../../../primitives/blockchain" sc-finality-grandpa = { version = "0.8.0-dev", path = "../../../client/finality-grandpa" } sc-finality-grandpa-rpc = { version = "0.8.0-dev", path = "../../../client/finality-grandpa/rpc" } sc-rpc-api = { version = "0.8.0-dev", path = "../../../client/rpc-api" } +sp-state-machine = { version = "0.8.0-dev", path = "../../../primitives/state-machine" } \ No newline at end of file diff --git a/bin/node/rpc/src/lib.rs b/bin/node/rpc/src/lib.rs index 259a792441d40..e5bb44476b2bf 100644 --- a/bin/node/rpc/src/lib.rs +++ b/bin/node/rpc/src/lib.rs @@ -43,7 +43,7 @@ use sp_consensus_babe::BabeApi; use sc_consensus_epochs::SharedEpochChanges; use sc_consensus_babe::{Config, Epoch}; use sc_consensus_babe_rpc::BabeRpcHandler; -use sc_finality_grandpa::{SharedVoterState, SharedAuthoritySet}; +use sc_finality_grandpa::{SharedVoterState, SharedAuthoritySet, FinalityProofProvider}; use sc_finality_grandpa_rpc::GrandpaRpcHandler; use sc_rpc_api::DenyUnsafe; @@ -70,15 +70,17 @@ pub struct BabeDeps { } /// Extra dependencies for GRANDPA -pub struct GrandpaDeps { +pub struct GrandpaDeps { /// Voting round info. pub shared_voter_state: SharedVoterState, /// Authority set info. pub shared_authority_set: SharedAuthoritySet, + /// Finality proof provider. + pub finality_provider: Arc>, } /// Full client dependencies. -pub struct FullDeps { +pub struct FullDeps { /// The client instance to use. pub client: Arc, /// Transaction pool instance. @@ -90,12 +92,12 @@ pub struct FullDeps { /// BABE specific dependencies. pub babe: BabeDeps, /// GRANDPA specific dependencies. - pub grandpa: GrandpaDeps, + pub grandpa: GrandpaDeps, } /// Instantiate all Full RPC extensions. -pub fn create_full( - deps: FullDeps, +pub fn create_full( + deps: FullDeps, ) -> jsonrpc_core::IoHandler where C: ProvideRuntimeApi, C: HeaderBackend + HeaderMetadata + 'static, @@ -108,6 +110,8 @@ pub fn create_full( P: TransactionPool + 'static, M: jsonrpc_core::Metadata + Default, SC: SelectChain +'static, + B: Send + Sync + 'static + sc_client_api::backend::Backend, + >::State: sp_state_machine::backend::Backend, { use substrate_frame_rpc_system::{FullSystem, SystemApi}; use pallet_contracts_rpc::{Contracts, ContractsApi}; @@ -130,6 +134,7 @@ pub fn create_full( let GrandpaDeps { shared_voter_state, shared_authority_set, + finality_provider, } = grandpa; io.extend_with( @@ -158,7 +163,7 @@ pub fn create_full( ); io.extend_with( sc_finality_grandpa_rpc::GrandpaApi::to_delegate( - GrandpaRpcHandler::new(shared_authority_set, shared_voter_state) + GrandpaRpcHandler::new(shared_authority_set, shared_voter_state, finality_provider) ) ); diff --git a/client/finality-grandpa/rpc/Cargo.toml b/client/finality-grandpa/rpc/Cargo.toml index 0eecec19f70c3..c3760bdfe83e2 100644 --- a/client/finality-grandpa/rpc/Cargo.toml +++ b/client/finality-grandpa/rpc/Cargo.toml @@ -10,6 +10,7 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0" [dependencies] sc-finality-grandpa = { version = "0.8.0-dev", path = "../" } finality-grandpa = { version = "0.12.3", features = ["derive-codec"] } +sp-runtime = { version = "2.0.0-dev", path = "../../../primitives/runtime" } jsonrpc-core = "14.0.3" jsonrpc-core-client = "14.0.3" jsonrpc-derive = "14.0.3" @@ -18,6 +19,10 @@ serde = { version = "1.0.105", features = ["derive"] } serde_json = "1.0.50" log = "0.4.8" derive_more = "0.99.2" +sc-network = { version = "0.8.0-dev", path = "../../network" } +sp-core = { version = "2.0.0-dev", path = "../../../primitives/core" } +sp-blockchain = { version = "2.0.0-dev", path = "../../../primitives/blockchain" } [dev-dependencies] sp-core = { version = "2.0.0-dev", path = "../../../primitives/core" } +sc-network-test = { version = "0.8.0-dev", path = "../../network/test" } diff --git a/client/finality-grandpa/rpc/src/error.rs b/client/finality-grandpa/rpc/src/error.rs index bfd0596fdf320..60f498c57cf5e 100644 --- a/client/finality-grandpa/rpc/src/error.rs +++ b/client/finality-grandpa/rpc/src/error.rs @@ -30,12 +30,17 @@ pub enum Error { /// GRANDPA reports voter state with round id or weights larger than 32-bits. #[display(fmt = "GRANDPA reports voter state as unreasonably large")] VoterStateReportsUnreasonablyLargeNumbers, + /// GRANDPA prove finality failed. + // WIP: extend this to forward more info and use a better description. + #[display(fmt = "WIP: GRANDPA prove finality rpc failed")] + ProveFinalityFailed, } impl From for jsonrpc_core::Error { fn from(error: Error) -> Self { jsonrpc_core::Error { message: format!("{}", error), + // WIP: needs updating to return different error codes code: jsonrpc_core::ErrorCode::ServerError(NOT_READY_ERROR_CODE), data: None, } diff --git a/client/finality-grandpa/rpc/src/lib.rs b/client/finality-grandpa/rpc/src/lib.rs index 1af84b7a84413..387c9b711f259 100644 --- a/client/finality-grandpa/rpc/src/lib.rs +++ b/client/finality-grandpa/rpc/src/lib.rs @@ -19,6 +19,8 @@ //! RPC API for GRANDPA. #![warn(missing_docs)] +use std::sync::Arc; + use futures::{FutureExt, TryFutureExt}; use jsonrpc_derive::rpc; @@ -26,6 +28,8 @@ mod error; mod report; use report::{ReportAuthoritySet, ReportVoterState, ReportedRoundStates}; +use sp_runtime::traits::{Block as BlockT}; +use sc_network::config::FinalityProofProvider; /// Returned when Grandpa RPC endpoint is not ready. pub const NOT_READY_ERROR_CODE: i64 = 1; @@ -35,30 +39,42 @@ type FutureResult = /// Provides RPC methods for interacting with GRANDPA. #[rpc] -pub trait GrandpaApi { +pub trait GrandpaApi +{ /// Returns the state of the current best round state as well as the /// ongoing background rounds. #[rpc(name = "grandpa_roundState")] fn round_state(&self) -> FutureResult; + + /// Prove finality for the hash, given a last known finalized hash. + // WIP: handle authoroties_set_id changes? + #[rpc(name = "grandpa_proveFinality")] + fn prove_finality(&self, hash: Hash, last_finalized: Hash, authorities_set_id: u64) -> FutureResult>>; } /// Implements the GrandpaApi RPC trait for interacting with GRANDPA. -pub struct GrandpaRpcHandler { +pub struct GrandpaRpcHandler { authority_set: AuthoritySet, voter_state: VoterState, + finality_proof_provider: Arc>, } -impl GrandpaRpcHandler { +impl GrandpaRpcHandler { /// Creates a new GrandpaRpcHander instance. - pub fn new(authority_set: AuthoritySet, voter_state: VoterState) -> Self { + pub fn new( + authority_set: AuthoritySet, + voter_state: VoterState, + finality_proof_provider: Arc> + ) -> Self { Self { authority_set, voter_state, + finality_proof_provider, } } } -impl GrandpaApi for GrandpaRpcHandler +impl GrandpaApi for GrandpaRpcHandler where VoterState: ReportVoterState + Send + Sync + 'static, AuthoritySet: ReportAuthoritySet + Send + Sync + 'static, @@ -68,6 +84,21 @@ where let future = async move { round_states }.boxed(); Box::new(future.map_err(jsonrpc_core::Error::from).compat()) } + + fn prove_finality(&self, hash: Block::Hash, last_finalized: Block::Hash, authorities_set_id: u64) -> FutureResult>> { + let request = sc_finality_grandpa::make_finality_proof_request(last_finalized, authorities_set_id); + let result = self.finality_proof_provider.prove_finality(hash, &request); + let future = async move { result }.boxed(); + Box::new( + future + .map_err(|e| match e { + // WIP: don't just swallow the error + _ => error::Error::ProveFinalityFailed, + }) + .map_err(jsonrpc_core::Error::from) + .compat() + ) + } } #[cfg(test)] @@ -77,11 +108,14 @@ mod tests { use sc_finality_grandpa::{report, AuthorityId}; use sp_core::crypto::Public; use std::{collections::HashSet, convert::TryInto}; + use sp_runtime::traits::{Block as BlockT}; struct TestAuthoritySet; struct TestVoterState; struct EmptyVoterState; + struct EmptyFinalityProofProvider; + fn voters() -> HashSet { let voter_id_1 = AuthorityId::from_slice(&[1; 32]); let voter_id_2 = AuthorityId::from_slice(&[2; 32]); @@ -101,6 +135,16 @@ mod tests { } } + impl sc_network::config::FinalityProofProvider for EmptyFinalityProofProvider { + fn prove_finality( + &self, + for_block: Block::Hash, + request: &[u8], + ) -> Result>, sp_blockchain::Error> { + todo!(); + } + } + impl ReportVoterState for TestVoterState { fn get(&self) -> Option> { let voter_id_1 = AuthorityId::from_slice(&[1; 32]); @@ -135,7 +179,11 @@ mod tests { #[test] fn uninitialized_rpc_handler() { - let handler = GrandpaRpcHandler::new(TestAuthoritySet, EmptyVoterState); + let handler: GrandpaRpcHandler<_, _, sc_network_test::Block> = GrandpaRpcHandler::new( + TestAuthoritySet, + EmptyVoterState, + Arc::new(EmptyFinalityProofProvider), + ); let mut io = IoHandler::new(); io.extend_with(GrandpaApi::to_delegate(handler)); @@ -147,7 +195,11 @@ mod tests { #[test] fn working_rpc_handler() { - let handler = GrandpaRpcHandler::new(TestAuthoritySet, TestVoterState); + let handler: GrandpaRpcHandler<_, _, sc_network_test::Block> = GrandpaRpcHandler::new( + TestAuthoritySet, + TestVoterState, + Arc::new(EmptyFinalityProofProvider), + ); let mut io = IoHandler::new(); io.extend_with(GrandpaApi::to_delegate(handler)); @@ -168,4 +220,24 @@ mod tests { assert_eq!(io.handle_request_sync(request), Some(response.into())); } + + #[test] + fn test_finality() { + let handler: GrandpaRpcHandler<_, _, sc_network_test::Block> = GrandpaRpcHandler::new( + TestAuthoritySet, + EmptyVoterState, + Arc::new(EmptyFinalityProofProvider), + ); + let mut io = IoHandler::new(); + io.extend_with(GrandpaApi::to_delegate(handler)); + + let request = "{\"jsonrpc\":\"2.0\",\"method\":\"grandpa_proveFinality\",\"params\":[\ + \"0x0000000000000000000000000000000000000000000000000000000000000000\",\ + \"0x0000000000000000000000000000000000000000000000000000000000000000\",\ + 42\ + ],\"id\":1}"; + let response = r#"{"jsonrpc":"2.0","error":{"code":1,"message":"GRANDPA RPC endpoint not ready"},"id":1}"#; + + assert_eq!(Some(response.into()), io.handle_request_sync(request)); + } } diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index 55f6376579db4..e9146f4f2d4b1 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -256,7 +256,7 @@ struct OriginalFinalityProofRequest { } /// Prepare data blob associated with finality proof request. -pub(crate) fn make_finality_proof_request(last_finalized: H, authorities_set_id: u64) -> Vec { +pub fn make_finality_proof_request(last_finalized: H, authorities_set_id: u64) -> Vec { FinalityProofRequest::Original(OriginalFinalityProofRequest { authorities_set_id, last_finalized, diff --git a/client/finality-grandpa/src/lib.rs b/client/finality-grandpa/src/lib.rs index 7b20d082a01ab..6795d470a49c1 100644 --- a/client/finality-grandpa/src/lib.rs +++ b/client/finality-grandpa/src/lib.rs @@ -118,7 +118,7 @@ mod until_imported; mod voting_rule; pub use authorities::SharedAuthoritySet; -pub use finality_proof::{FinalityProofProvider, StorageAndProofProvider}; +pub use finality_proof::{FinalityProofProvider, StorageAndProofProvider, make_finality_proof_request}; pub use import::GrandpaBlockImport; pub use justification::GrandpaJustification; pub use light_import::light_block_import; From 06b04f59d7f84aa07fbd0b9bd1b706206f8e2394 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Wed, 12 Aug 2020 10:03:41 +0200 Subject: [PATCH 02/26] grandpa-rpc: minor tidy --- bin/node/cli/src/service.rs | 1 + bin/node/rpc/src/lib.rs | 5 +++-- client/finality-grandpa/rpc/src/lib.rs | 23 +++++++++++++++++------ 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index 8db464534e8ce..c5cf48b58bcd5 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -176,6 +176,7 @@ pub fn new_full_base( other: (rpc_extensions_builder, import_setup, rpc_setup, finality_proof_provider), } = new_partial(&config)?; + // WIP: JON // let finality_proof_provider = // GrandpaFinalityProofProvider::new_for_service(backend.clone(), client.clone()); diff --git a/bin/node/rpc/src/lib.rs b/bin/node/rpc/src/lib.rs index 5a7a3c945836c..3929829be7b62 100644 --- a/bin/node/rpc/src/lib.rs +++ b/bin/node/rpc/src/lib.rs @@ -49,6 +49,7 @@ use sp_blockchain::{Error as BlockChainError, HeaderMetadata, HeaderBackend}; use sp_consensus::SelectChain; use sp_consensus_babe::BabeApi; use sp_transaction_pool::TransactionPool; +use sp_runtime::traits::BlakeTwo256; /// Light client extra dependencies. pub struct LightDeps { @@ -119,8 +120,8 @@ pub fn create_full( C::Api: BlockBuilder, P: TransactionPool + 'static, SC: SelectChain +'static, - B: Send + Sync + 'static + sc_client_api::backend::Backend, - >::State: sp_state_machine::backend::Backend, + B: Send + Sync + 'static + sc_client_api::Backend, + >::State: sp_state_machine::Backend, { use substrate_frame_rpc_system::{FullSystem, SystemApi}; use pallet_contracts_rpc::{Contracts, ContractsApi}; diff --git a/client/finality-grandpa/rpc/src/lib.rs b/client/finality-grandpa/rpc/src/lib.rs index d721e1b43d340..517b9a93dbf45 100644 --- a/client/finality-grandpa/rpc/src/lib.rs +++ b/client/finality-grandpa/rpc/src/lib.rs @@ -85,9 +85,14 @@ pub trait GrandpaApi { ) -> jsonrpc_core::Result; /// Prove finality for the hash, given a last known finalized hash. - // WIP: handle authoroties_set_id changes? + // WIP: handle authorities_set_id changes? #[rpc(name = "grandpa_proveFinality")] - fn prove_finality(&self, hash: Hash, last_finalized: Hash, authorities_set_id: u64) -> FutureResult>>; + fn prove_finality( + &self, + hash: Hash, + last_finalized: Hash, + authorities_set_id: u64 + ) -> FutureResult>>; } /// Implements the GrandpaApi RPC trait for interacting with GRANDPA. @@ -159,8 +164,14 @@ where Ok(self.manager.cancel(id)) } - fn prove_finality(&self, hash: Block::Hash, last_finalized: Block::Hash, authorities_set_id: u64) -> FutureResult>> { - let request = sc_finality_grandpa::make_finality_proof_request(last_finalized, authorities_set_id); + fn prove_finality( + &self, + hash: Block::Hash, + last_finalized: Block::Hash, + authorities_set_id: u64, + ) -> FutureResult>> { + let request = + sc_finality_grandpa::make_finality_proof_request(last_finalized, authorities_set_id); let result = self.finality_proof_provider.prove_finality(hash, &request); let future = async move { result }.boxed(); Box::new( @@ -224,8 +235,8 @@ mod tests { impl sc_network::config::FinalityProofProvider for EmptyFinalityProofProvider { fn prove_finality( &self, - for_block: Block::Hash, - request: &[u8], + _for_block: Block::Hash, + _request: &[u8], ) -> Result>, sp_blockchain::Error> { todo!(); } From 34522c181e4f6f773a63a5917ef0988e08bed2d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Wed, 12 Aug 2020 10:16:11 +0200 Subject: [PATCH 03/26] grandpa-rpc: remove dyn FinalityProofProvider --- client/finality-grandpa/rpc/src/lib.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/client/finality-grandpa/rpc/src/lib.rs b/client/finality-grandpa/rpc/src/lib.rs index 517b9a93dbf45..e7abd52442473 100644 --- a/client/finality-grandpa/rpc/src/lib.rs +++ b/client/finality-grandpa/rpc/src/lib.rs @@ -39,7 +39,6 @@ use sc_finality_grandpa::GrandpaJustificationStream; use sp_runtime::traits::Block as BlockT; use report::{ReportAuthoritySet, ReportVoterState, ReportedRoundStates}; -use sc_network::config::FinalityProofProvider; use notification::JustificationNotification; /// Returned when Grandpa RPC endpoint is not ready. @@ -96,22 +95,22 @@ pub trait GrandpaApi { } /// Implements the GrandpaApi RPC trait for interacting with GRANDPA. -pub struct GrandpaRpcHandler { +pub struct GrandpaRpcHandler { authority_set: AuthoritySet, voter_state: VoterState, justification_stream: GrandpaJustificationStream, manager: SubscriptionManager, - finality_proof_provider: Arc>, + finality_proof_provider: Arc, } -impl GrandpaRpcHandler { +impl GrandpaRpcHandler { /// Creates a new GrandpaRpcHandler instance. pub fn new( authority_set: AuthoritySet, voter_state: VoterState, justification_stream: GrandpaJustificationStream, manager: SubscriptionManager, - finality_proof_provider: Arc> + finality_proof_provider: Arc, ) -> Self { Self { authority_set, @@ -123,12 +122,13 @@ impl GrandpaRpcHandler GrandpaApi - for GrandpaRpcHandler +impl GrandpaApi + for GrandpaRpcHandler where VoterState: ReportVoterState + Send + Sync + 'static, AuthoritySet: ReportAuthoritySet + Send + Sync + 'static, Block: BlockT, + ProofProvider: sc_network::config::FinalityProofProvider + 'static, { type Metadata = sc_rpc::Metadata; From 9fe866ed010da341d332e75244fe08cfc1c854ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Wed, 12 Aug 2020 10:41:45 +0200 Subject: [PATCH 04/26] grandpa-rpc: remove unused dependencies --- client/finality-grandpa/rpc/Cargo.toml | 2 -- 1 file changed, 2 deletions(-) diff --git a/client/finality-grandpa/rpc/Cargo.toml b/client/finality-grandpa/rpc/Cargo.toml index c5ccc5ee4f979..4d0520c9880da 100644 --- a/client/finality-grandpa/rpc/Cargo.toml +++ b/client/finality-grandpa/rpc/Cargo.toml @@ -23,8 +23,6 @@ log = "0.4.8" derive_more = "0.99.2" parity-scale-codec = { version = "1.3.0", features = ["derive"] } sc-network = { version = "0.8.0-rc5", path = "../../network" } -sp-core = { version = "2.0.0-rc5", path = "../../../primitives/core" } -sp-blockchain = { version = "2.0.0-rc5", path = "../../../primitives/blockchain" } [dev-dependencies] sc-block-builder = { version = "0.8.0-rc5", path = "../../block-builder" } From 8591352ec1418b4a1043d21e9071fd0ddca2c360 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Wed, 12 Aug 2020 10:51:56 +0200 Subject: [PATCH 05/26] node: move finality_proof_provider setup --- bin/node/cli/src/service.rs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index c5cf48b58bcd5..c1d30cb929363 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -102,8 +102,10 @@ pub fn new_partial(config: &Configuration) -> Result Result Result Date: Wed, 12 Aug 2020 12:56:40 +0200 Subject: [PATCH 06/26] grandpa-rpc: print error reported by finality_proof_provider --- client/finality-grandpa/rpc/src/error.rs | 12 +++++++----- client/finality-grandpa/rpc/src/lib.rs | 10 ++++++---- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/client/finality-grandpa/rpc/src/error.rs b/client/finality-grandpa/rpc/src/error.rs index 60f498c57cf5e..45b881cacc17d 100644 --- a/client/finality-grandpa/rpc/src/error.rs +++ b/client/finality-grandpa/rpc/src/error.rs @@ -16,7 +16,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use crate::NOT_READY_ERROR_CODE; +use crate::{NOT_READY_ERROR_CODE, INTERNAL_ERROR}; #[derive(derive_more::Display, derive_more::From)] /// Top-level error type for the RPC handler @@ -31,17 +31,19 @@ pub enum Error { #[display(fmt = "GRANDPA reports voter state as unreasonably large")] VoterStateReportsUnreasonablyLargeNumbers, /// GRANDPA prove finality failed. - // WIP: extend this to forward more info and use a better description. - #[display(fmt = "WIP: GRANDPA prove finality rpc failed")] + #[display(fmt = "GRANDPA prove finality rpc failed")] ProveFinalityFailed, } impl From for jsonrpc_core::Error { fn from(error: Error) -> Self { + let code = match error { + Error::EndpointNotReady => NOT_READY_ERROR_CODE, + _ => INTERNAL_ERROR, + }; jsonrpc_core::Error { message: format!("{}", error), - // WIP: needs updating to return different error codes - code: jsonrpc_core::ErrorCode::ServerError(NOT_READY_ERROR_CODE), + code: jsonrpc_core::ErrorCode::ServerError(code), data: None, } } diff --git a/client/finality-grandpa/rpc/src/lib.rs b/client/finality-grandpa/rpc/src/lib.rs index e7abd52442473..6360764ebeebc 100644 --- a/client/finality-grandpa/rpc/src/lib.rs +++ b/client/finality-grandpa/rpc/src/lib.rs @@ -43,6 +43,8 @@ use notification::JustificationNotification; /// Returned when Grandpa RPC endpoint is not ready. pub const NOT_READY_ERROR_CODE: i64 = 1; +/// Returned for internal Grandpa RPC errors. +pub const INTERNAL_ERROR: i64 = 2; type FutureResult = Box + Send>; @@ -176,9 +178,9 @@ where let future = async move { result }.boxed(); Box::new( future - .map_err(|e| match e { - // WIP: don't just swallow the error - _ => error::Error::ProveFinalityFailed, + .map_err(|e| { + warn!("Error proving finality: {}", e); + error::Error::ProveFinalityFailed }) .map_err(jsonrpc_core::Error::from) .compat() @@ -479,7 +481,7 @@ mod tests { } #[test] - fn test_finality() { + fn test_prove_finality() { let (io, _) = setup_io_handler(TestVoterState); let request = "{\"jsonrpc\":\"2.0\",\"method\":\"grandpa_proveFinality\",\"params\":[\ From 136c257d9cce10e50280c8784daba817b7abb7ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Wed, 12 Aug 2020 14:54:38 +0200 Subject: [PATCH 07/26] grandpa-rpc: add note about unnecessary encode/decode --- client/finality-grandpa/rpc/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/client/finality-grandpa/rpc/src/lib.rs b/client/finality-grandpa/rpc/src/lib.rs index 6360764ebeebc..633214d6b42d8 100644 --- a/client/finality-grandpa/rpc/src/lib.rs +++ b/client/finality-grandpa/rpc/src/lib.rs @@ -172,6 +172,8 @@ where last_finalized: Block::Hash, authorities_set_id: u64, ) -> FutureResult>> { + // WIP: this encodes and immediately decodes the same blob. + // Important to refactor this away as a cleanup step in this PR. let request = sc_finality_grandpa::make_finality_proof_request(last_finalized, authorities_set_id); let result = self.finality_proof_provider.prove_finality(hash, &request); From eab8404b7aa91f452be8a69e354cc9b481a25bba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Fri, 14 Aug 2020 15:36:49 +0200 Subject: [PATCH 08/26] grandpa-rpc: dont encode/decode and use correct hash --- client/finality-grandpa/rpc/src/lib.rs | 22 +++++-------- client/finality-grandpa/src/finality_proof.rs | 33 ++++++++++++++++++- client/finality-grandpa/src/lib.rs | 4 ++- 3 files changed, 44 insertions(+), 15 deletions(-) diff --git a/client/finality-grandpa/rpc/src/lib.rs b/client/finality-grandpa/rpc/src/lib.rs index 633214d6b42d8..ddc2a39d6a916 100644 --- a/client/finality-grandpa/rpc/src/lib.rs +++ b/client/finality-grandpa/rpc/src/lib.rs @@ -35,7 +35,7 @@ mod error; mod notification; mod report; -use sc_finality_grandpa::GrandpaJustificationStream; +use sc_finality_grandpa::{RpcFinalityProofProvider, GrandpaJustificationStream}; use sp_runtime::traits::Block as BlockT; use report::{ReportAuthoritySet, ReportVoterState, ReportedRoundStates}; @@ -90,7 +90,6 @@ pub trait GrandpaApi { #[rpc(name = "grandpa_proveFinality")] fn prove_finality( &self, - hash: Hash, last_finalized: Hash, authorities_set_id: u64 ) -> FutureResult>>; @@ -130,7 +129,7 @@ where VoterState: ReportVoterState + Send + Sync + 'static, AuthoritySet: ReportAuthoritySet + Send + Sync + 'static, Block: BlockT, - ProofProvider: sc_network::config::FinalityProofProvider + 'static, + ProofProvider: RpcFinalityProofProvider + 'static, { type Metadata = sc_rpc::Metadata; @@ -168,15 +167,12 @@ where fn prove_finality( &self, - hash: Block::Hash, last_finalized: Block::Hash, authorities_set_id: u64, ) -> FutureResult>> { - // WIP: this encodes and immediately decodes the same blob. - // Important to refactor this away as a cleanup step in this PR. - let request = - sc_finality_grandpa::make_finality_proof_request(last_finalized, authorities_set_id); - let result = self.finality_proof_provider.prove_finality(hash, &request); + let result = self + .finality_proof_provider + .prove_finality_for_best_hash(authorities_set_id, last_finalized); let future = async move { result }.boxed(); Box::new( future @@ -236,11 +232,11 @@ mod tests { } } - impl sc_network::config::FinalityProofProvider for EmptyFinalityProofProvider { - fn prove_finality( + impl RpcFinalityProofProvider for EmptyFinalityProofProvider { + fn prove_finality_for_best_hash( &self, - _for_block: Block::Hash, - _request: &[u8], + _authoritites_set_id: u64, + _last_finalized: Block::Hash, ) -> Result>, sp_blockchain::Error> { todo!(); } diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index 0078e38e6bbac..514ccfc02e39c 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -180,7 +180,38 @@ impl FinalityProofProvider ) -> Arc { Arc::new(Self::new(backend, storage_and_proof_provider)) } +} + +/// WIP: TODO +pub trait RpcFinalityProofProvider: Send + Sync { + /// WIP: TODO + fn prove_finality_for_best_hash( + &self, + authorities_set_id: u64, + last_finalized: Block::Hash, + ) -> Result>, ClientError>; +} +impl RpcFinalityProofProvider for FinalityProofProvider + where + Block: BlockT, + NumberFor: BlockNumberOps, + B: Backend + Send + Sync + 'static, +{ + fn prove_finality_for_best_hash( + &self, + authorities_set_id: u64, + last_finalized: Block::Hash, + ) -> Result>, ClientError> { + use sp_blockchain::HeaderBackend; + prove_finality::<_, _, GrandpaJustification>( + &*self.backend.blockchain(), + &*self.authority_provider, + authorities_set_id, + last_finalized.clone(), + self.backend.blockchain().info().best_hash, + ) + } } impl sc_network::config::FinalityProofProvider for FinalityProofProvider @@ -281,7 +312,7 @@ pub fn make_finality_proof_request(last_finalized: H, author /// It is assumed that the caller already knows all blocks in the range (begin; end]. /// /// Returns None if there are no finalized blocks unknown to the caller. -pub(crate) fn prove_finality, J>( +fn prove_finality, J>( blockchain: &B, authorities_provider: &dyn AuthoritySetForFinalityProver, authorities_set_id: u64, diff --git a/client/finality-grandpa/src/lib.rs b/client/finality-grandpa/src/lib.rs index 64f854a639b4f..bbe9d6e9d6b1c 100644 --- a/client/finality-grandpa/src/lib.rs +++ b/client/finality-grandpa/src/lib.rs @@ -125,7 +125,9 @@ mod until_imported; mod voting_rule; pub use authorities::SharedAuthoritySet; -pub use finality_proof::{FinalityProofProvider, StorageAndProofProvider, make_finality_proof_request}; +pub use finality_proof::{ + FinalityProofProvider, StorageAndProofProvider, RpcFinalityProofProvider, +}; pub use notification::{GrandpaJustificationSender, GrandpaJustificationStream}; pub use import::GrandpaBlockImport; pub use justification::GrandpaJustification; From d3e8d305541406ce94fbadee3864ac3332b5cfa4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Tue, 18 Aug 2020 14:18:09 +0200 Subject: [PATCH 09/26] grandpa-rpc: set_id is optional --- client/finality-grandpa/rpc/src/lib.rs | 8 ++++---- client/finality-grandpa/src/finality_proof.rs | 11 +++++++++-- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/client/finality-grandpa/rpc/src/lib.rs b/client/finality-grandpa/rpc/src/lib.rs index ddc2a39d6a916..fe7ff806c93a7 100644 --- a/client/finality-grandpa/rpc/src/lib.rs +++ b/client/finality-grandpa/rpc/src/lib.rs @@ -91,7 +91,7 @@ pub trait GrandpaApi { fn prove_finality( &self, last_finalized: Hash, - authorities_set_id: u64 + authorities_set_id: Option, ) -> FutureResult>>; } @@ -168,11 +168,11 @@ where fn prove_finality( &self, last_finalized: Block::Hash, - authorities_set_id: u64, + authorities_set_id: Option, ) -> FutureResult>> { let result = self .finality_proof_provider - .prove_finality_for_best_hash(authorities_set_id, last_finalized); + .prove_finality_for_best_hash(last_finalized, authorities_set_id); let future = async move { result }.boxed(); Box::new( future @@ -235,8 +235,8 @@ mod tests { impl RpcFinalityProofProvider for EmptyFinalityProofProvider { fn prove_finality_for_best_hash( &self, - _authoritites_set_id: u64, _last_finalized: Block::Hash, + _authoritites_set_id: Option, ) -> Result>, sp_blockchain::Error> { todo!(); } diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index 514ccfc02e39c..3b9cb653a9761 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -187,8 +187,8 @@ pub trait RpcFinalityProofProvider: Send + Sync { /// WIP: TODO fn prove_finality_for_best_hash( &self, - authorities_set_id: u64, last_finalized: Block::Hash, + authorities_set_id: Option, ) -> Result>, ClientError>; } @@ -200,10 +200,17 @@ impl RpcFinalityProofProvider for FinalityProofProvider, ) -> Result>, ClientError> { use sp_blockchain::HeaderBackend; + + let authorities_set_id = if let Some(set_id) = authorities_set_id { + set_id + } else { + todo!(); + }; + prove_finality::<_, _, GrandpaJustification>( &*self.backend.blockchain(), &*self.authority_provider, From 801b3e6051688de1d2cc141ba6f3f2cd46007fdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Fri, 21 Aug 2020 15:12:48 +0200 Subject: [PATCH 10/26] grandpa-rpc: create test for prove_finality --- Cargo.lock | 1 + client/finality-grandpa/rpc/Cargo.toml | 1 + client/finality-grandpa/rpc/src/lib.rs | 83 ++++++++++++++++--- client/finality-grandpa/src/finality_proof.rs | 4 +- client/finality-grandpa/src/lib.rs | 1 + 5 files changed, 76 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c7c738ac0b832..5665151a9c17a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6630,6 +6630,7 @@ dependencies = [ "log", "parity-scale-codec", "sc-block-builder", + "sc-client-db", "sc-finality-grandpa", "sc-network", "sc-network-test", diff --git a/client/finality-grandpa/rpc/Cargo.toml b/client/finality-grandpa/rpc/Cargo.toml index 4d0520c9880da..4bc4cc22c200b 100644 --- a/client/finality-grandpa/rpc/Cargo.toml +++ b/client/finality-grandpa/rpc/Cargo.toml @@ -26,6 +26,7 @@ sc-network = { version = "0.8.0-rc5", path = "../../network" } [dev-dependencies] sc-block-builder = { version = "0.8.0-rc5", path = "../../block-builder" } +sc-client-db = { version = "0.8.0-rc5", features = ["test-helpers"], path = "../../db" } sc-network-test = { version = "0.8.0-rc5", path = "../../network/test" } sc-rpc = { version = "2.0.0-rc5", path = "../../rpc", features = ["test-helpers"] } sp-blockchain = { version = "2.0.0-rc5", path = "../../../primitives/blockchain" } diff --git a/client/finality-grandpa/rpc/src/lib.rs b/client/finality-grandpa/rpc/src/lib.rs index fe7ff806c93a7..21749875289bf 100644 --- a/client/finality-grandpa/rpc/src/lib.rs +++ b/client/finality-grandpa/rpc/src/lib.rs @@ -192,16 +192,18 @@ mod tests { use std::{collections::HashSet, convert::TryInto, sync::Arc}; use jsonrpc_core::{Notification, Output, types::Params}; - use parity_scale_codec::Decode; + use parity_scale_codec::{Encode, Decode}; use sc_block_builder::BlockBuilder; - use sc_finality_grandpa::{report, AuthorityId, GrandpaJustificationSender, GrandpaJustification}; + use sc_finality_grandpa::{ + report, AuthorityId, GrandpaJustificationSender, GrandpaJustification, + }; use sp_blockchain::HeaderBackend; use sp_consensus::RecordProof; use sp_core::crypto::Public; use sp_keyring::Ed25519Keyring; use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; use substrate_test_runtime_client::{ - runtime::Block, + runtime::{Block, Header, H256}, DefaultTestClientBuilderExt, TestClientBuilderExt, TestClientBuilder, @@ -211,7 +213,9 @@ mod tests { struct TestVoterState; struct EmptyVoterState; - struct EmptyFinalityProofProvider; + struct TestFinalityProofProvider { + finality_proofs: Vec>, + } fn voters() -> HashSet { let voter_id_1 = AuthorityId::from_slice(&[1; 32]); @@ -232,13 +236,26 @@ mod tests { } } - impl RpcFinalityProofProvider for EmptyFinalityProofProvider { + fn header(number: u64) -> Header { + let parent_hash = match number { + 0 => Default::default(), + _ => header(number - 1).hash(), + }; + Header::new( + number, + H256::from_low_u64_be(0), + H256::from_low_u64_be(0), + parent_hash, Default::default() + ) + } + + impl RpcFinalityProofProvider for TestFinalityProofProvider { fn prove_finality_for_best_hash( &self, _last_finalized: Block::Hash, _authoritites_set_id: Option, ) -> Result>, sp_blockchain::Error> { - todo!(); + Ok(Some(self.finality_proofs.encode())) } } @@ -279,16 +296,29 @@ mod tests { GrandpaJustificationSender, ) where VoterState: ReportVoterState + Send + Sync + 'static, + { + setup_io_handler_with_finality_proofs(voter_state, Default::default()) + } + + fn setup_io_handler_with_finality_proofs( + voter_state: VoterState, + finality_proofs: Vec>, + ) -> ( + jsonrpc_core::MetaIoHandler, + GrandpaJustificationSender, + ) where + VoterState: ReportVoterState + Send + Sync + 'static, { let (justification_sender, justification_stream) = GrandpaJustificationStream::channel(); let manager = SubscriptionManager::new(Arc::new(sc_rpc::testing::TaskExecutor)); + let finality_proof_provider = Arc::new(TestFinalityProofProvider { finality_proofs }); let handler = GrandpaRpcHandler::new( TestAuthoritySet, voter_state, justification_stream, manager, - Arc::new(EmptyFinalityProofProvider), + finality_proof_provider, ); let mut io = jsonrpc_core::MetaIoHandler::default(); @@ -479,17 +509,46 @@ mod tests { } #[test] - fn test_prove_finality() { - let (io, _) = setup_io_handler(TestVoterState); + fn prove_finality_with_test_finality_proof_provider() { + let finality_proofs = vec![sc_finality_grandpa::FinalityProofFragment { + block: header(42).hash(), + justification: create_justification().encode(), + unknown_headers: vec![header(2)], + authorities_proof: None, + }]; + let (io, _) = setup_io_handler_with_finality_proofs( + TestVoterState, + finality_proofs.clone(), + ); let request = "{\"jsonrpc\":\"2.0\",\"method\":\"grandpa_proveFinality\",\"params\":[\ - \"0x0000000000000000000000000000000000000000000000000000000000000000\",\ \"0x0000000000000000000000000000000000000000000000000000000000000000\",\ 42\ ],\"id\":1}"; - let response = r#"{"jsonrpc":"2.0","error":{"code":1,"message":"GRANDPA RPC endpoint not ready"},"id":1}"#; + let response = "{\"jsonrpc\":\"2.0\",\"result\":[\ + 4,130,100,253,46,137,220,14,213,229,74,246,107,63,105,84,82,3,229,255,64,239,\ + 205,252,81,42,182,86,255,23,139,214,0,233,2,1,0,0,0,0,0,0,0,169,96,2,156,106,\ + 88,215,93,172,162,191,86,187,255,138,227,123,240,44,147,110,145,99,200,3,136,\ + 200,174,177,252,38,4,1,0,0,0,0,0,0,0,4,169,96,2,156,106,88,215,93,172,162,191,\ + 86,187,255,138,227,123,240,44,147,110,145,99,200,3,136,200,174,177,252,38,4,1,\ + 0,0,0,0,0,0,0,131,2,35,74,166,100,120,130,13,44,11,231,205,157,118,166,130,215,\ + 241,219,234,52,238,163,35,21,41,36,176,43,215,49,219,27,180,158,131,150,4,104,\ + 201,22,172,233,249,88,122,242,200,25,243,99,46,198,254,180,166,145,152,16,2,141,\ + 179,0,136,220,52,23,213,5,142,196,180,80,62,12,18,234,26,10,137,190,32,15,233,137,\ + 34,66,61,67,52,1,79,166,176,238,0,4,185,226,146,135,126,116,181,99,47,249,203,114,\ + 83,32,76,136,16,147,43,236,75,71,19,160,58,65,197,75,11,36,94,4,8,0,0,0,0,0,0,0,0,\ + 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,\ + 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0\ + ],\"id\":1}"; let meta = sc_rpc::Metadata::default(); - assert_eq!(Some(response.into()), io.handle_request_sync(request, meta)); + let resp = io.handle_request_sync(request, meta); + assert_eq!(resp, Some(response.into())); + + let mut resp: serde_json::Value = serde_json::from_str(&resp.unwrap()).unwrap(); + let result: Vec = serde_json::from_value(resp["result"].take()).unwrap(); + let fragments: Vec> = + Decode::decode(&mut &result[..]).unwrap(); + assert_eq!(fragments, finality_proofs); } } diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index 3b9cb653a9761..b54e0a59d4dfd 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -270,8 +270,8 @@ pub struct FinalityEffects { /// 1) the justification for the descendant block F; /// 2) headers sub-chain (B; F] if B != F; /// 3) proof of GRANDPA::authorities() if the set changes at block F. -#[derive(Debug, PartialEq, Encode, Decode)] -pub(crate) struct FinalityProofFragment { +#[derive(Debug, PartialEq, Encode, Decode, Clone)] +pub struct FinalityProofFragment { /// The hash of block F for which justification is provided. pub block: Header::Hash, /// Justification of the block F. diff --git a/client/finality-grandpa/src/lib.rs b/client/finality-grandpa/src/lib.rs index bbe9d6e9d6b1c..8d9896b904503 100644 --- a/client/finality-grandpa/src/lib.rs +++ b/client/finality-grandpa/src/lib.rs @@ -127,6 +127,7 @@ mod voting_rule; pub use authorities::SharedAuthoritySet; pub use finality_proof::{ FinalityProofProvider, StorageAndProofProvider, RpcFinalityProofProvider, + FinalityProofFragment, }; pub use notification::{GrandpaJustificationSender, GrandpaJustificationStream}; pub use import::GrandpaBlockImport; From 56f32a375ef292f6c43aa84b525961bfab05bdd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Fri, 21 Aug 2020 15:30:25 +0200 Subject: [PATCH 11/26] grandpa-rpc: set visibility back to how it was --- 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 b54e0a59d4dfd..ba238cf3bf202 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -306,7 +306,7 @@ struct OriginalFinalityProofRequest { } /// Prepare data blob associated with finality proof request. -pub fn make_finality_proof_request(last_finalized: H, authorities_set_id: u64) -> Vec { +pub(crate) fn make_finality_proof_request(last_finalized: H, authorities_set_id: u64) -> Vec { FinalityProofRequest::Original(OriginalFinalityProofRequest { authorities_set_id, last_finalized, @@ -319,7 +319,7 @@ pub fn make_finality_proof_request(last_finalized: H, author /// It is assumed that the caller already knows all blocks in the range (begin; end]. /// /// Returns None if there are no finalized blocks unknown to the caller. -fn prove_finality, J>( +pub(crate) fn prove_finality, J>( blockchain: &B, authorities_provider: &dyn AuthoritySetForFinalityProver, authorities_set_id: u64, From c1e50d54c5990e4cac3e8038605f19190dde69da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Fri, 21 Aug 2020 15:33:52 +0200 Subject: [PATCH 12/26] grandpa-rpc: remove unused dependency --- Cargo.lock | 1 - client/finality-grandpa/rpc/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5665151a9c17a..c51298a8f36d7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6632,7 +6632,6 @@ dependencies = [ "sc-block-builder", "sc-client-db", "sc-finality-grandpa", - "sc-network", "sc-network-test", "sc-rpc", "serde", diff --git a/client/finality-grandpa/rpc/Cargo.toml b/client/finality-grandpa/rpc/Cargo.toml index 4bc4cc22c200b..95c13d716e39b 100644 --- a/client/finality-grandpa/rpc/Cargo.toml +++ b/client/finality-grandpa/rpc/Cargo.toml @@ -22,7 +22,6 @@ serde_json = "1.0.50" log = "0.4.8" derive_more = "0.99.2" parity-scale-codec = { version = "1.3.0", features = ["derive"] } -sc-network = { version = "0.8.0-rc5", path = "../../network" } [dev-dependencies] sc-block-builder = { version = "0.8.0-rc5", path = "../../block-builder" } From 3b5a3c6d579e41dd1cffb6c2985b9ac2779f4c9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Fri, 21 Aug 2020 15:37:47 +0200 Subject: [PATCH 13/26] grandpa-rpc: minor tidy --- client/finality-grandpa/rpc/src/lib.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/client/finality-grandpa/rpc/src/lib.rs b/client/finality-grandpa/rpc/src/lib.rs index 21749875289bf..bc317491f40b1 100644 --- a/client/finality-grandpa/rpc/src/lib.rs +++ b/client/finality-grandpa/rpc/src/lib.rs @@ -86,7 +86,6 @@ pub trait GrandpaApi { ) -> jsonrpc_core::Result; /// Prove finality for the hash, given a last known finalized hash. - // WIP: handle authorities_set_id changes? #[rpc(name = "grandpa_proveFinality")] fn prove_finality( &self, @@ -196,6 +195,7 @@ mod tests { use sc_block_builder::BlockBuilder; use sc_finality_grandpa::{ report, AuthorityId, GrandpaJustificationSender, GrandpaJustification, + FinalityProofFragment, }; use sp_blockchain::HeaderBackend; use sp_consensus::RecordProof; @@ -214,7 +214,7 @@ mod tests { struct EmptyVoterState; struct TestFinalityProofProvider { - finality_proofs: Vec>, + finality_proofs: Vec>, } fn voters() -> HashSet { @@ -245,7 +245,8 @@ mod tests { number, H256::from_low_u64_be(0), H256::from_low_u64_be(0), - parent_hash, Default::default() + parent_hash, + Default::default(), ) } @@ -302,7 +303,7 @@ mod tests { fn setup_io_handler_with_finality_proofs( voter_state: VoterState, - finality_proofs: Vec>, + finality_proofs: Vec>, ) -> ( jsonrpc_core::MetaIoHandler, GrandpaJustificationSender, @@ -510,7 +511,7 @@ mod tests { #[test] fn prove_finality_with_test_finality_proof_provider() { - let finality_proofs = vec![sc_finality_grandpa::FinalityProofFragment { + let finality_proofs = vec![FinalityProofFragment { block: header(42).hash(), justification: create_justification().encode(), unknown_headers: vec![header(2)], @@ -547,7 +548,7 @@ mod tests { let mut resp: serde_json::Value = serde_json::from_str(&resp.unwrap()).unwrap(); let result: Vec = serde_json::from_value(resp["result"].take()).unwrap(); - let fragments: Vec> = + let fragments: Vec> = Decode::decode(&mut &result[..]).unwrap(); assert_eq!(fragments, finality_proofs); } From a892236c7adc89f450e414f5b5df42a493145163 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Fri, 21 Aug 2020 15:45:25 +0200 Subject: [PATCH 14/26] grandpa: doc strings --- client/finality-grandpa/src/finality_proof.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index ba238cf3bf202..575cf763069fa 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -182,9 +182,10 @@ impl FinalityProofProvider } } -/// WIP: TODO +/// Provide finality proofs to the RPC API pub trait RpcFinalityProofProvider: Send + Sync { - /// WIP: TODO + /// Return finality proofs for the given authorities set id, if it is provided, otherwise the + /// current one will be used. fn prove_finality_for_best_hash( &self, last_finalized: Block::Hash, From 2db4ba0ae64cf3c5c53ef925e49eae25a69db1fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Fri, 21 Aug 2020 15:47:04 +0200 Subject: [PATCH 15/26] grandpa-rpc: rename to prove_finality --- client/finality-grandpa/rpc/src/lib.rs | 4 ++-- client/finality-grandpa/src/finality_proof.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/client/finality-grandpa/rpc/src/lib.rs b/client/finality-grandpa/rpc/src/lib.rs index bc317491f40b1..3bfac60202cb7 100644 --- a/client/finality-grandpa/rpc/src/lib.rs +++ b/client/finality-grandpa/rpc/src/lib.rs @@ -171,7 +171,7 @@ where ) -> FutureResult>> { let result = self .finality_proof_provider - .prove_finality_for_best_hash(last_finalized, authorities_set_id); + .prove_finality(last_finalized, authorities_set_id); let future = async move { result }.boxed(); Box::new( future @@ -251,7 +251,7 @@ mod tests { } impl RpcFinalityProofProvider for TestFinalityProofProvider { - fn prove_finality_for_best_hash( + fn prove_finality( &self, _last_finalized: Block::Hash, _authoritites_set_id: Option, diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index 575cf763069fa..6ab5864ec856c 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -186,7 +186,7 @@ impl FinalityProofProvider pub trait RpcFinalityProofProvider: Send + Sync { /// Return finality proofs for the given authorities set id, if it is provided, otherwise the /// current one will be used. - fn prove_finality_for_best_hash( + fn prove_finality( &self, last_finalized: Block::Hash, authorities_set_id: Option, @@ -199,7 +199,7 @@ impl RpcFinalityProofProvider for FinalityProofProvider: BlockNumberOps, B: Backend + Send + Sync + 'static, { - fn prove_finality_for_best_hash( + fn prove_finality( &self, last_finalized: Block::Hash, authorities_set_id: Option, From 3c543e658390a6fc4ca9f6d72adf5bb09a929d6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Fri, 21 Aug 2020 16:38:55 +0200 Subject: [PATCH 16/26] grandpa-rpc: use current set id if none is provided --- client/finality-grandpa/rpc/src/lib.rs | 5 ++++- client/finality-grandpa/src/finality_proof.rs | 11 ++--------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/client/finality-grandpa/rpc/src/lib.rs b/client/finality-grandpa/rpc/src/lib.rs index 3bfac60202cb7..a88c9585c0394 100644 --- a/client/finality-grandpa/rpc/src/lib.rs +++ b/client/finality-grandpa/rpc/src/lib.rs @@ -169,6 +169,9 @@ where last_finalized: Block::Hash, authorities_set_id: Option, ) -> FutureResult>> { + // If we are not provided a set_id, try with the current one. + let authorities_set_id = authorities_set_id + .unwrap_or_else(|| self.authority_set.get().0); let result = self .finality_proof_provider .prove_finality(last_finalized, authorities_set_id); @@ -254,7 +257,7 @@ mod tests { fn prove_finality( &self, _last_finalized: Block::Hash, - _authoritites_set_id: Option, + _authoritites_set_id: u64, ) -> Result>, sp_blockchain::Error> { Ok(Some(self.finality_proofs.encode())) } diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index 6ab5864ec856c..dbb28260670ff 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -189,7 +189,7 @@ pub trait RpcFinalityProofProvider: Send + Sync { fn prove_finality( &self, last_finalized: Block::Hash, - authorities_set_id: Option, + authorities_set_id: u64, ) -> Result>, ClientError>; } @@ -202,16 +202,9 @@ impl RpcFinalityProofProvider for FinalityProofProvider, + authorities_set_id: u64, ) -> Result>, ClientError> { use sp_blockchain::HeaderBackend; - - let authorities_set_id = if let Some(set_id) = authorities_set_id { - set_id - } else { - todo!(); - }; - prove_finality::<_, _, GrandpaJustification>( &*self.backend.blockchain(), &*self.authority_provider, From 7b1ed8fe5743c2ca8107b7ea357e856b29f78ce4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Fri, 21 Aug 2020 17:21:47 +0200 Subject: [PATCH 17/26] grandpa-rpc: remove unnecessary check in test --- client/finality-grandpa/rpc/src/lib.rs | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/client/finality-grandpa/rpc/src/lib.rs b/client/finality-grandpa/rpc/src/lib.rs index a88c9585c0394..f3300b30a3a2f 100644 --- a/client/finality-grandpa/rpc/src/lib.rs +++ b/client/finality-grandpa/rpc/src/lib.rs @@ -529,26 +529,9 @@ mod tests { \"0x0000000000000000000000000000000000000000000000000000000000000000\",\ 42\ ],\"id\":1}"; - let response = "{\"jsonrpc\":\"2.0\",\"result\":[\ - 4,130,100,253,46,137,220,14,213,229,74,246,107,63,105,84,82,3,229,255,64,239,\ - 205,252,81,42,182,86,255,23,139,214,0,233,2,1,0,0,0,0,0,0,0,169,96,2,156,106,\ - 88,215,93,172,162,191,86,187,255,138,227,123,240,44,147,110,145,99,200,3,136,\ - 200,174,177,252,38,4,1,0,0,0,0,0,0,0,4,169,96,2,156,106,88,215,93,172,162,191,\ - 86,187,255,138,227,123,240,44,147,110,145,99,200,3,136,200,174,177,252,38,4,1,\ - 0,0,0,0,0,0,0,131,2,35,74,166,100,120,130,13,44,11,231,205,157,118,166,130,215,\ - 241,219,234,52,238,163,35,21,41,36,176,43,215,49,219,27,180,158,131,150,4,104,\ - 201,22,172,233,249,88,122,242,200,25,243,99,46,198,254,180,166,145,152,16,2,141,\ - 179,0,136,220,52,23,213,5,142,196,180,80,62,12,18,234,26,10,137,190,32,15,233,137,\ - 34,66,61,67,52,1,79,166,176,238,0,4,185,226,146,135,126,116,181,99,47,249,203,114,\ - 83,32,76,136,16,147,43,236,75,71,19,160,58,65,197,75,11,36,94,4,8,0,0,0,0,0,0,0,0,\ - 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,\ - 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0\ - ],\"id\":1}"; let meta = sc_rpc::Metadata::default(); let resp = io.handle_request_sync(request, meta); - assert_eq!(resp, Some(response.into())); - let mut resp: serde_json::Value = serde_json::from_str(&resp.unwrap()).unwrap(); let result: Vec = serde_json::from_value(resp["result"].take()).unwrap(); let fragments: Vec> = From 9a3b43f55bc10dac16832bb9acf2ceb681a4acec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Mon, 24 Aug 2020 10:03:31 +0200 Subject: [PATCH 18/26] node: group finality_proof_provider in rpc_setup --- bin/node/cli/src/service.rs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index 8261cd35d425d..3cee23732cbc2 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -58,8 +58,10 @@ pub fn new_partial(config: &Configuration) -> Result, sc_consensus_babe::BabeLink, ), - grandpa::SharedVoterState, - Arc>, + ( + grandpa::SharedVoterState, + Arc>, + ), ) >, ServiceError> { let (client, backend, keystore, task_manager) = @@ -103,8 +105,6 @@ pub fn new_partial(config: &Configuration) -> Result Result Result Result Result Date: Mon, 24 Aug 2020 12:23:45 +0200 Subject: [PATCH 19/26] grandpa: make prove_finality concrete in FinalityProofProvider --- Cargo.lock | 1 + client/finality-grandpa/rpc/Cargo.toml | 3 +- client/finality-grandpa/rpc/src/lib.rs | 36 ++++++++++++++++--- client/finality-grandpa/src/finality_proof.rs | 17 +++------ client/finality-grandpa/src/lib.rs | 5 +-- 5 files changed, 39 insertions(+), 23 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d0db499c6b549..51964bc6a6b3c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6840,6 +6840,7 @@ dependencies = [ "log", "parity-scale-codec", "sc-block-builder", + "sc-client-api", "sc-finality-grandpa", "sc-network-test", "sc-rpc", diff --git a/client/finality-grandpa/rpc/Cargo.toml b/client/finality-grandpa/rpc/Cargo.toml index 28197405c8db7..d00576406e8f2 100644 --- a/client/finality-grandpa/rpc/Cargo.toml +++ b/client/finality-grandpa/rpc/Cargo.toml @@ -9,6 +9,7 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0" [dependencies] sc-rpc = { version = "2.0.0-rc6", path = "../../rpc" } +sp-blockchain = { version = "2.0.0-rc6", path = "../../../primitives/blockchain" } sp-runtime = { version = "2.0.0-rc6", path = "../../../primitives/runtime" } sc-finality-grandpa = { version = "0.8.0-rc6", path = "../" } finality-grandpa = { version = "0.12.3", features = ["derive-codec"] } @@ -22,12 +23,12 @@ serde_json = "1.0.50" log = "0.4.8" derive_more = "0.99.2" parity-scale-codec = { version = "1.3.0", features = ["derive"] } +sc-client-api = { version = "2.0.0-rc6", path = "../../api" } [dev-dependencies] sc-block-builder = { version = "0.8.0-rc6", path = "../../block-builder" } sc-network-test = { version = "0.8.0-rc6", path = "../../network/test" } sc-rpc = { version = "2.0.0-rc6", path = "../../rpc", features = ["test-helpers"] } -sp-blockchain = { version = "2.0.0-rc6", path = "../../../primitives/blockchain" } sp-consensus = { version = "0.8.0-rc6", path = "../../../primitives/consensus/common" } sp-core = { version = "2.0.0-rc6", path = "../../../primitives/core" } sp-finality-grandpa = { version = "2.0.0-rc6", path = "../../../primitives/finality-grandpa" } diff --git a/client/finality-grandpa/rpc/src/lib.rs b/client/finality-grandpa/rpc/src/lib.rs index f3300b30a3a2f..e6d51e7622e6a 100644 --- a/client/finality-grandpa/rpc/src/lib.rs +++ b/client/finality-grandpa/rpc/src/lib.rs @@ -35,8 +35,8 @@ mod error; mod notification; mod report; -use sc_finality_grandpa::{RpcFinalityProofProvider, GrandpaJustificationStream}; -use sp_runtime::traits::Block as BlockT; +use sc_finality_grandpa::{FinalityProofProvider, GrandpaJustificationStream}; +use sp_runtime::traits::{Block as BlockT, NumberFor}; use report::{ReportAuthoritySet, ReportVoterState, ReportedRoundStates}; use notification::JustificationNotification; @@ -128,7 +128,7 @@ where VoterState: ReportVoterState + Send + Sync + 'static, AuthoritySet: ReportAuthoritySet + Send + Sync + 'static, Block: BlockT, - ProofProvider: RpcFinalityProofProvider + 'static, + ProofProvider: RpcFinalityProofProvider + Send + Sync + 'static, { type Metadata = sc_rpc::Metadata; @@ -174,7 +174,7 @@ where .unwrap_or_else(|| self.authority_set.get().0); let result = self .finality_proof_provider - .prove_finality(last_finalized, authorities_set_id); + .rpc_prove_finality(last_finalized, authorities_set_id); let future = async move { result }.boxed(); Box::new( future @@ -188,6 +188,32 @@ where } } +/// Local trait mainly to allow mocking in tests. +pub trait RpcFinalityProofProvider { + /// Return finality proofs for the given authorities set id, if it is provided, otherwise the + /// current one will be used. + fn rpc_prove_finality( + &self, + last_finalized: Block::Hash, + authorities_set_id: u64, + ) -> Result>, sp_blockchain::Error>; +} + +impl RpcFinalityProofProvider for FinalityProofProvider +where + Block: BlockT, + NumberFor: finality_grandpa::BlockNumberOps, + B: sc_client_api::backend::Backend + Send + Sync + 'static, +{ + fn rpc_prove_finality( + &self, + last_finalized: Block::Hash, + authorities_set_id: u64, + ) -> Result>, sp_blockchain::Error> { + self.prove_finality(last_finalized, authorities_set_id) + } +} + #[cfg(test)] mod tests { use super::*; @@ -254,7 +280,7 @@ mod tests { } impl RpcFinalityProofProvider for TestFinalityProofProvider { - fn prove_finality( + fn rpc_prove_finality( &self, _last_finalized: Block::Hash, _authoritites_set_id: u64, diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index dbb28260670ff..242a0544d1394 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -182,24 +182,15 @@ impl FinalityProofProvider } } -/// Provide finality proofs to the RPC API -pub trait RpcFinalityProofProvider: Send + Sync { - /// Return finality proofs for the given authorities set id, if it is provided, otherwise the - /// current one will be used. - fn prove_finality( - &self, - last_finalized: Block::Hash, - authorities_set_id: u64, - ) -> Result>, ClientError>; -} - -impl RpcFinalityProofProvider for FinalityProofProvider +impl FinalityProofProvider where Block: BlockT, NumberFor: BlockNumberOps, B: Backend + Send + Sync + 'static, { - fn prove_finality( + /// Return finality proofs for the given authorities set id, if it is provided, otherwise the + /// current one will be used. + pub fn prove_finality( &self, last_finalized: Block::Hash, authorities_set_id: u64, diff --git a/client/finality-grandpa/src/lib.rs b/client/finality-grandpa/src/lib.rs index e1c4b55ed0e74..e358a268b641c 100644 --- a/client/finality-grandpa/src/lib.rs +++ b/client/finality-grandpa/src/lib.rs @@ -125,10 +125,7 @@ mod until_imported; mod voting_rule; pub use authorities::SharedAuthoritySet; -pub use finality_proof::{ - FinalityProofProvider, StorageAndProofProvider, RpcFinalityProofProvider, - FinalityProofFragment, -}; +pub use finality_proof::{FinalityProofProvider, StorageAndProofProvider, FinalityProofFragment}; pub use notification::{GrandpaJustificationSender, GrandpaJustificationStream}; pub use import::GrandpaBlockImport; pub use justification::GrandpaJustification; From 6c7adb1cf34b38c95bd6f19fae51df65315386da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Mon, 24 Aug 2020 14:30:59 +0200 Subject: [PATCH 20/26] grandpa-rpc: wrap finality output in struct and store as Bytes --- client/finality-grandpa/rpc/Cargo.toml | 1 + client/finality-grandpa/rpc/src/finality.rs | 52 +++++++++++++++++++++ client/finality-grandpa/rpc/src/lib.rs | 42 ++++------------- 3 files changed, 62 insertions(+), 33 deletions(-) create mode 100644 client/finality-grandpa/rpc/src/finality.rs diff --git a/client/finality-grandpa/rpc/Cargo.toml b/client/finality-grandpa/rpc/Cargo.toml index d00576406e8f2..0f9d9a15dd2d0 100644 --- a/client/finality-grandpa/rpc/Cargo.toml +++ b/client/finality-grandpa/rpc/Cargo.toml @@ -9,6 +9,7 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0" [dependencies] sc-rpc = { version = "2.0.0-rc6", path = "../../rpc" } +sp-core = { version = "2.0.0-rc6", path = "../../../primitives/core" } sp-blockchain = { version = "2.0.0-rc6", path = "../../../primitives/blockchain" } sp-runtime = { version = "2.0.0-rc6", path = "../../../primitives/runtime" } sc-finality-grandpa = { version = "0.8.0-rc6", path = "../" } diff --git a/client/finality-grandpa/rpc/src/finality.rs b/client/finality-grandpa/rpc/src/finality.rs new file mode 100644 index 0000000000000..266e5b5ceb995 --- /dev/null +++ b/client/finality-grandpa/rpc/src/finality.rs @@ -0,0 +1,52 @@ +// 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 . + +use serde::{Serialize, Deserialize}; + +use sc_finality_grandpa::FinalityProofProvider; +use sp_runtime::traits::{Block as BlockT, NumberFor}; + +#[derive(Serialize, Deserialize)] +pub struct EncodedFinalityProofs(pub sp_core::Bytes); + +/// Local trait mainly to allow mocking in tests. +pub trait RpcFinalityProofProvider { + /// Return finality proofs for the given authorities set id, if it is provided, otherwise the + /// current one will be used. + fn rpc_prove_finality( + &self, + last_finalized: Block::Hash, + authorities_set_id: u64, + ) -> Result, sp_blockchain::Error>; +} + +impl RpcFinalityProofProvider for FinalityProofProvider +where + Block: BlockT, + NumberFor: finality_grandpa::BlockNumberOps, + B: sc_client_api::backend::Backend + Send + Sync + 'static, +{ + fn rpc_prove_finality( + &self, + last_finalized: Block::Hash, + authorities_set_id: u64, + ) -> Result, sp_blockchain::Error> { + self.prove_finality(last_finalized, authorities_set_id) + .map(|x| x.map(|y| EncodedFinalityProofs(y.into()))) + } +} diff --git a/client/finality-grandpa/rpc/src/lib.rs b/client/finality-grandpa/rpc/src/lib.rs index e6d51e7622e6a..b3d5a3a66dc42 100644 --- a/client/finality-grandpa/rpc/src/lib.rs +++ b/client/finality-grandpa/rpc/src/lib.rs @@ -32,12 +32,14 @@ use jsonrpc_core::futures::{ }; mod error; +mod finality; mod notification; mod report; -use sc_finality_grandpa::{FinalityProofProvider, GrandpaJustificationStream}; -use sp_runtime::traits::{Block as BlockT, NumberFor}; +use sc_finality_grandpa::GrandpaJustificationStream; +use sp_runtime::traits::Block as BlockT; +use finality::{EncodedFinalityProofs, RpcFinalityProofProvider}; use report::{ReportAuthoritySet, ReportVoterState, ReportedRoundStates}; use notification::JustificationNotification; @@ -91,7 +93,7 @@ pub trait GrandpaApi { &self, last_finalized: Hash, authorities_set_id: Option, - ) -> FutureResult>>; + ) -> FutureResult>; } /// Implements the GrandpaApi RPC trait for interacting with GRANDPA. @@ -168,7 +170,7 @@ where &self, last_finalized: Block::Hash, authorities_set_id: Option, - ) -> FutureResult>> { + ) -> FutureResult> { // If we are not provided a set_id, try with the current one. let authorities_set_id = authorities_set_id .unwrap_or_else(|| self.authority_set.get().0); @@ -188,32 +190,6 @@ where } } -/// Local trait mainly to allow mocking in tests. -pub trait RpcFinalityProofProvider { - /// Return finality proofs for the given authorities set id, if it is provided, otherwise the - /// current one will be used. - fn rpc_prove_finality( - &self, - last_finalized: Block::Hash, - authorities_set_id: u64, - ) -> Result>, sp_blockchain::Error>; -} - -impl RpcFinalityProofProvider for FinalityProofProvider -where - Block: BlockT, - NumberFor: finality_grandpa::BlockNumberOps, - B: sc_client_api::backend::Backend + Send + Sync + 'static, -{ - fn rpc_prove_finality( - &self, - last_finalized: Block::Hash, - authorities_set_id: u64, - ) -> Result>, sp_blockchain::Error> { - self.prove_finality(last_finalized, authorities_set_id) - } -} - #[cfg(test)] mod tests { use super::*; @@ -284,8 +260,8 @@ mod tests { &self, _last_finalized: Block::Hash, _authoritites_set_id: u64, - ) -> Result>, sp_blockchain::Error> { - Ok(Some(self.finality_proofs.encode())) + ) -> Result, sp_blockchain::Error> { + Ok(Some(EncodedFinalityProofs(self.finality_proofs.encode().into()))) } } @@ -559,7 +535,7 @@ mod tests { let meta = sc_rpc::Metadata::default(); let resp = io.handle_request_sync(request, meta); let mut resp: serde_json::Value = serde_json::from_str(&resp.unwrap()).unwrap(); - let result: Vec = serde_json::from_value(resp["result"].take()).unwrap(); + let result: sp_core::Bytes = serde_json::from_value(resp["result"].take()).unwrap(); let fragments: Vec> = Decode::decode(&mut &result[..]).unwrap(); assert_eq!(fragments, finality_proofs); From 026f542bac2f6fc7652374c539c8a5c3c97a5689 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Mon, 24 Aug 2020 16:14:22 +0200 Subject: [PATCH 21/26] grandpa-rpc: exhaustive error codes and wrap --- client/finality-grandpa/rpc/src/error.rs | 39 ++++++++++++++++++------ client/finality-grandpa/rpc/src/lib.rs | 7 +---- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/client/finality-grandpa/rpc/src/error.rs b/client/finality-grandpa/rpc/src/error.rs index 45b881cacc17d..6464acbe10ea0 100644 --- a/client/finality-grandpa/rpc/src/error.rs +++ b/client/finality-grandpa/rpc/src/error.rs @@ -16,8 +16,6 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use crate::{NOT_READY_ERROR_CODE, INTERNAL_ERROR}; - #[derive(derive_more::Display, derive_more::From)] /// Top-level error type for the RPC handler pub enum Error { @@ -31,19 +29,40 @@ pub enum Error { #[display(fmt = "GRANDPA reports voter state as unreasonably large")] VoterStateReportsUnreasonablyLargeNumbers, /// GRANDPA prove finality failed. - #[display(fmt = "GRANDPA prove finality rpc failed")] - ProveFinalityFailed, + #[display(fmt = "GRANDPA prove finality rpc failed: {}", _0)] + ProveFinalityFailed(sp_blockchain::Error), +} + +/// The error codes returned by jsonrpc. +pub enum ErrorCode { + /// Returned when Grandpa RPC endpoint is not ready. + NotReady = 1, + /// Authority set ID is larger than 32-bits. + AuthoritySetTooLarge, + /// Voter state with round id or weights larger than 32-bits. + VoterStateTooLarge, + /// Failed to prove finality. + ProveFinality, +} + +impl From for ErrorCode { + fn from(error: Error) -> Self { + match error { + Error::EndpointNotReady => ErrorCode::NotReady, + Error::AuthoritySetIdReportedAsUnreasonablyLarge => ErrorCode::AuthoritySetTooLarge, + Error::VoterStateReportsUnreasonablyLargeNumbers => ErrorCode::VoterStateTooLarge, + Error::ProveFinalityFailed(_) => ErrorCode::ProveFinality, + } + } } impl From for jsonrpc_core::Error { fn from(error: Error) -> Self { - let code = match error { - Error::EndpointNotReady => NOT_READY_ERROR_CODE, - _ => INTERNAL_ERROR, - }; + let message = format!("{}", error); + let code = ErrorCode::from(error); jsonrpc_core::Error { - message: format!("{}", error), - code: jsonrpc_core::ErrorCode::ServerError(code), + message, + code: jsonrpc_core::ErrorCode::ServerError(code as i64), data: None, } } diff --git a/client/finality-grandpa/rpc/src/lib.rs b/client/finality-grandpa/rpc/src/lib.rs index b3d5a3a66dc42..9342490bed2fd 100644 --- a/client/finality-grandpa/rpc/src/lib.rs +++ b/client/finality-grandpa/rpc/src/lib.rs @@ -43,11 +43,6 @@ use finality::{EncodedFinalityProofs, RpcFinalityProofProvider}; use report::{ReportAuthoritySet, ReportVoterState, ReportedRoundStates}; use notification::JustificationNotification; -/// Returned when Grandpa RPC endpoint is not ready. -pub const NOT_READY_ERROR_CODE: i64 = 1; -/// Returned for internal Grandpa RPC errors. -pub const INTERNAL_ERROR: i64 = 2; - type FutureResult = Box + Send>; @@ -182,7 +177,7 @@ where future .map_err(|e| { warn!("Error proving finality: {}", e); - error::Error::ProveFinalityFailed + error::Error::ProveFinalityFailed(e) }) .map_err(jsonrpc_core::Error::from) .compat() From bdf931a1510cf8df41674f469144307b4143a173 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Thu, 27 Aug 2020 12:37:32 +0200 Subject: [PATCH 22/26] grandpa-rpc: let prove_finality take a range instead of a starting point --- client/finality-grandpa/rpc/src/finality.rs | 8 +++++--- client/finality-grandpa/rpc/src/lib.rs | 14 +++++++++----- client/finality-grandpa/src/finality_proof.rs | 8 ++++---- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/client/finality-grandpa/rpc/src/finality.rs b/client/finality-grandpa/rpc/src/finality.rs index 266e5b5ceb995..1f288b86a0e46 100644 --- a/client/finality-grandpa/rpc/src/finality.rs +++ b/client/finality-grandpa/rpc/src/finality.rs @@ -30,7 +30,8 @@ pub trait RpcFinalityProofProvider { /// current one will be used. fn rpc_prove_finality( &self, - last_finalized: Block::Hash, + begin: Block::Hash, + end: Block::Hash, authorities_set_id: u64, ) -> Result, sp_blockchain::Error>; } @@ -43,10 +44,11 @@ where { fn rpc_prove_finality( &self, - last_finalized: Block::Hash, + begin: Block::Hash, + end: Block::Hash, authorities_set_id: u64, ) -> Result, sp_blockchain::Error> { - self.prove_finality(last_finalized, authorities_set_id) + self.prove_finality(begin, end, authorities_set_id) .map(|x| x.map(|y| EncodedFinalityProofs(y.into()))) } } diff --git a/client/finality-grandpa/rpc/src/lib.rs b/client/finality-grandpa/rpc/src/lib.rs index 0edec2727f966..b0fd7bf4f8782 100644 --- a/client/finality-grandpa/rpc/src/lib.rs +++ b/client/finality-grandpa/rpc/src/lib.rs @@ -82,11 +82,13 @@ pub trait GrandpaApi { id: SubscriptionId ) -> jsonrpc_core::Result; - /// Prove finality for the hash, given a last known finalized hash. + /// Prove finality for the range (begin; end] hash. Returns None if there are no finalized blocks + /// unknown in the range. If no authorities set is provided, the current one will be attempted. #[rpc(name = "grandpa_proveFinality")] fn prove_finality( &self, - last_finalized: Hash, + begin: Hash, + end: Hash, authorities_set_id: Option, ) -> FutureResult>; } @@ -163,7 +165,8 @@ where fn prove_finality( &self, - last_finalized: Block::Hash, + begin: Block::Hash, + end: Block::Hash, authorities_set_id: Option, ) -> FutureResult> { // If we are not provided a set_id, try with the current one. @@ -171,7 +174,7 @@ where .unwrap_or_else(|| self.authority_set.get().0); let result = self .finality_proof_provider - .rpc_prove_finality(last_finalized, authorities_set_id); + .rpc_prove_finality(begin, end, authorities_set_id); let future = async move { result }.boxed(); Box::new( future @@ -253,7 +256,8 @@ mod tests { impl RpcFinalityProofProvider for TestFinalityProofProvider { fn rpc_prove_finality( &self, - _last_finalized: Block::Hash, + _begin: Block::Hash, + _end: Block::Hash, _authoritites_set_id: u64, ) -> Result, sp_blockchain::Error> { Ok(Some(EncodedFinalityProofs(self.finality_proofs.encode().into()))) diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index 242a0544d1394..d6246dda8f007 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -192,16 +192,16 @@ impl FinalityProofProvider /// current one will be used. pub fn prove_finality( &self, - last_finalized: Block::Hash, + begin: Block::Hash, + end: Block::Hash, authorities_set_id: u64, ) -> Result>, ClientError> { - use sp_blockchain::HeaderBackend; prove_finality::<_, _, GrandpaJustification>( &*self.backend.blockchain(), &*self.authority_provider, authorities_set_id, - last_finalized.clone(), - self.backend.blockchain().info().best_hash, + begin, + end, ) } } From 354c5898e0ee65ff626410aa935dbfa25dae811f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Thu, 27 Aug 2020 15:48:20 +0200 Subject: [PATCH 23/26] grandpa-rpc: fix test for changed API --- client/finality-grandpa/rpc/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/client/finality-grandpa/rpc/src/lib.rs b/client/finality-grandpa/rpc/src/lib.rs index b0fd7bf4f8782..0037a90fbee96 100644 --- a/client/finality-grandpa/rpc/src/lib.rs +++ b/client/finality-grandpa/rpc/src/lib.rs @@ -528,6 +528,7 @@ mod tests { let request = "{\"jsonrpc\":\"2.0\",\"method\":\"grandpa_proveFinality\",\"params\":[\ \"0x0000000000000000000000000000000000000000000000000000000000000000\",\ + \"0x0000000000000000000000000000000000000000000000000000000000000001\",\ 42\ ],\"id\":1}"; From c353f8160856112ea86960c92da930ac0c91b764 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Fri, 28 Aug 2020 16:26:12 +0200 Subject: [PATCH 24/26] grandpa-rpc: fix line length --- client/finality-grandpa/rpc/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/client/finality-grandpa/rpc/src/lib.rs b/client/finality-grandpa/rpc/src/lib.rs index 0037a90fbee96..8193116a3d003 100644 --- a/client/finality-grandpa/rpc/src/lib.rs +++ b/client/finality-grandpa/rpc/src/lib.rs @@ -102,7 +102,9 @@ pub struct GrandpaRpcHandler, } -impl GrandpaRpcHandler { +impl + GrandpaRpcHandler +{ /// Creates a new GrandpaRpcHandler instance. pub fn new( authority_set: AuthoritySet, From 37b9507c0017e7665184facd1bb3d895fa757eb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Mon, 14 Sep 2020 17:04:52 +0200 Subject: [PATCH 25/26] grandpa: fix reviewer nits --- bin/node/cli/src/service.rs | 1 - bin/node/rpc/src/lib.rs | 5 ++--- client/finality-grandpa/src/lib.rs | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index 5f62a4bbfe006..b15ace6181a8f 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -105,7 +105,6 @@ pub fn new_partial(config: &Configuration) -> Result { @@ -120,8 +119,8 @@ pub fn create_full( C::Api: BlockBuilder, P: TransactionPool + 'static, SC: SelectChain +'static, - B: Send + Sync + 'static + sc_client_api::Backend, - >::State: sp_state_machine::Backend, + B: sc_client_api::Backend + Send + Sync + 'static, + B::State: sc_client_api::backend::StateBackend>, { use substrate_frame_rpc_system::{FullSystem, SystemApi}; use pallet_contracts_rpc::{Contracts, ContractsApi}; diff --git a/client/finality-grandpa/src/lib.rs b/client/finality-grandpa/src/lib.rs index e358a268b641c..a15130942c30f 100644 --- a/client/finality-grandpa/src/lib.rs +++ b/client/finality-grandpa/src/lib.rs @@ -125,7 +125,7 @@ mod until_imported; mod voting_rule; pub use authorities::SharedAuthoritySet; -pub use finality_proof::{FinalityProofProvider, StorageAndProofProvider, FinalityProofFragment}; +pub use finality_proof::{FinalityProofFragment, FinalityProofProvider, StorageAndProofProvider}; pub use notification::{GrandpaJustificationSender, GrandpaJustificationStream}; pub use import::GrandpaBlockImport; pub use justification::GrandpaJustification; From 6c79a1ea496aeb25e63f8f81cf3d4ac92a90dd0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Wed, 16 Sep 2020 00:45:14 +0200 Subject: [PATCH 26/26] node/rpc: fix reviewer comments --- Cargo.lock | 1 - bin/node/rpc/Cargo.toml | 1 - client/finality-grandpa/src/finality_proof.rs | 4 ++-- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8b333bae924fa..18e690b65f92a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3823,7 +3823,6 @@ dependencies = [ "sp-consensus", "sp-consensus-babe", "sp-runtime", - "sp-state-machine", "sp-transaction-pool", "substrate-frame-rpc-system", ] diff --git a/bin/node/rpc/Cargo.toml b/bin/node/rpc/Cargo.toml index dd510d7dfa2d8..020fc88a3d0f2 100644 --- a/bin/node/rpc/Cargo.toml +++ b/bin/node/rpc/Cargo.toml @@ -31,6 +31,5 @@ sp-blockchain = { version = "2.0.0-rc6", path = "../../../primitives/blockchain" sp-consensus = { version = "0.8.0-rc6", path = "../../../primitives/consensus/common" } sp-consensus-babe = { version = "0.8.0-rc6", path = "../../../primitives/consensus/babe" } sp-runtime = { version = "2.0.0-rc6", path = "../../../primitives/runtime" } -sp-state-machine = { version = "0.8.0-rc6", path = "../../../primitives/state-machine" } sp-transaction-pool = { version = "2.0.0-rc6", path = "../../../primitives/transaction-pool" } substrate-frame-rpc-system = { version = "2.0.0-rc6", path = "../../../utils/frame/rpc/system" } diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index d6246dda8f007..33dd69cc11d6e 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -188,8 +188,8 @@ impl FinalityProofProvider NumberFor: BlockNumberOps, B: Backend + Send + Sync + 'static, { - /// Return finality proofs for the given authorities set id, if it is provided, otherwise the - /// current one will be used. + /// Prove finality for the range (begin; end] hash. Returns None if there are no finalized blocks + /// unknown in the range. pub fn prove_finality( &self, begin: Block::Hash,