Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

grandpa-rpc: use FinalityProofProvider to check finality for rpc #6215

Merged
37 commits merged into from
Sep 18, 2020
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
4eab2a4
grandpa-rpc: use FinalityProofProvider to check finality for rpc
octol May 26, 2020
5953803
Merge branch 'master' into jon/on-demand-grandpa-finality-api
octol Aug 6, 2020
0fc9cd5
Merge branch 'master' into jon/on-demand-grandpa-finality-api
octol Aug 10, 2020
06b04f5
grandpa-rpc: minor tidy
octol Aug 12, 2020
34522c1
grandpa-rpc: remove dyn FinalityProofProvider
octol Aug 12, 2020
9fe866e
grandpa-rpc: remove unused dependencies
octol Aug 12, 2020
8591352
node: move finality_proof_provider setup
octol Aug 12, 2020
80f6c53
grandpa-rpc: print error reported by finality_proof_provider
octol Aug 12, 2020
136c257
grandpa-rpc: add note about unnecessary encode/decode
octol Aug 12, 2020
eab8404
grandpa-rpc: dont encode/decode and use correct hash
octol Aug 14, 2020
d3e8d30
grandpa-rpc: set_id is optional
octol Aug 18, 2020
801b3e6
grandpa-rpc: create test for prove_finality
octol Aug 21, 2020
56f32a3
grandpa-rpc: set visibility back to how it was
octol Aug 21, 2020
c1e50d5
grandpa-rpc: remove unused dependency
octol Aug 21, 2020
3b5a3c6
grandpa-rpc: minor tidy
octol Aug 21, 2020
a892236
grandpa: doc strings
octol Aug 21, 2020
2db4ba0
grandpa-rpc: rename to prove_finality
octol Aug 21, 2020
3c543e6
grandpa-rpc: use current set id if none is provided
octol Aug 21, 2020
0e89cee
Merge branch 'master' into jon/on-demand-grandpa-finality-api
octol Aug 21, 2020
7b1ed8f
grandpa-rpc: remove unnecessary check in test
octol Aug 21, 2020
ca1e2a8
Merge branch 'master' into jon/on-demand-grandpa-finality-api
octol Aug 24, 2020
9a3b43f
node: group finality_proof_provider in rpc_setup
octol Aug 24, 2020
f294b47
grandpa: make prove_finality concrete in FinalityProofProvider
octol Aug 24, 2020
6c7adb1
grandpa-rpc: wrap finality output in struct and store as Bytes
octol Aug 24, 2020
026f542
grandpa-rpc: exhaustive error codes and wrap
octol Aug 24, 2020
cf2375d
Merge branch 'master' into jon/on-demand-grandpa-finality-api
octol Aug 25, 2020
bdf931a
grandpa-rpc: let prove_finality take a range instead of a starting point
octol Aug 27, 2020
9d43c11
Merge branch 'master' into jon/on-demand-grandpa-finality-api
octol Aug 27, 2020
354c589
grandpa-rpc: fix test for changed API
octol Aug 27, 2020
c353f81
grandpa-rpc: fix line length
octol Aug 28, 2020
de666ff
Merge branch 'master' into jon/on-demand-grandpa-finality-api
octol Aug 28, 2020
ff6d612
Merge branch 'master' into jon/on-demand-grandpa-finality-api
octol Sep 7, 2020
21afc7d
Merge branch 'master' into jon/on-demand-grandpa-finality-api
octol Sep 14, 2020
37b9507
grandpa: fix reviewer nits
octol Sep 14, 2020
1212782
Merge branch 'master' into jon/on-demand-grandpa-finality-api
octol Sep 15, 2020
6c79a1e
node/rpc: fix reviewer comments
octol Sep 15, 2020
c284665
Merge remote-tracking branch 'upstream/master' into jon/on-demand-gra…
octol Sep 15, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 10 additions & 5 deletions bin/node/cli/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,10 @@ pub fn new_partial(config: &Configuration) -> Result<sc_service::PartialComponen
grandpa::LinkHalf<Block, FullClient, FullSelectChain>,
sc_consensus_babe::BabeLink<Block>,
),
grandpa::SharedVoterState,
(
grandpa::SharedVoterState,
Arc<GrandpaFinalityProofProvider<FullBackend, Block>>,
),
)
>, ServiceError> {
let (client, backend, keystore, task_manager) =
Expand Down Expand Up @@ -102,14 +105,17 @@ pub fn new_partial(config: &Configuration) -> Result<sc_service::PartialComponen

let import_setup = (block_import, grandpa_link, babe_link);


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

let (rpc_extensions_builder, rpc_setup) = {
let (_, grandpa_link, babe_link) = &import_setup;

let justification_stream = grandpa_link.justification_stream();
let shared_authority_set = grandpa_link.shared_authority_set().clone();
let shared_voter_state = grandpa::SharedVoterState::empty();
let finality_proof_provider =
GrandpaFinalityProofProvider::new_for_service(backend.clone(), client.clone());
andresilva marked this conversation as resolved.
Show resolved Hide resolved

let rpc_setup = shared_voter_state.clone();
let rpc_setup = (shared_voter_state.clone(), finality_proof_provider.clone());

let babe_config = babe_link.config().clone();
let shared_epoch_changes = babe_link.epoch_changes().clone();
Expand All @@ -135,6 +141,7 @@ pub fn new_partial(config: &Configuration) -> Result<sc_service::PartialComponen
shared_authority_set: shared_authority_set.clone(),
justification_stream: justification_stream.clone(),
subscriptions,
finality_provider: finality_proof_provider.clone(),
},
};

