diff --git a/substrate/.maintain/monitoring/grafana-dashboards/substrate-dashboard.json b/substrate/.maintain/monitoring/grafana-dashboards/substrate-dashboard.json index 629b22617b22a970eb1f439179f62711d1bac9b4..a61e8a49bade752ea321ffe62528f24378442834 100644 --- a/substrate/.maintain/monitoring/grafana-dashboards/substrate-dashboard.json +++ b/substrate/.maintain/monitoring/grafana-dashboards/substrate-dashboard.json @@ -756,108 +756,6 @@ "alignLevel": null } }, - { - "aliasColors": {}, - "bars": false, - "dashLength": 10, - "dashes": false, - "datasource": null, - "fill": 1, - "fillGradient": 0, - "gridPos": { - "h": 6, - "w": 6, - "x": 0, - "y": 12 - }, - "hiddenSeries": false, - "id": 23, - "legend": { - "avg": false, - "current": false, - "max": false, - "min": false, - "show": true, - "total": false, - "values": false - }, - "lines": true, - "linewidth": 1, - "nullPointMode": "null", - "options": { - "dataLinks": [] - }, - "percentage": false, - "pointradius": 2, - "points": false, - "renderer": "flot", - "seriesOverrides": [], - "spaceLength": 10, - "stack": false, - "steppedLine": false, - "targets": [ - { - "expr": "[[metric_namespace]]_sync_extra_finality_proofs_active{instance=\"[[instance]]\",network=\"[[network]]\"}", - "legendFormat": "{{instance}} active", - "refId": "A" - }, - { - "expr": "[[metric_namespace]]_sync_extra_finality_proofs_failed{instance=\"[[instance]]\",network=\"[[network]]\"}", - "legendFormat": "{{instance}} failed", - "refId": "B" - }, - { - "expr": "[[metric_namespace]]_sync_extra_finality_proofs_importing{instance=\"[[instance]]\",network=\"[[network]]\"}", - "legendFormat": "{{instance}} importing", - "refId": "C" - }, - { - "expr": "[[metric_namespace]]_sync_extra_finality_proofs_pending{instance=\"[[instance]]\",network=\"[[network]]\"}", - "legendFormat": "{{instance}} pending", - "refId": "D" - } - ], - "thresholds": [], - "timeFrom": null, - "timeRegions": [], - "timeShift": null, - "title": "Sync Proof", - "tooltip": { - "shared": true, - "sort": 0, - "value_type": "individual" - }, - "type": "graph", - "xaxis": { - "buckets": null, - "mode": "time", - "name": null, - "show": true, - "values": [] - }, - "yaxes": [ - { - "format": "short", - "label": null, - "logBase": 1, - "max": null, - "min": null, - "show": true - }, - { - "format": "short", - "label": null, - "logBase": 1, - "max": null, - "min": null, - "show": true - } - ], - "yaxis": { - "align": false, - "alignLevel": null - } - }, { "aliasColors": {}, "bars": false, diff --git a/substrate/bin/node-template/node/src/service.rs b/substrate/bin/node-template/node/src/service.rs index d85de7c840dfd04c60f212e19ae947520ce0c87b..83481f8c152145b9641c4f4c0d6f08eceac1b73b 100644 --- a/substrate/bin/node-template/node/src/service.rs +++ b/substrate/bin/node-template/node/src/service.rs @@ -9,7 +9,7 @@ use sp_inherents::InherentDataProviders; use sc_executor::native_executor_instance; pub use sc_executor::NativeExecutor; use sp_consensus_aura::sr25519::{AuthorityPair as AuraPair}; -use sc_finality_grandpa::{FinalityProofProvider as GrandpaFinalityProofProvider, SharedVoterState}; +use sc_finality_grandpa::SharedVoterState; // Our native executor instance. native_executor_instance!( @@ -64,7 +64,6 @@ pub fn new_partial(config: &Configuration) -> Result<sc_service::PartialComponen sc_consensus_aura::slot_duration(&*client)?, aura_block_import.clone(), Some(Box::new(grandpa_block_import.clone())), - None, client.clone(), inherent_data_providers.clone(), &task_manager.spawn_handle(), @@ -87,9 +86,6 @@ pub fn new_full(config: Configuration) -> Result<TaskManager, ServiceError> { other: (block_import, grandpa_link), } = new_partial(&config)?; - let finality_proof_provider = - GrandpaFinalityProofProvider::new_for_service(backend.clone(), client.clone()); - let (network, network_status_sinks, system_rpc_tx, network_starter) = sc_service::build_network(sc_service::BuildNetworkParams { config: &config, @@ -99,8 +95,6 @@ pub fn new_full(config: Configuration) -> Result<TaskManager, ServiceError> { import_queue, on_demand: None, block_announce_validator_builder: None, - finality_proof_request_builder: None, - finality_proof_provider: Some(finality_proof_provider.clone()), })?; if config.offchain_worker.enabled { @@ -229,6 +223,8 @@ pub fn new_light(config: Configuration) -> Result<TaskManager, ServiceError> { let (client, backend, keystore_container, mut task_manager, on_demand) = sc_service::new_light_parts::<Block, RuntimeApi, Executor>(&config)?; + let select_chain = sc_consensus::LongestChain::new(backend.clone()); + let transaction_pool = Arc::new(sc_transaction_pool::BasicPool::new_light( config.transaction_pool.clone(), config.prometheus_registry(), @@ -237,19 +233,16 @@ pub fn new_light(config: Configuration) -> Result<TaskManager, ServiceError> { on_demand.clone(), )); - let grandpa_block_import = sc_finality_grandpa::light_block_import( - client.clone(), backend.clone(), &(client.clone() as Arc<_>), - Arc::new(on_demand.checker().clone()) as Arc<_>, + let (grandpa_block_import, _) = sc_finality_grandpa::block_import( + client.clone(), + &(client.clone() as Arc<_>), + select_chain.clone(), )?; - let finality_proof_import = grandpa_block_import.clone(); - let finality_proof_request_builder = - finality_proof_import.create_finality_proof_request_builder(); let import_queue = sc_consensus_aura::import_queue::<_, _, _, AuraPair, _, _>( sc_consensus_aura::slot_duration(&*client)?, - grandpa_block_import, - None, - Some(Box::new(finality_proof_import)), + grandpa_block_import.clone(), + Some(Box::new(grandpa_block_import)), client.clone(), InherentDataProviders::new(), &task_manager.spawn_handle(), @@ -257,9 +250,6 @@ pub fn new_light(config: Configuration) -> Result<TaskManager, ServiceError> { sp_consensus::NeverCanAuthor, )?; - let finality_proof_provider = - GrandpaFinalityProofProvider::new_for_service(backend.clone(), client.clone()); - let (network, network_status_sinks, system_rpc_tx, network_starter) = sc_service::build_network(sc_service::BuildNetworkParams { config: &config, @@ -269,8 +259,6 @@ pub fn new_light(config: Configuration) -> Result<TaskManager, ServiceError> { import_queue, on_demand: Some(on_demand.clone()), block_announce_validator_builder: None, - finality_proof_request_builder: Some(finality_proof_request_builder), - finality_proof_provider: Some(finality_proof_provider), })?; if config.offchain_worker.enabled { diff --git a/substrate/bin/node/cli/src/service.rs b/substrate/bin/node/cli/src/service.rs index 3bc406b84fc67481b94535829b06dc372c258b84..9d7c9bb1b7a66b865d1a3fda1811d230d2c78b7b 100644 --- a/substrate/bin/node/cli/src/service.rs +++ b/substrate/bin/node/cli/src/service.rs @@ -22,7 +22,6 @@ use std::sync::Arc; use sc_consensus_babe; -use grandpa::{self, FinalityProofProvider as GrandpaFinalityProofProvider}; use node_primitives::Block; use node_runtime::RuntimeApi; use sc_service::{ @@ -57,10 +56,7 @@ pub fn new_partial(config: &Configuration) -> Result<sc_service::PartialComponen grandpa::LinkHalf<Block, FullClient, FullSelectChain>, sc_consensus_babe::BabeLink<Block>, ), - ( - grandpa::SharedVoterState, - Arc<GrandpaFinalityProofProvider<FullBackend, Block>>, - ), + grandpa::SharedVoterState, ) >, ServiceError> { let (client, backend, keystore_container, task_manager) = @@ -93,7 +89,6 @@ pub fn new_partial(config: &Configuration) -> Result<sc_service::PartialComponen babe_link.clone(), block_import.clone(), Some(Box::new(justification_import)), - None, client.clone(), select_chain.clone(), inherent_data_providers.clone(), @@ -110,10 +105,10 @@ pub fn new_partial(config: &Configuration) -> Result<sc_service::PartialComponen 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()); + let rpc_setup = shared_voter_state.clone(); - let rpc_setup = (shared_voter_state.clone(), finality_proof_provider.clone()); + let finality_proof_provider = + grandpa::FinalityProofProvider::new_for_service(backend.clone(), client.clone()); let babe_config = babe_link.config().clone(); let shared_epoch_changes = babe_link.epoch_changes().clone(); @@ -181,7 +176,7 @@ pub fn new_full_base( other: (rpc_extensions_builder, import_setup, rpc_setup), } = new_partial(&config)?; - let (shared_voter_state, finality_proof_provider) = rpc_setup; + let shared_voter_state = rpc_setup; let (network, network_status_sinks, system_rpc_tx, network_starter) = sc_service::build_network(sc_service::BuildNetworkParams { @@ -192,8 +187,6 @@ pub fn new_full_base( import_queue, on_demand: None, block_announce_validator_builder: None, - finality_proof_request_builder: None, - finality_proof_provider: Some(finality_proof_provider.clone()), })?; if config.offchain_worker.enabled { @@ -363,14 +356,12 @@ pub fn new_light_base(config: Configuration) -> Result<( on_demand.clone(), )); - let grandpa_block_import = grandpa::light_block_import( - client.clone(), backend.clone(), &(client.clone() as Arc<_>), - Arc::new(on_demand.checker().clone()), + let (grandpa_block_import, _) = grandpa::block_import( + client.clone(), + &(client.clone() as Arc<_>), + select_chain.clone(), )?; - - let finality_proof_import = grandpa_block_import.clone(); - let finality_proof_request_builder = - finality_proof_import.create_finality_proof_request_builder(); + let justification_import = grandpa_block_import.clone(); let (babe_block_import, babe_link) = sc_consensus_babe::block_import( sc_consensus_babe::Config::get_or_compute(&*client)?, @@ -383,8 +374,7 @@ pub fn new_light_base(config: Configuration) -> Result<( let import_queue = sc_consensus_babe::import_queue( babe_link, babe_block_import, - None, - Some(Box::new(finality_proof_import)), + Some(Box::new(justification_import)), client.clone(), select_chain.clone(), inherent_data_providers.clone(), @@ -393,9 +383,6 @@ pub fn new_light_base(config: Configuration) -> Result<( sp_consensus::NeverCanAuthor, )?; - let finality_proof_provider = - GrandpaFinalityProofProvider::new_for_service(backend.clone(), client.clone()); - let (network, network_status_sinks, system_rpc_tx, network_starter) = sc_service::build_network(sc_service::BuildNetworkParams { config: &config, @@ -405,8 +392,6 @@ pub fn new_light_base(config: Configuration) -> Result<( import_queue, on_demand: Some(on_demand.clone()), block_announce_validator_builder: None, - finality_proof_request_builder: Some(finality_proof_request_builder), - finality_proof_provider: Some(finality_proof_provider), })?; network_starter.start_network(); diff --git a/substrate/bin/node/testing/src/bench.rs b/substrate/bin/node/testing/src/bench.rs index a123da25301d073214c7d6ec48b362e445143096..35af52a2f36c1ad3170ff418fb4900c2f3a758f0 100644 --- a/substrate/bin/node/testing/src/bench.rs +++ b/substrate/bin/node/testing/src/bench.rs @@ -695,7 +695,6 @@ impl BenchContext { clear_justification_requests: false, needs_justification: false, bad_justification: false, - needs_finality_proof: false, is_new_best: true, } ) diff --git a/substrate/client/consensus/aura/src/lib.rs b/substrate/client/consensus/aura/src/lib.rs index 97bfb217b9396f5d977bfa911c80a1dc61d4cfdc..246b39771277d3e54bc95db097e852a99447e68e 100644 --- a/substrate/client/consensus/aura/src/lib.rs +++ b/substrate/client/consensus/aura/src/lib.rs @@ -47,7 +47,7 @@ use sp_consensus::{ BlockOrigin, Error as ConsensusError, SelectChain, SlotData, BlockCheckParams, ImportResult }; use sp_consensus::import_queue::{ - Verifier, BasicQueue, DefaultImportQueue, BoxJustificationImport, BoxFinalityProofImport, + Verifier, BasicQueue, DefaultImportQueue, BoxJustificationImport, }; use sc_client_api::{backend::AuxStore, BlockOf}; use sp_blockchain::{ @@ -836,7 +836,6 @@ pub fn import_queue<B, I, C, P, S, CAW>( slot_duration: SlotDuration, block_import: I, justification_import: Option<BoxJustificationImport<B>>, - finality_proof_import: Option<BoxFinalityProofImport<B>>, client: Arc<C>, inherent_data_providers: InherentDataProviders, spawner: &S, @@ -868,7 +867,6 @@ pub fn import_queue<B, I, C, P, S, CAW>( verifier, Box::new(block_import), justification_import, - finality_proof_import, spawner, registry, )) diff --git a/substrate/client/consensus/babe/src/lib.rs b/substrate/client/consensus/babe/src/lib.rs index c672440d114b507b78f1cb929b2b784f37a50bcb..3f2a583482afb5d7486f44e0f6c45b7e0f5d6111 100644 --- a/substrate/client/consensus/babe/src/lib.rs +++ b/substrate/client/consensus/babe/src/lib.rs @@ -79,9 +79,7 @@ use std::{ any::Any, borrow::Cow, convert::TryInto, }; use sp_consensus::{ImportResult, CanAuthorWith}; -use sp_consensus::import_queue::{ - BoxJustificationImport, BoxFinalityProofImport, -}; +use sp_consensus::import_queue::BoxJustificationImport; use sp_core::crypto::Public; use sp_application_crypto::AppKey; use sp_keystore::{SyncCryptoStorePtr, SyncCryptoStore}; @@ -1484,7 +1482,6 @@ pub fn import_queue<Block: BlockT, Client, SelectChain, Inner, CAW>( babe_link: BabeLink<Block>, block_import: Inner, justification_import: Option<BoxJustificationImport<Block>>, - finality_proof_import: Option<BoxFinalityProofImport<Block>>, client: Arc<Client>, select_chain: SelectChain, inherent_data_providers: InherentDataProviders, @@ -1516,7 +1513,6 @@ pub fn import_queue<Block: BlockT, Client, SelectChain, Inner, CAW>( verifier, Box::new(block_import), justification_import, - finality_proof_import, spawner, registry, )) diff --git a/substrate/client/consensus/babe/src/tests.rs b/substrate/client/consensus/babe/src/tests.rs index b31699d13e0c865d6a98805853f8e231ce1f372c..6e0536c85ced76ff1242361e2ab4b4205a990642 100644 --- a/substrate/client/consensus/babe/src/tests.rs +++ b/substrate/client/consensus/babe/src/tests.rs @@ -37,11 +37,11 @@ use sc_consensus_slots::BackoffAuthoringOnFinalizedHeadLagging; use sc_block_builder::{BlockBuilder, BlockBuilderProvider}; use sp_consensus::{ NoNetwork as DummyOracle, Proposal, RecordProof, AlwaysCanAuthor, - import_queue::{BoxBlockImport, BoxJustificationImport, BoxFinalityProofImport}, + import_queue::{BoxBlockImport, BoxJustificationImport}, }; use sc_network_test::*; use sc_network_test::{Block as TestBlock, PeersClient}; -use sc_network::config::{BoxFinalityProofRequestBuilder, ProtocolConfig}; +use sc_network::config::ProtocolConfig; use sp_runtime::{generic::DigestItem, traits::{Block as BlockT, DigestFor}}; use sc_client_api::{BlockchainEvents, backend::TransactionFor}; use log::debug; @@ -272,8 +272,6 @@ impl TestNetFactory for BabeTestNet { -> ( BlockImportAdapter<Transaction>, Option<BoxJustificationImport<Block>>, - Option<BoxFinalityProofImport<Block>>, - Option<BoxFinalityProofRequestBuilder<Block>>, Option<PeerData>, ) { @@ -295,8 +293,6 @@ impl TestNetFactory for BabeTestNet { ( BlockImportAdapter::new_full(block_import), None, - None, - None, Some(PeerData { link, inherent_data_providers, block_import: data_block_import }), ) } diff --git a/substrate/client/consensus/manual-seal/src/lib.rs b/substrate/client/consensus/manual-seal/src/lib.rs index d025d6aaf689f823456ca58d500cffcf16ed7760..5a1cd0f79b47ddd1b2fc2a95c8f63732aee702e3 100644 --- a/substrate/client/consensus/manual-seal/src/lib.rs +++ b/substrate/client/consensus/manual-seal/src/lib.rs @@ -84,7 +84,6 @@ pub fn import_queue<Block, Transaction>( ManualSealVerifier, block_import, None, - None, spawner, registry, ) @@ -349,7 +348,6 @@ mod tests { clear_justification_requests: false, needs_justification: false, bad_justification: false, - needs_finality_proof: false, is_new_best: true, } } @@ -416,7 +414,6 @@ mod tests { clear_justification_requests: false, needs_justification: false, bad_justification: false, - needs_finality_proof: false, is_new_best: true, } } @@ -494,7 +491,6 @@ mod tests { clear_justification_requests: false, needs_justification: false, bad_justification: false, - needs_finality_proof: false, is_new_best: true } } diff --git a/substrate/client/consensus/pow/src/lib.rs b/substrate/client/consensus/pow/src/lib.rs index b73b9aa91f802a0cae87e9b590f224100b5b8325..e353ed6358a00fc94f5069bbeff356ba93347cd6 100644 --- a/substrate/client/consensus/pow/src/lib.rs +++ b/substrate/client/consensus/pow/src/lib.rs @@ -56,7 +56,7 @@ use sp_consensus::{ BlockCheckParams, ImportResult, }; use sp_consensus::import_queue::{ - BoxBlockImport, BasicQueue, Verifier, BoxJustificationImport, BoxFinalityProofImport, + BoxBlockImport, BasicQueue, Verifier, BoxJustificationImport, }; use codec::{Encode, Decode}; use prometheus_endpoint::Registry; @@ -503,7 +503,6 @@ pub type PowImportQueue<B, Transaction> = BasicQueue<B, Transaction>; pub fn import_queue<B, Transaction, Algorithm>( block_import: BoxBlockImport<B, Transaction>, justification_import: Option<BoxJustificationImport<B>>, - finality_proof_import: Option<BoxFinalityProofImport<B>>, algorithm: Algorithm, inherent_data_providers: InherentDataProviders, spawner: &impl sp_core::traits::SpawnNamed, @@ -524,7 +523,6 @@ pub fn import_queue<B, Transaction, Algorithm>( verifier, block_import, justification_import, - finality_proof_import, spawner, registry, )) diff --git a/substrate/client/finality-grandpa/src/aux_schema.rs b/substrate/client/finality-grandpa/src/aux_schema.rs index 4ed96d058ac6b42941705c046984852fdea0b795..97041f4081a720a35f9de0d0479f84a6071fd69b 100644 --- a/substrate/client/finality-grandpa/src/aux_schema.rs +++ b/substrate/client/finality-grandpa/src/aux_schema.rs @@ -17,7 +17,6 @@ //! Schema for stuff in the aux-db. use std::fmt::Debug; -use std::sync::Arc; use parity_scale_codec::{Encode, Decode}; use sc_client_api::backend::AuxStore; use sp_blockchain::{Result as ClientResult, Error as ClientError}; @@ -28,7 +27,6 @@ use log::{info, warn}; use sp_finality_grandpa::{AuthorityList, SetId, RoundNumber}; use crate::authorities::{AuthoritySet, SharedAuthoritySet, PendingChange, DelayKind}; -use crate::consensus_changes::{SharedConsensusChanges, ConsensusChanges}; use crate::environment::{ CompletedRound, CompletedRounds, CurrentRounds, HasVoted, SharedVoterSetState, VoterSetState, }; @@ -38,7 +36,6 @@ const VERSION_KEY: &[u8] = b"grandpa_schema_version"; const SET_STATE_KEY: &[u8] = b"grandpa_completed_round"; const CONCLUDED_ROUNDS: &[u8] = b"grandpa_concluded_rounds"; const AUTHORITY_SET_KEY: &[u8] = b"grandpa_voters"; -const CONSENSUS_CHANGES_KEY: &[u8] = b"grandpa_consensus_changes"; const CURRENT_VERSION: u32 = 2; @@ -122,7 +119,6 @@ pub(crate) fn load_decode<B: AuxStore, T: Decode>(backend: &B, key: &[u8]) -> Cl /// Persistent data kept between runs. pub(crate) struct PersistentData<Block: BlockT> { pub(crate) authority_set: SharedAuthoritySet<Block::Hash, NumberFor<Block>>, - pub(crate) consensus_changes: SharedConsensusChanges<Block::Hash, NumberFor<Block>>, pub(crate) set_state: SharedVoterSetState<Block>, } @@ -272,8 +268,6 @@ pub(crate) fn load_persistent<Block: BlockT, B, G>( G: FnOnce() -> ClientResult<AuthorityList>, { let version: Option<u32> = load_decode(backend, VERSION_KEY)?; - let consensus_changes = load_decode(backend, CONSENSUS_CHANGES_KEY)? - .unwrap_or_else(ConsensusChanges::<Block::Hash, NumberFor<Block>>::empty); let make_genesis_round = move || RoundState::genesis((genesis_hash, genesis_number)); @@ -282,7 +276,6 @@ pub(crate) fn load_persistent<Block: BlockT, B, G>( if let Some((new_set, set_state)) = migrate_from_version0::<Block, _, _>(backend, &make_genesis_round)? { return Ok(PersistentData { authority_set: new_set.into(), - consensus_changes: Arc::new(consensus_changes.into()), set_state: set_state.into(), }); } @@ -291,7 +284,6 @@ pub(crate) fn load_persistent<Block: BlockT, B, G>( if let Some((new_set, set_state)) = migrate_from_version1::<Block, _, _>(backend, &make_genesis_round)? { return Ok(PersistentData { authority_set: new_set.into(), - consensus_changes: Arc::new(consensus_changes.into()), set_state: set_state.into(), }); } @@ -321,7 +313,6 @@ pub(crate) fn load_persistent<Block: BlockT, B, G>( return Ok(PersistentData { authority_set: set.into(), - consensus_changes: Arc::new(consensus_changes.into()), set_state: set_state.into(), }); } @@ -359,7 +350,6 @@ pub(crate) fn load_persistent<Block: BlockT, B, G>( Ok(PersistentData { authority_set: genesis_set.into(), set_state: genesis_state.into(), - consensus_changes: Arc::new(consensus_changes.into()), }) } @@ -421,18 +411,6 @@ pub(crate) fn write_concluded_round<Block: BlockT, B: AuxStore>( backend.insert_aux(&[(&key[..], round_data.encode().as_slice())], &[]) } -/// Update the consensus changes. -pub(crate) fn update_consensus_changes<H, N, F, R>( - set: &ConsensusChanges<H, N>, - write_aux: F -) -> R where - H: Encode + Clone, - N: Encode + Clone, - F: FnOnce(&[(&'static [u8], &[u8])]) -> R, -{ - write_aux(&[(CONSENSUS_CHANGES_KEY, set.encode().as_slice())]) -} - #[cfg(test)] pub(crate) fn load_authorities<B: AuxStore, H: Decode, N: Decode>(backend: &B) -> Option<AuthoritySet<H, N>> { diff --git a/substrate/client/finality-grandpa/src/consensus_changes.rs b/substrate/client/finality-grandpa/src/consensus_changes.rs deleted file mode 100644 index 1ce7b551d0d7c577524b876e508467a8fc12bd75..0000000000000000000000000000000000000000 --- a/substrate/client/finality-grandpa/src/consensus_changes.rs +++ /dev/null @@ -1,78 +0,0 @@ -// Copyright 2018-2020 Parity Technologies (UK) Ltd. -// This file is part of Substrate. - -// Substrate is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// Substrate is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with Substrate. If not, see <http://www.gnu.org/licenses/>. - -use std::sync::Arc; -use parity_scale_codec::{Encode, Decode}; - -/// Consensus-related data changes tracker. -#[derive(Clone, Debug, Encode, Decode)] -pub(crate) struct ConsensusChanges<H, N> { - pending_changes: Vec<(N, H)>, -} - -impl<H, N> ConsensusChanges<H, N> { - /// Create empty consensus changes. - pub(crate) fn empty() -> Self { - ConsensusChanges { pending_changes: Vec::new(), } - } -} - -impl<H: Copy + PartialEq, N: Copy + Ord> ConsensusChanges<H, N> { - - /// Returns reference to all pending changes. - pub fn pending_changes(&self) -> &[(N, H)] { - &self.pending_changes - } - - /// Note unfinalized change of consensus-related data. - pub(crate) fn note_change(&mut self, at: (N, H)) { - let idx = self.pending_changes - .binary_search_by_key(&at.0, |change| change.0) - .unwrap_or_else(|i| i); - self.pending_changes.insert(idx, at); - } - - /// Finalize all pending consensus changes that are finalized by given block. - /// Returns true if there any changes were finalized. - pub(crate) fn finalize<F: Fn(N) -> ::sp_blockchain::Result<Option<H>>>( - &mut self, - block: (N, H), - canonical_at_height: F, - ) -> ::sp_blockchain::Result<(bool, bool)> { - let (split_idx, has_finalized_changes) = self.pending_changes.iter() - .enumerate() - .take_while(|(_, &(at_height, _))| at_height <= block.0) - .fold((None, Ok(false)), |(_, has_finalized_changes), (idx, ref at)| - ( - Some(idx), - has_finalized_changes - .and_then(|has_finalized_changes| if has_finalized_changes { - Ok(has_finalized_changes) - } else { - canonical_at_height(at.0).map(|can_hash| Some(at.1) == can_hash) - }), - )); - - let altered_changes = split_idx.is_some(); - if let Some(split_idx) = split_idx { - self.pending_changes = self.pending_changes.split_off(split_idx + 1); - } - has_finalized_changes.map(|has_finalized_changes| (altered_changes, has_finalized_changes)) - } -} - -/// Thread-safe consensus changes tracker reference. -pub(crate) type SharedConsensusChanges<H, N> = Arc<parking_lot::Mutex<ConsensusChanges<H, N>>>; diff --git a/substrate/client/finality-grandpa/src/environment.rs b/substrate/client/finality-grandpa/src/environment.rs index 9b3a656d0cd8fad741f82fe7e17ff43e653886c4..790be2a22178878dade0e4961193eade7703df41 100644 --- a/substrate/client/finality-grandpa/src/environment.rs +++ b/substrate/client/finality-grandpa/src/environment.rs @@ -34,10 +34,10 @@ use finality_grandpa::{ BlockNumberOps, Error as GrandpaError, round::State as RoundState, voter, voter_set::VoterSet, }; -use sp_blockchain::{HeaderBackend, HeaderMetadata, Error as ClientError}; +use sp_blockchain::HeaderMetadata; use sp_runtime::generic::BlockId; use sp_runtime::traits::{ - Block as BlockT, Header as HeaderT, NumberFor, One, Zero, + Block as BlockT, Header as HeaderT, NumberFor, Zero, }; use sc_telemetry::{telemetry, CONSENSUS_DEBUG, CONSENSUS_INFO}; @@ -50,7 +50,6 @@ use sp_consensus::SelectChain; use crate::authorities::{AuthoritySet, SharedAuthoritySet}; use crate::communication::Network as NetworkT; -use crate::consensus_changes::SharedConsensusChanges; use crate::notification::GrandpaJustificationSender; use crate::justification::GrandpaJustification; use crate::until_imported::UntilVoteTargetImported; @@ -440,7 +439,6 @@ pub(crate) struct Environment<Backend, Block: BlockT, C, N: NetworkT<Block>, SC, pub(crate) voters: Arc<VoterSet<AuthorityId>>, pub(crate) config: Config, pub(crate) authority_set: SharedAuthoritySet<Block::Hash, NumberFor<Block>>, - pub(crate) consensus_changes: SharedConsensusChanges<Block::Hash, NumberFor<Block>>, pub(crate) network: crate::communication::NetworkBridge<Block, N>, pub(crate) set_id: SetId, pub(crate) voter_set_state: SharedVoterSetState<Block>, @@ -1115,7 +1113,6 @@ where finalize_block( self.client.clone(), &self.authority_set, - &self.consensus_changes, Some(self.config.justification_period.into()), hash, number, @@ -1180,7 +1177,6 @@ impl<Block: BlockT> From<GrandpaJustification<Block>> for JustificationOrCommit< pub(crate) fn finalize_block<BE, Block, Client>( client: Arc<Client>, authority_set: &SharedAuthoritySet<Block::Hash, NumberFor<Block>>, - consensus_changes: &SharedConsensusChanges<Block::Hash, NumberFor<Block>>, justification_period: Option<NumberFor<Block>>, hash: Block::Hash, number: NumberFor<Block>, @@ -1215,15 +1211,6 @@ where // FIXME #1483: clone only when changed let old_authority_set = authority_set.clone(); - // holds the old consensus changes in case it is changed below, needed for - // reverting in case of failure - let mut old_consensus_changes = None; - - let mut consensus_changes = consensus_changes.lock(); - let canon_at_height = |canon_number| { - // "true" because the block is finalized - canonical_at_height(&*client, (hash, number), true, canon_number) - }; let update_res: Result<_, Error> = client.lock_import_and_run(|import_op| { let status = authority_set.apply_standard_changes( @@ -1233,26 +1220,6 @@ where initial_sync, ).map_err(|e| Error::Safety(e.to_string()))?; - // check if this is this is the first finalization of some consensus changes - let (alters_consensus_changes, finalizes_consensus_changes) = consensus_changes - .finalize((number, hash), &canon_at_height)?; - - if alters_consensus_changes { - old_consensus_changes = Some(consensus_changes.clone()); - - let write_result = crate::aux_schema::update_consensus_changes( - &*consensus_changes, - |insert| apply_aux(import_op, insert, &[]), - ); - - if let Err(e) = write_result { - warn!(target: "afg", "Failed to write updated consensus changes to disk. Bailing."); - warn!(target: "afg", "Node is in a potentially inconsistent state."); - - return Err(e.into()); - } - } - // send a justification notification if a sender exists and in case of error log it. fn notify_justification<Block: BlockT>( justification_sender: Option<&GrandpaJustificationSender<Block>>, @@ -1280,9 +1247,7 @@ where let mut justification_required = // justification is always required when block that enacts new authorities // set is finalized - status.new_set_block.is_some() || - // justification is required when consensus changes are finalized - finalizes_consensus_changes; + status.new_set_block.is_some(); // justification is required every N blocks to be able to prove blocks // finalization to remote nodes @@ -1387,57 +1352,7 @@ where Err(e) => { *authority_set = old_authority_set; - if let Some(old_consensus_changes) = old_consensus_changes { - *consensus_changes = old_consensus_changes; - } - Err(CommandOrError::Error(e)) } } } - -/// Using the given base get the block at the given height on this chain. The -/// target block must be an ancestor of base, therefore `height <= base.height`. -pub(crate) fn canonical_at_height<Block: BlockT, C: HeaderBackend<Block>>( - provider: &C, - base: (Block::Hash, NumberFor<Block>), - base_is_canonical: bool, - height: NumberFor<Block>, -) -> Result<Option<Block::Hash>, ClientError> { - if height > base.1 { - return Ok(None); - } - - if height == base.1 { - if base_is_canonical { - return Ok(Some(base.0)); - } else { - return Ok(provider.hash(height).unwrap_or(None)); - } - } else if base_is_canonical { - return Ok(provider.hash(height).unwrap_or(None)); - } - - let one = NumberFor::<Block>::one(); - - // start by getting _canonical_ block with number at parent position and then iterating - // backwards by hash. - let mut current = match provider.header(BlockId::Number(base.1 - one))? { - Some(header) => header, - _ => return Ok(None), - }; - - // we've already checked that base > height above. - let mut steps = base.1 - height - one; - - while steps > NumberFor::<Block>::zero() { - current = match provider.header(BlockId::Hash(*current.parent_hash()))? { - Some(header) => header, - _ => return Ok(None), - }; - - steps -= one; - } - - Ok(Some(current.hash())) -} diff --git a/substrate/client/finality-grandpa/src/finality_proof.rs b/substrate/client/finality-grandpa/src/finality_proof.rs index 33dd69cc11d6e73c328b2ceb4d0f594a40ba1fae..bf367ab3f4a55e4d6c5e7c06efec686abd46414a 100644 --- a/substrate/client/finality-grandpa/src/finality_proof.rs +++ b/substrate/client/finality-grandpa/src/finality_proof.rs @@ -16,6 +16,9 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see <https://www.gnu.org/licenses/>. +// NOTE: should be removed with: https://github.com/paritytech/substrate/pull/7339 +#![allow(dead_code)] + //! GRANDPA block finality proof generation and check. //! //! Finality of block B is proved by providing: @@ -37,7 +40,7 @@ //! of the U) could be returned. use std::sync::Arc; -use log::{trace, warn}; +use log::trace; use sp_blockchain::{Backend as BlockchainBackend, Error as ClientError, Result as ClientResult}; use sc_client_api::{ @@ -206,34 +209,6 @@ impl<B, Block> FinalityProofProvider<B, Block> } } -impl<B, Block> sc_network::config::FinalityProofProvider<Block> for FinalityProofProvider<B, Block> - where - Block: BlockT, - NumberFor<Block>: BlockNumberOps, - B: Backend<Block> + Send + Sync + 'static, -{ - fn prove_finality( - &self, - for_block: Block::Hash, - request: &[u8], - ) -> Result<Option<Vec<u8>>, ClientError> { - let request: FinalityProofRequest<Block::Hash> = Decode::decode(&mut &request[..]) - .map_err(|e| { - warn!(target: "afg", "Unable to decode finality proof request: {}", e.what()); - ClientError::Backend("Invalid finality proof request".to_string()) - })?; - match request { - FinalityProofRequest::Original(request) => prove_finality::<_, _, GrandpaJustification<Block>>( - &*self.backend.blockchain(), - &*self.authority_provider, - request.authorities_set_id, - request.last_finalized, - for_block, - ), - } - } -} - /// The effects of block finality. #[derive(Debug, PartialEq)] pub struct FinalityEffects<Header: HeaderT> { @@ -290,14 +265,6 @@ struct OriginalFinalityProofRequest<H: Encode + Decode> { pub last_finalized: H, } -/// Prepare data blob associated with finality proof request. -pub(crate) fn make_finality_proof_request<H: Encode + Decode>(last_finalized: H, authorities_set_id: u64) -> Vec<u8> { - FinalityProofRequest::Original(OriginalFinalityProofRequest { - authorities_set_id, - last_finalized, - }).encode() -} - /// Prepare proof-of-finality for the best possible block in the range: (begin; end]. /// /// It is assumed that the caller already have a proof-of-finality for the block 'begin'. diff --git a/substrate/client/finality-grandpa/src/import.rs b/substrate/client/finality-grandpa/src/import.rs index 04df95a3187e1bea91bea1c945394faaf6bccef6..89f9d0c16ad7c6d0b52fb8d08a46b5795d7bf646 100644 --- a/substrate/client/finality-grandpa/src/import.rs +++ b/substrate/client/finality-grandpa/src/import.rs @@ -41,7 +41,6 @@ use sp_runtime::traits::{ use crate::{Error, CommandOrError, NewAuthoritySet, VoterCommand}; use crate::authorities::{AuthoritySet, SharedAuthoritySet, DelayKind, PendingChange}; -use crate::consensus_changes::SharedConsensusChanges; use crate::environment::finalize_block; use crate::justification::GrandpaJustification; use crate::notification::GrandpaJustificationSender; @@ -61,7 +60,6 @@ pub struct GrandpaBlockImport<Backend, Block: BlockT, Client, SC> { select_chain: SC, authority_set: SharedAuthoritySet<Block::Hash, NumberFor<Block>>, send_voter_commands: TracingUnboundedSender<VoterCommand<Block::Hash, NumberFor<Block>>>, - consensus_changes: SharedConsensusChanges<Block::Hash, NumberFor<Block>>, authority_set_hard_forks: HashMap<Block::Hash, PendingChange<Block::Hash, NumberFor<Block>>>, justification_sender: GrandpaJustificationSender<Block>, _phantom: PhantomData<Backend>, @@ -76,7 +74,6 @@ impl<Backend, Block: BlockT, Client, SC: Clone> Clone for select_chain: self.select_chain.clone(), authority_set: self.authority_set.clone(), send_voter_commands: self.send_voter_commands.clone(), - consensus_changes: self.consensus_changes.clone(), authority_set_hard_forks: self.authority_set_hard_forks.clone(), justification_sender: self.justification_sender.clone(), _phantom: PhantomData, @@ -439,7 +436,6 @@ impl<BE, Block: BlockT, Client, SC> BlockImport<Block> // we don't want to finalize on `inner.import_block` let mut justification = block.justification.take(); - let enacts_consensus_change = !new_cache.is_empty(); let import_result = (&*self.inner).import_block(block, new_cache); let mut imported_aux = { @@ -517,7 +513,7 @@ impl<BE, Block: BlockT, Client, SC> BlockImport<Block> ); import_res.unwrap_or_else(|err| { - if needs_justification || enacts_consensus_change { + if needs_justification { debug!(target: "afg", "Imported block #{} that enacts authority set change with \ invalid justification: {:?}, requesting justification from peers.", number, err); imported_aux.bad_justification = true; @@ -535,12 +531,6 @@ impl<BE, Block: BlockT, Client, SC> BlockImport<Block> imported_aux.needs_justification = true; } - - // we have imported block with consensus data changes, but without justification - // => remember to create justification when next block will be finalized - if enacts_consensus_change { - self.consensus_changes.lock().note_change((number, hash)); - } } } @@ -561,7 +551,6 @@ impl<Backend, Block: BlockT, Client, SC> GrandpaBlockImport<Backend, Block, Clie select_chain: SC, authority_set: SharedAuthoritySet<Block::Hash, NumberFor<Block>>, send_voter_commands: TracingUnboundedSender<VoterCommand<Block::Hash, NumberFor<Block>>>, - consensus_changes: SharedConsensusChanges<Block::Hash, NumberFor<Block>>, authority_set_hard_forks: Vec<(SetId, PendingChange<Block::Hash, NumberFor<Block>>)>, justification_sender: GrandpaJustificationSender<Block>, ) -> GrandpaBlockImport<Backend, Block, Client, SC> { @@ -605,7 +594,6 @@ impl<Backend, Block: BlockT, Client, SC> GrandpaBlockImport<Backend, Block, Clie select_chain, authority_set, send_voter_commands, - consensus_changes, authority_set_hard_forks, justification_sender, _phantom: PhantomData, @@ -646,7 +634,6 @@ where let result = finalize_block( self.inner.clone(), &self.authority_set, - &self.consensus_changes, None, hash, number, diff --git a/substrate/client/finality-grandpa/src/lib.rs b/substrate/client/finality-grandpa/src/lib.rs index 18b439abf5e6882a02bb3ae4f7201d12c8da0de3..c5f89717a64d75838d2493a522b287562fd58303 100644 --- a/substrate/client/finality-grandpa/src/lib.rs +++ b/substrate/client/finality-grandpa/src/lib.rs @@ -112,12 +112,10 @@ macro_rules! afg_log { mod authorities; mod aux_schema; mod communication; -mod consensus_changes; mod environment; mod finality_proof; mod import; mod justification; -mod light_import; mod notification; mod observer; mod until_imported; @@ -128,7 +126,6 @@ pub use finality_proof::{FinalityProofFragment, FinalityProofProvider, StorageAn pub use notification::{GrandpaJustificationSender, GrandpaJustificationStream}; pub use import::GrandpaBlockImport; pub use justification::GrandpaJustification; -pub use light_import::{light_block_import, GrandpaLightBlockImport}; pub use voting_rule::{ BeforeBestBlockBy, ThreeQuartersOfTheUnfinalizedChain, VotingRule, VotingRulesBuilder }; @@ -588,7 +585,6 @@ where select_chain.clone(), persistent_data.authority_set.clone(), voter_commands_tx, - persistent_data.consensus_changes.clone(), authority_set_hard_forks, justification_sender.clone(), ), @@ -844,7 +840,6 @@ where network: network.clone(), set_id: persistent_data.authority_set.set_id(), authority_set: persistent_data.authority_set.clone(), - consensus_changes: persistent_data.consensus_changes.clone(), voter_set_state: persistent_data.set_state, metrics: metrics.as_ref().map(|m| m.environment.clone()), justification_sender: Some(justification_sender), @@ -989,7 +984,6 @@ where select_chain: self.env.select_chain.clone(), config: self.env.config.clone(), authority_set: self.env.authority_set.clone(), - consensus_changes: self.env.consensus_changes.clone(), network: self.env.network.clone(), voting_rule: self.env.voting_rule.clone(), metrics: self.env.metrics.clone(), diff --git a/substrate/client/finality-grandpa/src/light_import.rs b/substrate/client/finality-grandpa/src/light_import.rs deleted file mode 100644 index a7c9a655467c799ed500e6186fd64cc9825b1a7e..0000000000000000000000000000000000000000 --- a/substrate/client/finality-grandpa/src/light_import.rs +++ /dev/null @@ -1,880 +0,0 @@ -// Copyright 2019-2020 Parity Technologies (UK) Ltd. -// This file is part of Substrate. - -// Substrate is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// Substrate is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with Substrate. If not, see <http://www.gnu.org/licenses/>. - -use std::collections::HashMap; -use std::sync::Arc; -use log::{info, trace, warn}; -use parking_lot::RwLock; -use sc_client_api::backend::{AuxStore, Backend, Finalizer, TransactionFor}; -use sp_blockchain::{HeaderBackend, Error as ClientError, well_known_cache_keys}; -use parity_scale_codec::{Encode, Decode}; -use sp_consensus::{ - import_queue::Verifier, - BlockOrigin, BlockImport, FinalityProofImport, BlockImportParams, ImportResult, ImportedAux, - BlockCheckParams, Error as ConsensusError, -}; -use sc_network::config::{BoxFinalityProofRequestBuilder, FinalityProofRequestBuilder}; -use sp_runtime::Justification; -use sp_runtime::traits::{NumberFor, Block as BlockT, Header as HeaderT, DigestFor}; -use sp_finality_grandpa::{self, AuthorityList}; -use sp_runtime::generic::BlockId; - -use crate::GenesisAuthoritySetProvider; -use crate::aux_schema::load_decode; -use crate::consensus_changes::ConsensusChanges; -use crate::environment::canonical_at_height; -use crate::finality_proof::{ - AuthoritySetForFinalityChecker, ProvableJustification, make_finality_proof_request, -}; -use crate::justification::GrandpaJustification; - -/// LightAuthoritySet is saved under this key in aux storage. -const LIGHT_AUTHORITY_SET_KEY: &[u8] = b"grandpa_voters"; -/// ConsensusChanges is saver under this key in aux storage. -const LIGHT_CONSENSUS_CHANGES_KEY: &[u8] = b"grandpa_consensus_changes"; - -/// Create light block importer. -pub fn light_block_import<BE, Block: BlockT, Client>( - client: Arc<Client>, - backend: Arc<BE>, - genesis_authorities_provider: &dyn GenesisAuthoritySetProvider<Block>, - authority_set_provider: Arc<dyn AuthoritySetForFinalityChecker<Block>>, -) -> Result<GrandpaLightBlockImport<BE, Block, Client>, ClientError> - where - BE: Backend<Block>, - Client: crate::ClientForGrandpa<Block, BE>, -{ - let info = client.info(); - let import_data = load_aux_import_data( - info.finalized_hash, - &*client, - genesis_authorities_provider, - )?; - Ok(GrandpaLightBlockImport { - client, - backend, - authority_set_provider, - data: Arc::new(RwLock::new(import_data)), - }) -} - -/// A light block-import handler for GRANDPA. -/// -/// It is responsible for: -/// - checking GRANDPA justifications; -/// - fetching finality proofs for blocks that are enacting consensus changes. -pub struct GrandpaLightBlockImport<BE, Block: BlockT, Client> { - client: Arc<Client>, - backend: Arc<BE>, - authority_set_provider: Arc<dyn AuthoritySetForFinalityChecker<Block>>, - data: Arc<RwLock<LightImportData<Block>>>, -} - -impl<BE, Block: BlockT, Client> Clone for GrandpaLightBlockImport<BE, Block, Client> { - fn clone(&self) -> Self { - GrandpaLightBlockImport { - client: self.client.clone(), - backend: self.backend.clone(), - authority_set_provider: self.authority_set_provider.clone(), - data: self.data.clone(), - } - } -} - -/// Mutable data of light block importer. -struct LightImportData<Block: BlockT> { - last_finalized: Block::Hash, - authority_set: LightAuthoritySet, - consensus_changes: ConsensusChanges<Block::Hash, NumberFor<Block>>, -} - -/// Latest authority set tracker. -#[derive(Debug, Encode, Decode)] -struct LightAuthoritySet { - set_id: u64, - authorities: AuthorityList, -} - -impl<BE, Block: BlockT, Client> GrandpaLightBlockImport<BE, Block, Client> { - /// Create finality proof request builder. - pub fn create_finality_proof_request_builder(&self) -> BoxFinalityProofRequestBuilder<Block> { - Box::new(GrandpaFinalityProofRequestBuilder(self.data.clone())) as _ - } -} - -impl<BE, Block: BlockT, Client> BlockImport<Block> - for GrandpaLightBlockImport<BE, Block, Client> where - NumberFor<Block>: finality_grandpa::BlockNumberOps, - DigestFor<Block>: Encode, - BE: Backend<Block> + 'static, - for<'a> &'a Client: - HeaderBackend<Block> - + BlockImport<Block, Error = ConsensusError, Transaction = TransactionFor<BE, Block>> - + Finalizer<Block, BE> - + AuxStore, -{ - type Error = ConsensusError; - type Transaction = TransactionFor<BE, Block>; - - fn import_block( - &mut self, - block: BlockImportParams<Block, Self::Transaction>, - new_cache: HashMap<well_known_cache_keys::Id, Vec<u8>>, - ) -> Result<ImportResult, Self::Error> { - do_import_block::<_, _, _, GrandpaJustification<Block>>( - &*self.client, &mut *self.data.write(), block, new_cache - ) - } - - fn check_block( - &mut self, - block: BlockCheckParams<Block>, - ) -> Result<ImportResult, Self::Error> { - self.client.check_block(block) - } -} - -impl<BE, Block: BlockT, Client> FinalityProofImport<Block> - for GrandpaLightBlockImport<BE, Block, Client> where - NumberFor<Block>: finality_grandpa::BlockNumberOps, - DigestFor<Block>: Encode, - BE: Backend<Block> + 'static, - for<'a> &'a Client: - HeaderBackend<Block> - + BlockImport<Block, Error = ConsensusError, Transaction = TransactionFor<BE, Block>> - + Finalizer<Block, BE> - + AuxStore, -{ - type Error = ConsensusError; - - fn on_start(&mut self) -> Vec<(Block::Hash, NumberFor<Block>)> { - let mut out = Vec::new(); - let chain_info = (&*self.client).info(); - - let data = self.data.read(); - for (pending_number, pending_hash) in data.consensus_changes.pending_changes() { - if *pending_number > chain_info.finalized_number - && *pending_number <= chain_info.best_number - { - out.push((*pending_hash, *pending_number)); - } - } - - out - } - - fn import_finality_proof( - &mut self, - hash: Block::Hash, - number: NumberFor<Block>, - finality_proof: Vec<u8>, - verifier: &mut dyn Verifier<Block>, - ) -> Result<(Block::Hash, NumberFor<Block>), Self::Error> { - do_import_finality_proof::<_, _, _, GrandpaJustification<Block>>( - &*self.client, - self.backend.clone(), - &*self.authority_set_provider, - &mut *self.data.write(), - hash, - number, - finality_proof, - verifier, - ) - } -} - -impl LightAuthoritySet { - /// Get a genesis set with given authorities. - pub fn genesis(initial: AuthorityList) -> Self { - LightAuthoritySet { - set_id: sp_finality_grandpa::SetId::default(), - authorities: initial, - } - } - - /// Get latest set id. - pub fn set_id(&self) -> u64 { - self.set_id - } - - /// Get latest authorities set. - pub fn authorities(&self) -> AuthorityList { - self.authorities.clone() - } - - /// Set new authorities set. - pub fn update(&mut self, set_id: u64, authorities: AuthorityList) { - self.set_id = set_id; - self.authorities = authorities; - } -} - -struct GrandpaFinalityProofRequestBuilder<B: BlockT>(Arc<RwLock<LightImportData<B>>>); - -impl<B: BlockT> FinalityProofRequestBuilder<B> for GrandpaFinalityProofRequestBuilder<B> { - fn build_request_data(&mut self, _hash: &B::Hash) -> Vec<u8> { - let data = self.0.read(); - make_finality_proof_request( - data.last_finalized, - data.authority_set.set_id(), - ) - } -} - -/// Try to import new block. -fn do_import_block<B, C, Block: BlockT, J>( - mut client: C, - data: &mut LightImportData<Block>, - mut block: BlockImportParams<Block, TransactionFor<B, Block>>, - new_cache: HashMap<well_known_cache_keys::Id, Vec<u8>>, -) -> Result<ImportResult, ConsensusError> - where - C: HeaderBackend<Block> - + AuxStore - + Finalizer<Block, B> - + BlockImport<Block, Transaction = TransactionFor<B, Block>> - + Clone, - B: Backend<Block> + 'static, - NumberFor<Block>: finality_grandpa::BlockNumberOps, - DigestFor<Block>: Encode, - J: ProvableJustification<Block::Header>, -{ - let hash = block.post_hash(); - let number = *block.header.number(); - - // we don't want to finalize on `inner.import_block` - let justification = block.justification.take(); - let enacts_consensus_change = !new_cache.is_empty(); - let import_result = client.import_block(block, new_cache); - - let mut imported_aux = match import_result { - Ok(ImportResult::Imported(aux)) => aux, - Ok(r) => return Ok(r), - Err(e) => return Err(ConsensusError::ClientImport(e.to_string())), - }; - - match justification { - Some(justification) => { - trace!( - target: "afg", - "Imported block {}{}. Importing justification.", - if enacts_consensus_change { " which enacts consensus changes" } else { "" }, - hash, - ); - - do_import_justification::<_, _, _, J>(client, data, hash, number, justification) - }, - None if enacts_consensus_change => { - trace!( - target: "afg", - "Imported block {} which enacts consensus changes. Requesting finality proof.", - hash, - ); - - // remember that we need finality proof for this block - imported_aux.needs_finality_proof = true; - data.consensus_changes.note_change((number, hash)); - Ok(ImportResult::Imported(imported_aux)) - }, - None => Ok(ImportResult::Imported(imported_aux)), - } -} - -/// Try to import finality proof. -fn do_import_finality_proof<B, C, Block: BlockT, J>( - client: C, - backend: Arc<B>, - authority_set_provider: &dyn AuthoritySetForFinalityChecker<Block>, - data: &mut LightImportData<Block>, - _hash: Block::Hash, - _number: NumberFor<Block>, - finality_proof: Vec<u8>, - verifier: &mut dyn Verifier<Block>, -) -> Result<(Block::Hash, NumberFor<Block>), ConsensusError> - where - C: HeaderBackend<Block> - + AuxStore - + Finalizer<Block, B> - + BlockImport<Block, Transaction = TransactionFor<B, Block>> - + Clone, - B: Backend<Block> + 'static, - DigestFor<Block>: Encode, - NumberFor<Block>: finality_grandpa::BlockNumberOps, - J: ProvableJustification<Block::Header>, -{ - let authority_set_id = data.authority_set.set_id(); - let authorities = data.authority_set.authorities(); - let finality_effects = crate::finality_proof::check_finality_proof::<_, _, J>( - backend.blockchain(), - authority_set_id, - authorities, - authority_set_provider, - finality_proof, - ).map_err(|e| ConsensusError::ClientImport(e.to_string()))?; - - // try to import all new headers - let block_origin = BlockOrigin::NetworkBroadcast; - for header_to_import in finality_effects.headers_to_import { - let (block_to_import, new_authorities) = verifier.verify( - block_origin, - header_to_import, - None, - None, - ).map_err(|e| ConsensusError::ClientImport(e))?; - assert!( - block_to_import.justification.is_none(), - "We have passed None as justification to verifier.verify", - ); - - let mut cache = HashMap::new(); - if let Some(authorities) = new_authorities { - cache.insert(well_known_cache_keys::AUTHORITIES, authorities.encode()); - } - do_import_block::<_, _, _, J>( - client.clone(), - data, - block_to_import.convert_transaction(), - cache, - )?; - } - - // try to import latest justification - let finalized_block_hash = finality_effects.block; - let finalized_block_number = backend.blockchain() - .expect_block_number_from_id(&BlockId::Hash(finality_effects.block)) - .map_err(|e| ConsensusError::ClientImport(e.to_string()))?; - do_finalize_block( - client.clone(), - data, - finalized_block_hash, - finalized_block_number, - finality_effects.justification.encode(), - )?; - - // apply new authorities set - data.authority_set.update( - finality_effects.new_set_id, - finality_effects.new_authorities, - ); - - // store new authorities set - require_insert_aux( - &client, - LIGHT_AUTHORITY_SET_KEY, - &data.authority_set, - "authority set", - )?; - - Ok((finalized_block_hash, finalized_block_number)) -} - -/// Try to import justification. -fn do_import_justification<B, C, Block: BlockT, J>( - client: C, - data: &mut LightImportData<Block>, - hash: Block::Hash, - number: NumberFor<Block>, - justification: Justification, -) -> Result<ImportResult, ConsensusError> - where - C: HeaderBackend<Block> - + AuxStore - + Finalizer<Block, B> - + Clone, - B: Backend<Block> + 'static, - NumberFor<Block>: finality_grandpa::BlockNumberOps, - J: ProvableJustification<Block::Header>, -{ - // with justification, we have two cases - // - // optimistic: the same GRANDPA authorities set has generated intermediate justification - // => justification is verified using current authorities set + we could proceed further - // - // pessimistic scenario: the GRANDPA authorities set has changed - // => we need to fetch new authorities set (i.e. finality proof) from remote node - - // first, try to behave optimistically - let authority_set_id = data.authority_set.set_id(); - let justification = J::decode_and_verify( - &justification, - authority_set_id, - &data.authority_set.authorities(), - ); - - // BadJustification error means that justification has been successfully decoded, but - // it isn't valid within current authority set - let justification = match justification { - Err(ClientError::BadJustification(_)) => { - trace!( - target: "afg", - "Justification for {} is not valid within current authorities set. Requesting finality proof.", - hash, - ); - - let mut imported_aux = ImportedAux::default(); - imported_aux.needs_finality_proof = true; - return Ok(ImportResult::Imported(imported_aux)); - }, - Err(e) => { - trace!( - target: "afg", - "Justification for {} is not valid. Bailing.", - hash, - ); - - return Err(ConsensusError::ClientImport(e.to_string())); - }, - Ok(justification) => { - trace!( - target: "afg", - "Justification for {} is valid. Finalizing the block.", - hash, - ); - - justification - }, - }; - - // finalize the block - do_finalize_block(client, data, hash, number, justification.encode()) -} - -/// Finalize the block. -fn do_finalize_block<B, C, Block: BlockT>( - client: C, - data: &mut LightImportData<Block>, - hash: Block::Hash, - number: NumberFor<Block>, - justification: Justification, -) -> Result<ImportResult, ConsensusError> - where - C: HeaderBackend<Block> - + AuxStore - + Finalizer<Block, B> - + Clone, - B: Backend<Block> + 'static, - NumberFor<Block>: finality_grandpa::BlockNumberOps, -{ - // finalize the block - client.finalize_block(BlockId::Hash(hash), Some(justification), true).map_err(|e| { - warn!(target: "afg", "Error applying finality to block {:?}: {:?}", (hash, number), e); - ConsensusError::ClientImport(e.to_string()) - })?; - - // forget obsoleted consensus changes - let consensus_finalization_res = data.consensus_changes - .finalize( - (number, hash), - |at_height| canonical_at_height(&client, (hash, number), true, at_height) - ); - match consensus_finalization_res { - Ok((true, _)) => require_insert_aux( - &client, - LIGHT_CONSENSUS_CHANGES_KEY, - &data.consensus_changes, - "consensus changes", - )?, - Ok(_) => (), - Err(error) => return Err(on_post_finalization_error(error, "consensus changes")), - } - - // update last finalized block reference - data.last_finalized = hash; - - // we just finalized this block, so if we were importing it, it is now the new best - Ok(ImportResult::imported(true)) -} - -/// Load light import aux data from the store. -fn load_aux_import_data<B, Block>( - last_finalized: Block::Hash, - aux_store: &B, - genesis_authorities_provider: &dyn GenesisAuthoritySetProvider<Block>, -) -> Result<LightImportData<Block>, ClientError> - where - B: AuxStore, - Block: BlockT, -{ - let authority_set = match load_decode(aux_store, LIGHT_AUTHORITY_SET_KEY)? { - Some(authority_set) => authority_set, - None => { - info!(target: "afg", "Loading GRANDPA authorities \ - from genesis on what appears to be first startup."); - - // no authority set on disk: fetch authorities from genesis state - let genesis_authorities = genesis_authorities_provider.get()?; - - let authority_set = LightAuthoritySet::genesis(genesis_authorities); - let encoded = authority_set.encode(); - aux_store.insert_aux(&[(LIGHT_AUTHORITY_SET_KEY, &encoded[..])], &[])?; - - authority_set - }, - }; - - let consensus_changes = match load_decode(aux_store, LIGHT_CONSENSUS_CHANGES_KEY)? { - Some(consensus_changes) => consensus_changes, - None => { - let consensus_changes = ConsensusChanges::<Block::Hash, NumberFor<Block>>::empty(); - - let encoded = authority_set.encode(); - aux_store.insert_aux(&[(LIGHT_CONSENSUS_CHANGES_KEY, &encoded[..])], &[])?; - - consensus_changes - }, - }; - - Ok(LightImportData { - last_finalized, - authority_set, - consensus_changes, - }) -} - -/// Insert into aux store. If failed, return error && show inconsistency warning. -fn require_insert_aux<T: Encode, A: AuxStore>( - store: &A, - key: &[u8], - value: &T, - value_type: &str, -) -> Result<(), ConsensusError> { - let encoded = value.encode(); - let update_res = store.insert_aux(&[(key, &encoded[..])], &[]); - if let Err(error) = update_res { - return Err(on_post_finalization_error(error, value_type)); - } - - Ok(()) -} - -/// Display inconsistency warning. -fn on_post_finalization_error(error: ClientError, value_type: &str) -> ConsensusError { - warn!(target: "afg", "Failed to write updated {} to disk. Bailing.", value_type); - warn!(target: "afg", "Node is in a potentially inconsistent state."); - ConsensusError::ClientImport(error.to_string()) -} - -#[cfg(test)] -pub mod tests { - use super::*; - use sp_consensus::{import_queue::CacheKeyId, ForkChoiceStrategy, BlockImport}; - use sp_finality_grandpa::AuthorityId; - use sp_core::{H256, crypto::Public}; - use sc_client_api::{in_mem::Blockchain as InMemoryAuxStore, StorageProof, BlockBackend}; - use substrate_test_runtime_client::runtime::{Block, Header}; - use crate::tests::TestApi; - use crate::finality_proof::{ - FinalityProofFragment, - tests::{TestJustification, ClosureAuthoritySetForFinalityChecker}, - }; - - struct OkVerifier; - - impl Verifier<Block> for OkVerifier { - fn verify( - &mut self, - origin: BlockOrigin, - header: Header, - _justification: Option<Justification>, - _body: Option<Vec<<Block as BlockT>::Extrinsic>>, - ) -> Result<(BlockImportParams<Block, ()>, Option<Vec<(CacheKeyId, Vec<u8>)>>), String> { - Ok((BlockImportParams::new(origin, header), None)) - } - } - - pub struct NoJustificationsImport<BE, Block: BlockT, Client>( - pub GrandpaLightBlockImport<BE, Block, Client> - ); - - impl<BE, Block: BlockT, Client> Clone - for NoJustificationsImport<BE, Block, Client> where - NumberFor<Block>: finality_grandpa::BlockNumberOps, - DigestFor<Block>: Encode, - BE: Backend<Block> + 'static, - { - fn clone(&self) -> Self { - NoJustificationsImport(self.0.clone()) - } - } - - impl<BE, Block: BlockT, Client> BlockImport<Block> - for NoJustificationsImport<BE, Block, Client> where - NumberFor<Block>: finality_grandpa::BlockNumberOps, - DigestFor<Block>: Encode, - BE: Backend<Block> + 'static, - for <'a > &'a Client: - HeaderBackend<Block> - + BlockImport<Block, Error = ConsensusError, Transaction = TransactionFor<BE, Block>> - + Finalizer<Block, BE> - + AuxStore, - GrandpaLightBlockImport<BE, Block, Client>: - BlockImport<Block, Transaction = TransactionFor<BE, Block>, Error = ConsensusError> - { - type Error = ConsensusError; - type Transaction = TransactionFor<BE, Block>; - - fn import_block( - &mut self, - mut block: BlockImportParams<Block, Self::Transaction>, - new_cache: HashMap<well_known_cache_keys::Id, Vec<u8>>, - ) -> Result<ImportResult, Self::Error> { - block.justification.take(); - self.0.import_block(block, new_cache) - } - - fn check_block( - &mut self, - block: BlockCheckParams<Block>, - ) -> Result<ImportResult, Self::Error> { - self.0.check_block(block) - } - } - - impl<BE, Block: BlockT, Client> FinalityProofImport<Block> - for NoJustificationsImport<BE, Block, Client> where - NumberFor<Block>: finality_grandpa::BlockNumberOps, - BE: Backend<Block> + 'static, - DigestFor<Block>: Encode, - for <'a > &'a Client: - HeaderBackend<Block> - + BlockImport<Block, Error = ConsensusError, Transaction = TransactionFor<BE, Block>> - + Finalizer<Block, BE> - + AuxStore, - { - type Error = ConsensusError; - - fn on_start(&mut self) -> Vec<(Block::Hash, NumberFor<Block>)> { - self.0.on_start() - } - - fn import_finality_proof( - &mut self, - hash: Block::Hash, - number: NumberFor<Block>, - finality_proof: Vec<u8>, - verifier: &mut dyn Verifier<Block>, - ) -> Result<(Block::Hash, NumberFor<Block>), Self::Error> { - self.0.import_finality_proof(hash, number, finality_proof, verifier) - } - } - - /// Creates light block import that ignores justifications that came outside of finality proofs. - pub fn light_block_import_without_justifications<BE, Block: BlockT, Client>( - client: Arc<Client>, - backend: Arc<BE>, - genesis_authorities_provider: &dyn GenesisAuthoritySetProvider<Block>, - authority_set_provider: Arc<dyn AuthoritySetForFinalityChecker<Block>>, - ) -> Result<NoJustificationsImport<BE, Block, Client>, ClientError> - where - BE: Backend<Block> + 'static, - Client: crate::ClientForGrandpa<Block, BE>, - { - light_block_import(client, backend, genesis_authorities_provider, authority_set_provider) - .map(NoJustificationsImport) - } - - fn import_block( - new_cache: HashMap<well_known_cache_keys::Id, Vec<u8>>, - justification: Option<Justification>, - ) -> ( - ImportResult, - substrate_test_runtime_client::client::Client<substrate_test_runtime_client::LightBackend, substrate_test_runtime_client::LightExecutor, Block, substrate_test_runtime_client::runtime::RuntimeApi>, - Arc<substrate_test_runtime_client::LightBackend>, - ) { - let (client, backend) = substrate_test_runtime_client::new_light(); - let mut import_data = LightImportData { - last_finalized: Default::default(), - authority_set: LightAuthoritySet::genesis(vec![(AuthorityId::from_slice(&[1; 32]), 1)]), - consensus_changes: ConsensusChanges::empty(), - }; - let mut block = BlockImportParams::new( - BlockOrigin::Own, - Header { - number: 1, - parent_hash: client.chain_info().best_hash, - state_root: Default::default(), - digest: Default::default(), - extrinsics_root: Default::default(), - }, - ); - block.justification = justification; - block.fork_choice = Some(ForkChoiceStrategy::LongestChain); - - ( - do_import_block::<_, _, _, TestJustification>( - &client, - &mut import_data, - block, - new_cache, - ).unwrap(), - client, - backend, - ) - } - - #[test] - fn finality_proof_not_required_when_consensus_data_does_not_changes_and_no_justification_provided() { - assert_eq!(import_block(HashMap::new(), None).0, ImportResult::Imported(ImportedAux { - clear_justification_requests: false, - needs_justification: false, - bad_justification: false, - needs_finality_proof: false, - is_new_best: true, - header_only: false, - })); - } - - #[test] - fn finality_proof_not_required_when_consensus_data_does_not_changes_and_correct_justification_provided() { - let justification = TestJustification((0, vec![(AuthorityId::from_slice(&[1; 32]), 1)]), Vec::new()).encode(); - assert_eq!(import_block(HashMap::new(), Some(justification)).0, ImportResult::Imported(ImportedAux { - clear_justification_requests: false, - needs_justification: false, - bad_justification: false, - needs_finality_proof: false, - is_new_best: true, - header_only: false, - })); - } - - #[test] - fn finality_proof_required_when_consensus_data_changes_and_no_justification_provided() { - let mut cache = HashMap::new(); - cache.insert(well_known_cache_keys::AUTHORITIES, vec![AuthorityId::from_slice(&[2; 32])].encode()); - assert_eq!(import_block(cache, None).0, ImportResult::Imported(ImportedAux { - clear_justification_requests: false, - needs_justification: false, - bad_justification: false, - needs_finality_proof: true, - is_new_best: true, - header_only: false, - })); - } - - #[test] - fn finality_proof_required_when_consensus_data_changes_and_incorrect_justification_provided() { - let justification = TestJustification((0, vec![]), Vec::new()).encode(); - let mut cache = HashMap::new(); - cache.insert(well_known_cache_keys::AUTHORITIES, vec![AuthorityId::from_slice(&[2; 32])].encode()); - assert_eq!( - import_block(cache, Some(justification)).0, - ImportResult::Imported(ImportedAux { - clear_justification_requests: false, - needs_justification: false, - bad_justification: false, - needs_finality_proof: true, - is_new_best: false, - header_only: false, - }, - )); - } - - - #[test] - fn aux_data_updated_on_start() { - let aux_store = InMemoryAuxStore::<Block>::new(); - let api = TestApi::new(vec![(AuthorityId::from_slice(&[1; 32]), 1)]); - - // when aux store is empty initially - assert!(aux_store.get_aux(LIGHT_AUTHORITY_SET_KEY).unwrap().is_none()); - assert!(aux_store.get_aux(LIGHT_CONSENSUS_CHANGES_KEY).unwrap().is_none()); - - // it is updated on importer start - load_aux_import_data(Default::default(), &aux_store, &api).unwrap(); - assert!(aux_store.get_aux(LIGHT_AUTHORITY_SET_KEY).unwrap().is_some()); - assert!(aux_store.get_aux(LIGHT_CONSENSUS_CHANGES_KEY).unwrap().is_some()); - } - - #[test] - fn aux_data_loaded_on_restart() { - let aux_store = InMemoryAuxStore::<Block>::new(); - let api = TestApi::new(vec![(AuthorityId::from_slice(&[1; 32]), 1)]); - - // when aux store is non-empty initially - let mut consensus_changes = ConsensusChanges::<H256, u64>::empty(); - consensus_changes.note_change((42, Default::default())); - aux_store.insert_aux( - &[ - ( - LIGHT_AUTHORITY_SET_KEY, - LightAuthoritySet::genesis( - vec![(AuthorityId::from_slice(&[42; 32]), 2)] - ).encode().as_slice(), - ), - ( - LIGHT_CONSENSUS_CHANGES_KEY, - consensus_changes.encode().as_slice(), - ), - ], - &[], - ).unwrap(); - - // importer uses it on start - let data = load_aux_import_data(Default::default(), &aux_store, &api).unwrap(); - assert_eq!(data.authority_set.authorities(), vec![(AuthorityId::from_slice(&[42; 32]), 2)]); - assert_eq!(data.consensus_changes.pending_changes(), &[(42, Default::default())]); - } - - #[test] - fn authority_set_is_updated_on_finality_proof_import() { - let initial_set_id = 0; - let initial_set = vec![(AuthorityId::from_slice(&[1; 32]), 1)]; - let updated_set = vec![(AuthorityId::from_slice(&[2; 32]), 2)]; - let babe_set_signal = vec![AuthorityId::from_slice(&[42; 32])].encode(); - - // import block #1 without justification - let mut cache = HashMap::new(); - cache.insert(well_known_cache_keys::AUTHORITIES, babe_set_signal); - let (_, client, backend) = import_block(cache, None); - - // import finality proof for block #1 - let hash = client.block_hash(1).unwrap().unwrap(); - let mut verifier = OkVerifier; - let mut import_data = LightImportData { - last_finalized: Default::default(), - authority_set: LightAuthoritySet::genesis(initial_set.clone()), - consensus_changes: ConsensusChanges::empty(), - }; - - // import finality proof - do_import_finality_proof::<_, _, _, TestJustification>( - &client, - backend, - &ClosureAuthoritySetForFinalityChecker( - |_, _, _| Ok(updated_set.clone()) - ), - &mut import_data, - Default::default(), - Default::default(), - vec![ - FinalityProofFragment::<Header> { - block: hash, - justification: TestJustification( - (initial_set_id, initial_set.clone()), - Vec::new(), - ).encode(), - unknown_headers: Vec::new(), - authorities_proof: Some(StorageProof::new(vec![])), - }, - ].encode(), - &mut verifier, - ).unwrap(); - - // verify that new authorities set has been saved to the aux storage - let data = load_aux_import_data(Default::default(), &client, &TestApi::new(initial_set)).unwrap(); - assert_eq!(data.authority_set.authorities(), updated_set); - } -} diff --git a/substrate/client/finality-grandpa/src/observer.rs b/substrate/client/finality-grandpa/src/observer.rs index fd00b35c40a739fab6704d4e09fdac62b9eb276e..c61998225e32300f542f256f70acdc0aa20102a7 100644 --- a/substrate/client/finality-grandpa/src/observer.rs +++ b/substrate/client/finality-grandpa/src/observer.rs @@ -39,7 +39,6 @@ use crate::{ }; use crate::authorities::SharedAuthoritySet; use crate::communication::{Network as NetworkT, NetworkBridge}; -use crate::consensus_changes::SharedConsensusChanges; use crate::notification::GrandpaJustificationSender; use sp_finality_grandpa::AuthorityId; use std::marker::{PhantomData, Unpin}; @@ -68,7 +67,6 @@ impl<'a, Block, Client> finality_grandpa::Chain<Block::Hash, NumberFor<Block>> fn grandpa_observer<BE, Block: BlockT, Client, S, F>( client: &Arc<Client>, authority_set: &SharedAuthoritySet<Block::Hash, NumberFor<Block>>, - consensus_changes: &SharedConsensusChanges<Block::Hash, NumberFor<Block>>, voters: &Arc<VoterSet<AuthorityId>>, justification_sender: &Option<GrandpaJustificationSender<Block>>, last_finalized_number: NumberFor<Block>, @@ -83,7 +81,6 @@ where Client: crate::ClientForGrandpa<Block, BE>, { let authority_set = authority_set.clone(); - let consensus_changes = consensus_changes.clone(); let client = client.clone(); let voters = voters.clone(); let justification_sender = justification_sender.clone(); @@ -123,7 +120,6 @@ where match environment::finalize_block( client.clone(), &authority_set, - &consensus_changes, None, finalized_hash, finalized_number, @@ -293,7 +289,6 @@ where let observer = grandpa_observer( &self.client, &self.persistent_data.authority_set, - &self.persistent_data.consensus_changes, &voters, &self.justification_sender, last_finalized_number, diff --git a/substrate/client/finality-grandpa/src/tests.rs b/substrate/client/finality-grandpa/src/tests.rs index 44503d3c85d4404aeb432beaca16cd8334bd945a..ef8168e84f66cb535f7b2f1ec7543c185cccb638 100644 --- a/substrate/client/finality-grandpa/src/tests.rs +++ b/substrate/client/finality-grandpa/src/tests.rs @@ -25,7 +25,7 @@ use sc_network_test::{ Block, BlockImportAdapter, Hash, PassThroughVerifier, Peer, PeersClient, PeersFullClient, TestClient, TestNetFactory, FullPeerConfig, }; -use sc_network::config::{ProtocolConfig, BoxFinalityProofRequestBuilder}; +use sc_network::config::ProtocolConfig; use parking_lot::{RwLock, Mutex}; use futures_timer::Delay; use tokio::runtime::{Runtime, Handle}; @@ -36,22 +36,21 @@ use sp_api::{ApiRef, StorageProof, ProvideRuntimeApi}; use substrate_test_runtime_client::runtime::BlockNumber; use sp_consensus::{ BlockOrigin, ForkChoiceStrategy, ImportedAux, BlockImportParams, ImportResult, BlockImport, - import_queue::{BoxJustificationImport, BoxFinalityProofImport}, + import_queue::BoxJustificationImport, }; use std::{collections::{HashMap, HashSet}, pin::Pin}; use parity_scale_codec::Decode; use sp_runtime::traits::{Block as BlockT, Header as HeaderT, HashFor}; use sp_runtime::generic::{BlockId, DigestItem}; -use sp_core::{H256, crypto::Public}; +use sp_core::H256; use sp_keystore::{SyncCryptoStorePtr, SyncCryptoStore}; use sp_finality_grandpa::{GRANDPA_ENGINE_ID, AuthorityList, EquivocationProof, GrandpaApi, OpaqueKeyOwnershipProof}; use sp_state_machine::{InMemoryBackend, prove_read, read_proof_check}; use authorities::AuthoritySet; use finality_proof::{ - FinalityProofProvider, AuthoritySetForFinalityProver, AuthoritySetForFinalityChecker, + AuthoritySetForFinalityProver, AuthoritySetForFinalityChecker, }; -use consensus_changes::ConsensusChanges; use sc_block_builder::BlockBuilderProvider; use sc_consensus::LongestChain; use sc_keystore::LocalKeystore; @@ -117,8 +116,6 @@ impl TestNetFactory for GrandpaTestNet { -> ( BlockImportAdapter<Transaction>, Option<BoxJustificationImport<Block>>, - Option<BoxFinalityProofImport<Block>>, - Option<BoxFinalityProofRequestBuilder<Block>>, PeerData, ) { @@ -133,45 +130,12 @@ impl TestNetFactory for GrandpaTestNet { ( BlockImportAdapter::new_full(import), Some(justification_import), - None, - None, Mutex::new(Some(link)), ) }, - PeersClient::Light(ref client, ref backend) => { - use crate::light_import::tests::light_block_import_without_justifications; - - let authorities_provider = Arc::new(self.test_config.clone()); - // forbid direct finalization using justification that came with the block - // => light clients will try to fetch finality proofs - let import = light_block_import_without_justifications( - client.clone(), - backend.clone(), - &self.test_config, - authorities_provider, - ).expect("Could not create block import for fresh peer."); - let finality_proof_req_builder = import.0.create_finality_proof_request_builder(); - let proof_import = Box::new(import.clone()); - ( - BlockImportAdapter::new_light(import), - None, - Some(proof_import), - Some(finality_proof_req_builder), - Mutex::new(None), - ) - }, - } - } - - fn make_finality_proof_provider( - &self, - client: PeersClient - ) -> Option<Arc<dyn sc_network::config::FinalityProofProvider<Block>>> { - match client { - PeersClient::Full(_, ref backend) => { - Some(Arc::new(FinalityProofProvider::new(backend.clone(), self.test_config.clone()))) + PeersClient::Light(..) => { + panic!("Light client is not used in tests."); }, - PeersClient::Light(_, _) => None, } } @@ -679,24 +643,6 @@ fn transition_3_voters_twice_1_full_observer() { block_until_complete(wait_for, &net, &mut runtime); } -#[test] -fn justification_is_emitted_when_consensus_data_changes() { - let mut runtime = Runtime::new().unwrap(); - let peers = &[Ed25519Keyring::Alice, Ed25519Keyring::Bob, Ed25519Keyring::Charlie]; - let mut net = GrandpaTestNet::new(TestApi::new(make_ids(peers)), 3); - - // import block#1 WITH consensus data change - let new_authorities = vec![sp_consensus_babe::AuthorityId::from_slice(&[42; 32])]; - net.peer(0).push_authorities_change_block(new_authorities); - net.block_until_sync(); - let net = Arc::new(Mutex::new(net)); - run_to_completion(&mut runtime, 1, net.clone(), peers); - - // ... and check that there's justification for block#1 - assert!(net.lock().peer(0).client().justification(&BlockId::Number(1)).unwrap().is_some(), - "Missing justification for block#1"); -} - #[test] fn justification_is_generated_periodically() { let mut runtime = Runtime::new().unwrap(); @@ -717,25 +663,6 @@ fn justification_is_generated_periodically() { } } -#[test] -fn consensus_changes_works() { - let mut changes = ConsensusChanges::<H256, u64>::empty(); - - // pending changes are not finalized - changes.note_change((10, H256::from_low_u64_be(1))); - assert_eq!(changes.finalize((5, H256::from_low_u64_be(5)), |_| Ok(None)).unwrap(), (false, false)); - - // no change is selected from competing pending changes - changes.note_change((1, H256::from_low_u64_be(1))); - changes.note_change((1, H256::from_low_u64_be(101))); - assert_eq!(changes.finalize((10, H256::from_low_u64_be(10)), |_| Ok(Some(H256::from_low_u64_be(1001)))).unwrap(), (true, false)); - - // change is selected from competing pending changes - changes.note_change((1, H256::from_low_u64_be(1))); - changes.note_change((1, H256::from_low_u64_be(101))); - assert_eq!(changes.finalize((10, H256::from_low_u64_be(10)), |_| Ok(Some(H256::from_low_u64_be(1)))).unwrap(), (true, true)); -} - #[test] fn sync_justifications_on_change_blocks() { let mut runtime = Runtime::new().unwrap(); @@ -944,7 +871,6 @@ fn allows_reimporting_change_blocks() { needs_justification: true, clear_justification_requests: false, bad_justification: false, - needs_finality_proof: false, is_new_best: true, header_only: false, }), @@ -1069,7 +995,7 @@ fn voter_persists_its_votes() { Poll::Pending => return Poll::Pending, Poll::Ready(None) => return Poll::Ready(()), Poll::Ready(Some(())) => { - let (_block_import, _, _, _, link) = + let (_block_import, _, link) = this.net.lock() .make_block_import::< TransactionFor<substrate_test_runtime_client::Backend, Block> @@ -1144,7 +1070,7 @@ fn voter_persists_its_votes() { }; let set_state = { - let (_, _, _, _, link) = net.lock() + let (_, _, link) = net.lock() .make_block_import::< TransactionFor<substrate_test_runtime_client::Backend, Block> >(client); @@ -1311,100 +1237,6 @@ fn finalize_3_voters_1_light_observer() { }); } -#[test] -fn finality_proof_is_fetched_by_light_client_when_consensus_data_changes() { - sp_tracing::try_init_simple(); - let mut runtime = Runtime::new().unwrap(); - - let peers = &[Ed25519Keyring::Alice]; - let mut net = GrandpaTestNet::new(TestApi::new(make_ids(peers)), 1); - net.add_light_peer(); - - // import block#1 WITH consensus data change. Light client ignores justification - // && instead fetches finality proof for block #1 - net.peer(0).push_authorities_change_block(vec![sp_consensus_babe::AuthorityId::from_slice(&[42; 32])]); - let net = Arc::new(Mutex::new(net)); - run_to_completion(&mut runtime, 1, net.clone(), peers); - net.lock().block_until_sync(); - - // check that the block#1 is finalized on light client - runtime.block_on(futures::future::poll_fn(move |cx| { - if net.lock().peer(1).client().info().finalized_number == 1 { - Poll::Ready(()) - } else { - net.lock().poll(cx); - Poll::Pending - } - })); -} - -#[test] -fn empty_finality_proof_is_returned_to_light_client_when_authority_set_is_different() { - // for debug: to ensure that without forced change light client will sync finality proof - const FORCE_CHANGE: bool = true; - - sp_tracing::try_init_simple(); - let mut runtime = Runtime::new().unwrap(); - - // two of these guys are offline. - let genesis_authorities = if FORCE_CHANGE { - vec![ - Ed25519Keyring::Alice, - Ed25519Keyring::Bob, - Ed25519Keyring::Charlie, - Ed25519Keyring::One, - Ed25519Keyring::Two, - ] - } else { - vec![ - Ed25519Keyring::Alice, - Ed25519Keyring::Bob, - Ed25519Keyring::Charlie, - ] - }; - let peers_a = &[Ed25519Keyring::Alice, Ed25519Keyring::Bob, Ed25519Keyring::Charlie]; - let api = TestApi::new(make_ids(&genesis_authorities)); - - let voters = make_ids(peers_a); - let net = GrandpaTestNet::new(api, 3); - let net = Arc::new(Mutex::new(net)); - - // best is #1 - net.lock().peer(0).generate_blocks(1, BlockOrigin::File, |builder| { - // add a forced transition at block 5. - let mut block = builder.build().unwrap().block; - if FORCE_CHANGE { - add_forced_change(&mut block, 0, ScheduledChange { - next_authorities: voters.clone(), - delay: 3, - }); - } - block - }); - - // ensure block#10 enacts authorities set change => justification is generated - // normally it will reach light client, but because of the forced change, it will not - net.lock().peer(0).push_blocks(8, false); // best is #9 - net.lock().peer(0).push_authorities_change_block( - vec![sp_consensus_babe::AuthorityId::from_slice(&[42; 32])] - ); // #10 - net.lock().peer(0).push_blocks(1, false); // best is #11 - net.lock().block_until_sync(); - - // finalize block #11 on full clients - run_to_completion(&mut runtime, 11, net.clone(), peers_a); - - // request finalization by light client - net.lock().add_light_peer(); - net.lock().block_until_sync(); - - // check block, finalized on light client - assert_eq!( - net.lock().peer(3).client().info().finalized_number, - if FORCE_CHANGE { 0 } else { 10 }, - ); -} - #[test] fn voter_catches_up_to_latest_round_when_behind() { sp_tracing::try_init_simple(); @@ -1540,7 +1372,6 @@ where { let PersistentData { ref authority_set, - ref consensus_changes, ref set_state, .. } = link.persistent_data; @@ -1564,7 +1395,6 @@ where Environment { authority_set: authority_set.clone(), config: config.clone(), - consensus_changes: consensus_changes.clone(), client: link.client.clone(), select_chain: link.select_chain.clone(), set_id: authority_set.set_id(), diff --git a/substrate/client/network/build.rs b/substrate/client/network/build.rs index 8ed460f163eb4f9c0fbab6949bb0028b5e9dd808..2ccc72d99df9658b16d1c4c2be535e4d12993638 100644 --- a/substrate/client/network/build.rs +++ b/substrate/client/network/build.rs @@ -1,6 +1,5 @@ const PROTOS: &[&str] = &[ "src/schema/api.v1.proto", - "src/schema/finality.v1.proto", "src/schema/light.v1.proto" ]; diff --git a/substrate/client/network/src/behaviour.rs b/substrate/client/network/src/behaviour.rs index 41723d9068c2e8411ae486db4d605af3ee364916..b2914a5e0a72b5a7d81fb83b19e9a97c7e4a9ff2 100644 --- a/substrate/client/network/src/behaviour.rs +++ b/substrate/client/network/src/behaviour.rs @@ -15,9 +15,9 @@ // along with Substrate. If not, see <http://www.gnu.org/licenses/>. use crate::{ - config::{ProtocolId, Role}, block_requests, light_client_handler, finality_requests, + config::{ProtocolId, Role}, block_requests, light_client_handler, peer_info, request_responses, discovery::{DiscoveryBehaviour, DiscoveryConfig, DiscoveryOut}, - protocol::{message::{self, Roles}, CustomMessageOutcome, NotificationsSink, Protocol}, + protocol::{message::Roles, CustomMessageOutcome, NotificationsSink, Protocol}, ObservedRole, DhtEvent, ExHashT, }; @@ -58,8 +58,6 @@ pub struct Behaviour<B: BlockT, H: ExHashT> { request_responses: request_responses::RequestResponsesBehaviour, /// Block request handling. block_requests: block_requests::BlockRequests<B>, - /// Finality proof request handling. - finality_proof_requests: finality_requests::FinalityProofRequests<B>, /// Light client request handling. light_client_handler: light_client_handler::LightClientHandler<B>, @@ -76,7 +74,6 @@ pub struct Behaviour<B: BlockT, H: ExHashT> { pub enum BehaviourOut<B: BlockT> { BlockImport(BlockOrigin, Vec<IncomingBlock<B>>), JustificationImport(Origin, B::Hash, NumberFor<B>, Justification), - FinalityProofImport(Origin, B::Hash, NumberFor<B>, Vec<u8>), /// Started a random iterative Kademlia discovery query. RandomKademliaStarted(ProtocolId), @@ -182,7 +179,6 @@ impl<B: BlockT, H: ExHashT> Behaviour<B, H> { user_agent: String, local_public_key: PublicKey, block_requests: block_requests::BlockRequests<B>, - finality_proof_requests: finality_requests::FinalityProofRequests<B>, light_client_handler: light_client_handler::LightClientHandler<B>, disco_config: DiscoveryConfig, request_response_protocols: Vec<request_responses::ProtocolConfig>, @@ -194,7 +190,6 @@ impl<B: BlockT, H: ExHashT> Behaviour<B, H> { request_responses: request_responses::RequestResponsesBehaviour::new(request_response_protocols.into_iter())?, block_requests, - finality_proof_requests, light_client_handler, events: VecDeque::new(), role, @@ -334,8 +329,6 @@ Behaviour<B, H> { self.events.push_back(BehaviourOut::BlockImport(origin, blocks)), CustomMessageOutcome::JustificationImport(origin, hash, nb, justification) => self.events.push_back(BehaviourOut::JustificationImport(origin, hash, nb, justification)), - CustomMessageOutcome::FinalityProofImport(origin, hash, nb, proof) => - self.events.push_back(BehaviourOut::FinalityProofImport(origin, hash, nb, proof)), CustomMessageOutcome::BlockRequest { target, request } => { match self.block_requests.send_request(&target, request) { block_requests::SendRequestOutcome::Ok => { @@ -359,9 +352,6 @@ Behaviour<B, H> { block_requests::SendRequestOutcome::EncodeError(_) => {}, } }, - CustomMessageOutcome::FinalityProofRequest { target, block_hash, request } => { - self.finality_proof_requests.send_request(&target, block_hash, request); - }, CustomMessageOutcome::NotificationStreamOpened { remote, protocols, roles, notifications_sink } => { let role = reported_roles_to_observed_role(&self.role, &remote, roles); for protocol in protocols { @@ -454,26 +444,6 @@ impl<B: BlockT, H: ExHashT> NetworkBehaviourEventProcess<block_requests::Event<B } } -impl<B: BlockT, H: ExHashT> NetworkBehaviourEventProcess<finality_requests::Event<B>> for Behaviour<B, H> { - fn inject_event(&mut self, event: finality_requests::Event<B>) { - match event { - finality_requests::Event::Response { peer, block_hash, proof } => { - let response = message::FinalityProofResponse { - id: 0, - block: block_hash, - proof: if !proof.is_empty() { - Some(proof) - } else { - None - }, - }; - let ev = self.substrate.on_finality_proof_response(peer, response); - self.inject_event(ev); - } - } - } -} - impl<B: BlockT, H: ExHashT> NetworkBehaviourEventProcess<peer_info::PeerInfoEvent> for Behaviour<B, H> { fn inject_event(&mut self, event: peer_info::PeerInfoEvent) { diff --git a/substrate/client/network/src/chain.rs b/substrate/client/network/src/chain.rs index 20fbe0284397d513a909cbcf72733a265c060214..61d19c10dae513f109f4b29ee457a21d246b02cd 100644 --- a/substrate/client/network/src/chain.rs +++ b/substrate/client/network/src/chain.rs @@ -32,15 +32,3 @@ impl<Block: BlockT, T> Client<Block> for T T: HeaderBackend<Block> + ProofProvider<Block> + BlockIdTo<Block, Error = Error> + BlockBackend<Block> + HeaderMetadata<Block, Error = Error> + Send + Sync {} - -/// Finality proof provider. -pub trait FinalityProofProvider<Block: BlockT>: Send + Sync { - /// Prove finality of the block. - fn prove_finality(&self, for_block: Block::Hash, request: &[u8]) -> Result<Option<Vec<u8>>, Error>; -} - -impl<Block: BlockT> FinalityProofProvider<Block> for () { - fn prove_finality(&self, _for_block: Block::Hash, _request: &[u8]) -> Result<Option<Vec<u8>>, Error> { - Ok(None) - } -} diff --git a/substrate/client/network/src/config.rs b/substrate/client/network/src/config.rs index db33623a2e3302ec847fe5d9c66cae495ff82864..b7b113dc146926659fe1a3c2383aa92afd7a8eeb 100644 --- a/substrate/client/network/src/config.rs +++ b/substrate/client/network/src/config.rs @@ -21,7 +21,7 @@ //! The [`Params`] struct is the struct that must be passed in order to initialize the networking. //! See the documentation of [`Params`]. -pub use crate::chain::{Client, FinalityProofProvider}; +pub use crate::chain::Client; pub use crate::on_demand_layer::{AlwaysBadChecker, OnDemand}; pub use crate::request_responses::{IncomingRequest, ProtocolConfig as RequestResponseConfig}; pub use libp2p::{identity, core::PublicKey, wasm_ext::ExtTransport, build_multiaddr}; @@ -70,17 +70,6 @@ pub struct Params<B: BlockT, H: ExHashT> { /// Client that contains the blockchain. pub chain: Arc<dyn Client<B>>, - /// Finality proof provider. - /// - /// This object, if `Some`, is used when a node on the network requests a proof of finality - /// from us. - pub finality_proof_provider: Option<Arc<dyn FinalityProofProvider<B>>>, - - /// How to build requests for proofs of finality. - /// - /// This object, if `Some`, is used when we need a proof of finality from another node. - pub finality_proof_request_builder: Option<BoxFinalityProofRequestBuilder<B>>, - /// The `OnDemand` object acts as a "receiver" for block data requests from the client. /// If `Some`, the network worker will process these requests and answer them. /// Normally used only for light clients. @@ -153,25 +142,6 @@ impl fmt::Display for Role { } } -/// Finality proof request builder. -pub trait FinalityProofRequestBuilder<B: BlockT>: Send { - /// Build data blob, associated with the request. - fn build_request_data(&mut self, hash: &B::Hash) -> Vec<u8>; -} - -/// Implementation of `FinalityProofRequestBuilder` that builds a dummy empty request. -#[derive(Debug, Default)] -pub struct DummyFinalityProofRequestBuilder; - -impl<B: BlockT> FinalityProofRequestBuilder<B> for DummyFinalityProofRequestBuilder { - fn build_request_data(&mut self, _: &B::Hash) -> Vec<u8> { - Vec::new() - } -} - -/// Shared finality proof request builder struct used by the queue. -pub type BoxFinalityProofRequestBuilder<B> = Box<dyn FinalityProofRequestBuilder<B> + Send + Sync>; - /// Result of the transaction import. #[derive(Clone, Copy, Debug)] pub enum TransactionImport { diff --git a/substrate/client/network/src/finality_requests.rs b/substrate/client/network/src/finality_requests.rs deleted file mode 100644 index 55f56b9a0cc25c582f122e00d5fa3c933b508f14..0000000000000000000000000000000000000000 --- a/substrate/client/network/src/finality_requests.rs +++ /dev/null @@ -1,403 +0,0 @@ -// Copyright 2020 Parity Technologies (UK) Ltd. -// This file is part of Substrate. -// -// Substrate is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// Substrate is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with Substrate. If not, see <http://www.gnu.org/licenses/>. - -//! `NetworkBehaviour` implementation which handles incoming finality proof requests. -//! -//! Every request is coming in on a separate connection substream which gets -//! closed after we have sent the response back. Incoming requests are encoded -//! as protocol buffers (cf. `finality.v1.proto`). - -#![allow(unused)] - -use bytes::Bytes; -use codec::{Encode, Decode}; -use crate::{ - chain::FinalityProofProvider, - config::ProtocolId, - protocol::message, - schema, -}; -use futures::{future::BoxFuture, prelude::*, stream::FuturesUnordered}; -use libp2p::{ - core::{ - ConnectedPoint, - Multiaddr, - PeerId, - connection::ConnectionId, - upgrade::{InboundUpgrade, OutboundUpgrade, ReadOneError, UpgradeInfo, Negotiated}, - upgrade::{DeniedUpgrade, read_one, write_one} - }, - swarm::{ - NegotiatedSubstream, - NetworkBehaviour, - NetworkBehaviourAction, - NotifyHandler, - OneShotHandler, - OneShotHandlerConfig, - PollParameters, - SubstreamProtocol - } -}; -use prost::Message; -use sp_runtime::{generic::BlockId, traits::{Block, Header, One, Zero}}; -use std::{ - cmp::min, - collections::VecDeque, - io, - iter, - marker::PhantomData, - sync::Arc, - time::Duration, - task::{Context, Poll} -}; -use void::{Void, unreachable}; - -// Type alias for convenience. -pub type Error = Box<dyn std::error::Error + 'static>; - -/// Event generated by the finality proof requests behaviour. -#[derive(Debug)] -pub enum Event<B: Block> { - /// A response to a finality proof request has arrived. - Response { - peer: PeerId, - /// Block hash originally passed to `send_request`. - block_hash: B::Hash, - /// Finality proof returned by the remote. - proof: Vec<u8>, - }, -} - -/// Configuration options for `FinalityProofRequests`. -#[derive(Debug, Clone)] -pub struct Config { - max_request_len: usize, - max_response_len: usize, - inactivity_timeout: Duration, - protocol: Bytes, -} - -impl Config { - /// Create a fresh configuration with the following options: - /// - /// - max. request size = 1 MiB - /// - max. response size = 1 MiB - /// - inactivity timeout = 15s - pub fn new(id: &ProtocolId) -> Self { - let mut c = Config { - max_request_len: 1024 * 1024, - max_response_len: 1024 * 1024, - inactivity_timeout: Duration::from_secs(15), - protocol: Bytes::new(), - }; - c.set_protocol(id); - c - } - - /// Limit the max. length of incoming finality proof request bytes. - pub fn set_max_request_len(&mut self, v: usize) -> &mut Self { - self.max_request_len = v; - self - } - - /// Limit the max. length of incoming finality proof response bytes. - pub fn set_max_response_len(&mut self, v: usize) -> &mut Self { - self.max_response_len = v; - self - } - - /// Limit the max. duration the substream may remain inactive before closing it. - pub fn set_inactivity_timeout(&mut self, v: Duration) -> &mut Self { - self.inactivity_timeout = v; - self - } - - /// Set protocol to use for upgrade negotiation. - pub fn set_protocol(&mut self, id: &ProtocolId) -> &mut Self { - let mut v = Vec::new(); - v.extend_from_slice(b"/"); - v.extend_from_slice(id.as_ref().as_bytes()); - v.extend_from_slice(b"/finality-proof/1"); - self.protocol = v.into(); - self - } -} - -/// The finality proof request handling behaviour. -pub struct FinalityProofRequests<B: Block> { - /// This behaviour's configuration. - config: Config, - /// How to construct finality proofs. - finality_proof_provider: Option<Arc<dyn FinalityProofProvider<B>>>, - /// Futures sending back the finality proof request responses. - outgoing: FuturesUnordered<BoxFuture<'static, ()>>, - /// Events to return as soon as possible from `poll`. - pending_events: VecDeque<NetworkBehaviourAction<OutboundProtocol<B>, Event<B>>>, -} - -impl<B> FinalityProofRequests<B> -where - B: Block, -{ - /// Initializes the behaviour. - /// - /// If the proof provider is `None`, then the behaviour will not support the finality proof - /// requests protocol. - pub fn new(cfg: Config, finality_proof_provider: Option<Arc<dyn FinalityProofProvider<B>>>) -> Self { - FinalityProofRequests { - config: cfg, - finality_proof_provider, - outgoing: FuturesUnordered::new(), - pending_events: VecDeque::new(), - } - } - - /// Issue a new finality proof request. - /// - /// If the response doesn't arrive in time, or if the remote answers improperly, the target - /// will be disconnected. - pub fn send_request(&mut self, target: &PeerId, block_hash: B::Hash, request: Vec<u8>) { - let protobuf_rq = schema::v1::finality::FinalityProofRequest { - block_hash: block_hash.encode(), - request, - }; - - let mut buf = Vec::with_capacity(protobuf_rq.encoded_len()); - if let Err(err) = protobuf_rq.encode(&mut buf) { - log::warn!("failed to encode finality proof request {:?}: {:?}", protobuf_rq, err); - return; - } - - log::trace!("enqueueing finality proof request to {:?}: {:?}", target, protobuf_rq); - self.pending_events.push_back(NetworkBehaviourAction::NotifyHandler { - peer_id: target.clone(), - handler: NotifyHandler::Any, - event: OutboundProtocol { - request: buf, - block_hash, - max_response_size: self.config.max_response_len, - protocol: self.config.protocol.clone(), - }, - }); - } - - /// Callback, invoked when a new finality request has been received from remote. - fn on_finality_request(&mut self, peer: &PeerId, request: &schema::v1::finality::FinalityProofRequest) - -> Result<schema::v1::finality::FinalityProofResponse, Error> - { - let block_hash = Decode::decode(&mut request.block_hash.as_ref())?; - - log::trace!(target: "sync", "Finality proof request from {} for {}", peer, block_hash); - - // Note that an empty Vec is sent if no proof is available. - let finality_proof = if let Some(provider) = &self.finality_proof_provider { - provider - .prove_finality(block_hash, &request.request)? - .unwrap_or_default() - } else { - log::error!("Answering a finality proof request while finality provider is empty"); - return Err(From::from("Empty finality proof provider".to_string())) - }; - - Ok(schema::v1::finality::FinalityProofResponse { proof: finality_proof }) - } -} - -impl<B> NetworkBehaviour for FinalityProofRequests<B> -where - B: Block -{ - type ProtocolsHandler = OneShotHandler<InboundProtocol<B>, OutboundProtocol<B>, NodeEvent<B, NegotiatedSubstream>>; - type OutEvent = Event<B>; - - fn new_handler(&mut self) -> Self::ProtocolsHandler { - let p = InboundProtocol { - max_request_len: self.config.max_request_len, - protocol: if self.finality_proof_provider.is_some() { - Some(self.config.protocol.clone()) - } else { - None - }, - marker: PhantomData, - }; - let mut cfg = OneShotHandlerConfig::default(); - cfg.keep_alive_timeout = self.config.inactivity_timeout; - OneShotHandler::new(SubstreamProtocol::new(p, ()), cfg) - } - - fn addresses_of_peer(&mut self, _: &PeerId) -> Vec<Multiaddr> { - Vec::new() - } - - fn inject_connected(&mut self, _peer: &PeerId) { - } - - fn inject_disconnected(&mut self, _peer: &PeerId) { - } - - fn inject_event( - &mut self, - peer: PeerId, - connection: ConnectionId, - event: NodeEvent<B, NegotiatedSubstream> - ) { - match event { - NodeEvent::Request(request, mut stream) => { - match self.on_finality_request(&peer, &request) { - Ok(res) => { - log::trace!("enqueueing finality response for peer {}", peer); - let mut data = Vec::with_capacity(res.encoded_len()); - if let Err(e) = res.encode(&mut data) { - log::debug!("error encoding finality response for peer {}: {}", peer, e) - } else { - let future = async move { - if let Err(e) = write_one(&mut stream, data).await { - log::debug!("error writing finality response: {}", e) - } - }; - self.outgoing.push(future.boxed()) - } - } - Err(e) => log::debug!("error handling finality request from peer {}: {}", peer, e) - } - } - NodeEvent::Response(response, block_hash) => { - let ev = Event::Response { - peer, - block_hash, - proof: response.proof, - }; - self.pending_events.push_back(NetworkBehaviourAction::GenerateEvent(ev)); - } - } - } - - fn poll(&mut self, cx: &mut Context, _: &mut impl PollParameters) - -> Poll<NetworkBehaviourAction<OutboundProtocol<B>, Event<B>>> - { - if let Some(ev) = self.pending_events.pop_front() { - return Poll::Ready(ev); - } - - while let Poll::Ready(Some(_)) = self.outgoing.poll_next_unpin(cx) {} - Poll::Pending - } -} - -/// Output type of inbound and outbound substream upgrades. -#[derive(Debug)] -pub enum NodeEvent<B: Block, T> { - /// Incoming request from remote and substream to use for the response. - Request(schema::v1::finality::FinalityProofRequest, T), - /// Incoming response from remote. - Response(schema::v1::finality::FinalityProofResponse, B::Hash), -} - -/// Substream upgrade protocol. -/// -/// We attempt to parse an incoming protobuf encoded request (cf. `Request`) -/// which will be handled by the `FinalityProofRequests` behaviour, i.e. the request -/// will become visible via `inject_node_event` which then dispatches to the -/// relevant callback to process the message and prepare a response. -#[derive(Debug, Clone)] -pub struct InboundProtocol<B> { - /// The max. request length in bytes. - max_request_len: usize, - /// The protocol to use during upgrade negotiation. If `None`, then the incoming protocol - /// is simply disabled. - protocol: Option<Bytes>, - /// Marker to pin the block type. - marker: PhantomData<B>, -} - -impl<B: Block> UpgradeInfo for InboundProtocol<B> { - type Info = Bytes; - // This iterator will return either 0 elements if `self.protocol` is `None`, or 1 element if - // it is `Some`. - type InfoIter = std::option::IntoIter<Self::Info>; - - fn protocol_info(&self) -> Self::InfoIter { - self.protocol.clone().into_iter() - } -} - -impl<B, T> InboundUpgrade<T> for InboundProtocol<B> -where - B: Block, - T: AsyncRead + AsyncWrite + Unpin + Send + 'static -{ - type Output = NodeEvent<B, T>; - type Error = ReadOneError; - type Future = BoxFuture<'static, Result<Self::Output, Self::Error>>; - - fn upgrade_inbound(self, mut s: T, _: Self::Info) -> Self::Future { - async move { - let len = self.max_request_len; - let vec = read_one(&mut s, len).await?; - match schema::v1::finality::FinalityProofRequest::decode(&vec[..]) { - Ok(r) => Ok(NodeEvent::Request(r, s)), - Err(e) => Err(ReadOneError::Io(io::Error::new(io::ErrorKind::Other, e))) - } - }.boxed() - } -} - -/// Substream upgrade protocol. -/// -/// Sends a request to remote and awaits the response. -#[derive(Debug, Clone)] -pub struct OutboundProtocol<B: Block> { - /// The serialized protobuf request. - request: Vec<u8>, - /// Block hash that has been requested. - block_hash: B::Hash, - /// The max. response length in bytes. - max_response_size: usize, - /// The protocol to use for upgrade negotiation. - protocol: Bytes, -} - -impl<B: Block> UpgradeInfo for OutboundProtocol<B> { - type Info = Bytes; - type InfoIter = iter::Once<Self::Info>; - - fn protocol_info(&self) -> Self::InfoIter { - iter::once(self.protocol.clone()) - } -} - -impl<B, T> OutboundUpgrade<T> for OutboundProtocol<B> -where - B: Block, - T: AsyncRead + AsyncWrite + Unpin + Send + 'static -{ - type Output = NodeEvent<B, T>; - type Error = ReadOneError; - type Future = BoxFuture<'static, Result<Self::Output, Self::Error>>; - - fn upgrade_outbound(self, mut s: T, _: Self::Info) -> Self::Future { - async move { - write_one(&mut s, &self.request).await?; - let vec = read_one(&mut s, self.max_response_size).await?; - - schema::v1::finality::FinalityProofResponse::decode(&vec[..]) - .map(|r| NodeEvent::Response(r, self.block_hash)) - .map_err(|e| { - ReadOneError::Io(io::Error::new(io::ErrorKind::Other, e)) - }) - }.boxed() - } -} diff --git a/substrate/client/network/src/gossip/tests.rs b/substrate/client/network/src/gossip/tests.rs index e94052c0e4d29c30a17e7e87fec30bc30bd7acc9..93b69f7b64c8ec17d72b4dee99ef3dbf941d14c2 100644 --- a/substrate/client/network/src/gossip/tests.rs +++ b/substrate/client/network/src/gossip/tests.rs @@ -86,7 +86,6 @@ fn build_test_full_node(config: config::NetworkConfiguration) PassThroughVerifier(false), Box::new(client.clone()), None, - None, &sp_core::testing::TaskExecutor::new(), None, )); @@ -96,8 +95,6 @@ fn build_test_full_node(config: config::NetworkConfiguration) executor: None, network_config: config, chain: client.clone(), - finality_proof_provider: None, - finality_proof_request_builder: None, on_demand: None, transaction_pool: Arc::new(crate::config::EmptyTransactionPool), protocol_id: config::ProtocolId::from("/test-protocol-name"), diff --git a/substrate/client/network/src/lib.rs b/substrate/client/network/src/lib.rs index 3fd01c33dcf5f0d0dc8dd0846bc9700483ab6fa5..b050db8785ac17e332a3f21614486caf41da7a5a 100644 --- a/substrate/client/network/src/lib.rs +++ b/substrate/client/network/src/lib.rs @@ -249,7 +249,6 @@ mod block_requests; mod chain; mod peer_info; mod discovery; -mod finality_requests; mod light_client_handler; mod on_demand_layer; mod protocol; diff --git a/substrate/client/network/src/protocol.rs b/substrate/client/network/src/protocol.rs index d0b6b2823a2c8c7c11dfea28c4253562ab731389..597031b90182c812a5e8851b3e088e4feeef6d0e 100644 --- a/substrate/client/network/src/protocol.rs +++ b/substrate/client/network/src/protocol.rs @@ -19,7 +19,7 @@ use crate::{ ExHashT, chain::Client, - config::{BoxFinalityProofRequestBuilder, ProtocolId, TransactionPool, TransactionImportFuture, TransactionImport}, + config::{ProtocolId, TransactionPool, TransactionImportFuture, TransactionImport}, error, utils::{interval, LruHashSet}, }; @@ -131,7 +131,6 @@ struct Metrics { peers: Gauge<U64>, queued_blocks: Gauge<U64>, fork_targets: Gauge<U64>, - finality_proofs: GaugeVec<U64>, justifications: GaugeVec<U64>, propagated_transactions: Counter<U64>, } @@ -165,16 +164,6 @@ impl Metrics { )?; register(g, r)? }, - finality_proofs: { - let g = GaugeVec::new( - Opts::new( - "sync_extra_finality_proofs", - "Number of extra finality proof requests", - ), - &["status"], - )?; - register(g, r)? - }, propagated_transactions: register(Counter::new( "sync_propagated_transactions", "Number of transactions propagated to at least one peer", @@ -365,7 +354,6 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> { local_peer_id: PeerId, chain: Arc<dyn Client<B>>, transaction_pool: Arc<dyn TransactionPool<H, B>>, - finality_proof_request_builder: Option<BoxFinalityProofRequestBuilder<B>>, protocol_id: ProtocolId, peerset_config: sc_peerset::PeersetConfig, block_announce_validator: Box<dyn BlockAnnounceValidator<B> + Send>, @@ -377,7 +365,6 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> { config.roles, chain.clone(), &info, - finality_proof_request_builder, block_announce_validator, config.max_parallel_downloads, ); @@ -614,10 +601,7 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> { warn!(target: "sub-libp2p", "Received unexpected RemoteHeaderResponse"), GenericMessage::RemoteChangesResponse(_) => warn!(target: "sub-libp2p", "Received unexpected RemoteChangesResponse"), - GenericMessage::FinalityProofResponse(_) => - warn!(target: "sub-libp2p", "Received unexpected FinalityProofResponse"), GenericMessage::BlockRequest(_) | - GenericMessage::FinalityProofRequest(_) | GenericMessage::RemoteReadChildRequest(_) | GenericMessage::RemoteCallRequest(_) | GenericMessage::RemoteReadRequest(_) | @@ -1314,13 +1298,6 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> { self.sync.on_justification_import(hash, number, success) } - /// Request a finality proof for the given block. - /// - /// Queues a new finality proof request and tries to dispatch all pending requests. - pub fn request_finality_proof(&mut self, hash: &B::Hash, number: NumberFor<B>) { - self.sync.request_finality_proof(&hash, number) - } - /// Notify the protocol that we have learned about the existence of nodes. /// /// Can be called multiple times with the same `PeerId`s. @@ -1328,34 +1305,6 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> { self.behaviour.add_discovered_nodes(peer_ids) } - pub fn finality_proof_import_result( - &mut self, - request_block: (B::Hash, NumberFor<B>), - finalization_result: Result<(B::Hash, NumberFor<B>), ()>, - ) { - self.sync.on_finality_proof_import(request_block, finalization_result) - } - - /// Must be called after a [`CustomMessageOutcome::FinalityProofRequest`] has been emitted, - /// to notify of the response having arrived. - pub fn on_finality_proof_response( - &mut self, - who: PeerId, - response: message::FinalityProofResponse<B::Hash>, - ) -> CustomMessageOutcome<B> { - trace!(target: "sync", "Finality proof response from {} for {}", who, response.block); - match self.sync.on_block_finality_proof(who, response) { - Ok(sync::OnBlockFinalityProof::Nothing) => CustomMessageOutcome::None, - Ok(sync::OnBlockFinalityProof::Import { peer, hash, number, proof }) => - CustomMessageOutcome::FinalityProofImport(peer, hash, number, proof), - Err(sync::BadPeer(id, repu)) => { - self.behaviour.disconnect_peer(&id); - self.peerset_handle.report_peer(id, repu); - CustomMessageOutcome::None - } - } - } - fn format_stats(&self) -> String { let mut out = String::new(); for (id, stats) in &self.context_data.stats { @@ -1399,15 +1348,6 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> { .set(m.justifications.failed_requests.into()); metrics.justifications.with_label_values(&["importing"]) .set(m.justifications.importing_requests.into()); - - metrics.finality_proofs.with_label_values(&["pending"]) - .set(m.finality_proofs.pending_requests.into()); - metrics.finality_proofs.with_label_values(&["active"]) - .set(m.finality_proofs.active_requests.into()); - metrics.finality_proofs.with_label_values(&["failed"]) - .set(m.finality_proofs.failed_requests.into()); - metrics.finality_proofs.with_label_values(&["importing"]) - .set(m.finality_proofs.importing_requests.into()); } } } @@ -1418,7 +1358,6 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> { pub enum CustomMessageOutcome<B: BlockT> { BlockImport(BlockOrigin, Vec<IncomingBlock<B>>), JustificationImport(Origin, B::Hash, NumberFor<B>, Justification), - FinalityProofImport(Origin, B::Hash, NumberFor<B>, Vec<u8>), /// Notification protocols have been opened with a remote. NotificationStreamOpened { remote: PeerId, @@ -1443,12 +1382,6 @@ pub enum CustomMessageOutcome<B: BlockT> { /// must be silently discarded. /// It is the responsibility of the handler to ensure that a timeout exists. BlockRequest { target: PeerId, request: message::BlockRequest<B> }, - /// A new finality proof request must be emitted. - /// Once you have the response, you must call `Protocol::on_finality_proof_response`. - /// It is the responsibility of the handler to ensure that a timeout exists. - /// If the request times out, or the peer responds in an invalid way, the peer has to be - /// disconnect. This will inform the state machine that the request it has emitted is stale. - FinalityProofRequest { target: PeerId, block_hash: B::Hash, request: Vec<u8> }, /// Peer has a reported a new head of chain. PeerNewBest(PeerId, NumberFor<B>), None, @@ -1545,14 +1478,6 @@ impl<B: BlockT, H: ExHashT> NetworkBehaviour for Protocol<B, H> { }; self.pending_messages.push_back(event); } - for (id, r) in self.sync.finality_proof_requests() { - let event = CustomMessageOutcome::FinalityProofRequest { - target: id, - block_hash: r.block, - request: r.request, - }; - self.pending_messages.push_back(event); - } if let Poll::Ready(Some((tx_hash, result))) = self.pending_transactions.poll_next_unpin(cx) { if let Some(peers) = self.pending_transactions_peers.remove(&tx_hash) { peers.into_iter().for_each(|p| self.on_handle_transaction_import(p, result)); diff --git a/substrate/client/network/src/protocol/message.rs b/substrate/client/network/src/protocol/message.rs index dae7b86db8771a9eb3ed697324e34aaeb788a678..4213d56bbf022dbc442778d7bd73d2671f56897c 100644 --- a/substrate/client/network/src/protocol/message.rs +++ b/substrate/client/network/src/protocol/message.rs @@ -25,7 +25,6 @@ pub use self::generic::{ BlockAnnounce, RemoteCallRequest, RemoteReadRequest, RemoteHeaderRequest, RemoteHeaderResponse, RemoteChangesRequest, RemoteChangesResponse, - FinalityProofRequest, FinalityProofResponse, FromBlock, RemoteReadChildRequest, Roles, }; use sc_client_api::StorageProof; @@ -280,11 +279,10 @@ pub mod generic { RemoteChangesResponse(RemoteChangesResponse<Number, Hash>), /// Remote child storage read request. RemoteReadChildRequest(RemoteReadChildRequest<Hash>), - /// Finality proof request. - FinalityProofRequest(FinalityProofRequest<Hash>), - /// Finality proof response. - FinalityProofResponse(FinalityProofResponse<Hash>), /// Batch of consensus protocol messages. + // NOTE: index is incremented by 2 due to finality proof related + // messages that were removed. + #[codec(index = "17")] ConsensusBatch(Vec<ConsensusMessage>), } @@ -307,8 +305,6 @@ pub mod generic { Message::RemoteChangesRequest(_) => "RemoteChangesRequest", Message::RemoteChangesResponse(_) => "RemoteChangesResponse", Message::RemoteReadChildRequest(_) => "RemoteReadChildRequest", - Message::FinalityProofRequest(_) => "FinalityProofRequest", - Message::FinalityProofResponse(_) => "FinalityProofResponse", Message::ConsensusBatch(_) => "ConsensusBatch", } } @@ -546,26 +542,4 @@ pub mod generic { /// Missing changes tries roots proof. pub roots_proof: StorageProof, } - - #[derive(Debug, PartialEq, Eq, Clone, Encode, Decode)] - /// Finality proof request. - pub struct FinalityProofRequest<H> { - /// Unique request id. - pub id: RequestId, - /// Hash of the block to request proof for. - pub block: H, - /// Additional data blob (that both requester and provider understood) required for proving finality. - pub request: Vec<u8>, - } - - #[derive(Debug, PartialEq, Eq, Clone, Encode, Decode)] - /// Finality proof response. - pub struct FinalityProofResponse<H> { - /// Id of a request this response was made for. - pub id: RequestId, - /// Hash of the block (the same as in the FinalityProofRequest). - pub block: H, - /// Finality proof (if available). - pub proof: Option<Vec<u8>>, - } } diff --git a/substrate/client/network/src/protocol/sync.rs b/substrate/client/network/src/protocol/sync.rs index 03714b05ace0d2f3d51da667c0d20faf67cf41b3..ced789446da60dbbfc4c56dcda8ec23f4825b00e 100644 --- a/substrate/client/network/src/protocol/sync.rs +++ b/substrate/client/network/src/protocol/sync.rs @@ -35,9 +35,7 @@ use sp_consensus::{BlockOrigin, BlockStatus, import_queue::{IncomingBlock, BlockImportResult, BlockImportError} }; use crate::{ - config::BoxFinalityProofRequestBuilder, - protocol::message::{self, generic::FinalityProofRequest, BlockAnnounce, BlockAttributes, BlockRequest, BlockResponse, - FinalityProofResponse, Roles}, + protocol::message::{self, BlockAnnounce, BlockAttributes, BlockRequest, BlockResponse, Roles}, }; use either::Either; use extra_requests::ExtraRequests; @@ -116,9 +114,6 @@ mod rep { /// Reputation change for peers which send us a block with bad justifications. pub const BAD_JUSTIFICATION: Rep = Rep::new(-(1 << 16), "Bad justification"); - /// Reputation change for peers which send us a block with bad finality proof. - pub const BAD_FINALITY_PROOF: Rep = Rep::new(-(1 << 16), "Bad finality proof"); - /// Reputation change when a peer sent us invlid ancestry result. pub const UNKNOWN_ANCESTOR:Rep = Rep::new(-(1 << 16), "DB Error"); } @@ -185,8 +180,6 @@ pub struct ChainSync<B: BlockT> { /// What block attributes we require for this node, usually derived from /// what role we are, but could be customized required_block_attributes: message::BlockAttributes, - /// Any extra finality proof requests. - extra_finality_proofs: ExtraRequests<B>, /// Any extra justification requests. extra_justifications: ExtraRequests<B>, /// A set of hashes of blocks that are being downloaded or have been @@ -195,8 +188,6 @@ pub struct ChainSync<B: BlockT> { /// The best block number that was successfully imported into the chain. /// This can not decrease. best_imported_number: NumberFor<B>, - /// Finality proof handler. - request_builder: Option<BoxFinalityProofRequestBuilder<B>>, /// Fork sync targets. fork_targets: HashMap<B::Hash, ForkTarget<B>>, /// A set of peers for which there might be potential block requests @@ -270,8 +261,6 @@ pub enum PeerSyncState<B: BlockT> { DownloadingStale(B::Hash), /// Downloading justification for given block hash. DownloadingJustification(B::Hash), - /// Downloading finality proof for given block hash. - DownloadingFinalityProof(B::Hash) } impl<B: BlockT> PeerSyncState<B> { @@ -402,20 +391,6 @@ pub enum OnBlockJustification<B: BlockT> { } } -/// Result of [`ChainSync::on_block_finality_proof`]. -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum OnBlockFinalityProof<B: BlockT> { - /// The proof needs no further handling. - Nothing, - /// The proof should be imported. - Import { - peer: PeerId, - hash: B::Hash, - number: NumberFor<B>, - proof: Vec<u8> - } -} - /// Result of [`ChainSync::has_slot_for_block_announce_validation`]. enum HasSlotForBlockAnnounceValidation { /// Yes, there is a slot for the block announce validation. @@ -432,7 +407,6 @@ impl<B: BlockT> ChainSync<B> { role: Roles, client: Arc<dyn crate::chain::Client<B>>, info: &BlockchainInfo<B>, - request_builder: Option<BoxFinalityProofRequestBuilder<B>>, block_announce_validator: Box<dyn BlockAnnounceValidator<B> + Send>, max_parallel_downloads: u32, ) -> Self { @@ -449,12 +423,10 @@ impl<B: BlockT> ChainSync<B> { best_queued_hash: info.best_hash, best_queued_number: info.best_number, best_imported_number: info.best_number, - extra_finality_proofs: ExtraRequests::new("finality proof"), extra_justifications: ExtraRequests::new("justification"), role, required_block_attributes, queue_blocks: Default::default(), - request_builder, fork_targets: Default::default(), pending_requests: Default::default(), block_announce_validator, @@ -613,14 +585,6 @@ impl<B: BlockT> ChainSync<B> { }) } - /// Schedule a finality proof request for the given block. - pub fn request_finality_proof(&mut self, hash: &B::Hash, number: NumberFor<B>) { - let client = &self.client; - self.extra_finality_proofs.schedule((*hash, number), |base, block| { - is_descendent_of(&**client, base, block) - }) - } - /// Request syncing for the given block from given set of peers. // The implementation is similar to on_block_announce with unknown parent hash. pub fn set_sync_fork_request( @@ -700,30 +664,6 @@ impl<B: BlockT> ChainSync<B> { }) } - /// Get an iterator over all scheduled finality proof requests. - pub fn finality_proof_requests(&mut self) -> impl Iterator<Item = (PeerId, FinalityProofRequest<B::Hash>)> + '_ { - let peers = &mut self.peers; - let request_builder = &mut self.request_builder; - let mut matcher = self.extra_finality_proofs.matcher(); - std::iter::from_fn(move || { - if let Some((peer, request)) = matcher.next(&peers) { - peers.get_mut(&peer) - .expect("`Matcher::next` guarantees the `PeerId` comes from the given peers; qed") - .state = PeerSyncState::DownloadingFinalityProof(request.0); - let req = message::generic::FinalityProofRequest { - id: 0, - block: request.0, - request: request_builder.as_mut() - .map(|builder| builder.build_request_data(&request.0)) - .unwrap_or_default() - }; - Some((peer, req)) - } else { - None - } - }) - } - /// Get an iterator over all block requests of all peers. pub fn block_requests(&mut self) -> impl Iterator<Item = (&PeerId, BlockRequest<B>)> + '_ { if self.pending_requests.is_empty() { @@ -920,8 +860,7 @@ impl<B: BlockT> ChainSync<B> { } | PeerSyncState::Available - | PeerSyncState::DownloadingJustification(..) - | PeerSyncState::DownloadingFinalityProof(..) => Vec::new() + | PeerSyncState::DownloadingJustification(..) => Vec::new() } } else { // When request.is_none() this is a block announcement. Just accept blocks. @@ -1033,41 +972,6 @@ impl<B: BlockT> ChainSync<B> { Ok(OnBlockJustification::Nothing) } - /// Handle new finality proof data. - pub fn on_block_finality_proof - (&mut self, who: PeerId, resp: FinalityProofResponse<B::Hash>) -> Result<OnBlockFinalityProof<B>, BadPeer> - { - let peer = - if let Some(peer) = self.peers.get_mut(&who) { - peer - } else { - error!(target: "sync", "💔 Called on_block_finality_proof_data with a bad peer ID"); - return Ok(OnBlockFinalityProof::Nothing) - }; - - self.pending_requests.add(&who); - if let PeerSyncState::DownloadingFinalityProof(hash) = peer.state { - peer.state = PeerSyncState::Available; - - // We only request one finality proof at a time. - if hash != resp.block { - info!( - target: "sync", - "💔 Invalid block finality proof provided: requested: {:?} got: {:?}", - hash, - resp.block - ); - return Err(BadPeer(who, rep::BAD_FINALITY_PROOF)); - } - - if let Some((peer, hash, number, p)) = self.extra_finality_proofs.on_response(who, resp.proof) { - return Ok(OnBlockFinalityProof::Import { peer, hash, number, proof: p }) - } - } - - Ok(OnBlockFinalityProof::Nothing) - } - /// A batch of blocks have been processed, with or without errors. /// /// Call this when a batch of blocks have been processed by the import @@ -1122,11 +1026,6 @@ impl<B: BlockT> ChainSync<B> { } } - if aux.needs_finality_proof { - trace!(target: "sync", "Block imported but requires finality proof {}: {:?}", number, hash); - self.request_finality_proof(&hash, number); - } - if number > self.best_imported_number { self.best_imported_number = number; } @@ -1178,22 +1077,8 @@ impl<B: BlockT> ChainSync<B> { self.pending_requests.set_all(); } - pub fn on_finality_proof_import(&mut self, req: (B::Hash, NumberFor<B>), res: Result<(B::Hash, NumberFor<B>), ()>) { - self.extra_finality_proofs.try_finalize_root(req, res, true); - self.pending_requests.set_all(); - } - /// Notify about finalization of the given block. pub fn on_block_finalized(&mut self, hash: &B::Hash, number: NumberFor<B>) { - let client = &self.client; - let r = self.extra_finality_proofs.on_block_finalized(hash, number, |base, block| { - is_descendent_of(&**client, base, block) - }); - - if let Err(err) = r { - warn!(target: "sync", "💔 Error cleaning up pending extra finality proof data requests: {:?}", err) - } - let client = &self.client; let r = self.extra_justifications.on_block_finalized(hash, number, |base, block| { is_descendent_of(&**client, base, block) @@ -1506,14 +1391,12 @@ impl<B: BlockT> ChainSync<B> { self.blocks.clear_peer_download(who); self.peers.remove(who); self.extra_justifications.peer_disconnected(who); - self.extra_finality_proofs.peer_disconnected(who); self.pending_requests.set_all(); } /// Restart the sync process. This will reset all pending block requests and return an iterator /// of new block requests to make to peers. Peers that were downloading finality data (i.e. - /// their state was `DownloadingJustification` or `DownloadingFinalityProof`) are unaffected and - /// will stay in the same state. + /// their state was `DownloadingJustification`) are unaffected and will stay in the same state. fn restart<'a>( &'a mut self, ) -> impl Iterator<Item = Result<(PeerId, BlockRequest<B>), BadPeer>> + 'a { @@ -1526,11 +1409,10 @@ impl<B: BlockT> ChainSync<B> { let old_peers = std::mem::take(&mut self.peers); old_peers.into_iter().filter_map(move |(id, p)| { - // peers that were downloading justifications or finality proofs + // peers that were downloading justifications // should be kept in that state. match p.state { - PeerSyncState::DownloadingJustification(_) - | PeerSyncState::DownloadingFinalityProof(_) => { + PeerSyncState::DownloadingJustification(_) => { self.peers.insert(id, p); return None; } @@ -1570,7 +1452,6 @@ impl<B: BlockT> ChainSync<B> { Metrics { queued_blocks: self.queue_blocks.len().try_into().unwrap_or(std::u32::MAX), fork_targets: self.fork_targets.len().try_into().unwrap_or(std::u32::MAX), - finality_proofs: self.extra_finality_proofs.metrics(), justifications: self.extra_justifications.metrics(), _priv: () } @@ -1581,7 +1462,6 @@ impl<B: BlockT> ChainSync<B> { pub(crate) struct Metrics { pub(crate) queued_blocks: u32, pub(crate) fork_targets: u32, - pub(crate) finality_proofs: extra_requests::Metrics, pub(crate) justifications: extra_requests::Metrics, _priv: () } @@ -1835,7 +1715,6 @@ mod test { Roles::AUTHORITY, client.clone(), &info, - None, block_announce_validator, 1, ); @@ -1907,7 +1786,6 @@ mod test { Roles::AUTHORITY, client.clone(), &info, - None, Box::new(DefaultBlockAnnounceValidator), 1, ); @@ -1915,7 +1793,6 @@ mod test { let peer_id1 = PeerId::random(); let peer_id2 = PeerId::random(); let peer_id3 = PeerId::random(); - let peer_id4 = PeerId::random(); let mut new_blocks = |n| { for _ in 0..n { @@ -1928,7 +1805,6 @@ mod test { }; let (b1_hash, b1_number) = new_blocks(50); - let (b2_hash, b2_number) = new_blocks(10); // add 2 peers at blocks that we don't have locally sync.new_peer(peer_id1.clone(), Hash::random(), 42).unwrap(); @@ -1958,38 +1834,16 @@ mod test { PeerSyncState::DownloadingJustification(b1_hash), ); - // add another peer at a known later block - sync.new_peer(peer_id4.clone(), b2_hash, b2_number).unwrap(); - - // we request a finality proof for a block we have locally - sync.request_finality_proof(&b2_hash, b2_number); - - // the finality proof request should be scheduled to peer 4 - // which is at that block - assert!( - sync.finality_proof_requests().any(|(p, r)| { p == peer_id4 && r.block == b2_hash }) - ); - - assert_eq!( - sync.peers.get(&peer_id4).unwrap().state, - PeerSyncState::DownloadingFinalityProof(b2_hash), - ); - // we restart the sync state let block_requests = sync.restart(); // which should make us send out block requests to the first two peers assert!(block_requests.map(|r| r.unwrap()).all(|(p, _)| { p == peer_id1 || p == peer_id2 })); - // peer 3 and 4 should be unaffected as they were downloading finality data + // peer 3 should be unaffected it was downloading finality data assert_eq!( sync.peers.get(&peer_id3).unwrap().state, PeerSyncState::DownloadingJustification(b1_hash), ); - - assert_eq!( - sync.peers.get(&peer_id4).unwrap().state, - PeerSyncState::DownloadingFinalityProof(b2_hash), - ); } } diff --git a/substrate/client/network/src/protocol/sync/extra_requests.rs b/substrate/client/network/src/protocol/sync/extra_requests.rs index df336c25339fd8eb42b635735e7fab94fcd2feb6..79f10c4a3bf80ba59e856c0b8d677c9675c44435 100644 --- a/substrate/client/network/src/protocol/sync/extra_requests.rs +++ b/substrate/client/network/src/protocol/sync/extra_requests.rs @@ -528,13 +528,12 @@ mod tests { impl Arbitrary for ArbitraryPeerSyncState { fn arbitrary<G: Gen>(g: &mut G) -> Self { - let s = match g.gen::<u8>() % 5 { + let s = match g.gen::<u8>() % 4 { 0 => PeerSyncState::Available, // TODO: 1 => PeerSyncState::AncestorSearch(g.gen(), AncestorSearchState<B>), 1 => PeerSyncState::DownloadingNew(g.gen::<BlockNumber>()), 2 => PeerSyncState::DownloadingStale(Hash::random()), - 3 => PeerSyncState::DownloadingJustification(Hash::random()), - _ => PeerSyncState::DownloadingFinalityProof(Hash::random()) + _ => PeerSyncState::DownloadingJustification(Hash::random()), }; ArbitraryPeerSyncState(s) } diff --git a/substrate/client/network/src/schema.rs b/substrate/client/network/src/schema.rs index 44fbbffd25406d5b985707f46f8c2e9123ca992a..423d3ef5b41e4d8124138d8cdd0f8c7cf062f90d 100644 --- a/substrate/client/network/src/schema.rs +++ b/substrate/client/network/src/schema.rs @@ -20,9 +20,6 @@ pub mod v1 { include!(concat!(env!("OUT_DIR"), "/api.v1.rs")); - pub mod finality { - include!(concat!(env!("OUT_DIR"), "/api.v1.finality.rs")); - } pub mod light { include!(concat!(env!("OUT_DIR"), "/api.v1.light.rs")); } diff --git a/substrate/client/network/src/schema/finality.v1.proto b/substrate/client/network/src/schema/finality.v1.proto deleted file mode 100644 index 843bc4eca0990cc01b1479e19d68a721395266c4..0000000000000000000000000000000000000000 --- a/substrate/client/network/src/schema/finality.v1.proto +++ /dev/null @@ -1,19 +0,0 @@ -// Schema definition for finality proof request/responses. - -syntax = "proto3"; - -package api.v1.finality; - -// Request a finality proof from a peer. -message FinalityProofRequest { - // SCALE-encoded hash of the block to request. - bytes block_hash = 1; - // Opaque chain-specific additional request data. - bytes request = 2; -} - -// Response to a finality proof request. -message FinalityProofResponse { - // Opaque chain-specific finality proof. Empty if no such proof exists. - bytes proof = 1; // optional -} diff --git a/substrate/client/network/src/service.rs b/substrate/client/network/src/service.rs index 3296a97d71bbc10d6080332ae58f1aaf2f3b32f9..8ef76d4850699a774a09efc0e792b2506e744fcc 100644 --- a/substrate/client/network/src/service.rs +++ b/substrate/client/network/src/service.rs @@ -38,7 +38,7 @@ use crate::{ NetworkState, NotConnectedPeer as NetworkStateNotConnectedPeer, Peer as NetworkStatePeer, }, on_demand_layer::AlwaysBadChecker, - light_client_handler, block_requests, finality_requests, + light_client_handler, block_requests, protocol::{self, event::Event, NotifsHandlerError, NotificationsSink, Ready, sync::SyncState, PeerInfo, Protocol}, transport, ReputationChange, }; @@ -248,7 +248,6 @@ impl<B: BlockT + 'static, H: ExHashT> NetworkWorker<B, H> { local_peer_id.clone(), params.chain.clone(), params.transaction_pool, - params.finality_proof_request_builder, params.protocol_id.clone(), peerset_config, params.block_announce_validator, @@ -267,10 +266,6 @@ impl<B: BlockT + 'static, H: ExHashT> NetworkWorker<B, H> { let config = block_requests::Config::new(¶ms.protocol_id); block_requests::BlockRequests::new(config, params.chain.clone()) }; - let finality_proof_requests = { - let config = finality_requests::Config::new(¶ms.protocol_id); - finality_requests::FinalityProofRequests::new(config, params.finality_proof_provider.clone()) - }; let light_client_handler = { let config = light_client_handler::Config::new(¶ms.protocol_id); light_client_handler::LightClientHandler::new( @@ -310,7 +305,6 @@ impl<B: BlockT + 'static, H: ExHashT> NetworkWorker<B, H> { user_agent, local_public, block_requests, - finality_proof_requests, light_client_handler, discovery_config, params.network_config.request_response_protocols, @@ -1361,12 +1355,6 @@ impl<B: BlockT + 'static, H: ExHashT> Future for NetworkWorker<B, H> { } this.import_queue.import_justification(origin, hash, nb, justification); }, - Poll::Ready(SwarmEvent::Behaviour(BehaviourOut::FinalityProofImport(origin, hash, nb, proof))) => { - if let Some(metrics) = this.metrics.as_ref() { - metrics.import_queue_finality_proofs_submitted.inc(); - } - this.import_queue.import_finality_proof(origin, hash, nb, proof); - }, Poll::Ready(SwarmEvent::Behaviour(BehaviourOut::InboundRequest { protocol, result, .. })) => { if let Some(metrics) = this.metrics.as_ref() { match result { @@ -1563,11 +1551,11 @@ impl<B: BlockT + 'static, H: ExHashT> Future for NetworkWorker<B, H> { let reason = match cause { Some(ConnectionError::IO(_)) => "transport-error", Some(ConnectionError::Handler(NodeHandlerWrapperError::Handler(EitherError::A(EitherError::A( - EitherError::A(EitherError::A(EitherError::A(EitherError::B( - EitherError::A(PingFailure::Timeout)))))))))) => "ping-timeout", + EitherError::A(EitherError::A(EitherError::B( + EitherError::A(PingFailure::Timeout))))))))) => "ping-timeout", Some(ConnectionError::Handler(NodeHandlerWrapperError::Handler(EitherError::A(EitherError::A( - EitherError::A(EitherError::A(EitherError::A(EitherError::A( - NotifsHandlerError::SyncNotificationsClogged))))))))) => "sync-notifications-clogged", + EitherError::A(EitherError::A(EitherError::A( + NotifsHandlerError::SyncNotificationsClogged)))))))) => "sync-notifications-clogged", Some(ConnectionError::Handler(NodeHandlerWrapperError::Handler(_))) => "protocol-error", Some(ConnectionError::Handler(NodeHandlerWrapperError::KeepAliveTimeout)) => "keep-alive-timeout", None => "actively-closed", @@ -1752,23 +1740,6 @@ impl<'a, B: BlockT, H: ExHashT> Link<B> for NetworkLink<'a, B, H> { fn request_justification(&mut self, hash: &B::Hash, number: NumberFor<B>) { self.protocol.user_protocol_mut().request_justification(hash, number) } - fn request_finality_proof(&mut self, hash: &B::Hash, number: NumberFor<B>) { - self.protocol.user_protocol_mut().request_finality_proof(hash, number) - } - fn finality_proof_imported( - &mut self, - who: PeerId, - request_block: (B::Hash, NumberFor<B>), - finalization_result: Result<(B::Hash, NumberFor<B>), ()>, - ) { - let success = finalization_result.is_ok(); - self.protocol.user_protocol_mut().finality_proof_import_result(request_block, finalization_result); - if !success { - info!("💔 Invalid finality proof provided by {} for #{}", who, request_block.0); - self.protocol.user_protocol_mut().disconnect_peer(&who); - self.protocol.user_protocol_mut().report_peer(who, ReputationChange::new_fatal("Invalid finality proof")); - } - } } fn ensure_addresses_consistent_with_transport<'a>( diff --git a/substrate/client/network/src/service/metrics.rs b/substrate/client/network/src/service/metrics.rs index a63ce7a18a519d1a816eb08b12ecf70a64fef9ab..614c24b522de2902ed01c79b6cff25801ccec24c 100644 --- a/substrate/client/network/src/service/metrics.rs +++ b/substrate/client/network/src/service/metrics.rs @@ -56,7 +56,6 @@ pub struct Metrics { pub distinct_peers_connections_closed_total: Counter<U64>, pub distinct_peers_connections_opened_total: Counter<U64>, pub import_queue_blocks_submitted: Counter<U64>, - pub import_queue_finality_proofs_submitted: Counter<U64>, pub import_queue_justifications_submitted: Counter<U64>, pub incoming_connections_errors_total: CounterVec<U64>, pub incoming_connections_total: Counter<U64>, @@ -112,10 +111,6 @@ impl Metrics { "import_queue_blocks_submitted", "Number of blocks submitted to the import queue.", )?, registry)?, - import_queue_finality_proofs_submitted: prometheus::register(Counter::new( - "import_queue_finality_proofs_submitted", - "Number of finality proofs submitted to the import queue.", - )?, registry)?, import_queue_justifications_submitted: prometheus::register(Counter::new( "import_queue_justifications_submitted", "Number of justifications submitted to the import queue.", diff --git a/substrate/client/network/src/service/tests.rs b/substrate/client/network/src/service/tests.rs index 76a924748ad2a62270dc6ceb1da50401ab6c9ab5..225a3ae98ab5d9f0e35bbd8f69c8baa7532317cf 100644 --- a/substrate/client/network/src/service/tests.rs +++ b/substrate/client/network/src/service/tests.rs @@ -87,7 +87,6 @@ fn build_test_full_node(config: config::NetworkConfiguration) PassThroughVerifier(false), Box::new(client.clone()), None, - None, &sp_core::testing::TaskExecutor::new(), None, )); @@ -97,8 +96,6 @@ fn build_test_full_node(config: config::NetworkConfiguration) executor: None, network_config: config, chain: client.clone(), - finality_proof_provider: None, - finality_proof_request_builder: None, on_demand: None, transaction_pool: Arc::new(crate::config::EmptyTransactionPool), protocol_id: config::ProtocolId::from("/test-protocol-name"), diff --git a/substrate/client/network/test/src/block_import.rs b/substrate/client/network/test/src/block_import.rs index 1d2cd3d687de92f5f90a4c8e8d729b1e9460e5b9..a5d0600abefeaaa4d10f757d8ee6b193320c6063 100644 --- a/substrate/client/network/test/src/block_import.rs +++ b/substrate/client/network/test/src/block_import.rs @@ -107,7 +107,6 @@ fn async_import_queue_drops() { verifier, Box::new(substrate_test_runtime_client::new()), None, - None, &executor, None, ); diff --git a/substrate/client/network/test/src/lib.rs b/substrate/client/network/test/src/lib.rs index 1aec3dae22b920d1b523bc6a76b0ee920d85ef4e..6950ada4f8458c1413d0ee6ad3ada0b658d99038 100644 --- a/substrate/client/network/test/src/lib.rs +++ b/substrate/client/network/test/src/lib.rs @@ -29,7 +29,6 @@ use std::{ use libp2p::build_multiaddr; use log::trace; -use sc_network::config::FinalityProofProvider; use sp_blockchain::{ HeaderBackend, Result as ClientResult, well_known_cache_keys::{self, Id as CacheKeyId}, @@ -44,14 +43,14 @@ use sc_block_builder::{BlockBuilder, BlockBuilderProvider}; use sc_network::config::Role; use sp_consensus::block_validation::{DefaultBlockAnnounceValidator, BlockAnnounceValidator}; use sp_consensus::import_queue::{ - BasicQueue, BoxJustificationImport, Verifier, BoxFinalityProofImport, + BasicQueue, BoxJustificationImport, Verifier, }; use sp_consensus::block_import::{BlockImport, ImportResult}; use sp_consensus::Error as ConsensusError; use sp_consensus::{BlockOrigin, ForkChoiceStrategy, BlockImportParams, BlockCheckParams, JustificationImport}; use futures::prelude::*; use sc_network::{NetworkWorker, NetworkService, config::ProtocolId}; -use sc_network::config::{NetworkConfiguration, TransportConfig, BoxFinalityProofRequestBuilder}; +use sc_network::config::{NetworkConfiguration, TransportConfig}; use libp2p::PeerId; use parking_lot::Mutex; use sp_core::H256; @@ -586,20 +585,10 @@ pub trait TestNetFactory: Sized { -> ( BlockImportAdapter<Transaction>, Option<BoxJustificationImport<Block>>, - Option<BoxFinalityProofImport<Block>>, - Option<BoxFinalityProofRequestBuilder<Block>>, Self::PeerData, ) { - (client.as_block_import(), None, None, None, Default::default()) - } - - /// Get finality proof provider (if supported). - fn make_finality_proof_provider( - &self, - _client: PeersClient, - ) -> Option<Arc<dyn FinalityProofProvider<Block>>> { - None + (client.as_block_import(), None, Default::default()) } fn default_config() -> ProtocolConfig { @@ -636,8 +625,6 @@ pub trait TestNetFactory: Sized { let ( block_import, justification_import, - finality_proof_import, - finality_proof_request_builder, data, ) = self.make_block_import(PeersClient::Full(client.clone(), backend.clone())); @@ -652,7 +639,6 @@ pub trait TestNetFactory: Sized { verifier.clone(), Box::new(block_import.clone()), justification_import, - finality_proof_import, &sp_core::testing::TaskExecutor::new(), None, )); @@ -675,10 +661,6 @@ pub trait TestNetFactory: Sized { executor: None, network_config, chain: client.clone(), - finality_proof_provider: self.make_finality_proof_provider( - PeersClient::Full(client.clone(), backend.clone()), - ), - finality_proof_request_builder, on_demand: None, transaction_pool: Arc::new(EmptyTransactionPool), protocol_id: ProtocolId::from("test-protocol-name"), @@ -717,8 +699,6 @@ pub trait TestNetFactory: Sized { let ( block_import, justification_import, - finality_proof_import, - finality_proof_request_builder, data, ) = self.make_block_import(PeersClient::Light(client.clone(), backend.clone())); @@ -733,7 +713,6 @@ pub trait TestNetFactory: Sized { verifier.clone(), Box::new(block_import.clone()), justification_import, - finality_proof_import, &sp_core::testing::TaskExecutor::new(), None, )); @@ -755,10 +734,6 @@ pub trait TestNetFactory: Sized { executor: None, network_config, chain: client.clone(), - finality_proof_provider: self.make_finality_proof_provider( - PeersClient::Light(client.clone(), backend.clone()) - ), - finality_proof_request_builder, on_demand: None, transaction_pool: Arc::new(EmptyTransactionPool), protocol_id: ProtocolId::from("test-protocol-name"), @@ -989,16 +964,12 @@ impl TestNetFactory for JustificationTestNet { -> ( BlockImportAdapter<Transaction>, Option<BoxJustificationImport<Block>>, - Option<BoxFinalityProofImport<Block>>, - Option<BoxFinalityProofRequestBuilder<Block>>, Self::PeerData, ) { ( client.as_block_import(), Some(Box::new(ForceFinalized(client))), - None, - None, Default::default(), ) } diff --git a/substrate/client/service/src/builder.rs b/substrate/client/service/src/builder.rs index 7d613f2bc6292687023a570300af63126c59b2f6..d9dc0d1c6ba017cd38059a79d3a77cb3d06843a3 100644 --- a/substrate/client/service/src/builder.rs +++ b/substrate/client/service/src/builder.rs @@ -41,7 +41,7 @@ use futures::{ }; use sc_keystore::LocalKeystore; use log::{info, warn}; -use sc_network::config::{Role, FinalityProofProvider, OnDemand, BoxFinalityProofRequestBuilder}; +use sc_network::config::{Role, OnDemand}; use sc_network::NetworkService; use sp_runtime::generic::BlockId; use sp_runtime::traits::{ @@ -830,10 +830,6 @@ pub struct BuildNetworkParams<'a, TBl: BlockT, TExPool, TImpQu, TCl> { pub block_announce_validator_builder: Option<Box< dyn FnOnce(Arc<TCl>) -> Box<dyn BlockAnnounceValidator<TBl> + Send> + Send >>, - /// An optional finality proof request builder. - pub finality_proof_request_builder: Option<BoxFinalityProofRequestBuilder<TBl>>, - /// An optional, shared finality proof request provider. - pub finality_proof_provider: Option<Arc<dyn FinalityProofProvider<TBl>>>, } /// Build the network service, the network status sinks and an RPC sender. @@ -858,7 +854,7 @@ pub fn build_network<TBl, TExPool, TImpQu, TCl>( { let BuildNetworkParams { config, client, transaction_pool, spawn_handle, import_queue, on_demand, - block_announce_validator_builder, finality_proof_request_builder, finality_proof_provider, + block_announce_validator_builder, } = params; let transaction_pool_adapter = Arc::new(TransactionPoolAdapter { @@ -896,8 +892,6 @@ pub fn build_network<TBl, TExPool, TImpQu, TCl>( }, network_config: config.network.clone(), chain: client.clone(), - finality_proof_provider, - finality_proof_request_builder, on_demand: on_demand, transaction_pool: transaction_pool_adapter as _, import_queue: Box::new(import_queue), diff --git a/substrate/client/service/src/lib.rs b/substrate/client/service/src/lib.rs index a23ebf3d553d58573e8b960bc006f3cdd1019bcd..fd5ad9ebac917e15bef4bf20faf662aca7eb7292 100644 --- a/substrate/client/service/src/lib.rs +++ b/substrate/client/service/src/lib.rs @@ -73,7 +73,7 @@ pub use sc_executor::NativeExecutionDispatch; pub use std::{ops::Deref, result::Result, sync::Arc}; #[doc(hidden)] pub use sc_network::config::{ - FinalityProofProvider, OnDemand, BoxFinalityProofRequestBuilder, TransactionImport, + OnDemand, TransactionImport, TransactionImportFuture, }; pub use sc_tracing::TracingReceiver; diff --git a/substrate/primitives/consensus/common/src/block_import.rs b/substrate/primitives/consensus/common/src/block_import.rs index 5e593da1163d77a65be2ca7bfaa9cd9f4114d71b..0100041fc0a0ccdd8d9c765ba554c0302814fc23 100644 --- a/substrate/primitives/consensus/common/src/block_import.rs +++ b/substrate/primitives/consensus/common/src/block_import.rs @@ -26,7 +26,7 @@ use std::sync::Arc; use std::any::Any; use crate::Error; -use crate::import_queue::{Verifier, CacheKeyId}; +use crate::import_queue::CacheKeyId; /// Block import result. #[derive(Debug, PartialEq, Eq)] @@ -54,8 +54,6 @@ pub struct ImportedAux { pub needs_justification: bool, /// Received a bad justification. pub bad_justification: bool, - /// Request a finality proof for the given block. - pub needs_finality_proof: bool, /// Whether the block that was imported is the new best block. pub is_new_best: bool, } @@ -63,7 +61,7 @@ pub struct ImportedAux { impl ImportResult { /// Returns default value for `ImportResult::Imported` with /// `clear_justification_requests`, `needs_justification`, - /// `bad_justification` and `needs_finality_proof` set to false. + /// `bad_justification` set to false. pub fn imported(is_new_best: bool) -> ImportResult { let mut aux = ImportedAux::default(); aux.is_new_best = is_new_best; @@ -345,21 +343,3 @@ pub trait JustificationImport<B: BlockT> { justification: Justification, ) -> Result<(), Self::Error>; } - -/// Finality proof import trait. -pub trait FinalityProofImport<B: BlockT> { - type Error: std::error::Error + Send + 'static; - - /// Called by the import queue when it is started. Returns a list of finality proofs to request - /// from the network. - fn on_start(&mut self) -> Vec<(B::Hash, NumberFor<B>)> { Vec::new() } - - /// Import a Block justification and finalize the given block. Returns finalized block or error. - fn import_finality_proof( - &mut self, - hash: B::Hash, - number: NumberFor<B>, - finality_proof: Vec<u8>, - verifier: &mut dyn Verifier<B>, - ) -> Result<(B::Hash, NumberFor<B>), Self::Error>; -} diff --git a/substrate/primitives/consensus/common/src/import_queue.rs b/substrate/primitives/consensus/common/src/import_queue.rs index 92bd9966d75ec031778f71a638093c555a3f647c..3ad8c7c92e07f80028de8d052406e28ae5386c27 100644 --- a/substrate/primitives/consensus/common/src/import_queue.rs +++ b/substrate/primitives/consensus/common/src/import_queue.rs @@ -34,7 +34,7 @@ use crate::{ error::Error as ConsensusError, block_import::{ BlockImport, BlockOrigin, BlockImportParams, ImportedAux, JustificationImport, ImportResult, - BlockCheckParams, FinalityProofImport, + BlockCheckParams, }, metrics::Metrics, }; @@ -56,11 +56,6 @@ pub type BoxBlockImport<B, Transaction> = Box< /// Shared justification import struct used by the queue. pub type BoxJustificationImport<B> = Box<dyn JustificationImport<B, Error=ConsensusError> + Send + Sync>; -/// Shared finality proof import struct used by the queue. -pub type BoxFinalityProofImport<B> = Box< - dyn FinalityProofImport<B, Error = ConsensusError> + Send + Sync ->; - /// Maps to the Origin used by the network. pub type Origin = libp2p::PeerId; @@ -115,15 +110,6 @@ pub trait ImportQueue<B: BlockT>: Send { number: NumberFor<B>, justification: Justification ); - /// Import block finality proof. - fn import_finality_proof( - &mut self, - who: Origin, - hash: B::Hash, - number: NumberFor<B>, - finality_proof: Vec<u8> - ); - /// Polls for actions to perform on the network. /// /// This method should behave in a way similar to `Future::poll`. It can register the current @@ -146,19 +132,6 @@ pub trait Link<B: BlockT>: Send { fn justification_imported(&mut self, _who: Origin, _hash: &B::Hash, _number: NumberFor<B>, _success: bool) {} /// Request a justification for the given block. fn request_justification(&mut self, _hash: &B::Hash, _number: NumberFor<B>) {} - /// Finality proof import result. - /// - /// Even though we have asked for finality proof of block A, provider could return proof of - /// some earlier block B, if the proof for A was too large. The sync module should continue - /// asking for proof of A in this case. - fn finality_proof_imported( - &mut self, - _who: Origin, - _request_block: (B::Hash, NumberFor<B>), - _finalization_result: Result<(B::Hash, NumberFor<B>), ()>, - ) {} - /// Request a finality proof for the given block. - fn request_finality_proof(&mut self, _hash: &B::Hash, _number: NumberFor<B>) {} } /// Block import successful result. diff --git a/substrate/primitives/consensus/common/src/import_queue/basic_queue.rs b/substrate/primitives/consensus/common/src/import_queue/basic_queue.rs index ea0ca2cf3ee8848f9cfa9c12b02da5d966aac484..b426c39100e697d3def9d36c115bf7e794f08e47 100644 --- a/substrate/primitives/consensus/common/src/import_queue/basic_queue.rs +++ b/substrate/primitives/consensus/common/src/import_queue/basic_queue.rs @@ -25,7 +25,7 @@ use prometheus_endpoint::Registry; use crate::{ block_import::BlockOrigin, import_queue::{ - BlockImportResult, BlockImportError, Verifier, BoxBlockImport, BoxFinalityProofImport, + BlockImportResult, BlockImportError, Verifier, BoxBlockImport, BoxJustificationImport, ImportQueue, Link, Origin, IncomingBlock, import_single_block_metered, buffered_link::{self, BufferedLinkSender, BufferedLinkReceiver}, @@ -36,8 +36,8 @@ use crate::{ /// Interface to a basic block import queue that is importing blocks sequentially in a separate /// task, with plugable verification. pub struct BasicQueue<B: BlockT, Transaction> { - /// Channel to send finality work messages to the background task. - finality_sender: TracingUnboundedSender<worker_messages::Finality<B>>, + /// Channel to send justifcation import messages to the background task. + justification_sender: TracingUnboundedSender<worker_messages::ImportJustification<B>>, /// Channel to send block import messages to the background task. block_import_sender: TracingUnboundedSender<worker_messages::ImportBlocks<B>>, /// Results coming from the worker task. @@ -48,7 +48,7 @@ pub struct BasicQueue<B: BlockT, Transaction> { impl<B: BlockT, Transaction> Drop for BasicQueue<B, Transaction> { fn drop(&mut self) { // Flush the queue and close the receiver to terminate the future. - self.finality_sender.close_channel(); + self.justification_sender.close_channel(); self.block_import_sender.close_channel(); self.result_port.close(); } @@ -57,13 +57,11 @@ impl<B: BlockT, Transaction> Drop for BasicQueue<B, Transaction> { impl<B: BlockT, Transaction: Send + 'static> BasicQueue<B, Transaction> { /// Instantiate a new basic queue, with given verifier. /// - /// This creates a background task, and calls `on_start` on the justification importer and - /// finality proof importer. + /// This creates a background task, and calls `on_start` on the justification importer. pub fn new<V: 'static + Verifier<B>>( verifier: V, block_import: BoxBlockImport<B, Transaction>, justification_import: Option<BoxJustificationImport<B>>, - finality_proof_import: Option<BoxFinalityProofImport<B>>, spawner: &impl sp_core::traits::SpawnNamed, prometheus_registry: Option<&Registry>, ) -> Self { @@ -77,19 +75,18 @@ impl<B: BlockT, Transaction: Send + 'static> BasicQueue<B, Transaction> { .ok() }); - let (future, finality_sender, block_import_sender) = BlockImportWorker::new( + let (future, justification_sender, block_import_sender) = BlockImportWorker::new( result_sender, verifier, block_import, justification_import, - finality_proof_import, metrics, ); spawner.spawn_blocking("basic-block-import-worker", future.boxed()); Self { - finality_sender, + justification_sender, block_import_sender, result_port, _phantom: PhantomData, @@ -122,8 +119,8 @@ impl<B: BlockT, Transaction: Send> ImportQueue<B> for BasicQueue<B, Transaction> number: NumberFor<B>, justification: Justification, ) { - let res = self.finality_sender.unbounded_send( - worker_messages::Finality::ImportJustification(who, hash, number, justification), + let res = self.justification_sender.unbounded_send( + worker_messages::ImportJustification(who, hash, number, justification), ); if res.is_err() { @@ -134,26 +131,6 @@ impl<B: BlockT, Transaction: Send> ImportQueue<B> for BasicQueue<B, Transaction> } } - fn import_finality_proof( - &mut self, - who: Origin, - hash: B::Hash, - number: NumberFor<B>, - finality_proof: Vec<u8>, - ) { - trace!(target: "sync", "Scheduling finality proof of {}/{} for import", number, hash); - let res = self.finality_sender.unbounded_send( - worker_messages::Finality::ImportFinalityProof(who, hash, number, finality_proof), - ); - - if res.is_err() { - log::error!( - target: "sync", - "import_finality_proof: Background import task is no longer alive" - ); - } - } - fn poll_actions(&mut self, cx: &mut Context, link: &mut dyn Link<B>) { if self.result_port.poll_actions(cx, link).is_err() { log::error!(target: "sync", "poll_actions: Background import task is no longer alive"); @@ -166,17 +143,12 @@ mod worker_messages { use super::*; pub struct ImportBlocks<B: BlockT>(pub BlockOrigin, pub Vec<IncomingBlock<B>>); - - pub enum Finality<B: BlockT> { - ImportJustification(Origin, B::Hash, NumberFor<B>, Justification), - ImportFinalityProof(Origin, B::Hash, NumberFor<B>, Vec<u8>), - } + pub struct ImportJustification<B: BlockT>(pub Origin, pub B::Hash, pub NumberFor<B>, pub Justification); } struct BlockImportWorker<B: BlockT, Transaction> { result_sender: BufferedLinkSender<B>, justification_import: Option<BoxJustificationImport<B>>, - finality_proof_import: Option<BoxFinalityProofImport<B>>, delay_between_blocks: Duration, metrics: Option<Metrics>, _phantom: PhantomData<Transaction>, @@ -188,17 +160,16 @@ impl<B: BlockT, Transaction: Send> BlockImportWorker<B, Transaction> { verifier: V, block_import: BoxBlockImport<B, Transaction>, justification_import: Option<BoxJustificationImport<B>>, - finality_proof_import: Option<BoxFinalityProofImport<B>>, metrics: Option<Metrics>, ) -> ( impl Future<Output = ()> + Send, - TracingUnboundedSender<worker_messages::Finality<B>>, + TracingUnboundedSender<worker_messages::ImportJustification<B>>, TracingUnboundedSender<worker_messages::ImportBlocks<B>>, ) { use worker_messages::*; - let (finality_sender, mut finality_port) = - tracing_unbounded("mpsc_import_queue_worker_finality"); + let (justification_sender, mut justification_port) = + tracing_unbounded("mpsc_import_queue_worker_justification"); let (block_import_sender, mut block_import_port) = tracing_unbounded("mpsc_import_queue_worker_blocks"); @@ -206,23 +177,17 @@ impl<B: BlockT, Transaction: Send> BlockImportWorker<B, Transaction> { let mut worker = BlockImportWorker { result_sender, justification_import, - finality_proof_import, delay_between_blocks: Duration::new(0, 0), metrics, _phantom: PhantomData, }; - // Let's initialize `justification_import` and `finality_proof_import`. + // Let's initialize `justification_import` if let Some(justification_import) = worker.justification_import.as_mut() { for (hash, number) in justification_import.on_start() { worker.result_sender.request_justification(&hash, number); } } - if let Some(finality_proof_import) = worker.finality_proof_import.as_mut() { - for (hash, number) in finality_proof_import.on_start() { - worker.result_sender.request_finality_proof(&hash, number); - } - } // The future below has two possible states: // @@ -230,7 +195,7 @@ impl<B: BlockT, Transaction: Send> BlockImportWorker<B, Transaction> { // `Future`, and `block_import` is `None`. // - Something else, in which case `block_import` is `Some` and `importing` is None. // - // Additionally, the task will prioritize processing of finality work messages over + // Additionally, the task will prioritize processing of justification import messages over // block import messages, hence why two distinct channels are used. let mut block_import_verifier = Some((block_import, verifier)); let mut importing = None; @@ -243,28 +208,15 @@ impl<B: BlockT, Transaction: Send> BlockImportWorker<B, Transaction> { return Poll::Ready(()) } - // Grab the next finality action request sent to the import queue. - let finality_work = match Stream::poll_next(Pin::new(&mut finality_port), cx) { - Poll::Ready(Some(msg)) => Some(msg), - Poll::Ready(None) => return Poll::Ready(()), - Poll::Pending => None, - }; - - match finality_work { - Some(Finality::ImportFinalityProof(who, hash, number, proof)) => { - let (_, verif) = block_import_verifier - .as_mut() - .expect("block_import_verifier is always Some; qed"); - - worker.import_finality_proof(verif, who, hash, number, proof); - continue; - } - Some(Finality::ImportJustification(who, hash, number, justification)) => { + // Grab the next justification import request sent to the import queue. + match Stream::poll_next(Pin::new(&mut justification_port), cx) { + Poll::Ready(Some(ImportJustification(who, hash, number, justification))) => { worker.import_justification(who, hash, number, justification); continue; - } - None => {} - } + }, + Poll::Ready(None) => return Poll::Ready(()), + Poll::Pending => {}, + }; // If we are in the process of importing a bunch of blocks, let's resume this // process before doing anything more. @@ -299,7 +251,7 @@ impl<B: BlockT, Transaction: Send> BlockImportWorker<B, Transaction> { } }); - (future, finality_sender, block_import_sender) + (future, justification_sender, block_import_sender) } /// Returns a `Future` that imports the given blocks and sends the results on @@ -324,36 +276,6 @@ impl<B: BlockT, Transaction: Send> BlockImportWorker<B, Transaction> { }) } - fn import_finality_proof<V: 'static + Verifier<B>>( - &mut self, - verifier: &mut V, - who: Origin, - hash: B::Hash, - number: NumberFor<B>, - finality_proof: Vec<u8> - ) { - let started = wasm_timer::Instant::now(); - let result = self.finality_proof_import.as_mut().map(|finality_proof_import| { - finality_proof_import.import_finality_proof(hash, number, finality_proof, verifier) - .map_err(|e| { - debug!( - "Finality proof import failed with {:?} for hash: {:?} number: {:?} coming from node: {:?}", - e, - hash, - number, - who, - ); - }) - }).unwrap_or(Err(())); - - if let Some(metrics) = self.metrics.as_ref() { - metrics.finality_proof_import_time.observe(started.elapsed().as_secs_f64()); - } - - trace!(target: "sync", "Imported finality proof for {}/{}", number, hash); - self.result_sender.finality_proof_imported(who, (hash, number), result); - } - fn import_justification( &mut self, who: Origin, @@ -596,7 +518,7 @@ mod tests { let (result_sender, mut result_port) = buffered_link::buffered_link(); let (mut worker, mut finality_sender, mut block_import_sender) = - BlockImportWorker::new(result_sender, (), Box::new(()), Some(Box::new(())), None, None); + BlockImportWorker::new(result_sender, (), Box::new(()), Some(Box::new(())), None); let mut import_block = |n| { let header = Header { @@ -629,7 +551,7 @@ mod tests { let mut import_justification = || { let hash = Hash::random(); - block_on(finality_sender.send(worker_messages::Finality::ImportJustification( + block_on(finality_sender.send(worker_messages::ImportJustification( libp2p::PeerId::random(), hash, 1, diff --git a/substrate/primitives/consensus/common/src/import_queue/buffered_link.rs b/substrate/primitives/consensus/common/src/import_queue/buffered_link.rs index a37d4c53c260394dc335842d7723ace983d39472..db9bcc8f0ad63ce7e4985353da0d96834524d0e6 100644 --- a/substrate/primitives/consensus/common/src/import_queue/buffered_link.rs +++ b/substrate/primitives/consensus/common/src/import_queue/buffered_link.rs @@ -81,8 +81,6 @@ enum BlockImportWorkerMsg<B: BlockT> { BlocksProcessed(usize, usize, Vec<(Result<BlockImportResult<NumberFor<B>>, BlockImportError>, B::Hash)>), JustificationImported(Origin, B::Hash, NumberFor<B>, bool), RequestJustification(B::Hash, NumberFor<B>), - FinalityProofImported(Origin, (B::Hash, NumberFor<B>), Result<(B::Hash, NumberFor<B>), ()>), - RequestFinalityProof(B::Hash, NumberFor<B>), } impl<B: BlockT> Link<B> for BufferedLinkSender<B> { @@ -109,20 +107,6 @@ impl<B: BlockT> Link<B> for BufferedLinkSender<B> { fn request_justification(&mut self, hash: &B::Hash, number: NumberFor<B>) { let _ = self.tx.unbounded_send(BlockImportWorkerMsg::RequestJustification(hash.clone(), number)); } - - fn finality_proof_imported( - &mut self, - who: Origin, - request_block: (B::Hash, NumberFor<B>), - finalization_result: Result<(B::Hash, NumberFor<B>), ()>, - ) { - let msg = BlockImportWorkerMsg::FinalityProofImported(who, request_block, finalization_result); - let _ = self.tx.unbounded_send(msg); - } - - fn request_finality_proof(&mut self, hash: &B::Hash, number: NumberFor<B>) { - let _ = self.tx.unbounded_send(BlockImportWorkerMsg::RequestFinalityProof(hash.clone(), number)); - } } /// See [`buffered_link`]. @@ -154,10 +138,6 @@ impl<B: BlockT> BufferedLinkReceiver<B> { link.justification_imported(who, &hash, number, success), BlockImportWorkerMsg::RequestJustification(hash, number) => link.request_justification(&hash, number), - BlockImportWorkerMsg::FinalityProofImported(who, block, result) => - link.finality_proof_imported(who, block, result), - BlockImportWorkerMsg::RequestFinalityProof(hash, number) => - link.request_finality_proof(&hash, number), } } } diff --git a/substrate/primitives/consensus/common/src/lib.rs b/substrate/primitives/consensus/common/src/lib.rs index 988aa7a816c4244f861ff0ad59521a27d45a1fda..10fe8a2b315804fd850b9071327e00dfc67de852 100644 --- a/substrate/primitives/consensus/common/src/lib.rs +++ b/substrate/primitives/consensus/common/src/lib.rs @@ -49,7 +49,7 @@ mod metrics; pub use self::error::Error; pub use block_import::{ BlockImport, BlockOrigin, ForkChoiceStrategy, ImportedAux, BlockImportParams, BlockCheckParams, - ImportResult, JustificationImport, FinalityProofImport, + ImportResult, JustificationImport, }; pub use select_chain::SelectChain; pub use sp_state_machine::Backend as StateBackend; diff --git a/substrate/primitives/consensus/common/src/metrics.rs b/substrate/primitives/consensus/common/src/metrics.rs index a35b7c4968f7f6ee0d4e1995efbf1b9be8851f46..6e6b582e12594f489b2c39a04ff48c59edca41a1 100644 --- a/substrate/primitives/consensus/common/src/metrics.rs +++ b/substrate/primitives/consensus/common/src/metrics.rs @@ -30,7 +30,6 @@ pub(crate) struct Metrics { pub import_queue_processed: CounterVec<U64>, pub block_verification_time: HistogramVec, pub block_verification_and_import_time: Histogram, - pub finality_proof_import_time: Histogram, pub justification_import_time: Histogram, } @@ -63,15 +62,6 @@ impl Metrics { )?, registry, )?, - finality_proof_import_time: register( - Histogram::with_opts( - HistogramOpts::new( - "finality_proof_import_time", - "Time taken to import finality proofs", - ), - )?, - registry, - )?, justification_import_time: register( Histogram::with_opts( HistogramOpts::new(