From ecea78665b1a3a59d5de43426c4253d886ecba59 Mon Sep 17 00:00:00 2001 From: dmitrylavrenov Date: Fri, 10 Sep 2021 22:40:52 +0300 Subject: [PATCH 1/7] Add minimum required grandpa logic --- Cargo.lock | 110 +++++++++++++++++++++++++ crates/humanode-peer/Cargo.toml | 2 + crates/humanode-peer/src/chain_spec.rs | 23 ++++-- crates/humanode-peer/src/service.rs | 75 +++++++++++++++-- crates/humanode-runtime/Cargo.toml | 2 + crates/humanode-runtime/src/lib.rs | 58 ++++++++++++- 6 files changed, 256 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bb405467c..05ec3e721 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2330,6 +2330,7 @@ dependencies = [ "sc-consensus", "sc-consensus-aura", "sc-executor", + "sc-finality-grandpa", "sc-network", "sc-service", "sc-tracing 4.0.0-dev", @@ -2337,6 +2338,7 @@ dependencies = [ "sp-application-crypto", "sp-consensus", "sp-consensus-aura", + "sp-finality-grandpa", "sp-keystore", "sp-runtime", "sp-timestamp", @@ -2380,6 +2382,7 @@ dependencies = [ "pallet-aura", "pallet-balances", "pallet-bioauth", + "pallet-grandpa", "pallet-randomness-collective-flip", "pallet-sudo", "pallet-timestamp", @@ -4292,6 +4295,20 @@ dependencies = [ "sp-std 4.0.0-dev", ] +[[package]] +name = "pallet-authorship" +version = "4.0.0-dev" +source = "git+https://github.com/humanode-network/substrate?branch=master#b7409a2257c7c231b485ee827daa73d05d303047" +dependencies = [ + "frame-support", + "frame-system", + "impl-trait-for-tuples", + "parity-scale-codec", + "sp-authorship", + "sp-runtime", + "sp-std 4.0.0-dev", +] + [[package]] name = "pallet-balances" version = "4.0.0-dev" @@ -4322,6 +4339,28 @@ dependencies = [ "sp-std 4.0.0-dev", ] +[[package]] +name = "pallet-grandpa" +version = "4.0.0-dev" +source = "git+https://github.com/humanode-network/substrate?branch=master#b7409a2257c7c231b485ee827daa73d05d303047" +dependencies = [ + "frame-benchmarking", + "frame-support", + "frame-system", + "log", + "pallet-authorship", + "pallet-session", + "parity-scale-codec", + "sp-application-crypto", + "sp-core", + "sp-finality-grandpa", + "sp-io", + "sp-runtime", + "sp-session", + "sp-staking", + "sp-std 4.0.0-dev", +] + [[package]] name = "pallet-randomness-collective-flip" version = "4.0.0-dev" @@ -4352,6 +4391,7 @@ dependencies = [ "sp-session", "sp-staking", "sp-std 4.0.0-dev", + "sp-trie", ] [[package]] @@ -5938,6 +5978,47 @@ dependencies = [ "wasmi", ] +[[package]] +name = "sc-finality-grandpa" +version = "0.10.0-dev" +source = "git+https://github.com/humanode-network/substrate?branch=master#b7409a2257c7c231b485ee827daa73d05d303047" +dependencies = [ + "async-trait", + "derive_more", + "dyn-clone", + "finality-grandpa", + "fork-tree", + "futures 0.3.16", + "futures-timer 3.0.2", + "linked-hash-map", + "log", + "parity-scale-codec", + "parking_lot 0.11.1", + "pin-project 1.0.8", + "rand 0.8.4", + "sc-block-builder", + "sc-client-api", + "sc-consensus", + "sc-keystore", + "sc-network", + "sc-network-gossip", + "sc-telemetry 4.0.0-dev", + "serde_json", + "sp-api", + "sp-application-crypto", + "sp-arithmetic", + "sp-blockchain", + "sp-consensus", + "sp-core", + "sp-finality-grandpa", + "sp-inherents", + "sp-keystore", + "sp-runtime", + "sp-utils 4.0.0-dev", + "substrate-prometheus-endpoint", + "wasm-timer", +] + [[package]] name = "sc-informant" version = "0.10.0-dev" @@ -6050,6 +6131,23 @@ dependencies = [ "zeroize", ] +[[package]] +name = "sc-network-gossip" +version = "0.10.0-dev" +source = "git+https://github.com/humanode-network/substrate?branch=master#b7409a2257c7c231b485ee827daa73d05d303047" +dependencies = [ + "futures 0.3.16", + "futures-timer 3.0.2", + "libp2p 0.37.1", + "log", + "lru", + "sc-network", + "sp-runtime", + "substrate-prometheus-endpoint", + "tracing", + "wasm-timer", +] + [[package]] name = "sc-offchain" version = "4.0.0-dev" @@ -6930,6 +7028,18 @@ dependencies = [ "static_assertions", ] +[[package]] +name = "sp-authorship" +version = "4.0.0-dev" +source = "git+https://github.com/humanode-network/substrate?branch=master#b7409a2257c7c231b485ee827daa73d05d303047" +dependencies = [ + "async-trait", + "parity-scale-codec", + "sp-inherents", + "sp-runtime", + "sp-std 4.0.0-dev", +] + [[package]] name = "sp-block-builder" version = "4.0.0-dev" diff --git a/crates/humanode-peer/Cargo.toml b/crates/humanode-peer/Cargo.toml index 1369ba0fe..0349c0e3b 100644 --- a/crates/humanode-peer/Cargo.toml +++ b/crates/humanode-peer/Cargo.toml @@ -32,6 +32,7 @@ sc-client-api = { git = "https://github.com/humanode-network/substrate", branch sc-consensus = { git = "https://github.com/humanode-network/substrate", branch = "master" } sc-consensus-aura = { git = "https://github.com/humanode-network/substrate", branch = "master" } sc-executor = { git = "https://github.com/humanode-network/substrate", branch = "master" } +sc-finality-grandpa = { git = "https://github.com/humanode-network/substrate", branch = "master" } sc-network = { git = "https://github.com/humanode-network/substrate", branch = "master" } sc-service = { git = "https://github.com/humanode-network/substrate", branch = "master" } sc-tracing = { git = "https://github.com/humanode-network/substrate", branch = "master" } @@ -39,6 +40,7 @@ sc-transaction-pool = { git = "https://github.com/humanode-network/substrate", b sp-application-crypto = { git = "https://github.com/humanode-network/substrate", branch = "master" } sp-consensus = { git = "https://github.com/humanode-network/substrate", branch = "master" } sp-consensus-aura = { git = "https://github.com/humanode-network/substrate", branch = "master" } +sp-finality-grandpa = { git = "https://github.com/humanode-network/substrate", branch = "master" } sp-keystore = { git = "https://github.com/humanode-network/substrate", branch = "master" } sp-runtime = { git = "https://github.com/humanode-network/substrate", branch = "master" } sp-timestamp = { git = "https://github.com/humanode-network/substrate", branch = "master" } diff --git a/crates/humanode-peer/src/chain_spec.rs b/crates/humanode-peer/src/chain_spec.rs index c68c5ac5a..9b16406b9 100644 --- a/crates/humanode-peer/src/chain_spec.rs +++ b/crates/humanode-peer/src/chain_spec.rs @@ -2,12 +2,13 @@ use hex_literal::hex; use humanode_runtime::{ - AccountId, AuraConfig, BalancesConfig, BioauthConfig, GenesisConfig, RobonodePublicKeyWrapper, - Signature, SudoConfig, SystemConfig, WASM_BINARY, + AccountId, AuraConfig, BalancesConfig, BioauthConfig, GenesisConfig, GrandpaConfig, + RobonodePublicKeyWrapper, Signature, SudoConfig, SystemConfig, WASM_BINARY, }; use pallet_bioauth::StoredAuthTicket; use sc_service::ChainType; use sp_consensus_aura::sr25519::AuthorityId as AuraId; +use sp_finality_grandpa::AuthorityId as GrandpaId; use sp_runtime::{ app_crypto::{sr25519, Pair, Public}, traits::{IdentifyAccount, Verify}, @@ -35,8 +36,8 @@ where } /// Generate an Aura authority key. -pub fn authority_keys_from_seed(s: &str) -> AuraId { - get_from_seed::(s) +pub fn authority_keys_from_seed(s: &str) -> (AuraId, GrandpaId) { + (get_from_seed::(s), get_from_seed::(s)) } /// A configuration for local testnet. @@ -81,7 +82,7 @@ pub fn local_testnet_config() -> Result { get_account_id_from_seed::("Ferdie//stash"), ], vec![pallet_bioauth::StoredAuthTicket { - public_key: authority_keys_from_seed("Alice"), + public_key: authority_keys_from_seed("Alice").0, nonce: "1".as_bytes().to_vec(), }], robonode_public_key, @@ -130,7 +131,7 @@ pub fn development_config() -> Result { get_account_id_from_seed::("Bob//stash"), ], vec![pallet_bioauth::StoredAuthTicket { - public_key: authority_keys_from_seed("Alice"), + public_key: authority_keys_from_seed("Alice").0, nonce: "1".as_bytes().to_vec(), }], robonode_public_key, @@ -152,7 +153,7 @@ pub fn development_config() -> Result { /// Configure initial storage state for FRAME modules. fn testnet_genesis( wasm_binary: &[u8], - initial_authorities: Vec, + initial_authorities: Vec<(AuraId, GrandpaId)>, root_key: AccountId, endowed_accounts: Vec, stored_auth_tickets: Vec>, @@ -173,7 +174,13 @@ fn testnet_genesis( .collect(), }, aura: AuraConfig { - authorities: initial_authorities, + authorities: initial_authorities.iter().map(|x| (x.0.clone())).collect(), + }, + grandpa: GrandpaConfig { + authorities: initial_authorities + .iter() + .map(|x| (x.1.clone(), 1)) + .collect(), }, sudo: SudoConfig { // Assign network admin rights. diff --git a/crates/humanode-peer/src/service.rs b/crates/humanode-peer/src/service.rs index 2daa2b47b..7c3a03ad1 100644 --- a/crates/humanode-peer/src/service.rs +++ b/crates/humanode-peer/src/service.rs @@ -8,6 +8,7 @@ use sc_client_api::ExecutorProvider; use sc_consensus_aura::{ImportQueueParams, SlotDuration, SlotProportion, StartAuraParams}; use sc_executor::native_executor_instance; pub use sc_executor::NativeExecutor; +use sc_finality_grandpa::SharedVoterState; use sc_service::{Error as ServiceError, KeystoreContainer, PartialComponents, TaskManager}; use sp_consensus::SlotData; use sp_consensus_aura::sr25519::{AuthorityId as AuraId, AuthorityPair as AuraPair}; @@ -50,6 +51,13 @@ pub fn new_partial( sc_consensus::DefaultImportQueue, sc_transaction_pool::FullPool, ( + sc_finality_grandpa::GrandpaBlockImport< + FullBackend, + Block, + FullClient, + FullSelectChain, + >, + sc_finality_grandpa::LinkHalf, bioauth_consensus::BioauthBlockImport< FullBackend, Block, @@ -80,6 +88,14 @@ pub fn new_partial( ); let select_chain = sc_consensus::LongestChain::new(Arc::clone(&backend)); + + let (grandpa_block_import, grandpa_link) = sc_finality_grandpa::block_import( + Arc::clone(&client), + &(Arc::clone(&client) as Arc<_>), + select_chain.clone(), + None, + )?; + let bioauth_consensus_block_import: bioauth_consensus::BioauthBlockImport< FullBackend, _, @@ -98,7 +114,7 @@ pub fn new_partial( let import_queue = sc_consensus_aura::import_queue::(ImportQueueParams { block_import: bioauth_consensus_block_import.clone(), - justification_import: None, + justification_import: Some(Box::new(grandpa_block_import.clone())), client: Arc::clone(&client), create_inherent_data_providers: move |_, ()| async move { let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); @@ -129,6 +145,8 @@ pub fn new_partial( select_chain, transaction_pool, other: ( + grandpa_block_import, + grandpa_link, bioauth_consensus_block_import, slot_duration, raw_slot_duration, @@ -147,20 +165,41 @@ pub async fn new_full(config: Configuration) -> Result = None; + let prometheus_registry = config.prometheus_registry().cloned(); let proposer_factory = sc_basic_authorship::ProposerFactory::new( task_manager.spawn_handle(), @@ -179,7 +218,7 @@ pub async fn new_full(config: Configuration) -> Result Result Result>::Proof; + + type KeyOwnerIdentification = >::IdentificationTuple; + + type HandleEquivocation = (); + + type WeightInfo = (); +} + parameter_types! { pub const MinimumPeriod: u64 = SLOT_DURATION / 2; } @@ -356,6 +381,7 @@ construct_runtime!( TransactionPayment: pallet_transaction_payment::{Pallet, Storage}, Sudo: pallet_sudo::{Pallet, Call, Config, Storage, Event}, Bioauth: pallet_bioauth::{Pallet, Config, Call, Storage, Event, ValidateUnsigned}, + Grandpa: pallet_grandpa::{Pallet, Call, Storage, Config, Event}, } ); @@ -474,6 +500,36 @@ impl_runtime_apis! { } } + impl fg_primitives::GrandpaApi for Runtime { + fn grandpa_authorities() -> GrandpaAuthorityList { + Grandpa::grandpa_authorities() + } + + fn current_set_id() -> fg_primitives::SetId { + Grandpa::current_set_id() + } + + fn submit_report_equivocation_unsigned_extrinsic( + _equivocation_proof: fg_primitives::EquivocationProof< + ::Hash, + NumberFor, + >, + _key_owner_proof: fg_primitives::OpaqueKeyOwnershipProof, + ) -> Option<()> { + None + } + + fn generate_key_ownership_proof( + _set_id: fg_primitives::SetId, + _authority_id: GrandpaId, + ) -> Option { + // NOTE: this is the only implementation possible since we've + // defined our key owner proof type as a bottom type (i.e. a type + // with no values). + None + } + } + impl frame_system_rpc_runtime_api::AccountNonceApi for Runtime { fn account_nonce(account: AccountId) -> Index { System::account_nonce(account) From daf091ca34cea1819ee1532994ae9d76855e781b Mon Sep 17 00:00:00 2001 From: dmitrylavrenov Date: Tue, 14 Sep 2021 15:58:29 +0300 Subject: [PATCH 2/7] Integrate Grandpa consensus --- crates/bioauth-consensus/src/lib.rs | 30 ++++++++---- crates/bioauth-consensus/src/tests.rs | 66 +++++++++++++++++++++++++++ crates/humanode-peer/src/service.rs | 15 +++--- 3 files changed, 96 insertions(+), 15 deletions(-) diff --git a/crates/bioauth-consensus/src/lib.rs b/crates/bioauth-consensus/src/lib.rs index f509eb228..9b7ccae96 100644 --- a/crates/bioauth-consensus/src/lib.rs +++ b/crates/bioauth-consensus/src/lib.rs @@ -32,13 +32,15 @@ mod traits; pub use traits::*; /// A block-import handler for Bioauth. -pub struct BioauthBlockImport { +pub struct BioauthBlockImport { /// The client to interact with the chain. inner: Arc, /// The block author extractor. block_author_extractor: BAX, /// The bioauth auhtrization verifier. authorization_verifier: AV, + /// The finality consensus object. + finality_consensus: FC, /// A phantom data for Backend. _phantom_back_end: PhantomData, /// A phantom data for Block. @@ -63,9 +65,14 @@ where AuthorizationVerifier(AV), } -impl BioauthBlockImport { +impl BioauthBlockImport { /// Simple constructor. - pub fn new(inner: Arc, block_author_extractor: BAX, authorization_verifier: AV) -> Self + pub fn new( + inner: Arc, + block_author_extractor: BAX, + authorization_verifier: AV, + finality_consensus: FC, + ) -> Self where BE: Backend + 'static, { @@ -73,22 +80,26 @@ impl BioauthBlockImport Clone for BioauthBlockImport +impl Clone + for BioauthBlockImport where BAX: Clone, AV: Clone, + FC: Clone, { fn clone(&self) -> Self { Self { inner: Arc::clone(&self.inner), block_author_extractor: self.block_author_extractor.clone(), authorization_verifier: self.authorization_verifier.clone(), + finality_consensus: self.finality_consensus.clone(), _phantom_back_end: PhantomData, _phantom_block: PhantomData, } @@ -96,8 +107,8 @@ where } #[async_trait::async_trait] -impl BlockImport - for BioauthBlockImport +impl BlockImport + for BioauthBlockImport where Client: HeaderBackend + ProvideRuntimeApi + Send + Sync + Finalizer, for<'a> &'a Client: @@ -109,6 +120,9 @@ where ::Error: std::error::Error + Send + Sync + 'static, ::Error: std::error::Error + Send + Sync + 'static, BE: Backend, + FC: Send + + Sync + + BlockImport>, { type Error = ConsensusError; @@ -150,8 +164,8 @@ where return Err(mkerr(BioauthBlockImportError::NotBioauthAuthorized)); } - // Import a new block. - self.inner.import_block(block, cache).await + // Import a new block and apply finality with Grandpa. + self.finality_consensus.import_block(block, cache).await } } diff --git a/crates/bioauth-consensus/src/tests.rs b/crates/bioauth-consensus/src/tests.rs index af09b2f0a..128aeb64f 100644 --- a/crates/bioauth-consensus/src/tests.rs +++ b/crates/bioauth-consensus/src/tests.rs @@ -88,6 +88,27 @@ mock! { } } +mock! { + FinalityConsensus {} + + #[async_trait::async_trait] + impl BlockImport for FinalityConsensus { + type Error = ConsensusError; + type Transaction = TransactionFor; + + async fn check_block( + &mut self, + block: BlockCheckParams, + ) -> Result; + + async fn import_block( + &mut self, + block: BlockImportParams>, + cache: HashMap>, + ) -> Result; + } +} + // mockall doesn't allow implement trait for references inside mock #[async_trait::async_trait] impl<'a> BlockImport for &'a MockClient { @@ -233,16 +254,25 @@ async fn it_denies_block_import_with_error_extract_authorities() { let client = Arc::new(mock_client); + let mut mock_finality_consensus = MockFinalityConsensus::new(); + mock_finality_consensus + .expect_import_block() + .returning(|_, _| Ok(ImportResult::Imported(Default::default()))); + + let finality_consensus = mock_finality_consensus; + let mut bioauth_block_import: BioauthBlockImport< sc_service::TFullBackend, _, MockClient, _, _, + MockFinalityConsensus, > = BioauthBlockImport::new( Arc::clone(&client), crate::aura::BlockAuthorExtractor::new(Arc::clone(&client)), crate::bioauth::AuthorizationVerifier::new(Arc::clone(&client)), + finality_consensus, ); let res = bioauth_block_import @@ -282,16 +312,25 @@ async fn it_denies_block_import_with_invalid_slot_number() { let client = Arc::new(mock_client); + let mut mock_finality_consensus = MockFinalityConsensus::new(); + mock_finality_consensus + .expect_import_block() + .returning(|_, _| Ok(ImportResult::Imported(Default::default()))); + + let finality_consensus = mock_finality_consensus; + let mut bioauth_block_import: BioauthBlockImport< sc_service::TFullBackend, _, MockClient, _, _, + MockFinalityConsensus, > = BioauthBlockImport::new( Arc::clone(&client), crate::aura::BlockAuthorExtractor::new(Arc::clone(&client)), crate::bioauth::AuthorizationVerifier::new(Arc::clone(&client)), + finality_consensus, ); let res = bioauth_block_import @@ -335,16 +374,25 @@ async fn it_denies_block_import_with_error_extract_stored_auth_ticket() { let client = Arc::new(mock_client); + let mut mock_finality_consensus = MockFinalityConsensus::new(); + mock_finality_consensus + .expect_import_block() + .returning(|_, _| Ok(ImportResult::Imported(Default::default()))); + + let finality_consensus = mock_finality_consensus; + let mut bioauth_block_import: BioauthBlockImport< sc_service::TFullBackend, _, MockClient, _, _, + MockFinalityConsensus, > = BioauthBlockImport::new( Arc::clone(&client), crate::aura::BlockAuthorExtractor::new(Arc::clone(&client)), crate::bioauth::AuthorizationVerifier::new(Arc::clone(&client)), + finality_consensus, ); let res = bioauth_block_import @@ -398,16 +446,25 @@ async fn it_denies_block_import_with_not_bioauth_authorized() { let client = Arc::new(mock_client); + let mut mock_finality_consensus = MockFinalityConsensus::new(); + mock_finality_consensus + .expect_import_block() + .returning(|_, _| Ok(ImportResult::Imported(Default::default()))); + + let finality_consensus = mock_finality_consensus; + let mut bioauth_block_import: BioauthBlockImport< sc_service::TFullBackend, _, MockClient, _, _, + MockFinalityConsensus, > = BioauthBlockImport::new( Arc::clone(&client), crate::aura::BlockAuthorExtractor::new(Arc::clone(&client)), crate::bioauth::AuthorizationVerifier::new(Arc::clone(&client)), + finality_consensus, ); let res = bioauth_block_import @@ -467,16 +524,25 @@ async fn it_permits_block_import_with_valid_data() { let client = Arc::new(mock_client); + let mut mock_finality_consensus = MockFinalityConsensus::new(); + mock_finality_consensus + .expect_import_block() + .returning(|_, _| Ok(ImportResult::Imported(Default::default()))); + + let finality_consensus = mock_finality_consensus; + let mut bioauth_block_import: BioauthBlockImport< sc_service::TFullBackend, _, MockClient, _, _, + _, > = BioauthBlockImport::new( Arc::clone(&client), crate::aura::BlockAuthorExtractor::new(Arc::clone(&client)), crate::bioauth::AuthorizationVerifier::new(Arc::clone(&client)), + finality_consensus, ); let res = bioauth_block_import diff --git a/crates/humanode-peer/src/service.rs b/crates/humanode-peer/src/service.rs index 96269c82c..70ea02b57 100644 --- a/crates/humanode-peer/src/service.rs +++ b/crates/humanode-peer/src/service.rs @@ -30,6 +30,9 @@ type FullClient = sc_service::TFullClient; type FullBackend = sc_service::TFullBackend; /// Full node select chain type. type FullSelectChain = sc_consensus::LongestChain; +/// Full node Grandpa type. +type FullGrandpa = + sc_finality_grandpa::GrandpaBlockImport; /// Construct a bare keystore from the configuration. pub fn keystore_container( @@ -51,12 +54,7 @@ pub fn new_partial( sc_consensus::DefaultImportQueue, sc_transaction_pool::FullPool, ( - sc_finality_grandpa::GrandpaBlockImport< - FullBackend, - Block, - FullClient, - FullSelectChain, - >, + FullGrandpa, sc_finality_grandpa::LinkHalf, bioauth_consensus::BioauthBlockImport< FullBackend, @@ -64,6 +62,7 @@ pub fn new_partial( FullClient, bioauth_consensus::aura::BlockAuthorExtractor, bioauth_consensus::bioauth::AuthorizationVerifier, + FullGrandpa, >, SlotDuration, Duration, @@ -102,10 +101,12 @@ pub fn new_partial( _, _, _, + _, > = bioauth_consensus::BioauthBlockImport::new( Arc::clone(&client), bioauth_consensus::aura::BlockAuthorExtractor::new(Arc::clone(&client)), bioauth_consensus::bioauth::AuthorizationVerifier::new(Arc::clone(&client)), + grandpa_block_import.clone(), ); let slot_duration = sc_consensus_aura::slot_duration(&*client)?; @@ -167,7 +168,7 @@ pub async fn new_full(config: Configuration) -> Result Date: Wed, 15 Sep 2021 12:19:07 +0300 Subject: [PATCH 3/7] Add telemetry to grandpa --- crates/humanode-peer/src/service.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/humanode-peer/src/service.rs b/crates/humanode-peer/src/service.rs index be2a43b58..00607ef9b 100644 --- a/crates/humanode-peer/src/service.rs +++ b/crates/humanode-peer/src/service.rs @@ -113,7 +113,7 @@ pub fn new_partial( Arc::clone(&client), &(Arc::clone(&client) as Arc<_>), select_chain.clone(), - None, + telemetry.as_ref().map(|x| x.handle()), )?; let bioauth_consensus_block_import: bioauth_consensus::BioauthBlockImport< From e9e70579c943a881e014026aac8793e75b0a8182 Mon Sep 17 00:00:00 2001 From: dmitrylavrenov Date: Wed, 15 Sep 2021 14:34:20 +0300 Subject: [PATCH 4/7] Separate Client generic into Client and BI(BlockImport) --- crates/bioauth-consensus/src/lib.rs | 46 ++++++------ crates/bioauth-consensus/src/tests.rs | 101 +++++++------------------- crates/humanode-peer/src/service.rs | 4 +- 3 files changed, 51 insertions(+), 100 deletions(-) diff --git a/crates/bioauth-consensus/src/lib.rs b/crates/bioauth-consensus/src/lib.rs index 9b7ccae96..3ec88372a 100644 --- a/crates/bioauth-consensus/src/lib.rs +++ b/crates/bioauth-consensus/src/lib.rs @@ -8,7 +8,7 @@ use futures::future; use futures::future::FutureExt; -use sc_client_api::{backend::Backend, Finalizer}; +use sc_client_api::backend::Backend; use sc_consensus::{BlockCheckParams, BlockImport, BlockImportParams, ImportResult}; use sp_api::{HeaderT, ProvideRuntimeApi, TransactionFor}; use sp_blockchain::{well_known_cache_keys, HeaderBackend}; @@ -32,15 +32,15 @@ mod traits; pub use traits::*; /// A block-import handler for Bioauth. -pub struct BioauthBlockImport { +pub struct BioauthBlockImport { /// The client to interact with the chain. - inner: Arc, + client: Arc, + /// The inner block import to wrap. + inner: BI, /// The block author extractor. block_author_extractor: BAX, /// The bioauth auhtrization verifier. authorization_verifier: AV, - /// The finality consensus object. - finality_consensus: FC, /// A phantom data for Backend. _phantom_back_end: PhantomData, /// A phantom data for Block. @@ -65,41 +65,41 @@ where AuthorizationVerifier(AV), } -impl BioauthBlockImport { +impl BioauthBlockImport { /// Simple constructor. pub fn new( - inner: Arc, + client: Arc, + inner: BI, block_author_extractor: BAX, authorization_verifier: AV, - finality_consensus: FC, ) -> Self where BE: Backend + 'static, { Self { + client, inner, block_author_extractor, authorization_verifier, - finality_consensus, _phantom_back_end: PhantomData, _phantom_block: PhantomData, } } } -impl Clone - for BioauthBlockImport +impl Clone + for BioauthBlockImport where + BI: Clone, BAX: Clone, AV: Clone, - FC: Clone, { fn clone(&self) -> Self { Self { - inner: Arc::clone(&self.inner), + client: Arc::clone(&self.client), + inner: self.inner.clone(), block_author_extractor: self.block_author_extractor.clone(), authorization_verifier: self.authorization_verifier.clone(), - finality_consensus: self.finality_consensus.clone(), _phantom_back_end: PhantomData, _phantom_block: PhantomData, } @@ -107,22 +107,20 @@ where } #[async_trait::async_trait] -impl BlockImport - for BioauthBlockImport +impl BlockImport + for BioauthBlockImport where - Client: HeaderBackend + ProvideRuntimeApi + Send + Sync + Finalizer, - for<'a> &'a Client: - BlockImport>, + Client: HeaderBackend + ProvideRuntimeApi + Send + Sync, TransactionFor: 'static, + BI: BlockImport> + + Send + + Sync, BAX: BlockAuthorExtractor + Send, AV: AuthorizationVerifier + Send, ::PublicKeyType: Send + Sync, ::Error: std::error::Error + Send + Sync + 'static, ::Error: std::error::Error + Send + Sync + 'static, BE: Backend, - FC: Send - + Sync - + BlockImport>, { type Error = ConsensusError; @@ -144,7 +142,7 @@ where cache: HashMap>, ) -> Result { // Extract a number of the last imported block. - let at = sp_api::BlockId::Hash(self.inner.info().best_hash); + let at = sp_api::BlockId::Hash(self.client.info().best_hash); let mkerr = |err: BioauthBlockImportError| -> ConsensusError { ConsensusError::Other(Box::new(err)) @@ -165,7 +163,7 @@ where } // Import a new block and apply finality with Grandpa. - self.finality_consensus.import_block(block, cache).await + self.inner.import_block(block, cache).await } } diff --git a/crates/bioauth-consensus/src/tests.rs b/crates/bioauth-consensus/src/tests.rs index 128aeb64f..707fe77fc 100644 --- a/crates/bioauth-consensus/src/tests.rs +++ b/crates/bioauth-consensus/src/tests.rs @@ -2,7 +2,6 @@ use mockall::predicate::*; use mockall::*; use node_primitives::{Block, BlockNumber, Hash, Header}; use pallet_bioauth::{self, BioauthApi, StoredAuthTicket}; -use sc_client_api::Finalizer; use sc_consensus::{BlockCheckParams, BlockImport, BlockImportParams, ImportResult}; use sp_api::{ApiError, ApiRef, NativeOrEncoded, ProvideRuntimeApi, TransactionFor}; use sp_application_crypto::Pair; @@ -67,32 +66,13 @@ mock! { fn number(&self, _hash: Hash) -> sc_service::Result, sp_blockchain::Error>; fn hash(&self, _number: sp_api::NumberFor) -> sp_blockchain::Result>; } - - impl Finalizer> for Client { - fn apply_finality( - &self, - _operation: &mut sc_client_api::ClientImportOperation< - Block, - sc_service::TFullBackend, - >, - _id: sp_api::BlockId, - _justification: Option, - _notify: bool, - ) -> sp_blockchain::Result<()>; - fn finalize_block( - &self, - _id: sp_api::BlockId, - _justification: Option, - _notify: bool, - ) -> sp_blockchain::Result<()>; - } } mock! { - FinalityConsensus {} + BlockImportWrapper {} #[async_trait::async_trait] - impl BlockImport for FinalityConsensus { + impl BlockImport for BlockImportWrapper { type Error = ConsensusError; type Transaction = TransactionFor; @@ -109,29 +89,6 @@ mock! { } } -// mockall doesn't allow implement trait for references inside mock -#[async_trait::async_trait] -impl<'a> BlockImport for &'a MockClient { - type Error = ConsensusError; - - type Transaction = TransactionFor; - - async fn check_block( - &mut self, - block: BlockCheckParams, - ) -> Result { - (**self).check_block(block).await - } - - async fn import_block( - &mut self, - block: BlockImportParams>, - cache: HashMap>, - ) -> Result { - (**self).import_block(block, cache).await - } -} - sp_api::mock_impl_runtime_apis! { impl BioauthApi for MockWrapperRuntimeApi { #[advanced] @@ -254,25 +211,25 @@ async fn it_denies_block_import_with_error_extract_authorities() { let client = Arc::new(mock_client); - let mut mock_finality_consensus = MockFinalityConsensus::new(); - mock_finality_consensus + let mut mock_block_import = MockBlockImportWrapper::new(); + mock_block_import .expect_import_block() .returning(|_, _| Ok(ImportResult::Imported(Default::default()))); - let finality_consensus = mock_finality_consensus; + let block_import = mock_block_import; let mut bioauth_block_import: BioauthBlockImport< sc_service::TFullBackend, _, MockClient, + MockBlockImportWrapper, _, _, - MockFinalityConsensus, > = BioauthBlockImport::new( Arc::clone(&client), + block_import, crate::aura::BlockAuthorExtractor::new(Arc::clone(&client)), crate::bioauth::AuthorizationVerifier::new(Arc::clone(&client)), - finality_consensus, ); let res = bioauth_block_import @@ -312,25 +269,25 @@ async fn it_denies_block_import_with_invalid_slot_number() { let client = Arc::new(mock_client); - let mut mock_finality_consensus = MockFinalityConsensus::new(); - mock_finality_consensus + let mut mock_block_import = MockBlockImportWrapper::new(); + mock_block_import .expect_import_block() .returning(|_, _| Ok(ImportResult::Imported(Default::default()))); - let finality_consensus = mock_finality_consensus; + let block_import = mock_block_import; let mut bioauth_block_import: BioauthBlockImport< sc_service::TFullBackend, _, MockClient, + MockBlockImportWrapper, _, _, - MockFinalityConsensus, > = BioauthBlockImport::new( Arc::clone(&client), + block_import, crate::aura::BlockAuthorExtractor::new(Arc::clone(&client)), crate::bioauth::AuthorizationVerifier::new(Arc::clone(&client)), - finality_consensus, ); let res = bioauth_block_import @@ -374,25 +331,25 @@ async fn it_denies_block_import_with_error_extract_stored_auth_ticket() { let client = Arc::new(mock_client); - let mut mock_finality_consensus = MockFinalityConsensus::new(); - mock_finality_consensus + let mut mock_block_import = MockBlockImportWrapper::new(); + mock_block_import .expect_import_block() .returning(|_, _| Ok(ImportResult::Imported(Default::default()))); - let finality_consensus = mock_finality_consensus; + let block_import = mock_block_import; let mut bioauth_block_import: BioauthBlockImport< sc_service::TFullBackend, _, MockClient, + MockBlockImportWrapper, _, _, - MockFinalityConsensus, > = BioauthBlockImport::new( Arc::clone(&client), + block_import, crate::aura::BlockAuthorExtractor::new(Arc::clone(&client)), crate::bioauth::AuthorizationVerifier::new(Arc::clone(&client)), - finality_consensus, ); let res = bioauth_block_import @@ -446,25 +403,25 @@ async fn it_denies_block_import_with_not_bioauth_authorized() { let client = Arc::new(mock_client); - let mut mock_finality_consensus = MockFinalityConsensus::new(); - mock_finality_consensus + let mut mock_block_import = MockBlockImportWrapper::new(); + mock_block_import .expect_import_block() .returning(|_, _| Ok(ImportResult::Imported(Default::default()))); - let finality_consensus = mock_finality_consensus; + let block_import = mock_block_import; let mut bioauth_block_import: BioauthBlockImport< sc_service::TFullBackend, _, MockClient, + MockBlockImportWrapper, _, _, - MockFinalityConsensus, > = BioauthBlockImport::new( Arc::clone(&client), + block_import, crate::aura::BlockAuthorExtractor::new(Arc::clone(&client)), crate::bioauth::AuthorizationVerifier::new(Arc::clone(&client)), - finality_consensus, ); let res = bioauth_block_import @@ -510,10 +467,6 @@ async fn it_permits_block_import_with_valid_data() { let runtime_api = MockWrapperRuntimeApi(Arc::new(mock_runtime_api)); - mock_client - .expect_finalize_block() - .returning(|_, _, _| Ok(())); - mock_client .expect_import_block() .returning(|_, _| Ok(ImportResult::imported(Default::default()))); @@ -524,25 +477,25 @@ async fn it_permits_block_import_with_valid_data() { let client = Arc::new(mock_client); - let mut mock_finality_consensus = MockFinalityConsensus::new(); - mock_finality_consensus + let mut mock_block_import = MockBlockImportWrapper::new(); + mock_block_import .expect_import_block() .returning(|_, _| Ok(ImportResult::Imported(Default::default()))); - let finality_consensus = mock_finality_consensus; + let block_import = mock_block_import; let mut bioauth_block_import: BioauthBlockImport< sc_service::TFullBackend, _, MockClient, - _, + MockBlockImportWrapper, _, _, > = BioauthBlockImport::new( Arc::clone(&client), + block_import, crate::aura::BlockAuthorExtractor::new(Arc::clone(&client)), crate::bioauth::AuthorizationVerifier::new(Arc::clone(&client)), - finality_consensus, ); let res = bioauth_block_import diff --git a/crates/humanode-peer/src/service.rs b/crates/humanode-peer/src/service.rs index 00607ef9b..58cc5929e 100644 --- a/crates/humanode-peer/src/service.rs +++ b/crates/humanode-peer/src/service.rs @@ -61,9 +61,9 @@ pub fn new_partial( FullBackend, Block, FullClient, + FullGrandpa, bioauth_consensus::aura::BlockAuthorExtractor, bioauth_consensus::bioauth::AuthorizationVerifier, - FullGrandpa, >, SlotDuration, Duration, @@ -125,9 +125,9 @@ pub fn new_partial( _, > = bioauth_consensus::BioauthBlockImport::new( Arc::clone(&client), + grandpa_block_import.clone(), bioauth_consensus::aura::BlockAuthorExtractor::new(Arc::clone(&client)), bioauth_consensus::bioauth::AuthorizationVerifier::new(Arc::clone(&client)), - grandpa_block_import.clone(), ); let slot_duration = sc_consensus_aura::slot_duration(&*client)?; From c73b9c895540a852a31b3bf73a190b44764e4370 Mon Sep 17 00:00:00 2001 From: dmitrylavrenov Date: Wed, 15 Sep 2021 14:50:06 +0300 Subject: [PATCH 5/7] Add optional Grandpa usage --- crates/humanode-peer/src/service.rs | 31 ++++++++++++++++------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/crates/humanode-peer/src/service.rs b/crates/humanode-peer/src/service.rs index 58cc5929e..01e7bbf05 100644 --- a/crates/humanode-peer/src/service.rs +++ b/crates/humanode-peer/src/service.rs @@ -221,6 +221,7 @@ pub async fn new_full(config: Configuration) -> Result = None; let prometheus_registry = config.prometheus_registry().cloned(); @@ -342,20 +343,22 @@ pub async fn new_full(config: Configuration) -> Result Date: Wed, 15 Sep 2021 17:15:08 +0300 Subject: [PATCH 6/7] Remove redundant methods at MockClient in tests --- crates/bioauth-consensus/src/tests.rs | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/crates/bioauth-consensus/src/tests.rs b/crates/bioauth-consensus/src/tests.rs index 707fe77fc..b144b2169 100644 --- a/crates/bioauth-consensus/src/tests.rs +++ b/crates/bioauth-consensus/src/tests.rs @@ -40,18 +40,7 @@ struct MockWrapperRuntimeApi(Arc); mock! { #[derive(Debug)] - Client { - async fn check_block( - &self, - block: BlockCheckParams, - ) -> Result; - - async fn import_block( - &self, - block: BlockImportParams>, - cache: HashMap>, - ) -> Result; - } + Client {} impl ProvideRuntimeApi for Client { type Api = MockWrapperRuntimeApi; @@ -467,10 +456,6 @@ async fn it_permits_block_import_with_valid_data() { let runtime_api = MockWrapperRuntimeApi(Arc::new(mock_runtime_api)); - mock_client - .expect_import_block() - .returning(|_, _| Ok(ImportResult::imported(Default::default()))); - mock_client .expect_runtime_api() .returning(move || runtime_api.clone().into()); From 9dd326741644795a354f241efa091a86d944f9f3 Mon Sep 17 00:00:00 2001 From: dmitrylavrenov Date: Wed, 15 Sep 2021 17:19:42 +0300 Subject: [PATCH 7/7] Add reference to FIXME of Substrate code. --- crates/humanode-peer/src/service.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/humanode-peer/src/service.rs b/crates/humanode-peer/src/service.rs index 01e7bbf05..17d8234a4 100644 --- a/crates/humanode-peer/src/service.rs +++ b/crates/humanode-peer/src/service.rs @@ -333,7 +333,8 @@ pub async fn new_full(config: Configuration) -> Result