Expand Down Expand Up @@ -169,8 +176,7 @@ pub fn new_full_base(
other: (rpc_extensions_builder, import_setup, rpc_setup),
} = new_partial(&config)?;

let finality_proof_provider =
GrandpaFinalityProofProvider::new_for_service(backend.clone(), client.clone());
let (shared_voter_state, finality_proof_provider) = rpc_setup;

let (network, network_status_sinks, system_rpc_tx, network_starter) =
sc_service::build_network(sc_service::BuildNetworkParams {
Expand Down Expand Up @@ -215,7 +221,6 @@ pub fn new_full_base(
})?;

let (block_import, grandpa_link, babe_link) = import_setup;
let shared_voter_state = rpc_setup;

(with_startup_data)(&block_import, &babe_link);

Expand Down
2 changes: 2 additions & 0 deletions bin/node/rpc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,7 @@ sp-block-builder = { version = "2.0.0-rc6", path = "../../../primitives/block-bu
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" }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one isn't needed anymore.

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" }
21 changes: 15 additions & 6 deletions bin/node/rpc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ use node_primitives::{Block, BlockNumber, AccountId, Index, Balance, Hash};
use sc_consensus_babe::{Config, Epoch};
use sc_consensus_babe_rpc::BabeRpcHandler;
use sc_consensus_epochs::SharedEpochChanges;
use sc_finality_grandpa::{SharedVoterState, SharedAuthoritySet, GrandpaJustificationStream};
use sc_finality_grandpa::{
SharedVoterState, SharedAuthoritySet, FinalityProofProvider, GrandpaJustificationStream
};
use sc_finality_grandpa_rpc::GrandpaRpcHandler;
use sc_keystore::KeyStorePtr;
pub use sc_rpc_api::DenyUnsafe;
Expand All @@ -47,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;
Copy link
Contributor

@andresilva andresilva Sep 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be removed if we do the change I suggested below.


/// Light client extra dependencies.
pub struct LightDeps<C, F, P> {
Expand All @@ -71,7 +74,7 @@ pub struct BabeDeps {
}

/// Extra dependencies for GRANDPA
pub struct GrandpaDeps {
pub struct GrandpaDeps<B> {
/// Voting round info.
pub shared_voter_state: SharedVoterState,
/// Authority set info.
Expand All @@ -80,10 +83,12 @@ pub struct GrandpaDeps {
pub justification_stream: GrandpaJustificationStream<Block>,
/// Subscription manager to keep track of pubsub subscribers.
pub subscriptions: SubscriptionManager,
/// Finality proof provider.
pub finality_provider: Arc<FinalityProofProvider<B, Block>>,
}

/// Full client dependencies.
pub struct FullDeps<C, P, SC> {
pub struct FullDeps<C, P, SC, B> {
/// The client instance to use.
pub client: Arc<C>,
/// Transaction pool instance.
Expand All @@ -95,15 +100,15 @@ pub struct FullDeps<C, P, SC> {
/// BABE specific dependencies.
pub babe: BabeDeps,
/// GRANDPA specific dependencies.
pub grandpa: GrandpaDeps,
pub grandpa: GrandpaDeps<B>,
}

/// A IO handler that uses all Full RPC extensions.
pub type IoHandler = jsonrpc_core::IoHandler<sc_rpc::Metadata>;

/// Instantiate all Full RPC extensions.
pub fn create_full<C, P, SC>(
deps: FullDeps<C, P, SC>,
pub fn create_full<C, P, SC, B>(
deps: FullDeps<C, P, SC, B>,
) -> jsonrpc_core::IoHandler<sc_rpc_api::Metadata> where
C: ProvideRuntimeApi<Block>,
C: HeaderBackend<Block> + HeaderMetadata<Block, Error=BlockChainError> + 'static,
Expand All @@ -115,6 +120,8 @@ pub fn create_full<C, P, SC>(
C::Api: BlockBuilder<Block>,
P: TransactionPool + 'static,
SC: SelectChain<Block> +'static,
B: Send + Sync + 'static + sc_client_api::Backend<Block>,
<B as sc_client_api::Backend<Block>>::State: sp_state_machine::Backend<BlakeTwo256>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
B: Send + Sync + 'static + sc_client_api::Backend<Block>,
<B as sc_client_api::Backend<Block>>::State: sp_state_machine::Backend<BlakeTwo256>,
B: sc_client_api::Backend<Block> + Send + Sync + 'static,
B::State: sp_state_machine::Backend<sp_runtime::traits::HashFor<Block>>,

Copy link
Contributor

@andresilva andresilva Sep 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the reason we need to specify the state backend here is because Block isn't a generic type parameter and is actually the concrete block type (i.e. hash is already fixed to be H256). Either way doing it this way is more generic since we don't tie the bound to the concrete hashing we currently use.

{
use substrate_frame_rpc_system::{FullSystem, SystemApi};
use pallet_contracts_rpc::{Contracts, ContractsApi};
Expand All @@ -140,6 +147,7 @@ pub fn create_full<C, P, SC>(
shared_authority_set,
justification_stream,
subscriptions,
finality_provider,
} = grandpa;

io.extend_with(
Expand Down Expand Up @@ -173,6 +181,7 @@ pub fn create_full<C, P, SC>(
shared_voter_state,
justification_stream,
subscriptions,
finality_provider,
)
)
);
Expand Down
3 changes: 2 additions & 1 deletion client/finality-grandpa/rpc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0"
[dependencies]
sc-finality-grandpa = { version = "0.8.0-rc6", path = "../" }
sc-rpc = { version = "2.0.0-rc6", path = "../../rpc" }
sp-blockchain = { version = "2.0.0-rc6", path = "../../../primitives/blockchain" }
sp-core = { version = "2.0.0-rc6", path = "../../../primitives/core" }
sp-runtime = { version = "2.0.0-rc6", path = "../../../primitives/runtime" }
finality-grandpa = { version = "0.12.3", features = ["derive-codec"] }
Expand All @@ -23,12 +24,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" }
Expand Down
34 changes: 30 additions & 4 deletions client/finality-grandpa/rpc/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
// You should have received a copy of the GNU General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

use crate::NOT_READY_ERROR_CODE;

#[derive(derive_more::Display, derive_more::From)]
/// Top-level error type for the RPC handler
pub enum Error {
Expand All @@ -30,13 +28,41 @@ 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.
#[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<Error> 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<Error> for jsonrpc_core::Error {
fn from(error: Error) -> Self {
let message = format!("{}", error);
let code = ErrorCode::from(error);
jsonrpc_core::Error {
message: format!("{}", error),
code: jsonrpc_core::ErrorCode::ServerError(NOT_READY_ERROR_CODE),
message,
code: jsonrpc_core::ErrorCode::ServerError(code as i64),
data: None,
}
}
Expand Down
54 changes: 54 additions & 0 deletions client/finality-grandpa/rpc/src/finality.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// 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 <https://www.gnu.org/licenses/>.

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<Block: BlockT> {
/// 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,
begin: Block::Hash,
end: Block::Hash,
authorities_set_id: u64,
) -> Result<Option<EncodedFinalityProofs>, sp_blockchain::Error>;
}

impl<B, Block> RpcFinalityProofProvider<Block> for FinalityProofProvider<B, Block>
where
Block: BlockT,
NumberFor<Block>: finality_grandpa::BlockNumberOps,
B: sc_client_api::backend::Backend<Block> + Send + Sync + 'static,
{
fn rpc_prove_finality(
&self,
begin: Block::Hash,
end: Block::Hash,
authorities_set_id: u64,
) -> Result<Option<EncodedFinalityProofs>, sp_blockchain::Error> {
self.prove_finality(begin, end, authorities_set_id)
.map(|x| x.map(|y| EncodedFinalityProofs(y.into())))
}
}
Loading