From 0cf105717a55438e94f9c5027ad60a74441bd95b Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Tue, 6 Sep 2022 08:58:17 +0300 Subject: [PATCH 01/17] Move `ExHashT` to sc-network-common --- client/network/common/src/lib.rs | 6 ++++++ client/network/src/config.rs | 3 ++- client/network/src/lib.rs | 6 ------ client/network/src/service.rs | 3 ++- client/network/src/transactions.rs | 2 +- client/service/src/lib.rs | 2 +- 6 files changed, 12 insertions(+), 10 deletions(-) diff --git a/client/network/common/src/lib.rs b/client/network/common/src/lib.rs index 3a30d24900199..882d3f294d0a0 100644 --- a/client/network/common/src/lib.rs +++ b/client/network/common/src/lib.rs @@ -24,3 +24,9 @@ pub mod protocol; pub mod request_responses; pub mod service; pub mod sync; + +/// Minimum Requirements for a Hash within Networking +pub trait ExHashT: std::hash::Hash + Eq + std::fmt::Debug + Clone + Send + Sync + 'static {} + +impl ExHashT for T where T: std::hash::Hash + Eq + std::fmt::Debug + Clone + Send + Sync + 'static +{} diff --git a/client/network/src/config.rs b/client/network/src/config.rs index 3fe95deb0c1ed..79e65bc191225 100644 --- a/client/network/src/config.rs +++ b/client/network/src/config.rs @@ -27,11 +27,12 @@ pub use sc_network_common::{ IncomingRequest, OutgoingResponse, ProtocolConfig as RequestResponseConfig, }, sync::warp::WarpSyncProvider, + ExHashT, }; pub use libp2p::{build_multiaddr, core::PublicKey, identity}; -use crate::{bitswap::Bitswap, ExHashT}; +use crate::bitswap::Bitswap; use core::{fmt, iter}; use futures::future; diff --git a/client/network/src/lib.rs b/client/network/src/lib.rs index 35a91e77da524..21c7b1424035f 100644 --- a/client/network/src/lib.rs +++ b/client/network/src/lib.rs @@ -297,9 +297,3 @@ const MAX_CONNECTIONS_PER_PEER: usize = 2; /// The maximum number of concurrent established connections that were incoming. const MAX_CONNECTIONS_ESTABLISHED_INCOMING: u32 = 10_000; - -/// Minimum Requirements for a Hash within Networking -pub trait ExHashT: std::hash::Hash + Eq + std::fmt::Debug + Clone + Send + Sync + 'static {} - -impl ExHashT for T where T: std::hash::Hash + Eq + std::fmt::Debug + Clone + Send + Sync + 'static -{} diff --git a/client/network/src/service.rs b/client/network/src/service.rs index 4610b025dde0d..de5f01f366159 100644 --- a/client/network/src/service.rs +++ b/client/network/src/service.rs @@ -39,7 +39,7 @@ use crate::{ self, message::generic::Roles, NotificationsSink, NotifsHandlerError, PeerInfo, Protocol, Ready, }, - transactions, transport, ExHashT, ReputationChange, + transactions, transport, ReputationChange, }; use codec::Encode as _; @@ -60,6 +60,7 @@ use metrics::{Histogram, HistogramVec, MetricSources, Metrics}; use parking_lot::Mutex; use sc_consensus::{BlockImportError, BlockImportStatus, ImportQueue, Link}; use sc_network_common::{ + ExHashT, config::MultiaddrWithPeerId, protocol::{ event::{DhtEvent, Event}, diff --git a/client/network/src/transactions.rs b/client/network/src/transactions.rs index 1cf532f33ddc6..dfbd09e0dfd6d 100644 --- a/client/network/src/transactions.rs +++ b/client/network/src/transactions.rs @@ -32,7 +32,6 @@ use crate::{ protocol::message, service::NetworkService, utils::{interval, LruHashSet}, - ExHashT, }; use codec::{Decode, Encode}; @@ -41,6 +40,7 @@ use libp2p::{multiaddr, PeerId}; use log::{debug, trace, warn}; use prometheus_endpoint::{register, Counter, PrometheusError, Registry, U64}; use sc_network_common::{ + ExHashT, config::ProtocolId, protocol::{ event::{Event, ObservedRole}, diff --git a/client/service/src/lib.rs b/client/service/src/lib.rs index 19358c1e5bc4c..1c723469e4cc0 100644 --- a/client/service/src/lib.rs +++ b/client/service/src/lib.rs @@ -148,7 +148,7 @@ async fn build_network_future< + Send + Sync + 'static, - H: sc_network::ExHashT, + H: sc_network_common::ExHashT, >( role: Role, mut network: sc_network::NetworkWorker, From 6716646d80abc671b1d6cf52e902518e097a8bac Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Tue, 6 Sep 2022 09:25:04 +0300 Subject: [PATCH 02/17] Introduce sc-network-transactions Introduce a separate crate for the transaction protocol and move transaction configuration there. --- Cargo.lock | 12 ++++ Cargo.toml | 1 + client/network/Cargo.toml | 1 + client/network/src/config.rs | 63 +---------------- client/network/src/service/tests.rs | 3 +- client/network/src/transactions.rs | 3 +- client/network/test/Cargo.toml | 1 + client/network/test/src/lib.rs | 2 +- client/network/transactions/Cargo.toml | 19 +++++ client/network/transactions/src/config.rs | 84 +++++++++++++++++++++++ client/network/transactions/src/lib.rs | 19 +++++ client/service/Cargo.toml | 1 + client/service/src/lib.rs | 4 +- 13 files changed, 146 insertions(+), 67 deletions(-) create mode 100644 client/network/transactions/Cargo.toml create mode 100644 client/network/transactions/src/config.rs create mode 100644 client/network/transactions/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index f6c2df81d6ee1..8c604f8750694 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8417,6 +8417,7 @@ dependencies = [ "sc-network-common", "sc-network-light", "sc-network-sync", + "sc-network-transactions", "sc-peerset", "sc-utils", "serde", @@ -8552,6 +8553,7 @@ dependencies = [ "sc-network-common", "sc-network-light", "sc-network-sync", + "sc-network-transactions", "sc-service", "sp-blockchain", "sp-consensus", @@ -8563,6 +8565,15 @@ dependencies = [ "substrate-test-runtime-client", ] +[[package]] +name = "sc-network-transactions" +version = "0.10.0-dev" +dependencies = [ + "futures", + "sc-network-common", + "sp-runtime", +] + [[package]] name = "sc-offchain" version = "4.0.0-dev" @@ -8739,6 +8750,7 @@ dependencies = [ "sc-network-common", "sc-network-light", "sc-network-sync", + "sc-network-transactions", "sc-offchain", "sc-rpc", "sc-rpc-server", diff --git a/Cargo.toml b/Cargo.toml index e2907716ca9f2..4112783bc9559 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -47,6 +47,7 @@ members = [ "client/network/light", "client/network/sync", "client/network/test", + "client/network/transactions", "client/offchain", "client/peerset", "client/allocator", diff --git a/client/network/Cargo.toml b/client/network/Cargo.toml index 82f77fa44450f..a775b120b5497 100644 --- a/client/network/Cargo.toml +++ b/client/network/Cargo.toml @@ -51,6 +51,7 @@ sc-block-builder = { version = "0.10.0-dev", path = "../block-builder" } sc-client-api = { version = "4.0.0-dev", path = "../api" } sc-consensus = { version = "0.10.0-dev", path = "../consensus/common" } sc-network-common = { version = "0.10.0-dev", path = "./common" } +sc-network-transactions = { version = "0.10.0-dev", path = "./transactions" } sc-peerset = { version = "4.0.0-dev", path = "../peerset" } sc-utils = { version = "4.0.0-dev", path = "../utils" } sp-arithmetic = { version = "5.0.0", path = "../../primitives/arithmetic" } diff --git a/client/network/src/config.rs b/client/network/src/config.rs index 79e65bc191225..6261f340bd370 100644 --- a/client/network/src/config.rs +++ b/client/network/src/config.rs @@ -35,7 +35,6 @@ pub use libp2p::{build_multiaddr, core::PublicKey, identity}; use crate::bitswap::Bitswap; use core::{fmt, iter}; -use futures::future; use libp2p::{ identity::{ed25519, Keypair}, multiaddr, Multiaddr, @@ -43,9 +42,9 @@ use libp2p::{ use prometheus_endpoint::Registry; use sc_consensus::ImportQueue; use sc_network_common::{config::MultiaddrWithPeerId, protocol::ProtocolName, sync::ChainSync}; +use sc_network_transactions::config::TransactionPool; use sp_runtime::traits::Block as BlockT; use std::{ - collections::HashMap, error::Error, fs, future::Future, @@ -167,66 +166,6 @@ impl fmt::Display for Role { } } -/// Result of the transaction import. -#[derive(Clone, Copy, Debug)] -pub enum TransactionImport { - /// Transaction is good but already known by the transaction pool. - KnownGood, - /// Transaction is good and not yet known. - NewGood, - /// Transaction is invalid. - Bad, - /// Transaction import was not performed. - None, -} - -/// Future resolving to transaction import result. -pub type TransactionImportFuture = Pin + Send>>; - -/// Transaction pool interface -pub trait TransactionPool: Send + Sync { - /// Get transactions from the pool that are ready to be propagated. - fn transactions(&self) -> Vec<(H, B::Extrinsic)>; - /// Get hash of transaction. - fn hash_of(&self, transaction: &B::Extrinsic) -> H; - /// Import a transaction into the pool. - /// - /// This will return future. - fn import(&self, transaction: B::Extrinsic) -> TransactionImportFuture; - /// Notify the pool about transactions broadcast. - fn on_broadcasted(&self, propagations: HashMap>); - /// Get transaction by hash. - fn transaction(&self, hash: &H) -> Option; -} - -/// Dummy implementation of the [`TransactionPool`] trait for a transaction pool that is always -/// empty and discards all incoming transactions. -/// -/// Requires the "hash" type to implement the `Default` trait. -/// -/// Useful for testing purposes. -pub struct EmptyTransactionPool; - -impl TransactionPool for EmptyTransactionPool { - fn transactions(&self) -> Vec<(H, B::Extrinsic)> { - Vec::new() - } - - fn hash_of(&self, _transaction: &B::Extrinsic) -> H { - Default::default() - } - - fn import(&self, _transaction: B::Extrinsic) -> TransactionImportFuture { - Box::pin(future::ready(TransactionImport::KnownGood)) - } - - fn on_broadcasted(&self, _: HashMap>) {} - - fn transaction(&self, _h: &H) -> Option { - None - } -} - /// Sync operation mode. #[derive(Copy, Clone, Debug, Eq, PartialEq)] pub enum SyncMode { diff --git a/client/network/src/service/tests.rs b/client/network/src/service/tests.rs index 8770e14bf5338..3207f30ed475e 100644 --- a/client/network/src/service/tests.rs +++ b/client/network/src/service/tests.rs @@ -25,6 +25,7 @@ use sc_network_common::{ protocol::event::Event, service::{NetworkEventStream, NetworkNotification, NetworkPeers, NetworkStateInfo}, }; +use sc_network_transactions::config::EmptyTransactionPool; use sc_network_light::light_client_requests::handler::LightClientRequestHandler; use sc_network_sync::{ block_request_handler::BlockRequestHandler, state_request_handler::StateRequestHandler, @@ -140,7 +141,7 @@ fn build_test_full_node( }), network_config, chain: client.clone(), - transaction_pool: Arc::new(config::EmptyTransactionPool), + transaction_pool: Arc::new(EmptyTransactionPool), protocol_id, fork_id, import_queue, diff --git a/client/network/src/transactions.rs b/client/network/src/transactions.rs index dfbd09e0dfd6d..d9e1d37d97d15 100644 --- a/client/network/src/transactions.rs +++ b/client/network/src/transactions.rs @@ -27,7 +27,7 @@ //! `Future` that processes transactions. use crate::{ - config::{self, TransactionImport, TransactionImportFuture, TransactionPool}, + config, error, protocol::message, service::NetworkService, @@ -48,6 +48,7 @@ use sc_network_common::{ }, service::{NetworkEventStream, NetworkNotification, NetworkPeers}, }; +use sc_network_transactions::config::{TransactionImport, TransactionImportFuture, TransactionPool}; use sp_runtime::traits::Block as BlockT; use std::{ collections::{hash_map::Entry, HashMap}, diff --git a/client/network/test/Cargo.toml b/client/network/test/Cargo.toml index 990129229a40f..4e57e5f6caba8 100644 --- a/client/network/test/Cargo.toml +++ b/client/network/test/Cargo.toml @@ -28,6 +28,7 @@ sc-network = { version = "0.10.0-dev", path = "../" } sc-network-common = { version = "0.10.0-dev", path = "../common" } sc-network-light = { version = "0.10.0-dev", path = "../light" } sc-network-sync = { version = "0.10.0-dev", path = "../sync" } +sc-network-transactions = { version = "0.10.0-dev", path = "../transactions" } sc-service = { version = "0.10.0-dev", default-features = false, features = ["test-helpers"], path = "../../service" } sp-blockchain = { version = "4.0.0-dev", path = "../../../primitives/blockchain" } sp-consensus = { version = "0.10.0-dev", path = "../../../primitives/consensus/common" } diff --git a/client/network/test/src/lib.rs b/client/network/test/src/lib.rs index 837cdeed0f3d1..f4f0e07e4f78a 100644 --- a/client/network/test/src/lib.rs +++ b/client/network/test/src/lib.rs @@ -47,7 +47,7 @@ use sc_consensus::{ ForkChoiceStrategy, ImportResult, JustificationImport, JustificationSyncLink, LongestChain, Verifier, }; -pub use sc_network::config::EmptyTransactionPool; +pub use sc_network_transactions::config::EmptyTransactionPool; use sc_network::{ config::{ NetworkConfiguration, NonDefaultSetConfig, NonReservedPeerMode, Role, SyncMode, diff --git a/client/network/transactions/Cargo.toml b/client/network/transactions/Cargo.toml new file mode 100644 index 0000000000000..89e9f8759a394 --- /dev/null +++ b/client/network/transactions/Cargo.toml @@ -0,0 +1,19 @@ +[package] +description = "Substrate transaction protocol" +name = "sc-network-transactions" +version = "0.10.0-dev" +license = "GPL-3.0-or-later WITH Classpath-exception-2.0" +authors = ["Parity Technologies "] +edition = "2021" +homepage = "https://substrate.io" +repository = "https://github.com/paritytech/substrate/" +documentation = "https://docs.rs/sc-network-transactions" +readme = "README.md" + +[package.metadata.docs.rs] +targets = ["x86_64-unknown-linux-gnu"] + +[dependencies] +futures = "0.3.21" +sc-network-common = { version = "0.10.0-dev", path = "../common" } +sp-runtime = { version = "6.0.0", path = "../../../primitives/runtime" } diff --git a/client/network/transactions/src/config.rs b/client/network/transactions/src/config.rs new file mode 100644 index 0000000000000..72b6e8c633ce4 --- /dev/null +++ b/client/network/transactions/src/config.rs @@ -0,0 +1,84 @@ +// This file is part of Substrate. + +// Copyright (C) 2017-2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 + +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +//! Configuration of the transaction protocol + +use futures::prelude::*; +use sc_network_common::ExHashT; +use sp_runtime::traits::Block as BlockT; +use std::{collections::HashMap, future::Future, pin::Pin}; + +/// Result of the transaction import. +#[derive(Clone, Copy, Debug)] +pub enum TransactionImport { + /// Transaction is good but already known by the transaction pool. + KnownGood, + /// Transaction is good and not yet known. + NewGood, + /// Transaction is invalid. + Bad, + /// Transaction import was not performed. + None, +} + +/// Future resolving to transaction import result. +pub type TransactionImportFuture = Pin + Send>>; + +/// Transaction pool interface +pub trait TransactionPool: Send + Sync { + /// Get transactions from the pool that are ready to be propagated. + fn transactions(&self) -> Vec<(H, B::Extrinsic)>; + /// Get hash of transaction. + fn hash_of(&self, transaction: &B::Extrinsic) -> H; + /// Import a transaction into the pool. + /// + /// This will return future. + fn import(&self, transaction: B::Extrinsic) -> TransactionImportFuture; + /// Notify the pool about transactions broadcast. + fn on_broadcasted(&self, propagations: HashMap>); + /// Get transaction by hash. + fn transaction(&self, hash: &H) -> Option; +} + +/// Dummy implementation of the [`TransactionPool`] trait for a transaction pool that is always +/// empty and discards all incoming transactions. +/// +/// Requires the "hash" type to implement the `Default` trait. +/// +/// Useful for testing purposes. +pub struct EmptyTransactionPool; + +impl TransactionPool for EmptyTransactionPool { + fn transactions(&self) -> Vec<(H, B::Extrinsic)> { + Vec::new() + } + + fn hash_of(&self, _transaction: &B::Extrinsic) -> H { + Default::default() + } + + fn import(&self, _transaction: B::Extrinsic) -> TransactionImportFuture { + Box::pin(future::ready(TransactionImport::KnownGood)) + } + + fn on_broadcasted(&self, _: HashMap>) {} + + fn transaction(&self, _h: &H) -> Option { + None + } +} diff --git a/client/network/transactions/src/lib.rs b/client/network/transactions/src/lib.rs new file mode 100644 index 0000000000000..3a9da47ce562f --- /dev/null +++ b/client/network/transactions/src/lib.rs @@ -0,0 +1,19 @@ +// This file is part of Substrate. + +// Copyright (C) 2017-2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 + +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +pub mod config; diff --git a/client/service/Cargo.toml b/client/service/Cargo.toml index 0acdbb1b1b63e..772eea2c951ea 100644 --- a/client/service/Cargo.toml +++ b/client/service/Cargo.toml @@ -54,6 +54,7 @@ sc-network = { version = "0.10.0-dev", path = "../network" } sc-network-common = { version = "0.10.0-dev", path = "../network/common" } sc-network-light = { version = "0.10.0-dev", path = "../network/light" } sc-network-sync = { version = "0.10.0-dev", path = "../network/sync" } +sc-network-transactions = { version = "0.10.0-dev", path = "../network/transactions" } sc-chain-spec = { version = "4.0.0-dev", path = "../chain-spec" } sc-client-api = { version = "4.0.0-dev", path = "../api" } sp-api = { version = "4.0.0-dev", path = "../../primitives/api" } diff --git a/client/service/src/lib.rs b/client/service/src/lib.rs index 1c723469e4cc0..9e1c19294f0b8 100644 --- a/client/service/src/lib.rs +++ b/client/service/src/lib.rs @@ -72,7 +72,7 @@ pub use sc_chain_spec::{ pub use sc_consensus::ImportQueue; pub use sc_executor::NativeExecutionDispatch; #[doc(hidden)] -pub use sc_network::config::{TransactionImport, TransactionImportFuture}; +pub use sc_network_transactions::config::{TransactionImport, TransactionImportFuture}; pub use sc_rpc::{ RandomIntegerSubscriptionId, RandomStringSubscriptionId, RpcSubscriptionIdProvider, }; @@ -415,7 +415,7 @@ where .collect() } -impl sc_network::config::TransactionPool for TransactionPoolAdapter +impl sc_network_transactions::config::TransactionPool for TransactionPoolAdapter where C: HeaderBackend + BlockBackend From 2b75534719e870b67410f4196fb5272578be4732 Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Tue, 6 Sep 2022 10:16:38 +0300 Subject: [PATCH 03/17] Move types from sc-network to sc-network-common Move `SetConfig`, `NonDefaultSetConfig`, `TransportConfig` and `NonReservedPeerMode` to sc-network-common::config, and move sc-network::{utils, error} to sc-network-common. --- Cargo.lock | 5 + client/beefy/Cargo.toml | 1 + client/beefy/src/lib.rs | 4 +- client/cli/Cargo.toml | 1 + client/cli/src/params/network_params.rs | 5 +- client/finality-grandpa/src/lib.rs | 12 +- client/network/common/Cargo.toml | 3 + client/network/common/src/config.rs | 129 ++++++++++++++++++++++ client/network/{ => common}/src/error.rs | 6 +- client/network/common/src/lib.rs | 2 + client/network/{ => common}/src/utils.rs | 0 client/network/src/config.rs | 133 +---------------------- client/network/src/discovery.rs | 6 +- client/network/src/lib.rs | 2 - client/network/src/peer_info.rs | 2 +- client/network/src/protocol.rs | 13 +-- client/network/src/service.rs | 6 +- client/network/src/service/tests.rs | 46 ++++---- client/network/src/transactions.rs | 14 +-- client/network/test/src/lib.rs | 7 +- client/service/src/config.rs | 6 +- client/service/src/error.rs | 3 +- client/service/test/src/lib.rs | 4 +- 23 files changed, 212 insertions(+), 198 deletions(-) rename client/network/{ => common}/src/error.rs (96%) rename client/network/{ => common}/src/utils.rs (100%) diff --git a/Cargo.lock b/Cargo.lock index 8c604f8750694..e99bbcc592f64 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -456,6 +456,7 @@ dependencies = [ "sc-finality-grandpa", "sc-keystore", "sc-network", + "sc-network-common", "sc-network-gossip", "sc-network-test", "sc-utils", @@ -7856,6 +7857,7 @@ dependencies = [ "sc-client-db", "sc-keystore", "sc-network", + "sc-network-common", "sc-service", "sc-telemetry", "sc-tracing", @@ -8448,7 +8450,9 @@ dependencies = [ "bitflags", "bytes", "futures", + "futures-timer", "libp2p", + "linked_hash_set", "parity-scale-codec", "prost-build", "sc-consensus", @@ -8459,6 +8463,7 @@ dependencies = [ "sp-consensus", "sp-finality-grandpa", "sp-runtime", + "substrate-prometheus-endpoint", "thiserror", ] diff --git a/client/beefy/Cargo.toml b/client/beefy/Cargo.toml index 5cc2952e26ba5..a634b91f2113c 100644 --- a/client/beefy/Cargo.toml +++ b/client/beefy/Cargo.toml @@ -27,6 +27,7 @@ sc-consensus = { version = "0.10.0-dev", path = "../consensus/common" } sc-finality-grandpa = { version = "0.10.0-dev", path = "../../client/finality-grandpa" } sc-keystore = { version = "4.0.0-dev", path = "../keystore" } sc-network = { version = "0.10.0-dev", path = "../network" } +sc-network-common = { version = "0.10.0-dev", path = "../network/common" } sc-network-gossip = { version = "0.10.0-dev", path = "../network-gossip" } sc-utils = { version = "4.0.0-dev", path = "../utils" } sp-api = { version = "4.0.0-dev", path = "../../primitives/api" } diff --git a/client/beefy/src/lib.rs b/client/beefy/src/lib.rs index cdb7fca10320d..b5b3ece1d508d 100644 --- a/client/beefy/src/lib.rs +++ b/client/beefy/src/lib.rs @@ -82,8 +82,8 @@ pub(crate) mod beefy_protocol_name { /// For standard protocol name see [`beefy_protocol_name::standard_name`]. pub fn beefy_peers_set_config( protocol_name: ProtocolName, -) -> sc_network::config::NonDefaultSetConfig { - let mut cfg = sc_network::config::NonDefaultSetConfig::new(protocol_name, 1024 * 1024); +) -> sc_network_common::config::NonDefaultSetConfig { + let mut cfg = sc_network_common::config::NonDefaultSetConfig::new(protocol_name, 1024 * 1024); cfg.allow_non_reserved(25, 25); cfg.add_fallback_names(beefy_protocol_name::LEGACY_NAMES.iter().map(|&n| n.into()).collect()); diff --git a/client/cli/Cargo.toml b/client/cli/Cargo.toml index 50d110d40eabd..3cccc0bf41529 100644 --- a/client/cli/Cargo.toml +++ b/client/cli/Cargo.toml @@ -34,6 +34,7 @@ sc-client-api = { version = "4.0.0-dev", path = "../api" } sc-client-db = { version = "0.10.0-dev", default-features = false, path = "../db" } sc-keystore = { version = "4.0.0-dev", path = "../keystore" } sc-network = { version = "0.10.0-dev", path = "../network" } +sc-network-common = { version = "0.10.0-dev", path = "../network/common" } sc-service = { version = "0.10.0-dev", default-features = false, path = "../service" } sc-telemetry = { version = "4.0.0-dev", path = "../telemetry" } sc-tracing = { version = "4.0.0-dev", path = "../tracing" } diff --git a/client/cli/src/params/network_params.rs b/client/cli/src/params/network_params.rs index 74c2db92c3215..0450b5f0e2566 100644 --- a/client/cli/src/params/network_params.rs +++ b/client/cli/src/params/network_params.rs @@ -19,11 +19,10 @@ use crate::{arg_enums::SyncMode, params::node_key_params::NodeKeyParams}; use clap::Args; use sc_network::{ - config::{ - NetworkConfiguration, NodeKeyConfig, NonReservedPeerMode, SetConfig, TransportConfig, - }, + config::{NetworkConfiguration, NodeKeyConfig}, multiaddr::Protocol, }; +use sc_network_common::config::{NonReservedPeerMode, SetConfig, TransportConfig}; use sc_service::{ config::{Multiaddr, MultiaddrWithPeerId}, ChainSpec, ChainType, diff --git a/client/finality-grandpa/src/lib.rs b/client/finality-grandpa/src/lib.rs index 149cf1f89a222..062f3ed19574b 100644 --- a/client/finality-grandpa/src/lib.rs +++ b/client/finality-grandpa/src/lib.rs @@ -68,7 +68,9 @@ use sc_client_api::{ StorageProvider, TransactionFor, }; use sc_consensus::BlockImport; -use sc_network_common::protocol::ProtocolName; +use sc_network_common::{ + protocol::ProtocolName, +}; use sc_telemetry::{telemetry, TelemetryHandle, CONSENSUS_DEBUG, CONSENSUS_INFO}; use sc_utils::mpsc::{tracing_unbounded, TracingUnboundedReceiver}; use sp_api::ProvideRuntimeApi; @@ -725,18 +727,18 @@ pub struct GrandpaParams { /// For standard protocol name see [`crate::protocol_standard_name`]. pub fn grandpa_peers_set_config( protocol_name: ProtocolName, -) -> sc_network::config::NonDefaultSetConfig { +) -> sc_network_common::config::NonDefaultSetConfig { use communication::grandpa_protocol_name; - sc_network::config::NonDefaultSetConfig { + sc_network_common::config::NonDefaultSetConfig { notifications_protocol: protocol_name, fallback_names: grandpa_protocol_name::LEGACY_NAMES.iter().map(|&n| n.into()).collect(), // Notifications reach ~256kiB in size at the time of writing on Kusama and Polkadot. max_notification_size: 1024 * 1024, - set_config: sc_network::config::SetConfig { + set_config: sc_network_common::config::SetConfig { in_peers: 0, out_peers: 0, reserved_nodes: Vec::new(), - non_reserved_mode: sc_network::config::NonReservedPeerMode::Deny, + non_reserved_mode: sc_network_common::config::NonReservedPeerMode::Deny, }, } } diff --git a/client/network/common/Cargo.toml b/client/network/common/Cargo.toml index 47d43e8b4b03f..1ee7b15538366 100644 --- a/client/network/common/Cargo.toml +++ b/client/network/common/Cargo.toml @@ -24,7 +24,10 @@ codec = { package = "parity-scale-codec", version = "3.0.0", features = [ "derive", ] } futures = "0.3.21" +futures-timer = "3.0.2" libp2p = "0.46.1" +linked_hash_set = "0.1.3" +prometheus-endpoint = { package = "substrate-prometheus-endpoint", version = "0.10.0-dev", path = "../../../utils/prometheus" } smallvec = "1.8.0" sc-consensus = { version = "0.10.0-dev", path = "../../consensus/common" } sc-peerset = { version = "4.0.0-dev", path = "../../peerset" } diff --git a/client/network/common/src/config.rs b/client/network/common/src/config.rs index 8b7e045780d7d..eeee2ce1c0e1b 100644 --- a/client/network/common/src/config.rs +++ b/client/network/common/src/config.rs @@ -18,6 +18,8 @@ //! Configuration of the networking layer. +use crate::protocol; + use libp2p::{multiaddr, Multiaddr, PeerId}; use std::{fmt, str, str::FromStr}; @@ -171,3 +173,130 @@ impl From for ParseErr { Self::MultiaddrParse(err) } } + +/// Configuration for a set of nodes. +#[derive(Clone, Debug)] +pub struct SetConfig { + /// Maximum allowed number of incoming substreams related to this set. + pub in_peers: u32, + /// Number of outgoing substreams related to this set that we're trying to maintain. + pub out_peers: u32, + /// List of reserved node addresses. + pub reserved_nodes: Vec, + /// Whether nodes that aren't in [`SetConfig::reserved_nodes`] are accepted or automatically + /// refused. + pub non_reserved_mode: NonReservedPeerMode, +} + +impl Default for SetConfig { + fn default() -> Self { + Self { + in_peers: 25, + out_peers: 75, + reserved_nodes: Vec::new(), + non_reserved_mode: NonReservedPeerMode::Accept, + } + } +} + +/// Extension to [`SetConfig`] for sets that aren't the default set. +/// +/// > **Note**: As new fields might be added in the future, please consider using the `new` method +/// > and modifiers instead of creating this struct manually. +#[derive(Clone, Debug)] +pub struct NonDefaultSetConfig { + /// Name of the notifications protocols of this set. A substream on this set will be + /// considered established once this protocol is open. + /// + /// > **Note**: This field isn't present for the default set, as this is handled internally + /// > by the networking code. + pub notifications_protocol: protocol::ProtocolName, + /// If the remote reports that it doesn't support the protocol indicated in the + /// `notifications_protocol` field, then each of these fallback names will be tried one by + /// one. + /// + /// If a fallback is used, it will be reported in + /// [`crate::Event::NotificationStreamOpened::negotiated_fallback`]. + pub fallback_names: Vec, + /// Maximum allowed size of single notifications. + pub max_notification_size: u64, + /// Base configuration. + pub set_config: SetConfig, +} + +impl NonDefaultSetConfig { + /// Creates a new [`NonDefaultSetConfig`]. Zero slots and accepts only reserved nodes. + pub fn new(notifications_protocol: protocol::ProtocolName, max_notification_size: u64) -> Self { + Self { + notifications_protocol, + max_notification_size, + fallback_names: Vec::new(), + set_config: SetConfig { + in_peers: 0, + out_peers: 0, + reserved_nodes: Vec::new(), + non_reserved_mode: NonReservedPeerMode::Deny, + }, + } + } + + /// Modifies the configuration to allow non-reserved nodes. + pub fn allow_non_reserved(&mut self, in_peers: u32, out_peers: u32) { + self.set_config.in_peers = in_peers; + self.set_config.out_peers = out_peers; + self.set_config.non_reserved_mode = NonReservedPeerMode::Accept; + } + + /// Add a node to the list of reserved nodes. + pub fn add_reserved(&mut self, peer: MultiaddrWithPeerId) { + self.set_config.reserved_nodes.push(peer); + } + + /// Add a list of protocol names used for backward compatibility. + /// + /// See the explanations in [`NonDefaultSetConfig::fallback_names`]. + pub fn add_fallback_names(&mut self, fallback_names: Vec) { + self.fallback_names.extend(fallback_names); + } +} + +/// Configuration for the transport layer. +#[derive(Clone, Debug)] +pub enum TransportConfig { + /// Normal transport mode. + Normal { + /// If true, the network will use mDNS to discover other libp2p nodes on the local network + /// and connect to them if they support the same chain. + enable_mdns: bool, + + /// If true, allow connecting to private IPv4 addresses (as defined in + /// [RFC1918](https://tools.ietf.org/html/rfc1918)). Irrelevant for addresses that have + /// been passed in [`NetworkConfiguration::boot_nodes`]. + allow_private_ipv4: bool, + }, + + /// Only allow connections within the same process. + /// Only addresses of the form `/memory/...` will be supported. + MemoryOnly, +} + +/// The policy for connections to non-reserved peers. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum NonReservedPeerMode { + /// Accept them. This is the default. + Accept, + /// Deny them. + Deny, +} + +impl NonReservedPeerMode { + /// Attempt to parse the peer mode from a string. + pub fn parse(s: &str) -> Option { + match s { + "accept" => Some(Self::Accept), + "deny" => Some(Self::Deny), + _ => None, + } + } +} + diff --git a/client/network/src/error.rs b/client/network/common/src/error.rs similarity index 96% rename from client/network/src/error.rs rename to client/network/common/src/error.rs index b4287ffbd55db..e54e8c465783c 100644 --- a/client/network/src/error.rs +++ b/client/network/common/src/error.rs @@ -18,9 +18,11 @@ //! Substrate network possible errors. -use crate::config::TransportConfig; +use crate::{ + config::TransportConfig, + protocol::ProtocolName, +}; use libp2p::{Multiaddr, PeerId}; -use sc_network_common::protocol::ProtocolName; use std::fmt; diff --git a/client/network/common/src/lib.rs b/client/network/common/src/lib.rs index 882d3f294d0a0..36e67f11e5cff 100644 --- a/client/network/common/src/lib.rs +++ b/client/network/common/src/lib.rs @@ -19,11 +19,13 @@ //! Common data structures of the networking layer. pub mod config; +pub mod error; pub mod message; pub mod protocol; pub mod request_responses; pub mod service; pub mod sync; +pub mod utils; /// Minimum Requirements for a Hash within Networking pub trait ExHashT: std::hash::Hash + Eq + std::fmt::Debug + Clone + Send + Sync + 'static {} diff --git a/client/network/src/utils.rs b/client/network/common/src/utils.rs similarity index 100% rename from client/network/src/utils.rs rename to client/network/common/src/utils.rs diff --git a/client/network/src/config.rs b/client/network/src/config.rs index 6261f340bd370..894c87fa4e7fa 100644 --- a/client/network/src/config.rs +++ b/client/network/src/config.rs @@ -41,7 +41,11 @@ use libp2p::{ }; use prometheus_endpoint::Registry; use sc_consensus::ImportQueue; -use sc_network_common::{config::MultiaddrWithPeerId, protocol::ProtocolName, sync::ChainSync}; +use sc_network_common::{ + config::MultiaddrWithPeerId, + sync::ChainSync, + config::{SetConfig, NonDefaultSetConfig, TransportConfig} +}; use sc_network_transactions::config::TransactionPool; use sp_runtime::traits::Block as BlockT; use std::{ @@ -52,7 +56,6 @@ use std::{ net::Ipv4Addr, path::{Path, PathBuf}, pin::Pin, - str, sync::Arc, }; use zeroize::Zeroize; @@ -334,132 +337,6 @@ impl NetworkConfiguration { } } -/// Configuration for a set of nodes. -#[derive(Clone, Debug)] -pub struct SetConfig { - /// Maximum allowed number of incoming substreams related to this set. - pub in_peers: u32, - /// Number of outgoing substreams related to this set that we're trying to maintain. - pub out_peers: u32, - /// List of reserved node addresses. - pub reserved_nodes: Vec, - /// Whether nodes that aren't in [`SetConfig::reserved_nodes`] are accepted or automatically - /// refused. - pub non_reserved_mode: NonReservedPeerMode, -} - -impl Default for SetConfig { - fn default() -> Self { - Self { - in_peers: 25, - out_peers: 75, - reserved_nodes: Vec::new(), - non_reserved_mode: NonReservedPeerMode::Accept, - } - } -} - -/// Extension to [`SetConfig`] for sets that aren't the default set. -/// -/// > **Note**: As new fields might be added in the future, please consider using the `new` method -/// > and modifiers instead of creating this struct manually. -#[derive(Clone, Debug)] -pub struct NonDefaultSetConfig { - /// Name of the notifications protocols of this set. A substream on this set will be - /// considered established once this protocol is open. - /// - /// > **Note**: This field isn't present for the default set, as this is handled internally - /// > by the networking code. - pub notifications_protocol: ProtocolName, - /// If the remote reports that it doesn't support the protocol indicated in the - /// `notifications_protocol` field, then each of these fallback names will be tried one by - /// one. - /// - /// If a fallback is used, it will be reported in - /// [`crate::Event::NotificationStreamOpened::negotiated_fallback`]. - pub fallback_names: Vec, - /// Maximum allowed size of single notifications. - pub max_notification_size: u64, - /// Base configuration. - pub set_config: SetConfig, -} - -impl NonDefaultSetConfig { - /// Creates a new [`NonDefaultSetConfig`]. Zero slots and accepts only reserved nodes. - pub fn new(notifications_protocol: ProtocolName, max_notification_size: u64) -> Self { - Self { - notifications_protocol, - max_notification_size, - fallback_names: Vec::new(), - set_config: SetConfig { - in_peers: 0, - out_peers: 0, - reserved_nodes: Vec::new(), - non_reserved_mode: NonReservedPeerMode::Deny, - }, - } - } - - /// Modifies the configuration to allow non-reserved nodes. - pub fn allow_non_reserved(&mut self, in_peers: u32, out_peers: u32) { - self.set_config.in_peers = in_peers; - self.set_config.out_peers = out_peers; - self.set_config.non_reserved_mode = NonReservedPeerMode::Accept; - } - - /// Add a node to the list of reserved nodes. - pub fn add_reserved(&mut self, peer: MultiaddrWithPeerId) { - self.set_config.reserved_nodes.push(peer); - } - - /// Add a list of protocol names used for backward compatibility. - /// - /// See the explanations in [`NonDefaultSetConfig::fallback_names`]. - pub fn add_fallback_names(&mut self, fallback_names: Vec) { - self.fallback_names.extend(fallback_names); - } -} - -/// Configuration for the transport layer. -#[derive(Clone, Debug)] -pub enum TransportConfig { - /// Normal transport mode. - Normal { - /// If true, the network will use mDNS to discover other libp2p nodes on the local network - /// and connect to them if they support the same chain. - enable_mdns: bool, - - /// If true, allow connecting to private IPv4 addresses (as defined in - /// [RFC1918](https://tools.ietf.org/html/rfc1918)). Irrelevant for addresses that have - /// been passed in [`NetworkConfiguration::boot_nodes`]. - allow_private_ipv4: bool, - }, - - /// Only allow connections within the same process. - /// Only addresses of the form `/memory/...` will be supported. - MemoryOnly, -} - -/// The policy for connections to non-reserved peers. -#[derive(Clone, Debug, PartialEq, Eq)] -pub enum NonReservedPeerMode { - /// Accept them. This is the default. - Accept, - /// Deny them. - Deny, -} - -impl NonReservedPeerMode { - /// Attempt to parse the peer mode from a string. - pub fn parse(s: &str) -> Option { - match s { - "accept" => Some(Self::Accept), - "deny" => Some(Self::Deny), - _ => None, - } - } -} - /// The configuration of a node's secret key, describing the type of key /// and how it is obtained. A node's identity keypair is the result of /// the evaluation of the node key configuration. diff --git a/client/network/src/discovery.rs b/client/network/src/discovery.rs index ab93662968dc2..a4ac25dc60e5b 100644 --- a/client/network/src/discovery.rs +++ b/client/network/src/discovery.rs @@ -46,7 +46,6 @@ //! active mechanism that asks nodes for the addresses they are listening on. Whenever we learn //! of a node's address, you must call `add_self_reported_address`. -use crate::utils::LruHashSet; use futures::prelude::*; use futures_timer::Delay; use ip_network::IpNetwork; @@ -72,7 +71,10 @@ use libp2p::{ }, }; use log::{debug, error, info, trace, warn}; -use sc_network_common::config::ProtocolId; +use sc_network_common::{ + config::ProtocolId, + utils::LruHashSet, +}; use sp_core::hexdisplay::HexDisplay; use std::{ cmp, diff --git a/client/network/src/lib.rs b/client/network/src/lib.rs index 21c7b1424035f..745531164aca9 100644 --- a/client/network/src/lib.rs +++ b/client/network/src/lib.rs @@ -252,11 +252,9 @@ mod request_responses; mod schema; mod service; mod transport; -mod utils; pub mod bitswap; pub mod config; -pub mod error; pub mod network_state; pub mod transactions; diff --git a/client/network/src/peer_info.rs b/client/network/src/peer_info.rs index d668cb25ea455..2c7bdc0291f78 100644 --- a/client/network/src/peer_info.rs +++ b/client/network/src/peer_info.rs @@ -16,7 +16,6 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use crate::utils::interval; use fnv::FnvHashMap; use futures::prelude::*; use libp2p::{ @@ -32,6 +31,7 @@ use libp2p::{ }, Multiaddr, }; +use sc_network_common::utils::interval; use log::{debug, error, trace}; use smallvec::SmallVec; use std::{ diff --git a/client/network/src/protocol.rs b/client/network/src/protocol.rs index 9bcf363cd0692..6373c9255786f 100644 --- a/client/network/src/protocol.rs +++ b/client/network/src/protocol.rs @@ -16,10 +16,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use crate::{ - config, error, - utils::{interval, LruHashSet}, -}; +use crate::config; use bytes::Bytes; use codec::{Decode, DecodeAll, Encode}; @@ -43,7 +40,8 @@ use prometheus_endpoint::{register, Gauge, GaugeVec, Opts, PrometheusError, Regi use sc_client_api::HeaderBackend; use sc_consensus::import_queue::{BlockImportError, BlockImportStatus, IncomingBlock, Origin}; use sc_network_common::{ - config::ProtocolId, + config::{ProtocolId,NonReservedPeerMode}, + error, protocol::ProtocolName, request_responses::RequestFailure, sync::{ @@ -55,6 +53,7 @@ use sc_network_common::{ OpaqueBlockResponse, OpaqueStateRequest, OpaqueStateResponse, PollBlockAnnounceValidation, SyncStatus, }, + utils::{interval, LruHashSet}, }; use sp_arithmetic::traits::SaturatedConversion; use sp_consensus::BlockOrigin; @@ -339,7 +338,7 @@ where bootnodes, reserved_nodes: default_sets_reserved.clone(), reserved_only: network_config.default_peers_set.non_reserved_mode == - config::NonReservedPeerMode::Deny, + NonReservedPeerMode::Deny, }); for set_cfg in &network_config.extra_sets { @@ -350,7 +349,7 @@ where } let reserved_only = - set_cfg.set_config.non_reserved_mode == config::NonReservedPeerMode::Deny; + set_cfg.set_config.non_reserved_mode == NonReservedPeerMode::Deny; sets.push(sc_peerset::SetConfig { in_peers: set_cfg.set_config.in_peers, diff --git a/client/network/src/service.rs b/client/network/src/service.rs index de5f01f366159..83bd1d1226d88 100644 --- a/client/network/src/service.rs +++ b/client/network/src/service.rs @@ -29,9 +29,8 @@ use crate::{ behaviour::{self, Behaviour, BehaviourOut}, - config::{Params, TransportConfig}, + config::Params, discovery::DiscoveryConfig, - error::Error, network_state::{ NetworkState, NotConnectedPeer as NetworkStateNotConnectedPeer, Peer as NetworkStatePeer, }, @@ -61,7 +60,8 @@ use parking_lot::Mutex; use sc_consensus::{BlockImportError, BlockImportStatus, ImportQueue, Link}; use sc_network_common::{ ExHashT, - config::MultiaddrWithPeerId, + error::Error, + config::{MultiaddrWithPeerId, TransportConfig}, protocol::{ event::{DhtEvent, Event}, ProtocolName, diff --git a/client/network/src/service/tests.rs b/client/network/src/service/tests.rs index 3207f30ed475e..0af532bcedd89 100644 --- a/client/network/src/service/tests.rs +++ b/client/network/src/service/tests.rs @@ -21,7 +21,7 @@ use crate::{config, NetworkService, NetworkWorker}; use futures::prelude::*; use libp2p::PeerId; use sc_network_common::{ - config::{MultiaddrWithPeerId, ProtocolId}, + config::{MultiaddrWithPeerId, NonDefaultSetConfig, ProtocolId, SetConfig, TransportConfig}, protocol::event::Event, service::{NetworkEventStream, NetworkNotification, NetworkPeers, NetworkStateInfo}, }; @@ -179,23 +179,23 @@ fn build_nodes_one_proto() -> ( let listen_addr = config::build_multiaddr![Memory(rand::random::())]; let (node1, events_stream1) = build_test_full_node(config::NetworkConfiguration { - extra_sets: vec![config::NonDefaultSetConfig { + extra_sets: vec![NonDefaultSetConfig { notifications_protocol: PROTOCOL_NAME.into(), fallback_names: Vec::new(), max_notification_size: 1024 * 1024, set_config: Default::default(), }], listen_addresses: vec![listen_addr.clone()], - transport: config::TransportConfig::MemoryOnly, + transport: TransportConfig::MemoryOnly, ..config::NetworkConfiguration::new_local() }); let (node2, events_stream2) = build_test_full_node(config::NetworkConfiguration { - extra_sets: vec![config::NonDefaultSetConfig { + extra_sets: vec![NonDefaultSetConfig { notifications_protocol: PROTOCOL_NAME.into(), fallback_names: Vec::new(), max_notification_size: 1024 * 1024, - set_config: config::SetConfig { + set_config: SetConfig { reserved_nodes: vec![MultiaddrWithPeerId { multiaddr: listen_addr, peer_id: node1.local_peer_id(), @@ -204,7 +204,7 @@ fn build_nodes_one_proto() -> ( }, }], listen_addresses: vec![], - transport: config::TransportConfig::MemoryOnly, + transport: TransportConfig::MemoryOnly, ..config::NetworkConfiguration::new_local() }); @@ -369,13 +369,13 @@ fn lots_of_incoming_peers_works() { let (main_node, _) = build_test_full_node(config::NetworkConfiguration { listen_addresses: vec![listen_addr.clone()], - extra_sets: vec![config::NonDefaultSetConfig { + extra_sets: vec![NonDefaultSetConfig { notifications_protocol: PROTOCOL_NAME.into(), fallback_names: Vec::new(), max_notification_size: 1024 * 1024, - set_config: config::SetConfig { in_peers: u32::MAX, ..Default::default() }, + set_config: SetConfig { in_peers: u32::MAX, ..Default::default() }, }], - transport: config::TransportConfig::MemoryOnly, + transport: TransportConfig::MemoryOnly, ..config::NetworkConfiguration::new_local() }); @@ -388,11 +388,11 @@ fn lots_of_incoming_peers_works() { for _ in 0..32 { let (_dialing_node, event_stream) = build_test_full_node(config::NetworkConfiguration { listen_addresses: vec![], - extra_sets: vec![config::NonDefaultSetConfig { + extra_sets: vec![NonDefaultSetConfig { notifications_protocol: PROTOCOL_NAME.into(), fallback_names: Vec::new(), max_notification_size: 1024 * 1024, - set_config: config::SetConfig { + set_config: SetConfig { reserved_nodes: vec![MultiaddrWithPeerId { multiaddr: listen_addr.clone(), peer_id: main_node_peer_id, @@ -400,7 +400,7 @@ fn lots_of_incoming_peers_works() { ..Default::default() }, }], - transport: config::TransportConfig::MemoryOnly, + transport: TransportConfig::MemoryOnly, ..config::NetworkConfiguration::new_local() }); @@ -505,23 +505,23 @@ fn fallback_name_working() { let listen_addr = config::build_multiaddr![Memory(rand::random::())]; let (node1, mut events_stream1) = build_test_full_node(config::NetworkConfiguration { - extra_sets: vec![config::NonDefaultSetConfig { + extra_sets: vec![NonDefaultSetConfig { notifications_protocol: NEW_PROTOCOL_NAME.into(), fallback_names: vec![PROTOCOL_NAME.into()], max_notification_size: 1024 * 1024, set_config: Default::default(), }], listen_addresses: vec![listen_addr.clone()], - transport: config::TransportConfig::MemoryOnly, + transport: TransportConfig::MemoryOnly, ..config::NetworkConfiguration::new_local() }); let (_, mut events_stream2) = build_test_full_node(config::NetworkConfiguration { - extra_sets: vec![config::NonDefaultSetConfig { + extra_sets: vec![NonDefaultSetConfig { notifications_protocol: PROTOCOL_NAME.into(), fallback_names: Vec::new(), max_notification_size: 1024 * 1024, - set_config: config::SetConfig { + set_config: SetConfig { reserved_nodes: vec![MultiaddrWithPeerId { multiaddr: listen_addr, peer_id: node1.local_peer_id(), @@ -530,7 +530,7 @@ fn fallback_name_working() { }, }], listen_addresses: vec![], - transport: config::TransportConfig::MemoryOnly, + transport: TransportConfig::MemoryOnly, ..config::NetworkConfiguration::new_local() }); @@ -573,7 +573,7 @@ fn ensure_listen_addresses_consistent_with_transport_memory() { let _ = build_test_full_node(config::NetworkConfiguration { listen_addresses: vec![listen_addr.clone()], - transport: config::TransportConfig::MemoryOnly, + transport: TransportConfig::MemoryOnly, ..config::NetworkConfiguration::new("test-node", "test-client", Default::default(), None) }); } @@ -600,7 +600,7 @@ fn ensure_boot_node_addresses_consistent_with_transport_memory() { let _ = build_test_full_node(config::NetworkConfiguration { listen_addresses: vec![listen_addr.clone()], - transport: config::TransportConfig::MemoryOnly, + transport: TransportConfig::MemoryOnly, boot_nodes: vec![boot_node], ..config::NetworkConfiguration::new("test-node", "test-client", Default::default(), None) }); @@ -633,8 +633,8 @@ fn ensure_reserved_node_addresses_consistent_with_transport_memory() { let _ = build_test_full_node(config::NetworkConfiguration { listen_addresses: vec![listen_addr.clone()], - transport: config::TransportConfig::MemoryOnly, - default_peers_set: config::SetConfig { + transport: TransportConfig::MemoryOnly, + default_peers_set: SetConfig { reserved_nodes: vec![reserved_node], ..Default::default() }, @@ -653,7 +653,7 @@ fn ensure_reserved_node_addresses_consistent_with_transport_not_memory() { let _ = build_test_full_node(config::NetworkConfiguration { listen_addresses: vec![listen_addr.clone()], - default_peers_set: config::SetConfig { + default_peers_set: SetConfig { reserved_nodes: vec![reserved_node], ..Default::default() }, @@ -669,7 +669,7 @@ fn ensure_public_addresses_consistent_with_transport_memory() { let _ = build_test_full_node(config::NetworkConfiguration { listen_addresses: vec![listen_addr.clone()], - transport: config::TransportConfig::MemoryOnly, + transport: TransportConfig::MemoryOnly, public_addresses: vec![public_address], ..config::NetworkConfiguration::new("test-node", "test-client", Default::default(), None) }); diff --git a/client/network/src/transactions.rs b/client/network/src/transactions.rs index d9e1d37d97d15..1e399ca384d37 100644 --- a/client/network/src/transactions.rs +++ b/client/network/src/transactions.rs @@ -27,11 +27,8 @@ //! `Future` that processes transactions. use crate::{ - config, - error, protocol::message, service::NetworkService, - utils::{interval, LruHashSet}, }; use codec::{Decode, Encode}; @@ -41,12 +38,15 @@ use log::{debug, trace, warn}; use prometheus_endpoint::{register, Counter, PrometheusError, Registry, U64}; use sc_network_common::{ ExHashT, + error, config::ProtocolId, protocol::{ event::{Event, ObservedRole}, ProtocolName, }, service::{NetworkEventStream, NetworkNotification, NetworkPeers}, + config::{NonDefaultSetConfig, SetConfig, NonReservedPeerMode}, + utils::{interval, LruHashSet}, }; use sc_network_transactions::config::{TransactionImport, TransactionImportFuture, TransactionPool}; use sp_runtime::traits::Block as BlockT; @@ -158,16 +158,16 @@ impl TransactionsHandlerPrototype { } /// Returns the configuration of the set to put in the network configuration. - pub fn set_config(&self) -> config::NonDefaultSetConfig { - config::NonDefaultSetConfig { + pub fn set_config(&self) -> NonDefaultSetConfig { + NonDefaultSetConfig { notifications_protocol: self.protocol_name.clone(), fallback_names: self.fallback_protocol_names.clone(), max_notification_size: MAX_TRANSACTIONS_SIZE, - set_config: config::SetConfig { + set_config: SetConfig { in_peers: 0, out_peers: 0, reserved_nodes: Vec::new(), - non_reserved_mode: config::NonReservedPeerMode::Deny, + non_reserved_mode: NonReservedPeerMode::Deny, }, } } diff --git a/client/network/test/src/lib.rs b/client/network/test/src/lib.rs index f4f0e07e4f78a..540469bd25418 100644 --- a/client/network/test/src/lib.rs +++ b/client/network/test/src/lib.rs @@ -49,14 +49,11 @@ use sc_consensus::{ }; pub use sc_network_transactions::config::EmptyTransactionPool; use sc_network::{ - config::{ - NetworkConfiguration, NonDefaultSetConfig, NonReservedPeerMode, Role, SyncMode, - TransportConfig, - }, + config::{NetworkConfiguration, Role, SyncMode}, Multiaddr, NetworkService, NetworkWorker, }; use sc_network_common::{ - config::{MultiaddrWithPeerId, ProtocolId}, + config::{MultiaddrWithPeerId, ProtocolId, NonDefaultSetConfig, NonReservedPeerMode, TransportConfig}, protocol::ProtocolName, service::{NetworkBlock, NetworkStateInfo, NetworkSyncForkRequest}, sync::warp::{AuthorityList, EncodedProof, SetId, VerificationResult, WarpSyncProvider}, diff --git a/client/service/src/config.rs b/client/service/src/config.rs index 2fb3f820ebf15..ace908c835d74 100644 --- a/client/service/src/config.rs +++ b/client/service/src/config.rs @@ -24,13 +24,11 @@ pub use sc_executor::WasmExecutionMethod; #[cfg(feature = "wasmtime")] pub use sc_executor::WasmtimeInstantiationStrategy; pub use sc_network::{ - config::{ - NetworkConfiguration, NodeKeyConfig, NonDefaultSetConfig, Role, SetConfig, TransportConfig, - }, + config::{NetworkConfiguration, NodeKeyConfig, Role}, Multiaddr, }; pub use sc_network_common::{ - config::{MultiaddrWithPeerId, ProtocolId}, + config::{MultiaddrWithPeerId, NonDefaultSetConfig, ProtocolId, SetConfig, TransportConfig}, request_responses::{ IncomingRequest, OutgoingResponse, ProtocolConfig as RequestResponseConfig, }, diff --git a/client/service/src/error.rs b/client/service/src/error.rs index 0d702c7f37b98..001a83922d776 100644 --- a/client/service/src/error.rs +++ b/client/service/src/error.rs @@ -19,7 +19,6 @@ //! Errors that can occur during the service operation. use sc_keystore; -use sc_network; use sp_blockchain; use sp_consensus; @@ -41,7 +40,7 @@ pub enum Error { Consensus(#[from] sp_consensus::Error), #[error(transparent)] - Network(#[from] sc_network::error::Error), + Network(#[from] sc_network_common::error::Error), #[error(transparent)] Keystore(#[from] sc_keystore::Error), diff --git a/client/service/test/src/lib.rs b/client/service/test/src/lib.rs index 11c1cbaf7afb1..978363990d7cf 100644 --- a/client/service/test/src/lib.rs +++ b/client/service/test/src/lib.rs @@ -23,11 +23,11 @@ use log::{debug, info}; use parking_lot::Mutex; use sc_client_api::{Backend, CallExecutor}; use sc_network::{ - config::{NetworkConfiguration, TransportConfig}, + config::NetworkConfiguration, multiaddr, }; use sc_network_common::{ - config::MultiaddrWithPeerId, + config::{MultiaddrWithPeerId, TransportConfig}, service::{NetworkBlock, NetworkPeers, NetworkStateInfo}, }; use sc_service::{ From 8e389a95cb81cb356f529fc20d3ccf16dc904269 Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Tue, 6 Sep 2022 12:51:31 +0300 Subject: [PATCH 04/17] Move transaction code to sc-network-transactions --- Cargo.lock | 7 + client/network/src/lib.rs | 1 - client/network/src/protocol/message.rs | 7 +- client/network/src/service.rs | 6 +- client/network/src/transactions.rs | 491 ---------------------- client/network/transactions/Cargo.toml | 7 + client/network/transactions/src/config.rs | 16 +- client/network/transactions/src/lib.rs | 469 +++++++++++++++++++++ 8 files changed, 503 insertions(+), 501 deletions(-) delete mode 100644 client/network/src/transactions.rs diff --git a/Cargo.lock b/Cargo.lock index e99bbcc592f64..0f4a661251fe2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8575,8 +8575,15 @@ name = "sc-network-transactions" version = "0.10.0-dev" dependencies = [ "futures", + "hex", + "libp2p", + "log", + "parity-scale-codec", + "pin-project", "sc-network-common", + "sc-peerset", "sp-runtime", + "substrate-prometheus-endpoint", ] [[package]] diff --git a/client/network/src/lib.rs b/client/network/src/lib.rs index 745531164aca9..6895d886e8c21 100644 --- a/client/network/src/lib.rs +++ b/client/network/src/lib.rs @@ -256,7 +256,6 @@ mod transport; pub mod bitswap; pub mod config; pub mod network_state; -pub mod transactions; #[doc(inline)] pub use libp2p::{multiaddr, Multiaddr, PeerId}; diff --git a/client/network/src/protocol/message.rs b/client/network/src/protocol/message.rs index 50c4a264a5f95..9a0cb873ee1c6 100644 --- a/client/network/src/protocol/message.rs +++ b/client/network/src/protocol/message.rs @@ -36,9 +36,6 @@ pub type Message = generic::Message< ::Extrinsic, >; -/// A set of transactions. -pub type Transactions = Vec; - /// Remote call response. #[derive(Debug, PartialEq, Eq, Clone, Encode, Decode)] pub struct RemoteCallResponse { @@ -59,7 +56,7 @@ pub struct RemoteReadResponse { /// Generic types. pub mod generic { - use super::{RemoteCallResponse, RemoteReadResponse, Transactions}; + use super::{RemoteCallResponse, RemoteReadResponse}; use bitflags::bitflags; use codec::{Decode, Encode, Input, Output}; use sc_client_api::StorageProof; @@ -147,7 +144,7 @@ pub mod generic { /// Block announce. BlockAnnounce(BlockAnnounce
), /// Transactions. - Transactions(Transactions), + Transactions(sc_network_transactions::Transactions), /// Consensus protocol message. Consensus(ConsensusMessage), /// Remote method call request. diff --git a/client/network/src/service.rs b/client/network/src/service.rs index 83bd1d1226d88..86fa37f1cae0c 100644 --- a/client/network/src/service.rs +++ b/client/network/src/service.rs @@ -38,7 +38,7 @@ use crate::{ self, message::generic::Roles, NotificationsSink, NotifsHandlerError, PeerInfo, Protocol, Ready, }, - transactions, transport, ReputationChange, + transport, ReputationChange, }; use codec::Encode as _; @@ -211,7 +211,7 @@ where fs::create_dir_all(path)?; } - let transactions_handler_proto = transactions::TransactionsHandlerPrototype::new( + let transactions_handler_proto = sc_network_transactions::TransactionsHandlerPrototype::new( params.protocol_id.clone(), params .chain @@ -1317,7 +1317,7 @@ where /// that peer. Shared with the [`NetworkService`]. peers_notifications_sinks: Arc>>, /// Controller for the handler of incoming and outgoing transactions. - tx_handler_controller: transactions::TransactionsHandlerController, + tx_handler_controller: sc_network_transactions::TransactionsHandlerController, } impl Future for NetworkWorker diff --git a/client/network/src/transactions.rs b/client/network/src/transactions.rs deleted file mode 100644 index 1e399ca384d37..0000000000000 --- a/client/network/src/transactions.rs +++ /dev/null @@ -1,491 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) 2017-2022 Parity Technologies (UK) Ltd. -// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 - -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with this program. If not, see . - -//! Transactions handling to plug on top of the network service. -//! -//! Usage: -//! -//! - Use [`TransactionsHandlerPrototype::new`] to create a prototype. -//! - Pass the return value of [`TransactionsHandlerPrototype::set_config`] to the network -//! configuration as an extra peers set. -//! - Use [`TransactionsHandlerPrototype::build`] then [`TransactionsHandler::run`] to obtain a -//! `Future` that processes transactions. - -use crate::{ - protocol::message, - service::NetworkService, -}; - -use codec::{Decode, Encode}; -use futures::{channel::mpsc, prelude::*, stream::FuturesUnordered}; -use libp2p::{multiaddr, PeerId}; -use log::{debug, trace, warn}; -use prometheus_endpoint::{register, Counter, PrometheusError, Registry, U64}; -use sc_network_common::{ - ExHashT, - error, - config::ProtocolId, - protocol::{ - event::{Event, ObservedRole}, - ProtocolName, - }, - service::{NetworkEventStream, NetworkNotification, NetworkPeers}, - config::{NonDefaultSetConfig, SetConfig, NonReservedPeerMode}, - utils::{interval, LruHashSet}, -}; -use sc_network_transactions::config::{TransactionImport, TransactionImportFuture, TransactionPool}; -use sp_runtime::traits::Block as BlockT; -use std::{ - collections::{hash_map::Entry, HashMap}, - iter, - num::NonZeroUsize, - pin::Pin, - sync::{ - atomic::{AtomicBool, Ordering}, - Arc, - }, - task::Poll, - time, -}; - -/// Interval at which we propagate transactions; -const PROPAGATE_TIMEOUT: time::Duration = time::Duration::from_millis(2900); - -/// Maximum number of known transaction hashes to keep for a peer. -/// -/// This should be approx. 2 blocks full of transactions for the network to function properly. -const MAX_KNOWN_TRANSACTIONS: usize = 10240; // ~300kb per peer + overhead. - -/// Maximum allowed size for a transactions notification. -const MAX_TRANSACTIONS_SIZE: u64 = 16 * 1024 * 1024; - -/// Maximum number of transaction validation request we keep at any moment. -const MAX_PENDING_TRANSACTIONS: usize = 8192; - -mod rep { - use sc_peerset::ReputationChange as Rep; - /// Reputation change when a peer sends us any transaction. - /// - /// This forces node to verify it, thus the negative value here. Once transaction is verified, - /// reputation change should be refunded with `ANY_TRANSACTION_REFUND` - pub const ANY_TRANSACTION: Rep = Rep::new(-(1 << 4), "Any transaction"); - /// Reputation change when a peer sends us any transaction that is not invalid. - pub const ANY_TRANSACTION_REFUND: Rep = Rep::new(1 << 4, "Any transaction (refund)"); - /// Reputation change when a peer sends us an transaction that we didn't know about. - pub const GOOD_TRANSACTION: Rep = Rep::new(1 << 7, "Good transaction"); - /// Reputation change when a peer sends us a bad transaction. - pub const BAD_TRANSACTION: Rep = Rep::new(-(1 << 12), "Bad transaction"); -} - -struct Metrics { - propagated_transactions: Counter, -} - -impl Metrics { - fn register(r: &Registry) -> Result { - Ok(Self { - propagated_transactions: register( - Counter::new( - "substrate_sync_propagated_transactions", - "Number of transactions propagated to at least one peer", - )?, - r, - )?, - }) - } -} - -#[pin_project::pin_project] -struct PendingTransaction { - #[pin] - validation: TransactionImportFuture, - tx_hash: H, -} - -impl Future for PendingTransaction { - type Output = (H, TransactionImport); - - fn poll(self: Pin<&mut Self>, cx: &mut std::task::Context<'_>) -> Poll { - let mut this = self.project(); - - if let Poll::Ready(import_result) = Pin::new(&mut this.validation).poll_unpin(cx) { - return Poll::Ready((this.tx_hash.clone(), import_result)) - } - - Poll::Pending - } -} - -/// Prototype for a [`TransactionsHandler`]. -pub struct TransactionsHandlerPrototype { - protocol_name: ProtocolName, - fallback_protocol_names: Vec, -} - -impl TransactionsHandlerPrototype { - /// Create a new instance. - pub fn new>( - protocol_id: ProtocolId, - genesis_hash: Hash, - fork_id: Option, - ) -> Self { - let protocol_name = if let Some(fork_id) = fork_id { - format!("/{}/{}/transactions/1", hex::encode(genesis_hash), fork_id) - } else { - format!("/{}/transactions/1", hex::encode(genesis_hash)) - }; - let legacy_protocol_name = format!("/{}/transactions/1", protocol_id.as_ref()); - - Self { - protocol_name: protocol_name.into(), - fallback_protocol_names: iter::once(legacy_protocol_name.into()).collect(), - } - } - - /// Returns the configuration of the set to put in the network configuration. - pub fn set_config(&self) -> NonDefaultSetConfig { - NonDefaultSetConfig { - notifications_protocol: self.protocol_name.clone(), - fallback_names: self.fallback_protocol_names.clone(), - max_notification_size: MAX_TRANSACTIONS_SIZE, - set_config: SetConfig { - in_peers: 0, - out_peers: 0, - reserved_nodes: Vec::new(), - non_reserved_mode: NonReservedPeerMode::Deny, - }, - } - } - - /// Turns the prototype into the actual handler. Returns a controller that allows controlling - /// the behaviour of the handler while it's running. - /// - /// Important: the transactions handler is initially disabled and doesn't gossip transactions. - /// You must call [`TransactionsHandlerController::set_gossip_enabled`] to enable it. - pub fn build( - self, - service: Arc>, - transaction_pool: Arc>, - metrics_registry: Option<&Registry>, - ) -> error::Result<(TransactionsHandler, TransactionsHandlerController)> { - let event_stream = service.event_stream("transactions-handler"); - let (to_handler, from_controller) = mpsc::unbounded(); - let gossip_enabled = Arc::new(AtomicBool::new(false)); - - let handler = TransactionsHandler { - protocol_name: self.protocol_name, - propagate_timeout: Box::pin(interval(PROPAGATE_TIMEOUT)), - pending_transactions: FuturesUnordered::new(), - pending_transactions_peers: HashMap::new(), - gossip_enabled: gossip_enabled.clone(), - service, - event_stream, - peers: HashMap::new(), - transaction_pool, - from_controller, - metrics: if let Some(r) = metrics_registry { - Some(Metrics::register(r)?) - } else { - None - }, - }; - - let controller = TransactionsHandlerController { to_handler, gossip_enabled }; - - Ok((handler, controller)) - } -} - -/// Controls the behaviour of a [`TransactionsHandler`] it is connected to. -pub struct TransactionsHandlerController { - to_handler: mpsc::UnboundedSender>, - gossip_enabled: Arc, -} - -impl TransactionsHandlerController { - /// Controls whether transactions are being gossiped on the network. - pub fn set_gossip_enabled(&mut self, enabled: bool) { - self.gossip_enabled.store(enabled, Ordering::Relaxed); - } - - /// You may call this when new transactions are imported by the transaction pool. - /// - /// All transactions will be fetched from the `TransactionPool` that was passed at - /// initialization as part of the configuration and propagated to peers. - pub fn propagate_transactions(&self) { - let _ = self.to_handler.unbounded_send(ToHandler::PropagateTransactions); - } - - /// You must call when new a transaction is imported by the transaction pool. - /// - /// This transaction will be fetched from the `TransactionPool` that was passed at - /// initialization as part of the configuration and propagated to peers. - pub fn propagate_transaction(&self, hash: H) { - let _ = self.to_handler.unbounded_send(ToHandler::PropagateTransaction(hash)); - } -} - -enum ToHandler { - PropagateTransactions, - PropagateTransaction(H), -} - -/// Handler for transactions. Call [`TransactionsHandler::run`] to start the processing. -pub struct TransactionsHandler { - protocol_name: ProtocolName, - /// Interval at which we call `propagate_transactions`. - propagate_timeout: Pin + Send>>, - /// Pending transactions verification tasks. - pending_transactions: FuturesUnordered>, - /// As multiple peers can send us the same transaction, we group - /// these peers using the transaction hash while the transaction is - /// imported. This prevents that we import the same transaction - /// multiple times concurrently. - pending_transactions_peers: HashMap>, - /// Network service to use to send messages and manage peers. - service: Arc>, - /// Stream of networking events. - event_stream: Pin + Send>>, - // All connected peers - peers: HashMap>, - transaction_pool: Arc>, - gossip_enabled: Arc, - from_controller: mpsc::UnboundedReceiver>, - /// Prometheus metrics. - metrics: Option, -} - -/// Peer information -#[derive(Debug)] -struct Peer { - /// Holds a set of transactions known to this peer. - known_transactions: LruHashSet, - role: ObservedRole, -} - -impl TransactionsHandler { - /// Turns the [`TransactionsHandler`] into a future that should run forever and not be - /// interrupted. - pub async fn run(mut self) { - loop { - futures::select! { - _ = self.propagate_timeout.next().fuse() => { - self.propagate_transactions(); - }, - (tx_hash, result) = self.pending_transactions.select_next_some() => { - if let Some(peers) = self.pending_transactions_peers.remove(&tx_hash) { - peers.into_iter().for_each(|p| self.on_handle_transaction_import(p, result)); - } else { - warn!(target: "sub-libp2p", "Inconsistent state, no peers for pending transaction!"); - } - }, - network_event = self.event_stream.next().fuse() => { - if let Some(network_event) = network_event { - self.handle_network_event(network_event).await; - } else { - // Networking has seemingly closed. Closing as well. - return; - } - }, - message = self.from_controller.select_next_some().fuse() => { - match message { - ToHandler::PropagateTransaction(hash) => self.propagate_transaction(&hash), - ToHandler::PropagateTransactions => self.propagate_transactions(), - } - }, - } - } - } - - async fn handle_network_event(&mut self, event: Event) { - match event { - Event::Dht(_) => {}, - Event::SyncConnected { remote } => { - let addr = iter::once(multiaddr::Protocol::P2p(remote.into())) - .collect::(); - let result = self.service.add_peers_to_reserved_set( - self.protocol_name.clone(), - iter::once(addr).collect(), - ); - if let Err(err) = result { - log::error!(target: "sync", "Add reserved peer failed: {}", err); - } - }, - Event::SyncDisconnected { remote } => { - self.service.remove_peers_from_reserved_set( - self.protocol_name.clone(), - iter::once(remote).collect(), - ); - }, - - Event::NotificationStreamOpened { remote, protocol, role, .. } - if protocol == self.protocol_name => - { - let _was_in = self.peers.insert( - remote, - Peer { - known_transactions: LruHashSet::new( - NonZeroUsize::new(MAX_KNOWN_TRANSACTIONS).expect("Constant is nonzero"), - ), - role, - }, - ); - debug_assert!(_was_in.is_none()); - }, - Event::NotificationStreamClosed { remote, protocol } - if protocol == self.protocol_name => - { - let _peer = self.peers.remove(&remote); - debug_assert!(_peer.is_some()); - }, - - Event::NotificationsReceived { remote, messages } => { - for (protocol, message) in messages { - if protocol != self.protocol_name { - continue - } - - if let Ok(m) = as Decode>::decode( - &mut message.as_ref(), - ) { - self.on_transactions(remote, m); - } else { - warn!(target: "sub-libp2p", "Failed to decode transactions list"); - } - } - }, - - // Not our concern. - Event::NotificationStreamOpened { .. } | Event::NotificationStreamClosed { .. } => {}, - } - } - - /// Called when peer sends us new transactions - fn on_transactions(&mut self, who: PeerId, transactions: message::Transactions) { - // Accept transactions only when enabled - if !self.gossip_enabled.load(Ordering::Relaxed) { - trace!(target: "sync", "{} Ignoring transactions while disabled", who); - return - } - - trace!(target: "sync", "Received {} transactions from {}", transactions.len(), who); - if let Some(ref mut peer) = self.peers.get_mut(&who) { - for t in transactions { - if self.pending_transactions.len() > MAX_PENDING_TRANSACTIONS { - debug!( - target: "sync", - "Ignoring any further transactions that exceed `MAX_PENDING_TRANSACTIONS`({}) limit", - MAX_PENDING_TRANSACTIONS, - ); - break - } - - let hash = self.transaction_pool.hash_of(&t); - peer.known_transactions.insert(hash.clone()); - - self.service.report_peer(who, rep::ANY_TRANSACTION); - - match self.pending_transactions_peers.entry(hash.clone()) { - Entry::Vacant(entry) => { - self.pending_transactions.push(PendingTransaction { - validation: self.transaction_pool.import(t), - tx_hash: hash, - }); - entry.insert(vec![who]); - }, - Entry::Occupied(mut entry) => { - entry.get_mut().push(who); - }, - } - } - } - } - - fn on_handle_transaction_import(&mut self, who: PeerId, import: TransactionImport) { - match import { - TransactionImport::KnownGood => - self.service.report_peer(who, rep::ANY_TRANSACTION_REFUND), - TransactionImport::NewGood => self.service.report_peer(who, rep::GOOD_TRANSACTION), - TransactionImport::Bad => self.service.report_peer(who, rep::BAD_TRANSACTION), - TransactionImport::None => {}, - } - } - - /// Propagate one transaction. - pub fn propagate_transaction(&mut self, hash: &H) { - // Accept transactions only when enabled - if !self.gossip_enabled.load(Ordering::Relaxed) { - return - } - debug!(target: "sync", "Propagating transaction [{:?}]", hash); - if let Some(transaction) = self.transaction_pool.transaction(hash) { - let propagated_to = self.do_propagate_transactions(&[(hash.clone(), transaction)]); - self.transaction_pool.on_broadcasted(propagated_to); - } - } - - fn do_propagate_transactions( - &mut self, - transactions: &[(H, B::Extrinsic)], - ) -> HashMap> { - let mut propagated_to = HashMap::<_, Vec<_>>::new(); - let mut propagated_transactions = 0; - - for (who, peer) in self.peers.iter_mut() { - // never send transactions to the light node - if matches!(peer.role, ObservedRole::Light) { - continue - } - - let (hashes, to_send): (Vec<_>, Vec<_>) = transactions - .iter() - .filter(|&(ref hash, _)| peer.known_transactions.insert(hash.clone())) - .cloned() - .unzip(); - - propagated_transactions += hashes.len(); - - if !to_send.is_empty() { - for hash in hashes { - propagated_to.entry(hash).or_default().push(who.to_base58()); - } - trace!(target: "sync", "Sending {} transactions to {}", to_send.len(), who); - self.service - .write_notification(*who, self.protocol_name.clone(), to_send.encode()); - } - } - - if let Some(ref metrics) = self.metrics { - metrics.propagated_transactions.inc_by(propagated_transactions as _) - } - - propagated_to - } - - /// Call when we must propagate ready transactions to peers. - fn propagate_transactions(&mut self) { - // Accept transactions only when enabled - if !self.gossip_enabled.load(Ordering::Relaxed) { - return - } - debug!(target: "sync", "Propagating transactions"); - let transactions = self.transaction_pool.transactions(); - let propagated_to = self.do_propagate_transactions(&transactions); - self.transaction_pool.on_broadcasted(propagated_to); - } -} diff --git a/client/network/transactions/Cargo.toml b/client/network/transactions/Cargo.toml index 89e9f8759a394..c1cd728e8c0de 100644 --- a/client/network/transactions/Cargo.toml +++ b/client/network/transactions/Cargo.toml @@ -14,6 +14,13 @@ readme = "README.md" targets = ["x86_64-unknown-linux-gnu"] [dependencies] +codec = { package = "parity-scale-codec", version = "3.0.0", features = ["derive"] } futures = "0.3.21" +hex = "0.4.0" +libp2p = "0.46.1" +log = "0.4.17" +pin-project = "1.0.10" +prometheus-endpoint = { package = "substrate-prometheus-endpoint", version = "0.10.0-dev", path = "../../../utils/prometheus" } sc-network-common = { version = "0.10.0-dev", path = "../common" } +sc-peerset = { version = "4.0.0-dev", path = "../../peerset" } sp-runtime = { version = "6.0.0", path = "../../../primitives/runtime" } diff --git a/client/network/transactions/src/config.rs b/client/network/transactions/src/config.rs index 72b6e8c633ce4..abb8cccd301ac 100644 --- a/client/network/transactions/src/config.rs +++ b/client/network/transactions/src/config.rs @@ -21,7 +21,21 @@ use futures::prelude::*; use sc_network_common::ExHashT; use sp_runtime::traits::Block as BlockT; -use std::{collections::HashMap, future::Future, pin::Pin}; +use std::{collections::HashMap, future::Future, pin::Pin, time}; + +/// Interval at which we propagate transactions; +pub(crate) const PROPAGATE_TIMEOUT: time::Duration = time::Duration::from_millis(2900); + +/// Maximum number of known transaction hashes to keep for a peer. +/// +/// This should be approx. 2 blocks full of transactions for the network to function properly. +pub(crate) const MAX_KNOWN_TRANSACTIONS: usize = 10240; // ~300kb per peer + overhead. + +/// Maximum allowed size for a transactions notification. +pub(crate) const MAX_TRANSACTIONS_SIZE: u64 = 16 * 1024 * 1024; + +/// Maximum number of transaction validation request we keep at any moment. +pub(crate) const MAX_PENDING_TRANSACTIONS: usize = 8192; /// Result of the transaction import. #[derive(Clone, Copy, Debug)] diff --git a/client/network/transactions/src/lib.rs b/client/network/transactions/src/lib.rs index 3a9da47ce562f..806c206e786d0 100644 --- a/client/network/transactions/src/lib.rs +++ b/client/network/transactions/src/lib.rs @@ -16,4 +16,473 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . +//! Transactions handling to plug on top of the network service. +//! +//! Usage: +//! +//! - Use [`TransactionsHandlerPrototype::new`] to create a prototype. +//! - Pass the return value of [`TransactionsHandlerPrototype::set_config`] to the network +//! configuration as an extra peers set. +//! - Use [`TransactionsHandlerPrototype::build`] then [`TransactionsHandler::run`] to obtain a +//! `Future` that processes transactions. + +use crate::config::*; +use codec::{Decode, Encode}; +use futures::{channel::mpsc, prelude::*, stream::FuturesUnordered}; +use libp2p::{multiaddr, PeerId}; +use log::{debug, trace, warn}; +use prometheus_endpoint::{register, Counter, PrometheusError, Registry, U64}; +use sc_network_common::{ + config::{NonDefaultSetConfig, NonReservedPeerMode, ProtocolId, SetConfig}, + error, + protocol::{ + event::{Event, ObservedRole}, + ProtocolName, + }, + service::{NetworkEventStream, NetworkNotification, NetworkPeers}, + utils::{interval, LruHashSet}, + ExHashT, +}; +use sp_runtime::traits::Block as BlockT; +use std::{ + collections::{hash_map::Entry, HashMap}, + iter, + num::NonZeroUsize, + pin::Pin, + sync::{ + atomic::{AtomicBool, Ordering}, + Arc, + }, + task::Poll, +}; + pub mod config; + +/// A set of transactions. +pub type Transactions = Vec; + +mod rep { + use sc_peerset::ReputationChange as Rep; + /// Reputation change when a peer sends us any transaction. + /// + /// This forces node to verify it, thus the negative value here. Once transaction is verified, + /// reputation change should be refunded with `ANY_TRANSACTION_REFUND` + pub const ANY_TRANSACTION: Rep = Rep::new(-(1 << 4), "Any transaction"); + /// Reputation change when a peer sends us any transaction that is not invalid. + pub const ANY_TRANSACTION_REFUND: Rep = Rep::new(1 << 4, "Any transaction (refund)"); + /// Reputation change when a peer sends us an transaction that we didn't know about. + pub const GOOD_TRANSACTION: Rep = Rep::new(1 << 7, "Good transaction"); + /// Reputation change when a peer sends us a bad transaction. + pub const BAD_TRANSACTION: Rep = Rep::new(-(1 << 12), "Bad transaction"); +} + +struct Metrics { + propagated_transactions: Counter, +} + +impl Metrics { + fn register(r: &Registry) -> Result { + Ok(Self { + propagated_transactions: register( + Counter::new( + "substrate_sync_propagated_transactions", + "Number of transactions propagated to at least one peer", + )?, + r, + )?, + }) + } +} + +#[pin_project::pin_project] +struct PendingTransaction { + #[pin] + validation: TransactionImportFuture, + tx_hash: H, +} + +impl Future for PendingTransaction { + type Output = (H, TransactionImport); + + fn poll(self: Pin<&mut Self>, cx: &mut std::task::Context<'_>) -> Poll { + let mut this = self.project(); + + if let Poll::Ready(import_result) = Pin::new(&mut this.validation).poll_unpin(cx) { + return Poll::Ready((this.tx_hash.clone(), import_result)) + } + + Poll::Pending + } +} + +/// Prototype for a [`TransactionsHandler`]. +pub struct TransactionsHandlerPrototype { + protocol_name: ProtocolName, + fallback_protocol_names: Vec, +} + +impl TransactionsHandlerPrototype { + /// Create a new instance. + pub fn new>( + protocol_id: ProtocolId, + genesis_hash: Hash, + fork_id: Option, + ) -> Self { + let protocol_name = if let Some(fork_id) = fork_id { + format!("/{}/{}/transactions/1", hex::encode(genesis_hash), fork_id) + } else { + format!("/{}/transactions/1", hex::encode(genesis_hash)) + }; + let legacy_protocol_name = format!("/{}/transactions/1", protocol_id.as_ref()); + + Self { + protocol_name: protocol_name.into(), + fallback_protocol_names: iter::once(legacy_protocol_name.into()).collect(), + } + } + + /// Returns the configuration of the set to put in the network configuration. + pub fn set_config(&self) -> NonDefaultSetConfig { + NonDefaultSetConfig { + notifications_protocol: self.protocol_name.clone(), + fallback_names: self.fallback_protocol_names.clone(), + max_notification_size: MAX_TRANSACTIONS_SIZE, + set_config: SetConfig { + in_peers: 0, + out_peers: 0, + reserved_nodes: Vec::new(), + non_reserved_mode: NonReservedPeerMode::Deny, + }, + } + } + + /// Turns the prototype into the actual handler. Returns a controller that allows controlling + /// the behaviour of the handler while it's running. + /// + /// Important: the transactions handler is initially disabled and doesn't gossip transactions. + /// You must call [`TransactionsHandlerController::set_gossip_enabled`] to enable it. + pub fn build< + B: BlockT + 'static, + H: ExHashT, + S: NetworkPeers + NetworkEventStream + NetworkNotification, + >( + self, + service: S, + transaction_pool: Arc>, + metrics_registry: Option<&Registry>, + ) -> error::Result<(TransactionsHandler, TransactionsHandlerController)> { + let event_stream = service.event_stream("transactions-handler"); + let (to_handler, from_controller) = mpsc::unbounded(); + let gossip_enabled = Arc::new(AtomicBool::new(false)); + + let handler = TransactionsHandler { + protocol_name: self.protocol_name, + propagate_timeout: Box::pin(interval(PROPAGATE_TIMEOUT)), + pending_transactions: FuturesUnordered::new(), + pending_transactions_peers: HashMap::new(), + gossip_enabled: gossip_enabled.clone(), + service, + event_stream, + peers: HashMap::new(), + transaction_pool, + from_controller, + metrics: if let Some(r) = metrics_registry { + Some(Metrics::register(r)?) + } else { + None + }, + }; + + let controller = TransactionsHandlerController { to_handler, gossip_enabled }; + + Ok((handler, controller)) + } +} + +/// Controls the behaviour of a [`TransactionsHandler`] it is connected to. +pub struct TransactionsHandlerController { + to_handler: mpsc::UnboundedSender>, + gossip_enabled: Arc, +} + +impl TransactionsHandlerController { + /// Controls whether transactions are being gossiped on the network. + pub fn set_gossip_enabled(&mut self, enabled: bool) { + self.gossip_enabled.store(enabled, Ordering::Relaxed); + } + + /// You may call this when new transactions are imported by the transaction pool. + /// + /// All transactions will be fetched from the `TransactionPool` that was passed at + /// initialization as part of the configuration and propagated to peers. + pub fn propagate_transactions(&self) { + let _ = self.to_handler.unbounded_send(ToHandler::PropagateTransactions); + } + + /// You must call when new a transaction is imported by the transaction pool. + /// + /// This transaction will be fetched from the `TransactionPool` that was passed at + /// initialization as part of the configuration and propagated to peers. + pub fn propagate_transaction(&self, hash: H) { + let _ = self.to_handler.unbounded_send(ToHandler::PropagateTransaction(hash)); + } +} + +enum ToHandler { + PropagateTransactions, + PropagateTransaction(H), +} + +/// Handler for transactions. Call [`TransactionsHandler::run`] to start the processing. +pub struct TransactionsHandler< + B: BlockT + 'static, + H: ExHashT, + S: NetworkPeers + NetworkEventStream + NetworkNotification, +> { + protocol_name: ProtocolName, + /// Interval at which we call `propagate_transactions`. + propagate_timeout: Pin + Send>>, + /// Pending transactions verification tasks. + pending_transactions: FuturesUnordered>, + /// As multiple peers can send us the same transaction, we group + /// these peers using the transaction hash while the transaction is + /// imported. This prevents that we import the same transaction + /// multiple times concurrently. + pending_transactions_peers: HashMap>, + /// Network service to use to send messages and manage peers. + service: S, + /// Stream of networking events. + event_stream: Pin + Send>>, + // All connected peers + peers: HashMap>, + transaction_pool: Arc>, + gossip_enabled: Arc, + from_controller: mpsc::UnboundedReceiver>, + /// Prometheus metrics. + metrics: Option, +} + +/// Peer information +#[derive(Debug)] +struct Peer { + /// Holds a set of transactions known to this peer. + known_transactions: LruHashSet, + role: ObservedRole, +} + +impl TransactionsHandler +where + B: BlockT + 'static, + H: ExHashT, + S: NetworkPeers + NetworkEventStream + NetworkNotification, +{ + /// Turns the [`TransactionsHandler`] into a future that should run forever and not be + /// interrupted. + pub async fn run(mut self) { + loop { + futures::select! { + _ = self.propagate_timeout.next().fuse() => { + self.propagate_transactions(); + }, + (tx_hash, result) = self.pending_transactions.select_next_some() => { + if let Some(peers) = self.pending_transactions_peers.remove(&tx_hash) { + peers.into_iter().for_each(|p| self.on_handle_transaction_import(p, result)); + } else { + warn!(target: "sub-libp2p", "Inconsistent state, no peers for pending transaction!"); + } + }, + network_event = self.event_stream.next().fuse() => { + if let Some(network_event) = network_event { + self.handle_network_event(network_event).await; + } else { + // Networking has seemingly closed. Closing as well. + return; + } + }, + message = self.from_controller.select_next_some().fuse() => { + match message { + ToHandler::PropagateTransaction(hash) => self.propagate_transaction(&hash), + ToHandler::PropagateTransactions => self.propagate_transactions(), + } + }, + } + } + } + + async fn handle_network_event(&mut self, event: Event) { + match event { + Event::Dht(_) => {}, + Event::SyncConnected { remote } => { + let addr = iter::once(multiaddr::Protocol::P2p(remote.into())) + .collect::(); + let result = self.service.add_peers_to_reserved_set( + self.protocol_name.clone(), + iter::once(addr).collect(), + ); + if let Err(err) = result { + log::error!(target: "sync", "Add reserved peer failed: {}", err); + } + }, + Event::SyncDisconnected { remote } => { + self.service.remove_peers_from_reserved_set( + self.protocol_name.clone(), + iter::once(remote).collect(), + ); + }, + + Event::NotificationStreamOpened { remote, protocol, role, .. } + if protocol == self.protocol_name => + { + let _was_in = self.peers.insert( + remote, + Peer { + known_transactions: LruHashSet::new( + NonZeroUsize::new(MAX_KNOWN_TRANSACTIONS).expect("Constant is nonzero"), + ), + role, + }, + ); + debug_assert!(_was_in.is_none()); + }, + Event::NotificationStreamClosed { remote, protocol } + if protocol == self.protocol_name => + { + let _peer = self.peers.remove(&remote); + debug_assert!(_peer.is_some()); + }, + + Event::NotificationsReceived { remote, messages } => { + for (protocol, message) in messages { + if protocol != self.protocol_name { + continue + } + + if let Ok(m) = + as Decode>::decode(&mut message.as_ref()) + { + self.on_transactions(remote, m); + } else { + warn!(target: "sub-libp2p", "Failed to decode transactions list"); + } + } + }, + + // Not our concern. + Event::NotificationStreamOpened { .. } | Event::NotificationStreamClosed { .. } => {}, + } + } + + /// Called when peer sends us new transactions + fn on_transactions(&mut self, who: PeerId, transactions: Transactions) { + // Accept transactions only when enabled + if !self.gossip_enabled.load(Ordering::Relaxed) { + trace!(target: "sync", "{} Ignoring transactions while disabled", who); + return + } + + trace!(target: "sync", "Received {} transactions from {}", transactions.len(), who); + if let Some(ref mut peer) = self.peers.get_mut(&who) { + for t in transactions { + if self.pending_transactions.len() > MAX_PENDING_TRANSACTIONS { + debug!( + target: "sync", + "Ignoring any further transactions that exceed `MAX_PENDING_TRANSACTIONS`({}) limit", + MAX_PENDING_TRANSACTIONS, + ); + break + } + + let hash = self.transaction_pool.hash_of(&t); + peer.known_transactions.insert(hash.clone()); + + self.service.report_peer(who, rep::ANY_TRANSACTION); + + match self.pending_transactions_peers.entry(hash.clone()) { + Entry::Vacant(entry) => { + self.pending_transactions.push(PendingTransaction { + validation: self.transaction_pool.import(t), + tx_hash: hash, + }); + entry.insert(vec![who]); + }, + Entry::Occupied(mut entry) => { + entry.get_mut().push(who); + }, + } + } + } + } + + fn on_handle_transaction_import(&mut self, who: PeerId, import: TransactionImport) { + match import { + TransactionImport::KnownGood => + self.service.report_peer(who, rep::ANY_TRANSACTION_REFUND), + TransactionImport::NewGood => self.service.report_peer(who, rep::GOOD_TRANSACTION), + TransactionImport::Bad => self.service.report_peer(who, rep::BAD_TRANSACTION), + TransactionImport::None => {}, + } + } + + /// Propagate one transaction. + pub fn propagate_transaction(&mut self, hash: &H) { + // Accept transactions only when enabled + if !self.gossip_enabled.load(Ordering::Relaxed) { + return + } + debug!(target: "sync", "Propagating transaction [{:?}]", hash); + if let Some(transaction) = self.transaction_pool.transaction(hash) { + let propagated_to = self.do_propagate_transactions(&[(hash.clone(), transaction)]); + self.transaction_pool.on_broadcasted(propagated_to); + } + } + + fn do_propagate_transactions( + &mut self, + transactions: &[(H, B::Extrinsic)], + ) -> HashMap> { + let mut propagated_to = HashMap::<_, Vec<_>>::new(); + let mut propagated_transactions = 0; + + for (who, peer) in self.peers.iter_mut() { + // never send transactions to the light node + if matches!(peer.role, ObservedRole::Light) { + continue + } + + let (hashes, to_send): (Vec<_>, Vec<_>) = transactions + .iter() + .filter(|&(ref hash, _)| peer.known_transactions.insert(hash.clone())) + .cloned() + .unzip(); + + propagated_transactions += hashes.len(); + + if !to_send.is_empty() { + for hash in hashes { + propagated_to.entry(hash).or_default().push(who.to_base58()); + } + trace!(target: "sync", "Sending {} transactions to {}", to_send.len(), who); + self.service + .write_notification(*who, self.protocol_name.clone(), to_send.encode()); + } + } + + if let Some(ref metrics) = self.metrics { + metrics.propagated_transactions.inc_by(propagated_transactions as _) + } + + propagated_to + } + + /// Call when we must propagate ready transactions to peers. + fn propagate_transactions(&mut self) { + // Accept transactions only when enabled + if !self.gossip_enabled.load(Ordering::Relaxed) { + return + } + debug!(target: "sync", "Propagating transactions"); + let transactions = self.transaction_pool.transactions(); + let propagated_to = self.do_propagate_transactions(&transactions); + self.transaction_pool.on_broadcasted(propagated_to); + } +} From aa0bdfbd428f7f35ce2248d8b5b0f90ad167942d Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Tue, 6 Sep 2022 16:45:45 +0300 Subject: [PATCH 05/17] Update documentation --- client/network/common/src/config.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/network/common/src/config.rs b/client/network/common/src/config.rs index eeee2ce1c0e1b..12a0d56f4ac18 100644 --- a/client/network/common/src/config.rs +++ b/client/network/common/src/config.rs @@ -216,7 +216,7 @@ pub struct NonDefaultSetConfig { /// one. /// /// If a fallback is used, it will be reported in - /// [`crate::Event::NotificationStreamOpened::negotiated_fallback`]. + /// `sc_network::protocol::event::Event::NotificationStreamOpened::negotiated_fallback` pub fallback_names: Vec, /// Maximum allowed size of single notifications. pub max_notification_size: u64, @@ -271,7 +271,7 @@ pub enum TransportConfig { /// If true, allow connecting to private IPv4 addresses (as defined in /// [RFC1918](https://tools.ietf.org/html/rfc1918)). Irrelevant for addresses that have - /// been passed in [`NetworkConfiguration::boot_nodes`]. + /// been passed in `::sc_network::config::NetworkConfiguration::boot_nodes`. allow_private_ipv4: bool, }, From 5db7a269bfa70170d25d5bab3aba11c0b3e5cd4d Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Tue, 6 Sep 2022 17:59:54 +0300 Subject: [PATCH 06/17] Format code --- client/network/src/config.rs | 7 +++---- client/network/src/discovery.rs | 5 +---- client/network/src/peer_info.rs | 2 +- client/network/src/protocol.rs | 4 ++-- client/network/src/service.rs | 4 ++-- client/network/src/service/tests.rs | 12 +++--------- client/service/src/lib.rs | 3 ++- 7 files changed, 14 insertions(+), 23 deletions(-) diff --git a/client/network/src/config.rs b/client/network/src/config.rs index 894c87fa4e7fa..7cc3dee7c5f78 100644 --- a/client/network/src/config.rs +++ b/client/network/src/config.rs @@ -27,7 +27,7 @@ pub use sc_network_common::{ IncomingRequest, OutgoingResponse, ProtocolConfig as RequestResponseConfig, }, sync::warp::WarpSyncProvider, - ExHashT, + ExHashT, }; pub use libp2p::{build_multiaddr, core::PublicKey, identity}; @@ -42,9 +42,8 @@ use libp2p::{ use prometheus_endpoint::Registry; use sc_consensus::ImportQueue; use sc_network_common::{ - config::MultiaddrWithPeerId, - sync::ChainSync, - config::{SetConfig, NonDefaultSetConfig, TransportConfig} + config::{MultiaddrWithPeerId, NonDefaultSetConfig, SetConfig, TransportConfig}, + sync::ChainSync, }; use sc_network_transactions::config::TransactionPool; use sp_runtime::traits::Block as BlockT; diff --git a/client/network/src/discovery.rs b/client/network/src/discovery.rs index a4ac25dc60e5b..8422e34485125 100644 --- a/client/network/src/discovery.rs +++ b/client/network/src/discovery.rs @@ -71,10 +71,7 @@ use libp2p::{ }, }; use log::{debug, error, info, trace, warn}; -use sc_network_common::{ - config::ProtocolId, - utils::LruHashSet, -}; +use sc_network_common::{config::ProtocolId, utils::LruHashSet}; use sp_core::hexdisplay::HexDisplay; use std::{ cmp, diff --git a/client/network/src/peer_info.rs b/client/network/src/peer_info.rs index 2c7bdc0291f78..c62c2ea1c5d98 100644 --- a/client/network/src/peer_info.rs +++ b/client/network/src/peer_info.rs @@ -31,8 +31,8 @@ use libp2p::{ }, Multiaddr, }; -use sc_network_common::utils::interval; use log::{debug, error, trace}; +use sc_network_common::utils::interval; use smallvec::SmallVec; use std::{ collections::hash_map::Entry, diff --git a/client/network/src/protocol.rs b/client/network/src/protocol.rs index 6373c9255786f..9e88b02202c85 100644 --- a/client/network/src/protocol.rs +++ b/client/network/src/protocol.rs @@ -40,8 +40,8 @@ use prometheus_endpoint::{register, Gauge, GaugeVec, Opts, PrometheusError, Regi use sc_client_api::HeaderBackend; use sc_consensus::import_queue::{BlockImportError, BlockImportStatus, IncomingBlock, Origin}; use sc_network_common::{ - config::{ProtocolId,NonReservedPeerMode}, - error, + config::{NonReservedPeerMode, ProtocolId}, + error, protocol::ProtocolName, request_responses::RequestFailure, sync::{ diff --git a/client/network/src/service.rs b/client/network/src/service.rs index 86fa37f1cae0c..36affaf3d69cd 100644 --- a/client/network/src/service.rs +++ b/client/network/src/service.rs @@ -59,9 +59,8 @@ use metrics::{Histogram, HistogramVec, MetricSources, Metrics}; use parking_lot::Mutex; use sc_consensus::{BlockImportError, BlockImportStatus, ImportQueue, Link}; use sc_network_common::{ - ExHashT, - error::Error, config::{MultiaddrWithPeerId, TransportConfig}, + error::Error, protocol::{ event::{DhtEvent, Event}, ProtocolName, @@ -74,6 +73,7 @@ use sc_network_common::{ NotificationSenderReady as NotificationSenderReadyT, Signature, SigningError, }, sync::{SyncState, SyncStatus}, + ExHashT, }; use sc_peerset::PeersetHandle; use sc_utils::mpsc::{tracing_unbounded, TracingUnboundedReceiver, TracingUnboundedSender}; diff --git a/client/network/src/service/tests.rs b/client/network/src/service/tests.rs index 0af532bcedd89..627ab7bc6c8e0 100644 --- a/client/network/src/service/tests.rs +++ b/client/network/src/service/tests.rs @@ -25,12 +25,12 @@ use sc_network_common::{ protocol::event::Event, service::{NetworkEventStream, NetworkNotification, NetworkPeers, NetworkStateInfo}, }; -use sc_network_transactions::config::EmptyTransactionPool; use sc_network_light::light_client_requests::handler::LightClientRequestHandler; use sc_network_sync::{ block_request_handler::BlockRequestHandler, state_request_handler::StateRequestHandler, ChainSync, }; +use sc_network_transactions::config::EmptyTransactionPool; use sp_consensus::block_validation::DefaultBlockAnnounceValidator; use sp_runtime::traits::{Block as BlockT, Header as _}; use std::{sync::Arc, time::Duration}; @@ -634,10 +634,7 @@ fn ensure_reserved_node_addresses_consistent_with_transport_memory() { let _ = build_test_full_node(config::NetworkConfiguration { listen_addresses: vec![listen_addr.clone()], transport: TransportConfig::MemoryOnly, - default_peers_set: SetConfig { - reserved_nodes: vec![reserved_node], - ..Default::default() - }, + default_peers_set: SetConfig { reserved_nodes: vec![reserved_node], ..Default::default() }, ..config::NetworkConfiguration::new("test-node", "test-client", Default::default(), None) }); } @@ -653,10 +650,7 @@ fn ensure_reserved_node_addresses_consistent_with_transport_not_memory() { let _ = build_test_full_node(config::NetworkConfiguration { listen_addresses: vec![listen_addr.clone()], - default_peers_set: SetConfig { - reserved_nodes: vec![reserved_node], - ..Default::default() - }, + default_peers_set: SetConfig { reserved_nodes: vec![reserved_node], ..Default::default() }, ..config::NetworkConfiguration::new("test-node", "test-client", Default::default(), None) }); } diff --git a/client/service/src/lib.rs b/client/service/src/lib.rs index 9e1c19294f0b8..091b4bbe9fe5f 100644 --- a/client/service/src/lib.rs +++ b/client/service/src/lib.rs @@ -415,7 +415,8 @@ where .collect() } -impl sc_network_transactions::config::TransactionPool for TransactionPoolAdapter +impl sc_network_transactions::config::TransactionPool + for TransactionPoolAdapter where C: HeaderBackend + BlockBackend From 5a9025b7b01ad46f46b6d66c1d3e0cb03ecd31ca Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Tue, 6 Sep 2022 18:08:23 +0300 Subject: [PATCH 07/17] Format more code --- client/finality-grandpa/src/lib.rs | 4 +--- client/network/common/src/config.rs | 1 - client/network/common/src/error.rs | 5 +---- 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/client/finality-grandpa/src/lib.rs b/client/finality-grandpa/src/lib.rs index 062f3ed19574b..ce48ec3dd01e5 100644 --- a/client/finality-grandpa/src/lib.rs +++ b/client/finality-grandpa/src/lib.rs @@ -68,9 +68,7 @@ use sc_client_api::{ StorageProvider, TransactionFor, }; use sc_consensus::BlockImport; -use sc_network_common::{ - protocol::ProtocolName, -}; +use sc_network_common::protocol::ProtocolName; use sc_telemetry::{telemetry, TelemetryHandle, CONSENSUS_DEBUG, CONSENSUS_INFO}; use sc_utils::mpsc::{tracing_unbounded, TracingUnboundedReceiver}; use sp_api::ProvideRuntimeApi; diff --git a/client/network/common/src/config.rs b/client/network/common/src/config.rs index 12a0d56f4ac18..fb23cd0174922 100644 --- a/client/network/common/src/config.rs +++ b/client/network/common/src/config.rs @@ -299,4 +299,3 @@ impl NonReservedPeerMode { } } } - diff --git a/client/network/common/src/error.rs b/client/network/common/src/error.rs index e54e8c465783c..4326b1af52836 100644 --- a/client/network/common/src/error.rs +++ b/client/network/common/src/error.rs @@ -18,10 +18,7 @@ //! Substrate network possible errors. -use crate::{ - config::TransportConfig, - protocol::ProtocolName, -}; +use crate::{config::TransportConfig, protocol::ProtocolName}; use libp2p::{Multiaddr, PeerId}; use std::fmt; From f4960660853ca3b73b62c374b4127e8a78c20005 Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Tue, 6 Sep 2022 18:12:38 +0300 Subject: [PATCH 08/17] Format test code --- client/network/test/src/lib.rs | 6 ++++-- client/service/test/src/lib.rs | 5 +---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/client/network/test/src/lib.rs b/client/network/test/src/lib.rs index 540469bd25418..233e6bcac24b1 100644 --- a/client/network/test/src/lib.rs +++ b/client/network/test/src/lib.rs @@ -47,13 +47,14 @@ use sc_consensus::{ ForkChoiceStrategy, ImportResult, JustificationImport, JustificationSyncLink, LongestChain, Verifier, }; -pub use sc_network_transactions::config::EmptyTransactionPool; use sc_network::{ config::{NetworkConfiguration, Role, SyncMode}, Multiaddr, NetworkService, NetworkWorker, }; use sc_network_common::{ - config::{MultiaddrWithPeerId, ProtocolId, NonDefaultSetConfig, NonReservedPeerMode, TransportConfig}, + config::{ + MultiaddrWithPeerId, NonDefaultSetConfig, NonReservedPeerMode, ProtocolId, TransportConfig, + }, protocol::ProtocolName, service::{NetworkBlock, NetworkStateInfo, NetworkSyncForkRequest}, sync::warp::{AuthorityList, EncodedProof, SetId, VerificationResult, WarpSyncProvider}, @@ -63,6 +64,7 @@ use sc_network_sync::{ block_request_handler::BlockRequestHandler, state_request_handler::StateRequestHandler, warp_request_handler, ChainSync, }; +pub use sc_network_transactions::config::EmptyTransactionPool; use sc_service::client::Client; use sp_blockchain::{ well_known_cache_keys::{self, Id as CacheKeyId}, diff --git a/client/service/test/src/lib.rs b/client/service/test/src/lib.rs index 978363990d7cf..1e1b2ecbbc81c 100644 --- a/client/service/test/src/lib.rs +++ b/client/service/test/src/lib.rs @@ -22,10 +22,7 @@ use futures::{task::Poll, Future, TryFutureExt as _}; use log::{debug, info}; use parking_lot::Mutex; use sc_client_api::{Backend, CallExecutor}; -use sc_network::{ - config::NetworkConfiguration, - multiaddr, -}; +use sc_network::{config::NetworkConfiguration, multiaddr}; use sc_network_common::{ config::{MultiaddrWithPeerId, TransportConfig}, service::{NetworkBlock, NetworkPeers, NetworkStateInfo}, From 21eee686509f5447ee7b52d28054a90844f3d8f8 Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Wed, 7 Sep 2022 16:37:53 +0300 Subject: [PATCH 09/17] wip --- client/network/src/protocol/message.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/network/src/protocol/message.rs b/client/network/src/protocol/message.rs index 9a0cb873ee1c6..9b449d6d7d994 100644 --- a/client/network/src/protocol/message.rs +++ b/client/network/src/protocol/message.rs @@ -144,7 +144,7 @@ pub mod generic { /// Block announce. BlockAnnounce(BlockAnnounce
), /// Transactions. - Transactions(sc_network_transactions::Transactions), + Transactions, /// Consensus protocol message. Consensus(ConsensusMessage), /// Remote method call request. From 4a4e9f841d2614c404d7dfa970e8a3e045c4d96a Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Thu, 8 Sep 2022 17:13:25 +0300 Subject: [PATCH 10/17] Remove sc-network-transactions dependency from sc-network --- Cargo.lock | 1 - bin/node-template/node/src/service.rs | 3 +- bin/node/cli/src/service.rs | 3 +- client/network/Cargo.toml | 1 - client/network/src/config.rs | 13 +----- client/network/src/service.rs | 41 ++++-------------- client/network/src/service/tests.rs | 5 --- client/network/test/src/lib.rs | 4 -- client/network/transactions/src/lib.rs | 2 +- client/service/src/builder.rs | 59 ++++++++++++++++++-------- 10 files changed, 55 insertions(+), 77 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0f4a661251fe2..d8dda8c85f921 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8419,7 +8419,6 @@ dependencies = [ "sc-network-common", "sc-network-light", "sc-network-sync", - "sc-network-transactions", "sc-peerset", "sc-utils", "serde", diff --git a/bin/node-template/node/src/service.rs b/bin/node-template/node/src/service.rs index ffb2440caa0ed..c8a739e2781f7 100644 --- a/bin/node-template/node/src/service.rs +++ b/bin/node-template/node/src/service.rs @@ -194,7 +194,7 @@ pub fn new_full(mut config: Configuration) -> Result Vec::default(), )); - let (network, system_rpc_tx, network_starter) = + let (network, system_rpc_tx, tx_handler_controller, network_starter) = sc_service::build_network(sc_service::BuildNetworkParams { config: &config, client: client.clone(), @@ -241,6 +241,7 @@ pub fn new_full(mut config: Configuration) -> Result rpc_builder: rpc_extensions_builder, backend, system_rpc_tx, + tx_handler_controller, config, telemetry: telemetry.as_mut(), })?; diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index 13003c1a7a41f..dcbcebcf77c0f 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -355,7 +355,7 @@ pub fn new_full_base( Vec::default(), )); - let (network, system_rpc_tx, network_starter) = + let (network, system_rpc_tx, tx_handler_controller, network_starter) = sc_service::build_network(sc_service::BuildNetworkParams { config: &config, client: client.clone(), @@ -393,6 +393,7 @@ pub fn new_full_base( transaction_pool: transaction_pool.clone(), task_manager: &mut task_manager, system_rpc_tx, + tx_handler_controller, telemetry: telemetry.as_mut(), })?; diff --git a/client/network/Cargo.toml b/client/network/Cargo.toml index a775b120b5497..82f77fa44450f 100644 --- a/client/network/Cargo.toml +++ b/client/network/Cargo.toml @@ -51,7 +51,6 @@ sc-block-builder = { version = "0.10.0-dev", path = "../block-builder" } sc-client-api = { version = "4.0.0-dev", path = "../api" } sc-consensus = { version = "0.10.0-dev", path = "../consensus/common" } sc-network-common = { version = "0.10.0-dev", path = "./common" } -sc-network-transactions = { version = "0.10.0-dev", path = "./transactions" } sc-peerset = { version = "4.0.0-dev", path = "../peerset" } sc-utils = { version = "4.0.0-dev", path = "../utils" } sp-arithmetic = { version = "5.0.0", path = "../../primitives/arithmetic" } diff --git a/client/network/src/config.rs b/client/network/src/config.rs index 7cc3dee7c5f78..9a056c459a020 100644 --- a/client/network/src/config.rs +++ b/client/network/src/config.rs @@ -45,7 +45,6 @@ use sc_network_common::{ config::{MultiaddrWithPeerId, NonDefaultSetConfig, SetConfig, TransportConfig}, sync::ChainSync, }; -use sc_network_transactions::config::TransactionPool; use sp_runtime::traits::Block as BlockT; use std::{ error::Error, @@ -60,10 +59,9 @@ use std::{ use zeroize::Zeroize; /// Network initialization parameters. -pub struct Params +pub struct Params where B: BlockT + 'static, - H: ExHashT, { /// Assigned role for our node (full, light, ...). pub role: Role, @@ -72,9 +70,6 @@ where /// default. pub executor: Option + Send>>) + Send>>, - /// How to spawn the background task dedicated to the transactions handler. - pub transactions_handler_executor: Box + Send>>) + Send>, - /// Network layer configuration. pub network_config: NetworkConfiguration, @@ -84,12 +79,6 @@ where /// Bitswap block request protocol implementation. pub bitswap: Option>, - /// Pool of transactions. - /// - /// The network worker will fetch transactions from this object in order to propagate them on - /// the network. - pub transaction_pool: Arc>, - /// Legacy name of the protocol to use on the wire. Should be different for each chain. pub protocol_id: ProtocolId, diff --git a/client/network/src/service.rs b/client/network/src/service.rs index 36affaf3d69cd..fecee772cc7d2 100644 --- a/client/network/src/service.rs +++ b/client/network/src/service.rs @@ -145,7 +145,7 @@ where /// Returns a `NetworkWorker` that implements `Future` and must be regularly polled in order /// for the network processing to advance. From it, you can extract a `NetworkService` using /// `worker.service()`. The `NetworkService` can be shared through the codebase. - pub fn new(mut params: Params) -> Result { + pub fn new(mut params: Params) -> Result { // Private and public keys configuration. let local_identity = params.network_config.node_key.clone().into_keypair()?; let local_public = local_identity.public(); @@ -211,21 +211,6 @@ where fs::create_dir_all(path)?; } - let transactions_handler_proto = sc_network_transactions::TransactionsHandlerPrototype::new( - params.protocol_id.clone(), - params - .chain - .hash(0u32.into()) - .ok() - .flatten() - .expect("Genesis block exists; qed"), - params.fork_id.clone(), - ); - params - .network_config - .extra_sets - .insert(0, transactions_handler_proto.set_config()); - info!( target: "sub-libp2p", "🏷 Local node identity is: {}", @@ -242,7 +227,7 @@ where ¶ms.network_config, iter::once(Vec::new()) .chain( - (0..params.network_config.extra_sets.len() - 1) + (0..params.network_config.extra_sets.len().saturating_sub(1)) .map(|_| default_notif_handshake_message.clone()), ) .collect(), @@ -462,13 +447,6 @@ where _marker: PhantomData, }); - let (tx_handler, tx_handler_controller) = transactions_handler_proto.build( - service.clone(), - params.transaction_pool, - params.metrics_registry.as_ref(), - )?; - (params.transactions_handler_executor)(tx_handler.run().boxed()); - Ok(NetworkWorker { external_addresses, num_connected, @@ -479,9 +457,9 @@ where from_service, event_streams: out_events::OutChannels::new(params.metrics_registry.as_ref())?, peers_notifications_sinks, - tx_handler_controller, metrics, boot_node_ids, + _marker: Default::default(), }) } @@ -1316,8 +1294,9 @@ where /// For each peer and protocol combination, an object that allows sending notifications to /// that peer. Shared with the [`NetworkService`]. peers_notifications_sinks: Arc>>, - /// Controller for the handler of incoming and outgoing transactions. - tx_handler_controller: sc_network_transactions::TransactionsHandlerController, + /// Marker to pin the `H` generic. Serves no purpose except to not break backwards + /// compatibility. + _marker: PhantomData, } impl Future for NetworkWorker @@ -1373,10 +1352,8 @@ where .behaviour_mut() .user_protocol_mut() .clear_justification_requests(), - ServiceToWorkerMsg::PropagateTransaction(hash) => - this.tx_handler_controller.propagate_transaction(hash), - ServiceToWorkerMsg::PropagateTransactions => - this.tx_handler_controller.propagate_transactions(), + ServiceToWorkerMsg::PropagateTransaction(_) => {}, + ServiceToWorkerMsg::PropagateTransactions => {}, ServiceToWorkerMsg::GetValue(key) => this.network_service.behaviour_mut().get_value(key), ServiceToWorkerMsg::PutValue(key, value) => @@ -1923,8 +1900,6 @@ where SyncState::Downloading => true, }; - this.tx_handler_controller.set_gossip_enabled(!is_major_syncing); - this.is_major_syncing.store(is_major_syncing, Ordering::Relaxed); if let Some(metrics) = this.metrics.as_ref() { diff --git a/client/network/src/service/tests.rs b/client/network/src/service/tests.rs index 627ab7bc6c8e0..33f6dd95465fb 100644 --- a/client/network/src/service/tests.rs +++ b/client/network/src/service/tests.rs @@ -30,7 +30,6 @@ use sc_network_sync::{ block_request_handler::BlockRequestHandler, state_request_handler::StateRequestHandler, ChainSync, }; -use sc_network_transactions::config::EmptyTransactionPool; use sp_consensus::block_validation::DefaultBlockAnnounceValidator; use sp_runtime::traits::{Block as BlockT, Header as _}; use std::{sync::Arc, time::Duration}; @@ -136,12 +135,8 @@ fn build_test_full_node( let worker = NetworkWorker::new(config::Params { role: config::Role::Full, executor: None, - transactions_handler_executor: Box::new(|task| { - async_std::task::spawn(task); - }), network_config, chain: client.clone(), - transaction_pool: Arc::new(EmptyTransactionPool), protocol_id, fork_id, import_queue, diff --git a/client/network/test/src/lib.rs b/client/network/test/src/lib.rs index 233e6bcac24b1..0ed380c02f988 100644 --- a/client/network/test/src/lib.rs +++ b/client/network/test/src/lib.rs @@ -871,12 +871,8 @@ where let network = NetworkWorker::new(sc_network::config::Params { role: if config.is_authority { Role::Authority } else { Role::Full }, executor: None, - transactions_handler_executor: Box::new(|task| { - async_std::task::spawn(task); - }), network_config, chain: client.clone(), - transaction_pool: Arc::new(EmptyTransactionPool), protocol_id, fork_id, import_queue, diff --git a/client/network/transactions/src/lib.rs b/client/network/transactions/src/lib.rs index 806c206e786d0..fe9aa129466d9 100644 --- a/client/network/transactions/src/lib.rs +++ b/client/network/transactions/src/lib.rs @@ -126,7 +126,7 @@ impl TransactionsHandlerPrototype { pub fn new>( protocol_id: ProtocolId, genesis_hash: Hash, - fork_id: Option, + fork_id: Option<&str>, ) -> Self { let protocol_name = if let Some(fork_id) = fork_id { format!("/{}/{}/transactions/1", hex::encode(genesis_hash), fork_id) diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index f03ba6de2866d..7fb33bb5ffcad 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -367,6 +367,9 @@ pub struct SpawnTasksParams<'a, TBl: BlockT, TCl, TExPool, TRpc, Backend> { pub network: Arc>, /// A Sender for RPC requests. pub system_rpc_tx: TracingUnboundedSender>, + /// Controller for transactions handlers + pub tx_handler_controller: + sc_network_transactions::TransactionsHandlerController<::Hash>, /// Telemetry instance for this node. pub telemetry: Option<&'a mut Telemetry>, } @@ -445,6 +448,7 @@ where rpc_builder, network, system_rpc_tx, + tx_handler_controller, telemetry, } = params; @@ -480,7 +484,11 @@ where spawn_handle.spawn( "on-transaction-imported", Some("transaction-pool"), - transaction_notifications(transaction_pool.clone(), network.clone(), telemetry.clone()), + transaction_notifications( + transaction_pool.clone(), + tx_handler_controller, + telemetry.clone(), + ), ); // Prometheus metrics. @@ -543,20 +551,21 @@ where Ok(rpc_handlers) } -async fn transaction_notifications( +async fn transaction_notifications( transaction_pool: Arc, - network: Network, + tx_handler_controller: sc_network_transactions::TransactionsHandlerController< + ::Hash, + >, telemetry: Option, ) where Block: BlockT, ExPool: MaintainedTransactionPool::Hash>, - Network: NetworkTransaction<::Hash> + Send + Sync, { // transaction notifications transaction_pool .import_notification_stream() .for_each(move |hash| { - network.propagate_transaction(hash); + tx_handler_controller.propagate_transaction(hash); let status = transaction_pool.status(); telemetry!( telemetry; @@ -718,6 +727,7 @@ pub fn build_network( ( Arc::Hash>>, TracingUnboundedSender>, + sc_network_transactions::TransactionsHandlerController<::Hash>, NetworkStarter, ), Error, @@ -758,9 +768,6 @@ where } } - let transaction_pool_adapter = - Arc::new(TransactionPoolAdapter { pool: transaction_pool, client: client.clone() }); - let protocol_id = config.protocol_id(); let block_announce_validator = if let Some(f) = block_announce_validator_builder { @@ -835,7 +842,8 @@ where config.network.max_parallel_downloads, warp_sync_provider, )?; - let network_params = sc_network::config::Params { + + let mut network_params = sc_network::config::Params { role: config.role.clone(), executor: { let spawn_handle = Clone::clone(&spawn_handle); @@ -843,16 +851,9 @@ where spawn_handle.spawn("libp2p-node", Some("networking"), fut); })) }, - transactions_handler_executor: { - let spawn_handle = Clone::clone(&spawn_handle); - Box::new(move |fut| { - spawn_handle.spawn("network-transactions-handler", Some("networking"), fut); - }) - }, network_config: config.network.clone(), chain: client.clone(), - transaction_pool: transaction_pool_adapter as _, - protocol_id, + protocol_id: protocol_id.clone(), fork_id: config.chain_spec.fork_id().map(ToOwned::to_owned), import_queue: Box::new(import_queue), chain_sync: Box::new(chain_sync), @@ -864,10 +865,32 @@ where light_client_request_protocol_config, }; + // crate transactions protocol and add it to the list of supported protocols of `network_params` + let transactions_handler_proto = sc_network_transactions::TransactionsHandlerPrototype::new( + protocol_id.clone(), + client + .block_hash(0u32.into()) + .ok() + .flatten() + .expect("Genesis block exists; qed"), + config.chain_spec.fork_id(), + ); + network_params + .network_config + .extra_sets + .insert(0, transactions_handler_proto.set_config()); + let has_bootnodes = !network_params.network_config.boot_nodes.is_empty(); let network_mut = sc_network::NetworkWorker::new(network_params)?; let network = network_mut.service().clone(); + let (tx_handler, tx_handler_controller) = transactions_handler_proto.build( + network.clone(), + Arc::new(TransactionPoolAdapter { pool: transaction_pool, client: client.clone() }), + config.prometheus_config.as_ref().map(|config| config.registry.clone()).as_ref(), + )?; + spawn_handle.spawn("network-transactions-handler", Some("networking"), tx_handler.run()); + let (system_rpc_tx, system_rpc_rx) = tracing_unbounded("mpsc_system_rpc"); let future = build_network_future( @@ -915,7 +938,7 @@ where future.await }); - Ok((network, system_rpc_tx, NetworkStarter(network_start_tx))) + Ok((network, system_rpc_tx, tx_handler_controller, NetworkStarter(network_start_tx))) } /// Object used to start the network. From b9d99e876e30415a3249c094411a45107b3f0224 Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Thu, 8 Sep 2022 18:50:22 +0300 Subject: [PATCH 11/17] Adjust trait bounds --- Cargo.lock | 1 + client/network/transactions/Cargo.toml | 1 + client/network/transactions/src/lib.rs | 38 ++++++++++---------------- 3 files changed, 17 insertions(+), 23 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7a5962b4a9291..587412ba47d4a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8566,6 +8566,7 @@ dependencies = [ "pin-project", "sc-network-common", "sc-peerset", + "sp-consensus", "sp-runtime", "substrate-prometheus-endpoint", ] diff --git a/client/network/transactions/Cargo.toml b/client/network/transactions/Cargo.toml index c1cd728e8c0de..b9a1627c12dc0 100644 --- a/client/network/transactions/Cargo.toml +++ b/client/network/transactions/Cargo.toml @@ -24,3 +24,4 @@ prometheus-endpoint = { package = "substrate-prometheus-endpoint", version = "0. sc-network-common = { version = "0.10.0-dev", path = "../common" } sc-peerset = { version = "4.0.0-dev", path = "../../peerset" } sp-runtime = { version = "6.0.0", path = "../../../primitives/runtime" } +sp-consensus = { version = "0.10.0-dev", path = "../../../primitives/consensus/common" } diff --git a/client/network/transactions/src/lib.rs b/client/network/transactions/src/lib.rs index fe9aa129466d9..695a3f6fb65d9 100644 --- a/client/network/transactions/src/lib.rs +++ b/client/network/transactions/src/lib.rs @@ -49,10 +49,7 @@ use std::{ iter, num::NonZeroUsize, pin::Pin, - sync::{ - atomic::{AtomicBool, Ordering}, - Arc, - }, + sync::Arc, task::Poll, }; @@ -159,12 +156,14 @@ impl TransactionsHandlerPrototype { /// Turns the prototype into the actual handler. Returns a controller that allows controlling /// the behaviour of the handler while it's running. /// + /// TODO: update doc + /// /// Important: the transactions handler is initially disabled and doesn't gossip transactions. /// You must call [`TransactionsHandlerController::set_gossip_enabled`] to enable it. pub fn build< B: BlockT + 'static, H: ExHashT, - S: NetworkPeers + NetworkEventStream + NetworkNotification, + S: NetworkPeers + NetworkEventStream + NetworkNotification + sp_consensus::SyncOracle, >( self, service: S, @@ -173,14 +172,12 @@ impl TransactionsHandlerPrototype { ) -> error::Result<(TransactionsHandler, TransactionsHandlerController)> { let event_stream = service.event_stream("transactions-handler"); let (to_handler, from_controller) = mpsc::unbounded(); - let gossip_enabled = Arc::new(AtomicBool::new(false)); let handler = TransactionsHandler { protocol_name: self.protocol_name, propagate_timeout: Box::pin(interval(PROPAGATE_TIMEOUT)), pending_transactions: FuturesUnordered::new(), pending_transactions_peers: HashMap::new(), - gossip_enabled: gossip_enabled.clone(), service, event_stream, peers: HashMap::new(), @@ -193,7 +190,7 @@ impl TransactionsHandlerPrototype { }, }; - let controller = TransactionsHandlerController { to_handler, gossip_enabled }; + let controller = TransactionsHandlerController { to_handler }; Ok((handler, controller)) } @@ -202,15 +199,9 @@ impl TransactionsHandlerPrototype { /// Controls the behaviour of a [`TransactionsHandler`] it is connected to. pub struct TransactionsHandlerController { to_handler: mpsc::UnboundedSender>, - gossip_enabled: Arc, } impl TransactionsHandlerController { - /// Controls whether transactions are being gossiped on the network. - pub fn set_gossip_enabled(&mut self, enabled: bool) { - self.gossip_enabled.store(enabled, Ordering::Relaxed); - } - /// You may call this when new transactions are imported by the transaction pool. /// /// All transactions will be fetched from the `TransactionPool` that was passed at @@ -237,7 +228,7 @@ enum ToHandler { pub struct TransactionsHandler< B: BlockT + 'static, H: ExHashT, - S: NetworkPeers + NetworkEventStream + NetworkNotification, + S: NetworkPeers + NetworkEventStream + NetworkNotification + sp_consensus::SyncOracle, > { protocol_name: ProtocolName, /// Interval at which we call `propagate_transactions`. @@ -256,7 +247,6 @@ pub struct TransactionsHandler< // All connected peers peers: HashMap>, transaction_pool: Arc>, - gossip_enabled: Arc, from_controller: mpsc::UnboundedReceiver>, /// Prometheus metrics. metrics: Option, @@ -274,7 +264,7 @@ impl TransactionsHandler where B: BlockT + 'static, H: ExHashT, - S: NetworkPeers + NetworkEventStream + NetworkNotification, + S: NetworkPeers + NetworkEventStream + NetworkNotification + sp_consensus::SyncOracle, { /// Turns the [`TransactionsHandler`] into a future that should run forever and not be /// interrupted. @@ -374,8 +364,8 @@ where /// Called when peer sends us new transactions fn on_transactions(&mut self, who: PeerId, transactions: Transactions) { - // Accept transactions only when enabled - if !self.gossip_enabled.load(Ordering::Relaxed) { + // Accept transactions only when node is not major syncing + if self.service.is_major_syncing() { trace!(target: "sync", "{} Ignoring transactions while disabled", who); return } @@ -425,10 +415,11 @@ where /// Propagate one transaction. pub fn propagate_transaction(&mut self, hash: &H) { - // Accept transactions only when enabled - if !self.gossip_enabled.load(Ordering::Relaxed) { + // Accept transactions only when node is not major syncing + if self.service.is_major_syncing() { return } + debug!(target: "sync", "Propagating transaction [{:?}]", hash); if let Some(transaction) = self.transaction_pool.transaction(hash) { let propagated_to = self.do_propagate_transactions(&[(hash.clone(), transaction)]); @@ -476,10 +467,11 @@ where /// Call when we must propagate ready transactions to peers. fn propagate_transactions(&mut self) { - // Accept transactions only when enabled - if !self.gossip_enabled.load(Ordering::Relaxed) { + // Accept transactions only when node is not major syncing + if self.service.is_major_syncing() { return } + debug!(target: "sync", "Propagating transactions"); let transactions = self.transaction_pool.transactions(); let propagated_to = self.do_propagate_transactions(&transactions); From 60ab91e3d6a7e943766cbe8169fc8e46ab5bc258 Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Fri, 9 Sep 2022 09:11:46 +0300 Subject: [PATCH 12/17] Remove unneeded dependency --- Cargo.lock | 1 - client/network/test/Cargo.toml | 1 - client/network/test/src/lib.rs | 1 - client/service/src/builder.rs | 2 ++ 4 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 587412ba47d4a..5183fd91b6a46 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8542,7 +8542,6 @@ dependencies = [ "sc-network-common", "sc-network-light", "sc-network-sync", - "sc-network-transactions", "sc-service", "sp-blockchain", "sp-consensus", diff --git a/client/network/test/Cargo.toml b/client/network/test/Cargo.toml index 4e57e5f6caba8..990129229a40f 100644 --- a/client/network/test/Cargo.toml +++ b/client/network/test/Cargo.toml @@ -28,7 +28,6 @@ sc-network = { version = "0.10.0-dev", path = "../" } sc-network-common = { version = "0.10.0-dev", path = "../common" } sc-network-light = { version = "0.10.0-dev", path = "../light" } sc-network-sync = { version = "0.10.0-dev", path = "../sync" } -sc-network-transactions = { version = "0.10.0-dev", path = "../transactions" } sc-service = { version = "0.10.0-dev", default-features = false, features = ["test-helpers"], path = "../../service" } sp-blockchain = { version = "4.0.0-dev", path = "../../../primitives/blockchain" } sp-consensus = { version = "0.10.0-dev", path = "../../../primitives/consensus/common" } diff --git a/client/network/test/src/lib.rs b/client/network/test/src/lib.rs index 0ed380c02f988..8cf56576eae22 100644 --- a/client/network/test/src/lib.rs +++ b/client/network/test/src/lib.rs @@ -64,7 +64,6 @@ use sc_network_sync::{ block_request_handler::BlockRequestHandler, state_request_handler::StateRequestHandler, warp_request_handler, ChainSync, }; -pub use sc_network_transactions::config::EmptyTransactionPool; use sc_service::client::Client; use sp_blockchain::{ well_known_cache_keys::{self, Id as CacheKeyId}, diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index 7fb33bb5ffcad..a7b04d07623d1 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -880,6 +880,8 @@ where .extra_sets .insert(0, transactions_handler_proto.set_config()); + info!("extra sets", network_params .network_config .extra_sets); + let has_bootnodes = !network_params.network_config.boot_nodes.is_empty(); let network_mut = sc_network::NetworkWorker::new(network_params)?; let network = network_mut.service().clone(); From e731ad07cfa7c00882f89361c1756ee0b3f8fb7a Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 13 Sep 2022 13:32:34 +0200 Subject: [PATCH 13/17] benches: disable caching per default (#12232) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Disable cache for storage benches Signed-off-by: Oliver Tale-Yazdi * Disable caching per default Signed-off-by: Oliver Tale-Yazdi * Update utils/frame/benchmarking-cli/src/storage/cmd.rs Co-authored-by: Bastian Köcher * Add --enable-trie-cache to 'storage' command Signed-off-by: Oliver Tale-Yazdi Signed-off-by: Oliver Tale-Yazdi Co-authored-by: Bastian Köcher --- utils/frame/benchmarking-cli/src/block/cmd.rs | 14 ++++++++++++++ utils/frame/benchmarking-cli/src/extrinsic/cmd.rs | 14 ++++++++++++++ utils/frame/benchmarking-cli/src/overhead/cmd.rs | 14 ++++++++++++++ utils/frame/benchmarking-cli/src/storage/cmd.rs | 14 ++++++++++---- 4 files changed, 52 insertions(+), 4 deletions(-) diff --git a/utils/frame/benchmarking-cli/src/block/cmd.rs b/utils/frame/benchmarking-cli/src/block/cmd.rs index e4e1716b1c5ac..14cbb1b438686 100644 --- a/utils/frame/benchmarking-cli/src/block/cmd.rs +++ b/utils/frame/benchmarking-cli/src/block/cmd.rs @@ -67,6 +67,12 @@ pub struct BlockCmd { #[allow(missing_docs)] #[clap(flatten)] pub params: BenchmarkParams, + + /// Enable the Trie cache. + /// + /// This should only be used for performance analysis and not for final results. + #[clap(long)] + pub enable_trie_cache: bool, } impl BlockCmd { @@ -98,4 +104,12 @@ impl CliConfiguration for BlockCmd { fn import_params(&self) -> Option<&ImportParams> { Some(&self.import_params) } + + fn trie_cache_maximum_size(&self) -> Result> { + if self.enable_trie_cache { + Ok(self.import_params().map(|x| x.trie_cache_maximum_size()).unwrap_or_default()) + } else { + Ok(None) + } + } } diff --git a/utils/frame/benchmarking-cli/src/extrinsic/cmd.rs b/utils/frame/benchmarking-cli/src/extrinsic/cmd.rs index 2c94a6be50255..339d6126bcd09 100644 --- a/utils/frame/benchmarking-cli/src/extrinsic/cmd.rs +++ b/utils/frame/benchmarking-cli/src/extrinsic/cmd.rs @@ -72,6 +72,12 @@ pub struct ExtrinsicParams { /// Extrinsic to benchmark. #[clap(long, value_name = "EXTRINSIC", required_unless_present = "list")] pub extrinsic: Option, + + /// Enable the Trie cache. + /// + /// This should only be used for performance analysis and not for final results. + #[clap(long)] + pub enable_trie_cache: bool, } impl ExtrinsicCmd { @@ -132,4 +138,12 @@ impl CliConfiguration for ExtrinsicCmd { fn import_params(&self) -> Option<&ImportParams> { Some(&self.import_params) } + + fn trie_cache_maximum_size(&self) -> Result> { + if self.params.enable_trie_cache { + Ok(self.import_params().map(|x| x.trie_cache_maximum_size()).unwrap_or_default()) + } else { + Ok(None) + } + } } diff --git a/utils/frame/benchmarking-cli/src/overhead/cmd.rs b/utils/frame/benchmarking-cli/src/overhead/cmd.rs index 1c6753b4a20ea..93b0e7c472647 100644 --- a/utils/frame/benchmarking-cli/src/overhead/cmd.rs +++ b/utils/frame/benchmarking-cli/src/overhead/cmd.rs @@ -75,6 +75,12 @@ pub struct OverheadParams { /// Good for adding LICENSE headers. #[clap(long, value_name = "PATH")] pub header: Option, + + /// Enable the Trie cache. + /// + /// This should only be used for performance analysis and not for final results. + #[clap(long)] + pub enable_trie_cache: bool, } /// Type of a benchmark. @@ -156,4 +162,12 @@ impl CliConfiguration for OverheadCmd { fn import_params(&self) -> Option<&ImportParams> { Some(&self.import_params) } + + fn trie_cache_maximum_size(&self) -> Result> { + if self.params.enable_trie_cache { + Ok(self.import_params().map(|x| x.trie_cache_maximum_size()).unwrap_or_default()) + } else { + Ok(None) + } + } } diff --git a/utils/frame/benchmarking-cli/src/storage/cmd.rs b/utils/frame/benchmarking-cli/src/storage/cmd.rs index d28ac4102152f..1d91e8f0b0517 100644 --- a/utils/frame/benchmarking-cli/src/storage/cmd.rs +++ b/utils/frame/benchmarking-cli/src/storage/cmd.rs @@ -105,9 +105,15 @@ pub struct StorageParams { /// Trie cache size in bytes. /// /// Providing `0` will disable the cache. - #[clap(long, default_value = "1024")] + #[clap(long, value_name = "Bytes", default_value = "67108864")] pub trie_cache_size: usize, + /// Enable the Trie cache. + /// + /// This should only be used for performance analysis and not for final results. + #[clap(long)] + pub enable_trie_cache: bool, + /// Include child trees in benchmark. #[clap(long)] pub include_child_trees: bool, @@ -220,10 +226,10 @@ impl CliConfiguration for StorageCmd { } fn trie_cache_maximum_size(&self) -> Result> { - if self.params.trie_cache_size == 0 { - Ok(None) - } else { + if self.params.enable_trie_cache && self.params.trie_cache_size > 0 { Ok(Some(self.params.trie_cache_size)) + } else { + Ok(None) } } } From 426c686bf6c1bf3c595a452f9144f5d75f5ce5de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 13 Sep 2022 12:48:46 +0100 Subject: [PATCH 14/17] Make `BasePath::new_temp_dir` return the same path for the program lifetime (#12246) * Make `BasePath::new_temp_dir` return the same path for the program lifetime Instead of returning always a different path, this now returns the same path for the entire lifetime of the program. We still ensure that the path is cleared at the end of the program. * Update client/service/src/config.rs Co-authored-by: Koute * Update client/service/src/config.rs Co-authored-by: Nitwit <47109040+nitwit69@users.noreply.github.com> * FMT Co-authored-by: Koute Co-authored-by: Nitwit <47109040+nitwit69@users.noreply.github.com> --- Cargo.lock | 37 +++++++++++++++++++++++++++++++++- client/service/Cargo.toml | 1 + client/service/src/config.rs | 39 ++++++++++++++++++++++-------------- 3 files changed, 61 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a05f04c8d0a47..42e2df92d77ef 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -874,6 +874,12 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" +[[package]] +name = "cfg_aliases" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fd16c4719339c4530435d38e511904438d07cce7950afa3718a84ac36c10e89e" + [[package]] name = "chacha20" version = "0.8.1" @@ -8845,6 +8851,7 @@ dependencies = [ "sp-transaction-storage-proof", "sp-trie", "sp-version", + "static_init", "substrate-prometheus-endpoint", "substrate-test-runtime", "substrate-test-runtime-client", @@ -10364,6 +10371,34 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" +[[package]] +name = "static_init" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8a2a1c578e98c1c16fc3b8ec1328f7659a500737d7a0c6d625e73e830ff9c1f6" +dependencies = [ + "bitflags", + "cfg_aliases", + "libc", + "parking_lot 0.11.2", + "parking_lot_core 0.8.5", + "static_init_macro", + "winapi", +] + +[[package]] +name = "static_init_macro" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "70a2595fc3aa78f2d0e45dd425b22282dd863273761cc77780914b2cf3003acf" +dependencies = [ + "cfg_aliases", + "memchr", + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "statrs" version = "0.15.0" @@ -11233,7 +11268,7 @@ checksum = "97fee6b57c6a41524a810daee9286c02d7752c4253064d0b05472833a438f675" dependencies = [ "cfg-if 1.0.0", "digest 0.10.3", - "rand 0.7.3", + "rand 0.8.5", "static_assertions", ] diff --git a/client/service/Cargo.toml b/client/service/Cargo.toml index c27ab0c0de127..3305c377343d8 100644 --- a/client/service/Cargo.toml +++ b/client/service/Cargo.toml @@ -86,6 +86,7 @@ async-trait = "0.1.57" tokio = { version = "1.17.0", features = ["time", "rt-multi-thread", "parking_lot"] } tempfile = "3.1.0" directories = "4.0.1" +static_init = "1.0.3" [dev-dependencies] substrate-test-runtime-client = { version = "2.0.0", path = "../../test-utils/runtime/client" } diff --git a/client/service/src/config.rs b/client/service/src/config.rs index ace908c835d74..bca0697bcbd08 100644 --- a/client/service/src/config.rs +++ b/client/service/src/config.rs @@ -260,31 +260,43 @@ impl Default for RpcMethods { } } -/// The base path that is used for everything that needs to be write on disk to run a node. +#[static_init::dynamic(drop, lazy)] +static mut BASE_PATH_TEMP: Option = None; + +/// The base path that is used for everything that needs to be written on disk to run a node. #[derive(Debug)] -pub enum BasePath { - /// A temporary directory is used as base path and will be deleted when dropped. - Temporary(TempDir), - /// A path on the disk. - Permanenent(PathBuf), +pub struct BasePath { + path: PathBuf, } impl BasePath { /// Create a `BasePath` instance using a temporary directory prefixed with "substrate" and use /// it as base path. /// - /// Note: the temporary directory will be created automatically and deleted when the `BasePath` - /// instance is dropped. + /// Note: The temporary directory will be created automatically and deleted when the program + /// exits. Every call to this function will return the same path for the lifetime of the + /// program. pub fn new_temp_dir() -> io::Result { - Ok(BasePath::Temporary(tempfile::Builder::new().prefix("substrate").tempdir()?)) + let mut temp = BASE_PATH_TEMP.write(); + + match &*temp { + Some(p) => Ok(Self::new(p.path())), + None => { + let temp_dir = tempfile::Builder::new().prefix("substrate").tempdir()?; + let path = PathBuf::from(temp_dir.path()); + + *temp = Some(temp_dir); + Ok(Self::new(path)) + }, + } } /// Create a `BasePath` instance based on an existing path on disk. /// /// Note: this function will not ensure that the directory exist nor create the directory. It /// will also not delete the directory when the instance is dropped. - pub fn new>(path: P) -> BasePath { - BasePath::Permanenent(path.as_ref().to_path_buf()) + pub fn new>(path: P) -> BasePath { + Self { path: path.into() } } /// Create a base path from values describing the project. @@ -298,10 +310,7 @@ impl BasePath { /// Retrieve the base path. pub fn path(&self) -> &Path { - match self { - BasePath::Temporary(temp_dir) => temp_dir.path(), - BasePath::Permanenent(path) => path.as_path(), - } + &self.path } /// Returns the configuration directory inside this base path. From 32cec32d43c3c97561cf3e93e1ce4d38f0a9e806 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Tue, 13 Sep 2022 14:19:26 +0200 Subject: [PATCH 15/17] Remove CanAuthorWith trait (#12213) * Remove CanAuthorWith trait CanAuthotWith trait removed. Also all dependencies, parameters, type paramers were removed. This is related to removal of native runtime. * Remove commented code * Fix code formatting * trigger CI job * trigger CI job * trigger CI job * trigger CI job * trigger CI job * trigger CI job * trigger CI job --- bin/node-template/node/src/service.rs | 13 ++--- bin/node/cli/src/service.rs | 7 +-- client/consensus/aura/src/import_queue.rs | 53 ++++++-------------- client/consensus/aura/src/lib.rs | 24 +++------ client/consensus/babe/src/lib.rs | 41 ++++------------ client/consensus/babe/src/tests.rs | 5 +- client/consensus/pow/src/lib.rs | 45 +++-------------- client/consensus/slots/src/lib.rs | 25 ++-------- primitives/consensus/common/src/lib.rs | 59 ----------------------- 9 files changed, 45 insertions(+), 227 deletions(-) diff --git a/bin/node-template/node/src/service.rs b/bin/node-template/node/src/service.rs index c8a739e2781f7..b291ebd100404 100644 --- a/bin/node-template/node/src/service.rs +++ b/bin/node-template/node/src/service.rs @@ -1,7 +1,7 @@ //! Service and ServiceFactory implementation. Specialized wrapper over substrate service. use node_template_runtime::{self, opaque::Block, RuntimeApi}; -use sc_client_api::{BlockBackend, ExecutorProvider}; +use sc_client_api::BlockBackend; use sc_consensus_aura::{ImportQueueParams, SlotProportion, StartAuraParams}; pub use sc_executor::NativeElseWasmExecutor; use sc_finality_grandpa::SharedVoterState; @@ -113,7 +113,7 @@ pub fn new_partial( let slot_duration = sc_consensus_aura::slot_duration(&*client)?; let import_queue = - sc_consensus_aura::import_queue::(ImportQueueParams { + sc_consensus_aura::import_queue::(ImportQueueParams { block_import: grandpa_block_import.clone(), justification_import: Some(Box::new(grandpa_block_import.clone())), client: client.clone(), @@ -129,9 +129,6 @@ pub fn new_partial( Ok((timestamp, slot)) }, spawner: &task_manager.spawn_essential_handle(), - can_author_with: sp_consensus::CanAuthorWithNativeVersion::new( - client.executor().clone(), - ), registry: config.prometheus_registry(), check_for_equivocation: Default::default(), telemetry: telemetry.as_ref().map(|x| x.handle()), @@ -255,12 +252,9 @@ pub fn new_full(mut config: Configuration) -> Result telemetry.as_ref().map(|x| x.handle()), ); - let can_author_with = - sp_consensus::CanAuthorWithNativeVersion::new(client.executor().clone()); - let slot_duration = sc_consensus_aura::slot_duration(&*client)?; - let aura = sc_consensus_aura::start_aura::( + let aura = sc_consensus_aura::start_aura::( StartAuraParams { slot_duration, client, @@ -281,7 +275,6 @@ pub fn new_full(mut config: Configuration) -> Result force_authoring, backoff_authoring_blocks, keystore: keystore_container.sync_keystore(), - can_author_with, sync_oracle: network.clone(), justification_sync_link: network.clone(), block_proposal_slot_portion: SlotProportion::new(2f32 / 3f32), diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index 65124688b0c12..53c67d3e9a433 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -26,7 +26,7 @@ use futures::prelude::*; use kitchensink_runtime::RuntimeApi; use node_executor::ExecutorDispatch; use node_primitives::Block; -use sc_client_api::{BlockBackend, ExecutorProvider}; +use sc_client_api::BlockBackend; use sc_consensus_babe::{self, SlotProportion}; use sc_executor::NativeElseWasmExecutor; use sc_network::NetworkService; @@ -227,7 +227,6 @@ pub fn new_partial( }, &task_manager.spawn_essential_handle(), config.prometheus_registry(), - sp_consensus::CanAuthorWithNativeVersion::new(client.executor().clone()), telemetry.as_ref().map(|x| x.handle()), )?; @@ -423,9 +422,6 @@ pub fn new_full_base( telemetry.as_ref().map(|x| x.handle()), ); - let can_author_with = - sp_consensus::CanAuthorWithNativeVersion::new(client.executor().clone()); - let client_clone = client.clone(); let slot_duration = babe_link.config().slot_duration(); let babe_config = sc_consensus_babe::BabeParams { @@ -464,7 +460,6 @@ pub fn new_full_base( force_authoring, backoff_authoring_blocks, babe_link, - can_author_with, block_proposal_slot_portion: SlotProportion::new(0.5), max_block_proposal_slot_portion: None, telemetry: telemetry.as_ref().map(|x| x.handle()), diff --git a/client/consensus/aura/src/import_queue.rs b/client/consensus/aura/src/import_queue.rs index 30554006732c0..31fe7bbaa1095 100644 --- a/client/consensus/aura/src/import_queue.rs +++ b/client/consensus/aura/src/import_queue.rs @@ -35,7 +35,7 @@ use sp_blockchain::{ well_known_cache_keys::{self, Id as CacheKeyId}, HeaderBackend, }; -use sp_consensus::{CanAuthorWith, Error as ConsensusError}; +use sp_consensus::Error as ConsensusError; use sp_consensus_aura::{ digests::CompatibleDigestItem, inherents::AuraInherentData, AuraApi, ConsensusLog, AURA_ENGINE_ID, @@ -109,27 +109,24 @@ where } /// A verifier for Aura blocks. -pub struct AuraVerifier { +pub struct AuraVerifier { client: Arc, phantom: PhantomData

, create_inherent_data_providers: CIDP, - can_author_with: CAW, check_for_equivocation: CheckForEquivocation, telemetry: Option, } -impl AuraVerifier { +impl AuraVerifier { pub(crate) fn new( client: Arc, create_inherent_data_providers: CIDP, - can_author_with: CAW, check_for_equivocation: CheckForEquivocation, telemetry: Option, ) -> Self { Self { client, create_inherent_data_providers, - can_author_with, check_for_equivocation, telemetry, phantom: PhantomData, @@ -137,10 +134,9 @@ impl AuraVerifier { } } -impl AuraVerifier +impl AuraVerifier where P: Send + Sync + 'static, - CAW: Send + Sync + 'static, CIDP: Send, { async fn check_inherents( @@ -154,19 +150,8 @@ where where C: ProvideRuntimeApi, C::Api: BlockBuilderApi, - CAW: CanAuthorWith, CIDP: CreateInherentDataProviders, { - if let Err(e) = self.can_author_with.can_author_with(&block_id) { - debug!( - target: "aura", - "Skipping `check_inherents` as authoring version is not compatible: {}", - e, - ); - - return Ok(()) - } - let inherent_res = self .client .runtime_api() @@ -187,14 +172,13 @@ where } #[async_trait::async_trait] -impl Verifier for AuraVerifier +impl Verifier for AuraVerifier where C: ProvideRuntimeApi + Send + Sync + sc_client_api::backend::AuxStore + BlockOf, C::Api: BlockBuilderApi + AuraApi> + ApiExt, P: Pair + Send + Sync + 'static, P::Public: Send + Sync + Hash + Eq + Clone + Decode + Encode + Debug + 'static, P::Signature: Encode + Decode, - CAW: CanAuthorWith + Send + Sync + 'static, CIDP: CreateInherentDataProviders + Send + Sync, CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync, { @@ -338,7 +322,7 @@ impl Default for CheckForEquivocation { } /// Parameters of [`import_queue`]. -pub struct ImportQueueParams<'a, Block, I, C, S, CAW, CIDP> { +pub struct ImportQueueParams<'a, Block, I, C, S, CIDP> { /// The block import to use. pub block_import: I, /// The justification import. @@ -351,8 +335,6 @@ pub struct ImportQueueParams<'a, Block, I, C, S, CAW, CIDP> { pub spawner: &'a S, /// The prometheus registry. pub registry: Option<&'a Registry>, - /// Can we author with the current node? - pub can_author_with: CAW, /// Should we check for equivocation? pub check_for_equivocation: CheckForEquivocation, /// Telemetry instance used to report telemetry metrics. @@ -360,7 +342,7 @@ pub struct ImportQueueParams<'a, Block, I, C, S, CAW, CIDP> { } /// Start an import queue for the Aura consensus algorithm. -pub fn import_queue( +pub fn import_queue( ImportQueueParams { block_import, justification_import, @@ -368,10 +350,9 @@ pub fn import_queue( create_inherent_data_providers, spawner, registry, - can_author_with, check_for_equivocation, telemetry, - }: ImportQueueParams, + }: ImportQueueParams, ) -> Result, sp_consensus::Error> where Block: BlockT, @@ -392,14 +373,12 @@ where P::Public: Clone + Eq + Send + Sync + Hash + Debug + Encode + Decode, P::Signature: Encode + Decode, S: sp_core::traits::SpawnEssentialNamed, - CAW: CanAuthorWith + Send + Sync + 'static, CIDP: CreateInherentDataProviders + Sync + Send + 'static, CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync, { - let verifier = build_verifier::(BuildVerifierParams { + let verifier = build_verifier::(BuildVerifierParams { client, create_inherent_data_providers, - can_author_with, check_for_equivocation, telemetry, }); @@ -408,13 +387,11 @@ where } /// Parameters of [`build_verifier`]. -pub struct BuildVerifierParams { +pub struct BuildVerifierParams { /// The client to interact with the chain. pub client: Arc, /// Something that can create the inherent data providers. pub create_inherent_data_providers: CIDP, - /// Can we author with the current node? - pub can_author_with: CAW, /// Should we check for equivocation? pub check_for_equivocation: CheckForEquivocation, /// Telemetry instance used to report telemetry metrics. @@ -422,19 +399,17 @@ pub struct BuildVerifierParams { } /// Build the [`AuraVerifier`] -pub fn build_verifier( +pub fn build_verifier( BuildVerifierParams { client, create_inherent_data_providers, - can_author_with, check_for_equivocation, telemetry, - }: BuildVerifierParams, -) -> AuraVerifier { - AuraVerifier::<_, P, _, _>::new( + }: BuildVerifierParams, +) -> AuraVerifier { + AuraVerifier::<_, P, _>::new( client, create_inherent_data_providers, - can_author_with, check_for_equivocation, telemetry, ) diff --git a/client/consensus/aura/src/lib.rs b/client/consensus/aura/src/lib.rs index 92fe1fa3cf29d..0bdc663815051 100644 --- a/client/consensus/aura/src/lib.rs +++ b/client/consensus/aura/src/lib.rs @@ -47,9 +47,7 @@ use sc_telemetry::TelemetryHandle; use sp_api::ProvideRuntimeApi; use sp_application_crypto::{AppKey, AppPublic}; use sp_blockchain::{HeaderBackend, Result as CResult}; -use sp_consensus::{ - BlockOrigin, CanAuthorWith, Environment, Error as ConsensusError, Proposer, SelectChain, -}; +use sp_consensus::{BlockOrigin, Environment, Error as ConsensusError, Proposer, SelectChain}; use sp_consensus_slots::Slot; use sp_core::crypto::{ByteArray, Pair, Public}; use sp_inherents::CreateInherentDataProviders; @@ -108,7 +106,7 @@ fn slot_author(slot: Slot, authorities: &[AuthorityId

]) -> Option<&A } /// Parameters of [`start_aura`]. -pub struct StartAuraParams { +pub struct StartAuraParams { /// The duration of a slot. pub slot_duration: SlotDuration, /// The client to interact with the chain. @@ -131,8 +129,6 @@ pub struct StartAuraParams { pub backoff_authoring_blocks: Option, /// The keystore used by the node. pub keystore: SyncCryptoStorePtr, - /// Can we author a block with this node? - pub can_author_with: CAW, /// The proportion of the slot dedicated to proposing. /// /// The block proposing will be limited to this proportion of the slot from the starting of the @@ -147,7 +143,7 @@ pub struct StartAuraParams { } /// Start the aura worker. The returned future should be run in a futures executor. -pub fn start_aura( +pub fn start_aura( StartAuraParams { slot_duration, client, @@ -160,11 +156,10 @@ pub fn start_aura( force_authoring, backoff_authoring_blocks, keystore, - can_author_with, block_proposal_slot_portion, max_block_proposal_slot_portion, telemetry, - }: StartAuraParams, + }: StartAuraParams, ) -> Result, sp_consensus::Error> where P: Pair + Send + Sync, @@ -182,7 +177,6 @@ where CIDP: CreateInherentDataProviders + Send, CIDP::InherentDataProviders: InherentDataProviderExt + Send, BS: BackoffAuthoringBlocksStrategy> + Send + Sync + 'static, - CAW: CanAuthorWith + Send, Error: std::error::Error + Send + From + 'static, { let worker = build_aura_worker::(BuildAuraWorkerParams { @@ -205,7 +199,6 @@ where SimpleSlotWorkerToSlotWorker(worker), sync_oracle, create_inherent_data_providers, - can_author_with, )) } @@ -568,9 +561,7 @@ mod tests { use sc_keystore::LocalKeystore; use sc_network_test::{Block as TestBlock, *}; use sp_application_crypto::key_types::AURA; - use sp_consensus::{ - AlwaysCanAuthor, DisableProofRecording, NoNetwork as DummyOracle, Proposal, - }; + use sp_consensus::{DisableProofRecording, NoNetwork as DummyOracle, Proposal}; use sp_consensus_aura::sr25519::AuthorityPair; use sp_inherents::InherentData; use sp_keyring::sr25519::Keyring; @@ -633,7 +624,6 @@ mod tests { type AuraVerifier = import_queue::AuraVerifier< PeersFullClient, AuthorityPair, - AlwaysCanAuthor, Box< dyn CreateInherentDataProviders< TestBlock, @@ -670,7 +660,6 @@ mod tests { Ok((timestamp, slot)) }), - AlwaysCanAuthor, CheckForEquivocation::Yes, None, ) @@ -738,7 +727,7 @@ mod tests { let slot_duration = slot_duration(&*client).expect("slot duration available"); aura_futures.push( - start_aura::(StartAuraParams { + start_aura::(StartAuraParams { slot_duration, block_import: client.clone(), select_chain, @@ -760,7 +749,6 @@ mod tests { BackoffAuthoringOnFinalizedHeadLagging::default(), ), keystore, - can_author_with: sp_consensus::AlwaysCanAuthor, block_proposal_slot_portion: SlotProportion::new(0.5), max_block_proposal_slot_portion: None, telemetry: None, diff --git a/client/consensus/babe/src/lib.rs b/client/consensus/babe/src/lib.rs index 1303915efee49..198de865bb290 100644 --- a/client/consensus/babe/src/lib.rs +++ b/client/consensus/babe/src/lib.rs @@ -115,8 +115,7 @@ use sp_blockchain::{ Backend as _, Error as ClientError, HeaderBackend, HeaderMetadata, Result as ClientResult, }; use sp_consensus::{ - BlockOrigin, CacheKeyId, CanAuthorWith, Environment, Error as ConsensusError, Proposer, - SelectChain, + BlockOrigin, CacheKeyId, Environment, Error as ConsensusError, Proposer, SelectChain, }; use sp_consensus_babe::inherents::BabeInherentData; use sp_consensus_slots::Slot; @@ -370,7 +369,7 @@ where } /// Parameters for BABE. -pub struct BabeParams { +pub struct BabeParams { /// The keystore that manages the keys of the node. pub keystore: SyncCryptoStorePtr, @@ -406,9 +405,6 @@ pub struct BabeParams { /// The source of timestamps for relative slots pub babe_link: BabeLink, - /// Checks if the current native implementation can author with a runtime at a given block. - pub can_author_with: CAW, - /// The proportion of the slot dedicated to proposing. /// /// The block proposing will be limited to this proportion of the slot from the starting of the @@ -425,7 +421,7 @@ pub struct BabeParams { } /// Start the babe worker. -pub fn start_babe( +pub fn start_babe( BabeParams { keystore, client, @@ -438,11 +434,10 @@ pub fn start_babe( force_authoring, backoff_authoring_blocks, babe_link, - can_author_with, block_proposal_slot_portion, max_block_proposal_slot_portion, telemetry, - }: BabeParams, + }: BabeParams, ) -> Result, sp_consensus::Error> where B: BlockT, @@ -468,7 +463,6 @@ where CIDP: CreateInherentDataProviders + Send + Sync + 'static, CIDP::InherentDataProviders: InherentDataProviderExt + Send, BS: BackoffAuthoringBlocksStrategy> + Send + Sync + 'static, - CAW: CanAuthorWith + Send + Sync + 'static, Error: std::error::Error + Send + From + From + 'static, { const HANDLE_BUFFER_SIZE: usize = 1024; @@ -500,7 +494,6 @@ where sc_consensus_slots::SimpleSlotWorkerToSlotWorker(worker), sync_oracle, create_inherent_data_providers, - can_author_with, ); let (worker_tx, worker_rx) = channel(HANDLE_BUFFER_SIZE); @@ -1009,23 +1002,21 @@ impl BabeLink { } /// A verifier for Babe blocks. -pub struct BabeVerifier { +pub struct BabeVerifier { client: Arc, select_chain: SelectChain, create_inherent_data_providers: CIDP, config: BabeConfiguration, epoch_changes: SharedEpochChanges, - can_author_with: CAW, telemetry: Option, } -impl BabeVerifier +impl BabeVerifier where Block: BlockT, Client: AuxStore + HeaderBackend + HeaderMetadata + ProvideRuntimeApi, Client::Api: BlockBuilderApi + BabeApi, SelectChain: sp_consensus::SelectChain, - CAW: CanAuthorWith, CIDP: CreateInherentDataProviders, { async fn check_inherents( @@ -1036,16 +1027,6 @@ where create_inherent_data_providers: CIDP::InherentDataProviders, execution_context: ExecutionContext, ) -> Result<(), Error> { - if let Err(e) = self.can_author_with.can_author_with(&block_id) { - debug!( - target: "babe", - "Skipping `check_inherents` as authoring version is not compatible: {}", - e, - ); - - return Ok(()) - } - let inherent_res = self .client .runtime_api() @@ -1150,8 +1131,8 @@ type BlockVerificationResult = Result<(BlockImportParams, Option)>>), String>; #[async_trait::async_trait] -impl Verifier - for BabeVerifier +impl Verifier + for BabeVerifier where Block: BlockT, Client: HeaderMetadata @@ -1162,7 +1143,6 @@ where + AuxStore, Client::Api: BlockBuilderApi + BabeApi, SelectChain: sp_consensus::SelectChain, - CAW: CanAuthorWith + Send + Sync, CIDP: CreateInherentDataProviders + Send + Sync, CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync, { @@ -1773,7 +1753,7 @@ where /// /// The block import object provided must be the `BabeBlockImport` or a wrapper /// of it, otherwise crucial import logic will be omitted. -pub fn import_queue( +pub fn import_queue( babe_link: BabeLink, block_import: Inner, justification_import: Option>, @@ -1782,7 +1762,6 @@ pub fn import_queue( create_inherent_data_providers: CIDP, spawner: &impl sp_core::traits::SpawnEssentialNamed, registry: Option<&Registry>, - can_author_with: CAW, telemetry: Option, ) -> ClientResult> where @@ -1802,7 +1781,6 @@ where + 'static, Client::Api: BlockBuilderApi + BabeApi + ApiExt, SelectChain: sp_consensus::SelectChain + 'static, - CAW: CanAuthorWith + Send + Sync + 'static, CIDP: CreateInherentDataProviders + Send + Sync + 'static, CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync, { @@ -1811,7 +1789,6 @@ where create_inherent_data_providers, config: babe_link.config, epoch_changes: babe_link.epoch_changes, - can_author_with, telemetry, client, }; diff --git a/client/consensus/babe/src/tests.rs b/client/consensus/babe/src/tests.rs index 7207b7a36c3d4..ab3805138482c 100644 --- a/client/consensus/babe/src/tests.rs +++ b/client/consensus/babe/src/tests.rs @@ -31,7 +31,7 @@ use sc_consensus_slots::BackoffAuthoringOnFinalizedHeadLagging; use sc_keystore::LocalKeystore; use sc_network_test::{Block as TestBlock, *}; use sp_application_crypto::key_types::BABE; -use sp_consensus::{AlwaysCanAuthor, DisableProofRecording, NoNetwork as DummyOracle, Proposal}; +use sp_consensus::{DisableProofRecording, NoNetwork as DummyOracle, Proposal}; use sp_consensus_babe::{ inherents::InherentDataProvider, make_transcript, make_transcript_data, AllowedSlots, AuthorityPair, Slot, @@ -235,7 +235,6 @@ pub struct TestVerifier { TestBlock, PeersFullClient, TestSelectChain, - AlwaysCanAuthor, Box< dyn CreateInherentDataProviders< TestBlock, @@ -332,7 +331,6 @@ impl TestNetFactory for BabeTestNet { }), config: data.link.config.clone(), epoch_changes: data.link.epoch_changes.clone(), - can_author_with: AlwaysCanAuthor, telemetry: None, }, mutator: MUTATOR.with(|m| m.borrow().clone()), @@ -447,7 +445,6 @@ fn run_one_test(mutator: impl Fn(&mut TestHeader, Stage) + Send + Sync + 'static backoff_authoring_blocks: Some(BackoffAuthoringOnFinalizedHeadLagging::default()), babe_link: data.link.clone(), keystore, - can_author_with: sp_consensus::AlwaysCanAuthor, justification_sync_link: (), block_proposal_slot_portion: SlotProportion::new(0.5), max_block_proposal_slot_portion: None, diff --git a/client/consensus/pow/src/lib.rs b/client/consensus/pow/src/lib.rs index f63e453a48026..6c492931d445c 100644 --- a/client/consensus/pow/src/lib.rs +++ b/client/consensus/pow/src/lib.rs @@ -56,9 +56,7 @@ use sc_consensus::{ use sp_api::ProvideRuntimeApi; use sp_block_builder::BlockBuilder as BlockBuilderApi; use sp_blockchain::{well_known_cache_keys::Id as CacheKeyId, HeaderBackend}; -use sp_consensus::{ - CanAuthorWith, Environment, Error as ConsensusError, Proposer, SelectChain, SyncOracle, -}; +use sp_consensus::{Environment, Error as ConsensusError, Proposer, SelectChain, SyncOracle}; use sp_consensus_pow::{Seal, TotalDifficulty, POW_ENGINE_ID}; use sp_core::ExecutionContext; use sp_inherents::{CreateInherentDataProviders, InherentDataProvider}; @@ -214,18 +212,17 @@ pub trait PowAlgorithm { } /// A block importer for PoW. -pub struct PowBlockImport { +pub struct PowBlockImport { algorithm: Algorithm, inner: I, select_chain: S, client: Arc, create_inherent_data_providers: Arc, check_inherents_after: <::Header as HeaderT>::Number, - can_author_with: CAW, } -impl Clone - for PowBlockImport +impl Clone + for PowBlockImport { fn clone(&self) -> Self { Self { @@ -235,12 +232,11 @@ impl Clone client: self.client.clone(), create_inherent_data_providers: self.create_inherent_data_providers.clone(), check_inherents_after: self.check_inherents_after, - can_author_with: self.can_author_with.clone(), } } } -impl PowBlockImport +impl PowBlockImport where B: BlockT, I: BlockImport> + Send + Sync, @@ -248,7 +244,6 @@ where C: ProvideRuntimeApi + Send + Sync + HeaderBackend + AuxStore + BlockOf, C::Api: BlockBuilderApi, Algorithm: PowAlgorithm, - CAW: CanAuthorWith, CIDP: CreateInherentDataProviders, { /// Create a new block import suitable to be used in PoW @@ -259,7 +254,6 @@ where check_inherents_after: <::Header as HeaderT>::Number, select_chain: S, create_inherent_data_providers: CIDP, - can_author_with: CAW, ) -> Self { Self { inner, @@ -268,7 +262,6 @@ where check_inherents_after, select_chain, create_inherent_data_providers: Arc::new(create_inherent_data_providers), - can_author_with, } } @@ -283,16 +276,6 @@ where return Ok(()) } - if let Err(e) = self.can_author_with.can_author_with(&block_id) { - debug!( - target: "pow", - "Skipping `check_inherents` as authoring version is not compatible: {}", - e, - ); - - return Ok(()) - } - let inherent_data = inherent_data_providers .create_inherent_data() .map_err(|e| Error::CreateInherents(e))?; @@ -317,8 +300,7 @@ where } #[async_trait::async_trait] -impl BlockImport - for PowBlockImport +impl BlockImport for PowBlockImport where B: BlockT, I: BlockImport> + Send + Sync, @@ -328,7 +310,6 @@ where C::Api: BlockBuilderApi, Algorithm: PowAlgorithm + Send + Sync, Algorithm::Difficulty: 'static + Send, - CAW: CanAuthorWith + Send + Sync, CIDP: CreateInherentDataProviders + Send + Sync, { type Error = ConsensusError; @@ -512,7 +493,7 @@ where /// /// `pre_runtime` is a parameter that allows a custom additional pre-runtime digest to be inserted /// for blocks being built. This can encode authorship information, or just be a graffiti. -pub fn start_mining_worker( +pub fn start_mining_worker( block_import: BoxBlockImport>, client: Arc, select_chain: S, @@ -524,7 +505,6 @@ pub fn start_mining_worker( create_inherent_data_providers: CIDP, timeout: Duration, build_time: Duration, - can_author_with: CAW, ) -> ( MiningHandle>::Proof>, impl Future, @@ -541,7 +521,6 @@ where SO: SyncOracle + Clone + Send + Sync + 'static, L: sc_consensus::JustificationSyncLink, CIDP: CreateInherentDataProviders, - CAW: CanAuthorWith + Clone + Send + 'static, { let mut timer = UntilImportedOrTimeout::new(client.import_notification_stream(), timeout); let worker = MiningHandle::new(algorithm.clone(), block_import, justification_sync_link); @@ -573,16 +552,6 @@ where }; let best_hash = best_header.hash(); - if let Err(err) = can_author_with.can_author_with(&BlockId::Hash(best_hash)) { - warn!( - target: "pow", - "Skipping proposal `can_author_with` returned: {} \ - Probably a node update is required!", - err, - ); - continue - } - if worker.best_hash() == Some(best_hash) { continue } diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index 39b40a32f18ca..b9f8c03f2ac88 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -38,13 +38,10 @@ use log::{debug, info, warn}; use sc_consensus::{BlockImport, JustificationSyncLink}; use sc_telemetry::{telemetry, TelemetryHandle, CONSENSUS_DEBUG, CONSENSUS_INFO, CONSENSUS_WARN}; use sp_arithmetic::traits::BaseArithmetic; -use sp_consensus::{CanAuthorWith, Proposal, Proposer, SelectChain, SyncOracle}; +use sp_consensus::{Proposal, Proposer, SelectChain, SyncOracle}; use sp_consensus_slots::{Slot, SlotDuration}; use sp_inherents::CreateInherentDataProviders; -use sp_runtime::{ - generic::BlockId, - traits::{Block as BlockT, HashFor, Header as HeaderT}, -}; +use sp_runtime::traits::{Block as BlockT, HashFor, Header as HeaderT}; use sp_timestamp::Timestamp; use std::{fmt::Debug, ops::Deref, time::Duration}; @@ -486,13 +483,12 @@ impl_inherent_data_provider_ext_tuple!(T, S, A, B, C, D, E, F, G, H, I, J); /// /// Every time a new slot is triggered, `worker.on_slot` is called and the future it returns is /// polled until completion, unless we are major syncing. -pub async fn start_slot_worker( +pub async fn start_slot_worker( slot_duration: SlotDuration, client: C, mut worker: W, sync_oracle: SO, create_inherent_data_providers: CIDP, - can_author_with: CAW, ) where B: BlockT, C: SelectChain, @@ -500,7 +496,6 @@ pub async fn start_slot_worker( SO: SyncOracle + Send, CIDP: CreateInherentDataProviders + Send, CIDP::InherentDataProviders: InherentDataProviderExt + Send, - CAW: CanAuthorWith + Send, { let mut slots = Slots::new(slot_duration.as_duration(), create_inherent_data_providers, client); @@ -518,19 +513,7 @@ pub async fn start_slot_worker( continue } - if let Err(err) = - can_author_with.can_author_with(&BlockId::Hash(slot_info.chain_head.hash())) - { - warn!( - target: "slots", - "Unable to author block in slot {},. `can_author_with` returned: {} \ - Probably a node update is required!", - slot_info.slot, - err, - ); - } else { - let _ = worker.on_slot(slot_info).await; - } + let _ = worker.on_slot(slot_info).await; } } diff --git a/primitives/consensus/common/src/lib.rs b/primitives/consensus/common/src/lib.rs index 043533cbf2258..458a5eee259a9 100644 --- a/primitives/consensus/common/src/lib.rs +++ b/primitives/consensus/common/src/lib.rs @@ -25,7 +25,6 @@ use std::{sync::Arc, time::Duration}; use futures::prelude::*; use sp_runtime::{ - generic::BlockId, traits::{Block as BlockT, HashFor}, Digest, }; @@ -268,61 +267,3 @@ where T::is_offline(self) } } - -/// Checks if the current active native block authoring implementation can author with the runtime -/// at the given block. -pub trait CanAuthorWith { - /// See trait docs for more information. - /// - /// # Return - /// - /// - Returns `Ok(())` when authoring is supported. - /// - Returns `Err(_)` when authoring is not supported. - fn can_author_with(&self, at: &BlockId) -> Result<(), String>; -} - -/// Checks if the node can author blocks by using -/// [`NativeVersion::can_author_with`](sp_version::NativeVersion::can_author_with). -#[derive(Clone)] -pub struct CanAuthorWithNativeVersion(T); - -impl CanAuthorWithNativeVersion { - /// Creates a new instance of `Self`. - pub fn new(inner: T) -> Self { - Self(inner) - } -} - -impl + sp_version::GetNativeVersion, Block: BlockT> - CanAuthorWith for CanAuthorWithNativeVersion -{ - fn can_author_with(&self, at: &BlockId) -> Result<(), String> { - match self.0.runtime_version(at) { - Ok(version) => self.0.native_version().can_author_with(&version), - Err(e) => Err(format!( - "Failed to get runtime version at `{}` and will disable authoring. Error: {}", - at, e, - )), - } - } -} - -/// Returns always `true` for `can_author_with`. This is useful for tests. -#[derive(Clone)] -pub struct AlwaysCanAuthor; - -impl CanAuthorWith for AlwaysCanAuthor { - fn can_author_with(&self, _: &BlockId) -> Result<(), String> { - Ok(()) - } -} - -/// Never can author. -#[derive(Clone)] -pub struct NeverCanAuthor; - -impl CanAuthorWith for NeverCanAuthor { - fn can_author_with(&self, _: &BlockId) -> Result<(), String> { - Err("Authoring is always disabled.".to_string()) - } -} From 17f7ff851c4193f0c291e939db4ac1c877e17a55 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 13 Sep 2022 21:23:44 +0800 Subject: [PATCH 16/17] Create sp-weights crate to store weight primitives (#12219) * Create sp-weights crate to store weight primitives * Fix templates * Fix templates * Fixes * Fixes * cargo fmt * Fixes * Fixes * Use deprecated type alias instead of deprecated unit types * Use deprecated subtraits instead of deprecated hollow new traits * Fixes * Allow deprecation in macro expansion * Add missing where clause during call macro expansion * cargo fmt * Fixes * cargo fmt * Fixes * Fixes * Fixes * Fixes * Move FRAME-specific weight files back to frame_support * Fixes * Update frame/support/src/dispatch.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Update frame/support/src/dispatch.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Update frame/support/src/dispatch.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Add missing header * Rewrite module docs * Fixes * Fixes * Fixes * Fixes * cargo fmt Co-authored-by: Shawn Tabrizi Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- Cargo.lock | 18 + Cargo.toml | 1 + bin/node/executor/tests/basic.rs | 3 +- bin/node/executor/tests/fees.rs | 3 +- bin/node/runtime/src/impls.rs | 5 +- bin/node/runtime/src/lib.rs | 3 +- frame/babe/src/lib.rs | 4 +- frame/babe/src/tests.rs | 2 +- frame/balances/src/tests_composite.rs | 3 +- frame/balances/src/tests_local.rs | 3 +- frame/collective/src/lib.rs | 7 +- frame/collective/src/tests.rs | 5 +- frame/contracts/src/lib.rs | 4 +- frame/contracts/src/tests.rs | 4 +- frame/contracts/src/wasm/runtime.rs | 2 +- .../election-provider-multi-phase/src/lib.rs | 3 +- .../election-provider-support/src/onchain.rs | 2 +- frame/examples/basic/src/lib.rs | 4 +- frame/examples/basic/src/tests.rs | 5 +- frame/executive/src/lib.rs | 4 +- frame/grandpa/src/lib.rs | 4 +- frame/grandpa/src/tests.rs | 2 +- frame/multisig/src/lib.rs | 5 +- frame/preimage/src/lib.rs | 2 +- frame/proxy/src/lib.rs | 3 +- frame/ranked-collective/src/lib.rs | 3 +- frame/recovery/src/lib.rs | 3 +- frame/scheduler/src/lib.rs | 4 +- frame/staking/src/pallet/impls.rs | 3 +- frame/staking/src/tests.rs | 3 +- frame/sudo/src/lib.rs | 2 +- frame/support/Cargo.toml | 2 + .../src/construct_runtime/expand/call.rs | 7 +- .../procedural/src/pallet/expand/call.rs | 4 + frame/support/src/dispatch.rs | 754 +++++++++++++- frame/support/src/lib.rs | 7 +- frame/support/src/traits/misc.rs | 4 +- frame/support/src/weights.rs | 976 ++---------------- frame/support/src/weights/block_weights.rs | 8 +- .../support/src/weights/extrinsic_weights.rs | 8 +- frame/support/src/weights/paritydb_weights.rs | 9 +- frame/support/src/weights/rocksdb_weights.rs | 9 +- frame/support/test/tests/construct_runtime.rs | 6 +- frame/support/test/tests/origin.rs | 2 +- frame/support/test/tests/pallet.rs | 6 +- frame/support/test/tests/pallet_instance.rs | 3 +- frame/system/Cargo.toml | 2 + frame/system/benchmarking/src/lib.rs | 2 +- .../system/src/extensions/check_mortality.rs | 5 +- .../src/extensions/check_non_zero_sender.rs | 2 +- frame/system/src/extensions/check_nonce.rs | 2 +- frame/system/src/extensions/check_weight.rs | 8 +- frame/system/src/lib.rs | 10 +- frame/system/src/limits.rs | 5 +- frame/system/src/tests.rs | 3 +- .../asset-tx-payment/src/lib.rs | 3 +- .../asset-tx-payment/src/tests.rs | 3 +- frame/transaction-payment/src/lib.rs | 17 +- frame/transaction-payment/src/types.rs | 2 +- frame/utility/src/lib.rs | 3 +- frame/utility/src/tests.rs | 4 +- frame/whitelist/src/lib.rs | 3 +- primitives/runtime/Cargo.toml | 1 + primitives/runtime/src/traits.rs | 6 + primitives/weights/Cargo.toml | 40 + primitives/weights/src/lib.rs | 299 ++++++ .../weights/src}/weight_v2.rs | 204 +--- .../benchmarking-cli/src/overhead/weights.hbs | 8 +- .../benchmarking-cli/src/storage/weights.hbs | 9 +- 69 files changed, 1328 insertions(+), 1237 deletions(-) create mode 100644 primitives/weights/Cargo.toml create mode 100644 primitives/weights/src/lib.rs rename {frame/support/src/weights => primitives/weights/src}/weight_v2.rs (68%) diff --git a/Cargo.lock b/Cargo.lock index 42e2df92d77ef..f00f87212d780 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2294,6 +2294,7 @@ dependencies = [ "sp-state-machine", "sp-std", "sp-tracing", + "sp-weights", "tt-call", ] @@ -2391,6 +2392,7 @@ dependencies = [ "sp-runtime", "sp-std", "sp-version", + "sp-weights", "substrate-test-runtime-client", ] @@ -10033,6 +10035,7 @@ dependencies = [ "sp-state-machine", "sp-std", "sp-tracing", + "sp-weights", "substrate-test-runtime-client", "zstd", ] @@ -10338,6 +10341,21 @@ dependencies = [ "wasmtime", ] +[[package]] +name = "sp-weights" +version = "4.0.0" +dependencies = [ + "impl-trait-for-tuples", + "parity-scale-codec", + "scale-info", + "serde", + "smallvec", + "sp-arithmetic", + "sp-core", + "sp-debug-derive", + "sp-std", +] + [[package]] name = "spin" version = "0.5.2" diff --git a/Cargo.toml b/Cargo.toml index af2b90d9964d3..294bc27ade3da 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -213,6 +213,7 @@ members = [ "primitives/version", "primitives/version/proc-macro", "primitives/wasm-interface", + "primitives/weights", "test-utils/client", "test-utils/derive", "test-utils/runtime", diff --git a/bin/node/executor/tests/basic.rs b/bin/node/executor/tests/basic.rs index 33f71568ec6c5..99a9b83596acf 100644 --- a/bin/node/executor/tests/basic.rs +++ b/bin/node/executor/tests/basic.rs @@ -17,8 +17,9 @@ use codec::{Decode, Encode, Joiner}; use frame_support::{ + dispatch::{DispatchClass, DispatchInfo, GetDispatchInfo}, traits::Currency, - weights::{DispatchClass, DispatchInfo, GetDispatchInfo, Weight}, + weights::Weight, }; use frame_system::{self, AccountInfo, EventRecord, Phase}; use sp_core::{storage::well_known_keys, traits::Externalities}; diff --git a/bin/node/executor/tests/fees.rs b/bin/node/executor/tests/fees.rs index ae1bfa371469c..6932cb2cea867 100644 --- a/bin/node/executor/tests/fees.rs +++ b/bin/node/executor/tests/fees.rs @@ -17,8 +17,9 @@ use codec::{Encode, Joiner}; use frame_support::{ + dispatch::GetDispatchInfo, traits::Currency, - weights::{constants::ExtrinsicBaseWeight, GetDispatchInfo, IdentityFee, WeightToFee}, + weights::{constants::ExtrinsicBaseWeight, IdentityFee, WeightToFee}, }; use kitchensink_runtime::{ constants::{currency::*, time::SLOT_DURATION}, diff --git a/bin/node/runtime/src/impls.rs b/bin/node/runtime/src/impls.rs index 3509592ebf04a..fb2f3cec65290 100644 --- a/bin/node/runtime/src/impls.rs +++ b/bin/node/runtime/src/impls.rs @@ -129,7 +129,10 @@ mod multiplier_tests { AdjustmentVariable, MinimumMultiplier, Runtime, RuntimeBlockWeights as BlockWeights, System, TargetBlockFullness, TransactionPayment, }; - use frame_support::weights::{DispatchClass, Weight, WeightToFee}; + use frame_support::{ + dispatch::DispatchClass, + weights::{Weight, WeightToFee}, + }; fn max_normal() -> Weight { BlockWeights::get() diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 966a2e17ea475..2f0efcaa56740 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -28,6 +28,7 @@ use frame_election_provider_support::{ }; use frame_support::{ construct_runtime, + dispatch::DispatchClass, pallet_prelude::Get, parameter_types, traits::{ @@ -37,7 +38,7 @@ use frame_support::{ }, weights::{ constants::{BlockExecutionWeight, ExtrinsicBaseWeight, RocksDbWeight, WEIGHT_PER_SECOND}, - ConstantMultiplier, DispatchClass, IdentityFee, Weight, + ConstantMultiplier, IdentityFee, Weight, }, PalletId, RuntimeDebug, }; diff --git a/frame/babe/src/lib.rs b/frame/babe/src/lib.rs index 9a99d8ed995ab..eadaa036332fa 100644 --- a/frame/babe/src/lib.rs +++ b/frame/babe/src/lib.rs @@ -23,13 +23,13 @@ use codec::{Decode, Encode}; use frame_support::{ - dispatch::DispatchResultWithPostInfo, + dispatch::{DispatchResultWithPostInfo, Pays}, ensure, traits::{ ConstU32, DisabledValidators, FindAuthor, Get, KeyOwnerProofSystem, OnTimestampSet, OneSessionHandler, }, - weights::{Pays, Weight}, + weights::Weight, BoundedVec, WeakBoundedVec, }; use sp_application_crypto::ByteArray; diff --git a/frame/babe/src/tests.rs b/frame/babe/src/tests.rs index 065105eda1c32..84e55ed5d050b 100644 --- a/frame/babe/src/tests.rs +++ b/frame/babe/src/tests.rs @@ -20,8 +20,8 @@ use super::{Call, *}; use frame_support::{ assert_err, assert_noop, assert_ok, + dispatch::{GetDispatchInfo, Pays}, traits::{Currency, EstimateNextSessionRotation, OnFinalize}, - weights::{GetDispatchInfo, Pays}, }; use mock::*; use pallet_session::ShouldEndSession; diff --git a/frame/balances/src/tests_composite.rs b/frame/balances/src/tests_composite.rs index fa1d2db8b1e49..1276a812c7861 100644 --- a/frame/balances/src/tests_composite.rs +++ b/frame/balances/src/tests_composite.rs @@ -21,9 +21,10 @@ use crate::{self as pallet_balances, decl_tests, Config, Pallet}; use frame_support::{ + dispatch::DispatchInfo, parameter_types, traits::{ConstU32, ConstU64, ConstU8}, - weights::{DispatchInfo, IdentityFee, Weight}, + weights::{IdentityFee, Weight}, }; use pallet_transaction_payment::CurrencyAdapter; use sp_core::H256; diff --git a/frame/balances/src/tests_local.rs b/frame/balances/src/tests_local.rs index 3301d90027682..b391b03ed6b6a 100644 --- a/frame/balances/src/tests_local.rs +++ b/frame/balances/src/tests_local.rs @@ -21,9 +21,10 @@ use crate::{self as pallet_balances, decl_tests, Config, Pallet}; use frame_support::{ + dispatch::DispatchInfo, parameter_types, traits::{ConstU32, ConstU64, ConstU8, StorageMapShim}, - weights::{DispatchInfo, IdentityFee, Weight}, + weights::{IdentityFee, Weight}, }; use pallet_transaction_payment::CurrencyAdapter; use sp_core::H256; diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index 7944f826255ec..49da641e3e204 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -49,12 +49,15 @@ use sp_std::{marker::PhantomData, prelude::*, result}; use frame_support::{ codec::{Decode, Encode, MaxEncodedLen}, - dispatch::{DispatchError, DispatchResultWithPostInfo, Dispatchable, PostDispatchInfo}, + dispatch::{ + DispatchError, DispatchResultWithPostInfo, Dispatchable, GetDispatchInfo, Pays, + PostDispatchInfo, + }, ensure, traits::{ Backing, ChangeMembers, EnsureOrigin, Get, GetBacking, InitializeMembers, StorageVersion, }, - weights::{GetDispatchInfo, Pays, Weight}, + weights::Weight, }; #[cfg(test)] diff --git a/frame/collective/src/tests.rs b/frame/collective/src/tests.rs index b063cc21bd426..13b9a24f7889d 100644 --- a/frame/collective/src/tests.rs +++ b/frame/collective/src/tests.rs @@ -18,9 +18,10 @@ use super::{Event as CollectiveEvent, *}; use crate as pallet_collective; use frame_support::{ - assert_noop, assert_ok, parameter_types, + assert_noop, assert_ok, + dispatch::Pays, + parameter_types, traits::{ConstU32, ConstU64, GenesisBuild, StorageVersion}, - weights::Pays, Hashable, }; use frame_system::{EventRecord, Phase}; diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 9890bd840cc3a..48f1668440387 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -107,10 +107,10 @@ use crate::{ }; use codec::{Encode, HasCompact}; use frame_support::{ - dispatch::Dispatchable, + dispatch::{DispatchClass, Dispatchable, GetDispatchInfo, Pays, PostDispatchInfo}, ensure, traits::{ConstU32, Contains, Currency, Get, Randomness, ReservableCurrency, Time}, - weights::{DispatchClass, GetDispatchInfo, Pays, PostDispatchInfo, Weight}, + weights::Weight, BoundedVec, }; use frame_system::{limits::BlockWeights, Pallet as System}; diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index 1193d17676e1d..c40968eb2ea57 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -32,14 +32,14 @@ use assert_matches::assert_matches; use codec::Encode; use frame_support::{ assert_err, assert_err_ignore_postinfo, assert_noop, assert_ok, - dispatch::DispatchErrorWithPostInfo, + dispatch::{DispatchClass, DispatchErrorWithPostInfo, PostDispatchInfo}, parameter_types, storage::child, traits::{ BalanceStatus, ConstU32, ConstU64, Contains, Currency, Get, OnIdle, OnInitialize, ReservableCurrency, }, - weights::{constants::WEIGHT_PER_SECOND, DispatchClass, PostDispatchInfo, Weight}, + weights::{constants::WEIGHT_PER_SECOND, Weight}, }; use frame_system::{self as system, EventRecord, Phase}; use pretty_assertions::{assert_eq, assert_ne}; diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index 5b94611843169..7caa7c5c6fa1f 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -2314,7 +2314,7 @@ pub mod env { call_ptr: u32, call_len: u32, ) -> Result { - use frame_support::{dispatch::GetDispatchInfo, weights::extract_actual_weight}; + use frame_support::dispatch::{extract_actual_weight, GetDispatchInfo}; ctx.charge_gas(RuntimeCosts::CopyFromContract(call_len))?; let call: ::RuntimeCall = ctx.read_sandbox_memory_as_unbounded(call_ptr, call_len)?; diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index f0378ea7dc83f..1188e3353bc96 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -234,9 +234,10 @@ use frame_election_provider_support::{ ElectionDataProvider, ElectionProvider, InstantElectionProvider, NposSolution, }; use frame_support::{ + dispatch::DispatchClass, ensure, traits::{Currency, Get, OnUnbalanced, ReservableCurrency}, - weights::{DispatchClass, Weight}, + weights::Weight, }; use frame_system::{ensure_none, offchain::SendTransactionTypes}; use scale_info::TypeInfo; diff --git a/frame/election-provider-support/src/onchain.rs b/frame/election-provider-support/src/onchain.rs index c899e021f974a..05bbd11e1ae9d 100644 --- a/frame/election-provider-support/src/onchain.rs +++ b/frame/election-provider-support/src/onchain.rs @@ -22,7 +22,7 @@ use crate::{ Debug, ElectionDataProvider, ElectionProvider, InstantElectionProvider, NposSolver, WeightInfo, }; -use frame_support::{traits::Get, weights::DispatchClass}; +use frame_support::{dispatch::DispatchClass, traits::Get}; use sp_npos_elections::*; use sp_std::{collections::btree_map::BTreeMap, marker::PhantomData, prelude::*}; diff --git a/frame/examples/basic/src/lib.rs b/frame/examples/basic/src/lib.rs index 756333b197090..caa2d825262db 100644 --- a/frame/examples/basic/src/lib.rs +++ b/frame/examples/basic/src/lib.rs @@ -273,9 +273,9 @@ use codec::{Decode, Encode}; use frame_support::{ - dispatch::DispatchResult, + dispatch::{ClassifyDispatch, DispatchClass, DispatchResult, Pays, PaysFee, WeighData}, traits::IsSubType, - weights::{ClassifyDispatch, DispatchClass, Pays, PaysFee, WeighData, Weight}, + weights::Weight, }; use frame_system::ensure_signed; use log::info; diff --git a/frame/examples/basic/src/tests.rs b/frame/examples/basic/src/tests.rs index 78cdb3794f8d3..793cb4287f505 100644 --- a/frame/examples/basic/src/tests.rs +++ b/frame/examples/basic/src/tests.rs @@ -19,9 +19,10 @@ use crate::*; use frame_support::{ - assert_ok, parameter_types, + assert_ok, + dispatch::{DispatchInfo, GetDispatchInfo}, + parameter_types, traits::{ConstU64, OnInitialize}, - weights::{DispatchInfo, GetDispatchInfo}, }; use sp_core::H256; // The testing primitives are very useful for avoiding having to work with signatures diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 7b71224904378..96e71512e54bc 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -118,12 +118,12 @@ use codec::{Codec, Encode}; use frame_support::{ - dispatch::PostDispatchInfo, + dispatch::{DispatchClass, DispatchInfo, GetDispatchInfo, PostDispatchInfo}, traits::{ EnsureInherentsAreFirst, ExecuteBlock, OffchainWorker, OnFinalize, OnIdle, OnInitialize, OnRuntimeUpgrade, }, - weights::{DispatchClass, DispatchInfo, GetDispatchInfo, Weight}, + weights::Weight, }; use sp_runtime::{ generic::Digest, diff --git a/frame/grandpa/src/lib.rs b/frame/grandpa/src/lib.rs index 1fcf5c67150b5..fe5b9861853bf 100644 --- a/frame/grandpa/src/lib.rs +++ b/frame/grandpa/src/lib.rs @@ -40,11 +40,11 @@ use fg_primitives::{ GRANDPA_ENGINE_ID, }; use frame_support::{ - dispatch::DispatchResultWithPostInfo, + dispatch::{DispatchResultWithPostInfo, Pays}, pallet_prelude::Get, storage, traits::{KeyOwnerProofSystem, OneSessionHandler}, - weights::{Pays, Weight}, + weights::Weight, WeakBoundedVec, }; use scale_info::TypeInfo; diff --git a/frame/grandpa/src/tests.rs b/frame/grandpa/src/tests.rs index 07c4fcad3078d..baf71dfff71dc 100644 --- a/frame/grandpa/src/tests.rs +++ b/frame/grandpa/src/tests.rs @@ -25,8 +25,8 @@ use codec::Encode; use fg_primitives::ScheduledChange; use frame_support::{ assert_err, assert_noop, assert_ok, + dispatch::{GetDispatchInfo, Pays}, traits::{Currency, OnFinalize, OneSessionHandler}, - weights::{GetDispatchInfo, Pays}, }; use frame_system::{EventRecord, Phase}; use sp_core::H256; diff --git a/frame/multisig/src/lib.rs b/frame/multisig/src/lib.rs index 4593d6dade690..0c7e931e5ea2e 100644 --- a/frame/multisig/src/lib.rs +++ b/frame/multisig/src/lib.rs @@ -53,11 +53,12 @@ pub mod weights; use codec::{Decode, Encode}; use frame_support::{ dispatch::{ - DispatchErrorWithPostInfo, DispatchResult, DispatchResultWithPostInfo, PostDispatchInfo, + DispatchErrorWithPostInfo, DispatchResult, DispatchResultWithPostInfo, GetDispatchInfo, + PostDispatchInfo, }, ensure, traits::{Currency, Get, ReservableCurrency, WrapperKeepOpaque}, - weights::{GetDispatchInfo, Weight}, + weights::Weight, RuntimeDebug, }; use frame_system::{self as system, RawOrigin}; diff --git a/frame/preimage/src/lib.rs b/frame/preimage/src/lib.rs index e4616078b0761..749e5924cc885 100644 --- a/frame/preimage/src/lib.rs +++ b/frame/preimage/src/lib.rs @@ -41,10 +41,10 @@ use sp_std::prelude::*; use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{ + dispatch::Pays, ensure, pallet_prelude::Get, traits::{Currency, PreimageProvider, PreimageRecipient, ReservableCurrency}, - weights::Pays, BoundedVec, }; use scale_info::TypeInfo; diff --git a/frame/proxy/src/lib.rs b/frame/proxy/src/lib.rs index 7467fb0988dd6..dcc19e85085f8 100644 --- a/frame/proxy/src/lib.rs +++ b/frame/proxy/src/lib.rs @@ -35,10 +35,9 @@ pub mod weights; use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{ - dispatch::DispatchError, + dispatch::{DispatchError, GetDispatchInfo}, ensure, traits::{Currency, Get, InstanceFilter, IsSubType, IsType, OriginTrait, ReservableCurrency}, - weights::GetDispatchInfo, RuntimeDebug, }; use frame_system::{self as system}; diff --git a/frame/ranked-collective/src/lib.rs b/frame/ranked-collective/src/lib.rs index 7f212b3859776..44b942d000012 100644 --- a/frame/ranked-collective/src/lib.rs +++ b/frame/ranked-collective/src/lib.rs @@ -52,10 +52,9 @@ use sp_std::{marker::PhantomData, prelude::*}; use frame_support::{ codec::{Decode, Encode, MaxEncodedLen}, - dispatch::{DispatchError, DispatchResultWithPostInfo}, + dispatch::{DispatchError, DispatchResultWithPostInfo, PostDispatchInfo}, ensure, traits::{EnsureOrigin, PollStatus, Polling, VoteTally}, - weights::PostDispatchInfo, CloneNoBound, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound, }; diff --git a/frame/recovery/src/lib.rs b/frame/recovery/src/lib.rs index dd9e7208320c4..7b85c119e68c5 100644 --- a/frame/recovery/src/lib.rs +++ b/frame/recovery/src/lib.rs @@ -160,9 +160,8 @@ use sp_runtime::traits::{CheckedAdd, CheckedMul, Dispatchable, SaturatedConversi use sp_std::prelude::*; use frame_support::{ - dispatch::PostDispatchInfo, + dispatch::{GetDispatchInfo, PostDispatchInfo}, traits::{BalanceStatus, Currency, ReservableCurrency}, - weights::GetDispatchInfo, BoundedVec, RuntimeDebug, }; diff --git a/frame/scheduler/src/lib.rs b/frame/scheduler/src/lib.rs index 39dbac17cedd2..7e9b8e1b92fb7 100644 --- a/frame/scheduler/src/lib.rs +++ b/frame/scheduler/src/lib.rs @@ -60,12 +60,12 @@ pub mod weights; use codec::{Codec, Decode, Encode}; use frame_support::{ - dispatch::{DispatchError, DispatchResult, Dispatchable, Parameter}, + dispatch::{DispatchError, DispatchResult, Dispatchable, GetDispatchInfo, Parameter}, traits::{ schedule::{self, DispatchTime, MaybeHashed}, EnsureOrigin, Get, IsType, OriginTrait, PalletInfoAccess, PrivilegeCmp, StorageVersion, }, - weights::{GetDispatchInfo, Weight}, + weights::Weight, }; use frame_system::{self as system, ensure_signed}; pub use pallet::*; diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 386c413132f6c..13038527bc2f8 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -22,12 +22,13 @@ use frame_election_provider_support::{ Supports, VoteWeight, VoterOf, }; use frame_support::{ + dispatch::WithPostDispatchInfo, pallet_prelude::*, traits::{ Currency, CurrencyToVote, Defensive, EstimateNextNewSession, Get, Imbalance, LockableCurrency, OnUnbalanced, UnixTime, WithdrawReasons, }, - weights::{Weight, WithPostDispatchInfo}, + weights::Weight, }; use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin}; use pallet_session::historical; diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 9807cffd43909..85badfac769af 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -21,10 +21,9 @@ use super::{ConfigOp, Event, MaxUnlockingChunks, *}; use frame_election_provider_support::{ElectionProvider, SortedListProvider, Support}; use frame_support::{ assert_noop, assert_ok, assert_storage_noop, bounded_vec, - dispatch::WithPostDispatchInfo, + dispatch::{extract_actual_weight, GetDispatchInfo, WithPostDispatchInfo}, pallet_prelude::*, traits::{Currency, Get, ReservableCurrency}, - weights::{extract_actual_weight, GetDispatchInfo}, }; use mock::*; use pallet_balances::Error as BalancesError; diff --git a/frame/sudo/src/lib.rs b/frame/sudo/src/lib.rs index 96cd2d1615449..69eac2c1b93c4 100644 --- a/frame/sudo/src/lib.rs +++ b/frame/sudo/src/lib.rs @@ -95,7 +95,7 @@ use sp_runtime::{traits::StaticLookup, DispatchResult}; use sp_std::prelude::*; -use frame_support::{traits::UnfilteredDispatchable, weights::GetDispatchInfo}; +use frame_support::{dispatch::GetDispatchInfo, traits::UnfilteredDispatchable}; #[cfg(test)] mod mock; diff --git a/frame/support/Cargo.toml b/frame/support/Cargo.toml index 6f0831b751b45..d0a40a09f2211 100644 --- a/frame/support/Cargo.toml +++ b/frame/support/Cargo.toml @@ -26,6 +26,7 @@ sp-core = { version = "6.0.0", default-features = false, path = "../../primitive sp-arithmetic = { version = "5.0.0", default-features = false, path = "../../primitives/arithmetic" } sp-inherents = { version = "4.0.0-dev", default-features = false, path = "../../primitives/inherents" } sp-staking = { version = "4.0.0-dev", default-features = false, path = "../../primitives/staking" } +sp-weights = { version = "4.0.0", default-features = false, path = "../../primitives/weights" } tt-call = "1.0.8" frame-support-procedural = { version = "4.0.0-dev", default-features = false, path = "./procedural" } paste = "1.0" @@ -62,6 +63,7 @@ std = [ "sp-inherents/std", "sp-staking/std", "sp-state-machine", + "sp-weights/std", "frame-support-procedural/std", "log/std", ] diff --git a/frame/support/procedural/src/construct_runtime/expand/call.rs b/frame/support/procedural/src/construct_runtime/expand/call.rs index b415132ab0426..101f0c371382f 100644 --- a/frame/support/procedural/src/construct_runtime/expand/call.rs +++ b/frame/support/procedural/src/construct_runtime/expand/call.rs @@ -123,6 +123,9 @@ pub fn expand_outer_dispatch( } } } + // Deprecated, but will warn when used + #[allow(deprecated)] + impl #scrate::weights::GetDispatchInfo for RuntimeCall {} impl #scrate::dispatch::GetCallMetadata for RuntimeCall { fn get_call_metadata(&self) -> #scrate::dispatch::CallMetadata { use #scrate::dispatch::GetCallName; @@ -161,8 +164,8 @@ pub fn expand_outer_dispatch( impl #scrate::dispatch::Dispatchable for RuntimeCall { type Origin = Origin; type Config = RuntimeCall; - type Info = #scrate::weights::DispatchInfo; - type PostInfo = #scrate::weights::PostDispatchInfo; + type Info = #scrate::dispatch::DispatchInfo; + type PostInfo = #scrate::dispatch::PostDispatchInfo; fn dispatch(self, origin: Origin) -> #scrate::dispatch::DispatchResultWithPostInfo { if !::filter_call(&origin, &self) { return #scrate::sp_std::result::Result::Err( diff --git a/frame/support/procedural/src/pallet/expand/call.rs b/frame/support/procedural/src/pallet/expand/call.rs index 8b2922b5fa4f2..d5c84fa37510d 100644 --- a/frame/support/procedural/src/pallet/expand/call.rs +++ b/frame/support/procedural/src/pallet/expand/call.rs @@ -255,6 +255,10 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream { } } + // Deprecated, but will warn when used + #[allow(deprecated)] + impl<#type_impl_gen> #frame_support::weights::GetDispatchInfo for #call_ident<#type_use_gen> #where_clause {} + impl<#type_impl_gen> #frame_support::dispatch::GetCallName for #call_ident<#type_use_gen> #where_clause { diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index 9a6654d5524d4..1d8930f96fbe5 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -31,18 +31,22 @@ pub use crate::{ traits::{ CallMetadata, GetCallMetadata, GetCallName, GetStorageVersion, UnfilteredDispatchable, }, - weights::{ - ClassifyDispatch, DispatchInfo, GetDispatchInfo, PaysFee, PostDispatchInfo, - TransactionPriority, WeighData, Weight, WithPostDispatchInfo, - }, }; -pub use sp_runtime::{traits::Dispatchable, DispatchError, RuntimeDebug}; +#[cfg(feature = "std")] +use serde::{Deserialize, Serialize}; +use sp_runtime::{ + generic::{CheckedExtrinsic, UncheckedExtrinsic}, + traits::SignedExtension, +}; +pub use sp_runtime::{ + traits::Dispatchable, transaction_validity::TransactionPriority, DispatchError, RuntimeDebug, +}; +pub use sp_weights::Weight; /// The return type of a `Dispatchable` in frame. When returned explicitly from /// a dispatchable function it allows overriding the default `PostDispatchInfo` /// returned from a dispatch. -pub type DispatchResultWithPostInfo = - sp_runtime::DispatchResultWithInfo; +pub type DispatchResultWithPostInfo = sp_runtime::DispatchResultWithInfo; /// Unaugmented version of `DispatchResultWithPostInfo` that can be returned from /// dispatchable functions and is automatically converted to the augmented type. Should be @@ -51,8 +55,7 @@ pub type DispatchResultWithPostInfo = pub type DispatchResult = Result<(), sp_runtime::DispatchError>; /// The error type contained in a `DispatchResultWithPostInfo`. -pub type DispatchErrorWithPostInfo = - sp_runtime::DispatchErrorWithPostInfo; +pub type DispatchErrorWithPostInfo = sp_runtime::DispatchErrorWithPostInfo; /// Serializable version of pallet dispatchable. pub trait Callable { @@ -91,6 +94,552 @@ impl From> for RawOrigin { pub trait Parameter: Codec + EncodeLike + Clone + Eq + fmt::Debug + scale_info::TypeInfo {} impl Parameter for T where T: Codec + EncodeLike + Clone + Eq + fmt::Debug + scale_info::TypeInfo {} +/// Means of classifying a dispatchable function. +pub trait ClassifyDispatch { + /// Classify the dispatch function based on input data `target` of type `T`. When implementing + /// this for a dispatchable, `T` will be a tuple of all arguments given to the function (except + /// origin). + fn classify_dispatch(&self, target: T) -> DispatchClass; +} + +/// Indicates if dispatch function should pay fees or not. +/// +/// If set to `Pays::No`, the block resource limits are applied, yet no fee is deducted. +pub trait PaysFee { + fn pays_fee(&self, _target: T) -> Pays; +} + +/// Explicit enum to denote if a transaction pays fee or not. +#[derive(Clone, Copy, Eq, PartialEq, RuntimeDebug, Encode, Decode, TypeInfo)] +pub enum Pays { + /// Transactor will pay related fees. + Yes, + /// Transactor will NOT pay related fees. + No, +} + +impl Default for Pays { + fn default() -> Self { + Self::Yes + } +} + +impl From for PostDispatchInfo { + fn from(pays_fee: Pays) -> Self { + Self { actual_weight: None, pays_fee } + } +} + +/// A generalized group of dispatch types. +/// +/// NOTE whenever upgrading the enum make sure to also update +/// [DispatchClass::all] and [DispatchClass::non_mandatory] helper functions. +#[cfg_attr(feature = "std", derive(Serialize, Deserialize))] +#[cfg_attr(feature = "std", serde(rename_all = "camelCase"))] +#[derive(PartialEq, Eq, Clone, Copy, Encode, Decode, RuntimeDebug, TypeInfo)] +pub enum DispatchClass { + /// A normal dispatch. + Normal, + /// An operational dispatch. + Operational, + /// A mandatory dispatch. These kinds of dispatch are always included regardless of their + /// weight, therefore it is critical that they are separately validated to ensure that a + /// malicious validator cannot craft a valid but impossibly heavy block. Usually this just + /// means ensuring that the extrinsic can only be included once and that it is always very + /// light. + /// + /// Do *NOT* use it for extrinsics that can be heavy. + /// + /// The only real use case for this is inherent extrinsics that are required to execute in a + /// block for the block to be valid, and it solves the issue in the case that the block + /// initialization is sufficiently heavy to mean that those inherents do not fit into the + /// block. Essentially, we assume that in these exceptional circumstances, it is better to + /// allow an overweight block to be created than to not allow any block at all to be created. + Mandatory, +} + +impl Default for DispatchClass { + fn default() -> Self { + Self::Normal + } +} + +impl DispatchClass { + /// Returns an array containing all dispatch classes. + pub fn all() -> &'static [DispatchClass] { + &[DispatchClass::Normal, DispatchClass::Operational, DispatchClass::Mandatory] + } + + /// Returns an array of all dispatch classes except `Mandatory`. + pub fn non_mandatory() -> &'static [DispatchClass] { + &[DispatchClass::Normal, DispatchClass::Operational] + } +} + +/// A trait that represents one or many values of given type. +/// +/// Useful to accept as parameter type to let the caller pass either a single value directly +/// or an iterator. +pub trait OneOrMany { + /// The iterator type. + type Iter: Iterator; + /// Convert this item into an iterator. + fn into_iter(self) -> Self::Iter; +} + +impl OneOrMany for DispatchClass { + type Iter = sp_std::iter::Once; + fn into_iter(self) -> Self::Iter { + sp_std::iter::once(self) + } +} + +impl<'a> OneOrMany for &'a [DispatchClass] { + type Iter = sp_std::iter::Cloned>; + fn into_iter(self) -> Self::Iter { + self.iter().cloned() + } +} + +/// A bundle of static information collected from the `#[pallet::weight]` attributes. +#[derive(Clone, Copy, Eq, PartialEq, Default, RuntimeDebug, Encode, Decode, TypeInfo)] +pub struct DispatchInfo { + /// Weight of this transaction. + pub weight: Weight, + /// Class of this transaction. + pub class: DispatchClass, + /// Does this transaction pay fees. + pub pays_fee: Pays, +} + +/// A `Dispatchable` function (aka transaction) that can carry some static information along with +/// it, using the `#[pallet::weight]` attribute. +pub trait GetDispatchInfo { + /// Return a `DispatchInfo`, containing relevant information of this dispatch. + /// + /// This is done independently of its encoded size. + fn get_dispatch_info(&self) -> DispatchInfo; +} + +impl GetDispatchInfo for () { + fn get_dispatch_info(&self) -> DispatchInfo { + DispatchInfo::default() + } +} + +/// Extract the actual weight from a dispatch result if any or fall back to the default weight. +pub fn extract_actual_weight(result: &DispatchResultWithPostInfo, info: &DispatchInfo) -> Weight { + match result { + Ok(post_info) => post_info, + Err(err) => &err.post_info, + } + .calc_actual_weight(info) +} + +/// Extract the actual pays_fee from a dispatch result if any or fall back to the default weight. +pub fn extract_actual_pays_fee(result: &DispatchResultWithPostInfo, info: &DispatchInfo) -> Pays { + match result { + Ok(post_info) => post_info, + Err(err) => &err.post_info, + } + .pays_fee(info) +} + +/// Weight information that is only available post dispatch. +/// NOTE: This can only be used to reduce the weight or fee, not increase it. +#[derive(Clone, Copy, Eq, PartialEq, Default, RuntimeDebug, Encode, Decode, TypeInfo)] +pub struct PostDispatchInfo { + /// Actual weight consumed by a call or `None` which stands for the worst case static weight. + pub actual_weight: Option, + /// Whether this transaction should pay fees when all is said and done. + pub pays_fee: Pays, +} + +impl PostDispatchInfo { + /// Calculate how much (if any) weight was not used by the `Dispatchable`. + pub fn calc_unspent(&self, info: &DispatchInfo) -> Weight { + info.weight - self.calc_actual_weight(info) + } + + /// Calculate how much weight was actually spent by the `Dispatchable`. + pub fn calc_actual_weight(&self, info: &DispatchInfo) -> Weight { + if let Some(actual_weight) = self.actual_weight { + actual_weight.min(info.weight) + } else { + info.weight + } + } + + /// Determine if user should actually pay fees at the end of the dispatch. + pub fn pays_fee(&self, info: &DispatchInfo) -> Pays { + // If they originally were not paying fees, or the post dispatch info + // says they should not pay fees, then they don't pay fees. + // This is because the pre dispatch information must contain the + // worst case for weight and fees paid. + if info.pays_fee == Pays::No || self.pays_fee == Pays::No { + Pays::No + } else { + // Otherwise they pay. + Pays::Yes + } + } +} + +impl From<()> for PostDispatchInfo { + fn from(_: ()) -> Self { + Self { actual_weight: None, pays_fee: Default::default() } + } +} + +impl sp_runtime::traits::Printable for PostDispatchInfo { + fn print(&self) { + "actual_weight=".print(); + match self.actual_weight { + Some(weight) => weight.print(), + None => "max-weight".print(), + }; + "pays_fee=".print(); + match self.pays_fee { + Pays::Yes => "Yes".print(), + Pays::No => "No".print(), + } + } +} + +/// Allows easy conversion from `DispatchError` to `DispatchErrorWithPostInfo` for dispatchables +/// that want to return a custom a posterior weight on error. +pub trait WithPostDispatchInfo { + /// Call this on your modules custom errors type in order to return a custom weight on error. + /// + /// # Example + /// + /// ```ignore + /// let who = ensure_signed(origin).map_err(|e| e.with_weight(Weight::from_ref_time(100)))?; + /// ensure!(who == me, Error::::NotMe.with_weight(200_000)); + /// ``` + fn with_weight(self, actual_weight: Weight) -> DispatchErrorWithPostInfo; +} + +impl WithPostDispatchInfo for T +where + T: Into, +{ + fn with_weight(self, actual_weight: Weight) -> DispatchErrorWithPostInfo { + DispatchErrorWithPostInfo { + post_info: PostDispatchInfo { + actual_weight: Some(actual_weight), + pays_fee: Default::default(), + }, + error: self.into(), + } + } +} + +/// Implementation for unchecked extrinsic. +impl GetDispatchInfo + for UncheckedExtrinsic +where + Call: GetDispatchInfo, + Extra: SignedExtension, +{ + fn get_dispatch_info(&self) -> DispatchInfo { + self.function.get_dispatch_info() + } +} + +/// Implementation for checked extrinsic. +impl GetDispatchInfo for CheckedExtrinsic +where + Call: GetDispatchInfo, +{ + fn get_dispatch_info(&self) -> DispatchInfo { + self.function.get_dispatch_info() + } +} + +/// Implementation for test extrinsic. +#[cfg(feature = "std")] +impl GetDispatchInfo for sp_runtime::testing::TestXt { + fn get_dispatch_info(&self) -> DispatchInfo { + // for testing: weight == size. + DispatchInfo { + weight: Weight::from_ref_time(self.encode().len() as _), + pays_fee: Pays::Yes, + ..Default::default() + } + } +} + +/// A struct holding value for each `DispatchClass`. +#[derive(Clone, Eq, PartialEq, Default, RuntimeDebug, Encode, Decode, TypeInfo, MaxEncodedLen)] +pub struct PerDispatchClass { + /// Value for `Normal` extrinsics. + normal: T, + /// Value for `Operational` extrinsics. + operational: T, + /// Value for `Mandatory` extrinsics. + mandatory: T, +} + +impl PerDispatchClass { + /// Create new `PerDispatchClass` with the same value for every class. + pub fn new(val: impl Fn(DispatchClass) -> T) -> Self { + Self { + normal: val(DispatchClass::Normal), + operational: val(DispatchClass::Operational), + mandatory: val(DispatchClass::Mandatory), + } + } + + /// Get a mutable reference to current value of given class. + pub fn get_mut(&mut self, class: DispatchClass) -> &mut T { + match class { + DispatchClass::Operational => &mut self.operational, + DispatchClass::Normal => &mut self.normal, + DispatchClass::Mandatory => &mut self.mandatory, + } + } + + /// Get current value for given class. + pub fn get(&self, class: DispatchClass) -> &T { + match class { + DispatchClass::Normal => &self.normal, + DispatchClass::Operational => &self.operational, + DispatchClass::Mandatory => &self.mandatory, + } + } +} + +impl PerDispatchClass { + /// Set the value of given class. + pub fn set(&mut self, new: T, class: impl OneOrMany) { + for class in class.into_iter() { + *self.get_mut(class) = new.clone(); + } + } +} + +impl PerDispatchClass { + /// Returns the total weight consumed by all extrinsics in the block. + pub fn total(&self) -> Weight { + let mut sum = Weight::zero(); + for class in DispatchClass::all() { + sum = sum.saturating_add(*self.get(*class)); + } + sum + } + + /// Add some weight of a specific dispatch class, saturating at the numeric bounds of `Weight`. + pub fn add(&mut self, weight: Weight, class: DispatchClass) { + let value = self.get_mut(class); + *value = value.saturating_add(weight); + } + + /// Try to add some weight of a specific dispatch class, returning Err(()) if overflow would + /// occur. + pub fn checked_add(&mut self, weight: Weight, class: DispatchClass) -> Result<(), ()> { + let value = self.get_mut(class); + *value = value.checked_add(&weight).ok_or(())?; + Ok(()) + } + + /// Subtract some weight of a specific dispatch class, saturating at the numeric bounds of + /// `Weight`. + pub fn sub(&mut self, weight: Weight, class: DispatchClass) { + let value = self.get_mut(class); + *value = value.saturating_sub(weight); + } +} + +/// Means of weighing some particular kind of data (`T`). +pub trait WeighData { + /// Weigh the data `T` given by `target`. When implementing this for a dispatchable, `T` will be + /// a tuple of all arguments given to the function (except origin). + fn weigh_data(&self, target: T) -> Weight; +} + +impl WeighData for Weight { + fn weigh_data(&self, _: T) -> Weight { + return *self + } +} + +impl PaysFee for (Weight, DispatchClass, Pays) { + fn pays_fee(&self, _: T) -> Pays { + self.2 + } +} + +impl WeighData for (Weight, DispatchClass) { + fn weigh_data(&self, args: T) -> Weight { + return self.0.weigh_data(args) + } +} + +impl WeighData for (Weight, DispatchClass, Pays) { + fn weigh_data(&self, args: T) -> Weight { + return self.0.weigh_data(args) + } +} + +impl ClassifyDispatch for (Weight, DispatchClass) { + fn classify_dispatch(&self, _: T) -> DispatchClass { + self.1 + } +} + +impl PaysFee for (Weight, DispatchClass) { + fn pays_fee(&self, _: T) -> Pays { + Pays::Yes + } +} + +impl WeighData for (Weight, Pays) { + fn weigh_data(&self, args: T) -> Weight { + return self.0.weigh_data(args) + } +} + +impl ClassifyDispatch for (Weight, Pays) { + fn classify_dispatch(&self, _: T) -> DispatchClass { + DispatchClass::Normal + } +} + +impl PaysFee for (Weight, Pays) { + fn pays_fee(&self, _: T) -> Pays { + self.1 + } +} + +impl From<(Option, Pays)> for PostDispatchInfo { + fn from(post_weight_info: (Option, Pays)) -> Self { + let (actual_weight, pays_fee) = post_weight_info; + Self { actual_weight, pays_fee } + } +} + +impl From> for PostDispatchInfo { + fn from(actual_weight: Option) -> Self { + Self { actual_weight, pays_fee: Default::default() } + } +} + +impl ClassifyDispatch for Weight { + fn classify_dispatch(&self, _: T) -> DispatchClass { + DispatchClass::Normal + } +} + +impl PaysFee for Weight { + fn pays_fee(&self, _: T) -> Pays { + Pays::Yes + } +} + +impl ClassifyDispatch for (Weight, DispatchClass, Pays) { + fn classify_dispatch(&self, _: T) -> DispatchClass { + self.1 + } +} + +// TODO: Eventually remove these + +impl From> for PostDispatchInfo { + fn from(maybe_actual_computation: Option) -> Self { + let actual_weight = match maybe_actual_computation { + Some(actual_computation) => Some(Weight::zero().set_ref_time(actual_computation)), + None => None, + }; + Self { actual_weight, pays_fee: Default::default() } + } +} + +impl From<(Option, Pays)> for PostDispatchInfo { + fn from(post_weight_info: (Option, Pays)) -> Self { + let (maybe_actual_time, pays_fee) = post_weight_info; + let actual_weight = match maybe_actual_time { + Some(actual_time) => Some(Weight::zero().set_ref_time(actual_time)), + None => None, + }; + Self { actual_weight, pays_fee } + } +} + +impl ClassifyDispatch for u64 { + fn classify_dispatch(&self, _: T) -> DispatchClass { + DispatchClass::Normal + } +} + +impl PaysFee for u64 { + fn pays_fee(&self, _: T) -> Pays { + Pays::Yes + } +} + +impl WeighData for u64 { + fn weigh_data(&self, _: T) -> Weight { + return Weight::zero().set_ref_time(*self) + } +} + +impl WeighData for (u64, DispatchClass, Pays) { + fn weigh_data(&self, args: T) -> Weight { + return self.0.weigh_data(args) + } +} + +impl ClassifyDispatch for (u64, DispatchClass, Pays) { + fn classify_dispatch(&self, _: T) -> DispatchClass { + self.1 + } +} + +impl PaysFee for (u64, DispatchClass, Pays) { + fn pays_fee(&self, _: T) -> Pays { + self.2 + } +} + +impl WeighData for (u64, DispatchClass) { + fn weigh_data(&self, args: T) -> Weight { + return self.0.weigh_data(args) + } +} + +impl ClassifyDispatch for (u64, DispatchClass) { + fn classify_dispatch(&self, _: T) -> DispatchClass { + self.1 + } +} + +impl PaysFee for (u64, DispatchClass) { + fn pays_fee(&self, _: T) -> Pays { + Pays::Yes + } +} + +impl WeighData for (u64, Pays) { + fn weigh_data(&self, args: T) -> Weight { + return self.0.weigh_data(args) + } +} + +impl ClassifyDispatch for (u64, Pays) { + fn classify_dispatch(&self, _: T) -> DispatchClass { + DispatchClass::Normal + } +} + +impl PaysFee for (u64, Pays) { + fn pays_fee(&self, _: T) -> Pays { + self.1 + } +} + +// END TODO + /// Declares a `Module` struct and a `Call` enum, which implements the dispatch logic. /// /// ## Declaration @@ -2629,13 +3178,14 @@ macro_rules! __check_reserved_fn_name { mod tests { use super::*; use crate::{ + dispatch::{DispatchClass, DispatchInfo, Pays}, metadata::*, traits::{ CrateVersion, Get, GetCallName, IntegrityTest, OnFinalize, OnIdle, OnInitialize, OnRuntimeUpgrade, PalletInfo, }, - weights::{DispatchClass, DispatchInfo, Pays, RuntimeDbWeight}, }; + use sp_weights::RuntimeDbWeight; pub trait Config: system::Config + Sized where @@ -2940,3 +3490,187 @@ mod tests { Call::::new_call_variant_aux_0(); } } + +#[cfg(test)] +// Do not complain about unused `dispatch` and `dispatch_aux`. +#[allow(dead_code)] +mod weight_tests { + use super::*; + use sp_core::{parameter_types, Get}; + use sp_weights::RuntimeDbWeight; + + pub trait Config: 'static { + type Origin; + type Balance; + type BlockNumber; + type DbWeight: Get; + type PalletInfo: crate::traits::PalletInfo; + } + + pub struct TraitImpl {} + + parameter_types! { + pub const DbWeight: RuntimeDbWeight = RuntimeDbWeight { + read: 100, + write: 1000, + }; + } + + impl Config for TraitImpl { + type Origin = u32; + type BlockNumber = u32; + type Balance = u32; + type DbWeight = DbWeight; + type PalletInfo = crate::tests::PanicPalletInfo; + } + + decl_module! { + pub struct Module for enum Call where origin: T::Origin, system=self { + // no arguments, fixed weight + #[weight = 1000] + fn f00(_origin) { unimplemented!(); } + + #[weight = (1000, DispatchClass::Mandatory)] + fn f01(_origin) { unimplemented!(); } + + #[weight = (1000, Pays::No)] + fn f02(_origin) { unimplemented!(); } + + #[weight = (1000, DispatchClass::Operational, Pays::No)] + fn f03(_origin) { unimplemented!(); } + + // weight = a x 10 + b + #[weight = ((_a * 10 + _eb * 1) as u64, DispatchClass::Normal, Pays::Yes)] + fn f11(_origin, _a: u32, _eb: u32) { unimplemented!(); } + + #[weight = (0, DispatchClass::Operational, Pays::Yes)] + fn f12(_origin, _a: u32, _eb: u32) { unimplemented!(); } + + #[weight = T::DbWeight::get().reads(3) + T::DbWeight::get().writes(2) + Weight::from_ref_time(10_000)] + fn f20(_origin) { unimplemented!(); } + + #[weight = T::DbWeight::get().reads_writes(6, 5) + Weight::from_ref_time(40_000)] + fn f21(_origin) { unimplemented!(); } + + } + } + + #[test] + fn weights_are_correct() { + // #[weight = 1000] + let info = Call::::f00 {}.get_dispatch_info(); + assert_eq!(info.weight, Weight::from_ref_time(1000)); + assert_eq!(info.class, DispatchClass::Normal); + assert_eq!(info.pays_fee, Pays::Yes); + + // #[weight = (1000, DispatchClass::Mandatory)] + let info = Call::::f01 {}.get_dispatch_info(); + assert_eq!(info.weight, Weight::from_ref_time(1000)); + assert_eq!(info.class, DispatchClass::Mandatory); + assert_eq!(info.pays_fee, Pays::Yes); + + // #[weight = (1000, Pays::No)] + let info = Call::::f02 {}.get_dispatch_info(); + assert_eq!(info.weight, Weight::from_ref_time(1000)); + assert_eq!(info.class, DispatchClass::Normal); + assert_eq!(info.pays_fee, Pays::No); + + // #[weight = (1000, DispatchClass::Operational, Pays::No)] + let info = Call::::f03 {}.get_dispatch_info(); + assert_eq!(info.weight, Weight::from_ref_time(1000)); + assert_eq!(info.class, DispatchClass::Operational); + assert_eq!(info.pays_fee, Pays::No); + + // #[weight = ((_a * 10 + _eb * 1) as Weight, DispatchClass::Normal, Pays::Yes)] + let info = Call::::f11 { _a: 13, _eb: 20 }.get_dispatch_info(); + assert_eq!(info.weight, Weight::from_ref_time(150)); // 13*10 + 20 + assert_eq!(info.class, DispatchClass::Normal); + assert_eq!(info.pays_fee, Pays::Yes); + + // #[weight = (0, DispatchClass::Operational, Pays::Yes)] + let info = Call::::f12 { _a: 10, _eb: 20 }.get_dispatch_info(); + assert_eq!(info.weight, Weight::from_ref_time(0)); + assert_eq!(info.class, DispatchClass::Operational); + assert_eq!(info.pays_fee, Pays::Yes); + + // #[weight = T::DbWeight::get().reads(3) + T::DbWeight::get().writes(2) + 10_000] + let info = Call::::f20 {}.get_dispatch_info(); + assert_eq!(info.weight, Weight::from_ref_time(12300)); // 100*3 + 1000*2 + 10_1000 + assert_eq!(info.class, DispatchClass::Normal); + assert_eq!(info.pays_fee, Pays::Yes); + + // #[weight = T::DbWeight::get().reads_writes(6, 5) + 40_000] + let info = Call::::f21 {}.get_dispatch_info(); + assert_eq!(info.weight, Weight::from_ref_time(45600)); // 100*6 + 1000*5 + 40_1000 + assert_eq!(info.class, DispatchClass::Normal); + assert_eq!(info.pays_fee, Pays::Yes); + } + + #[test] + fn extract_actual_weight_works() { + let pre = DispatchInfo { weight: Weight::from_ref_time(1000), ..Default::default() }; + assert_eq!(extract_actual_weight(&Ok(Some(7).into()), &pre), Weight::from_ref_time(7)); + assert_eq!( + extract_actual_weight(&Ok(Some(1000).into()), &pre), + Weight::from_ref_time(1000) + ); + assert_eq!( + extract_actual_weight( + &Err(DispatchError::BadOrigin.with_weight(Weight::from_ref_time(9))), + &pre + ), + Weight::from_ref_time(9) + ); + } + + #[test] + fn extract_actual_weight_caps_at_pre_weight() { + let pre = DispatchInfo { weight: Weight::from_ref_time(1000), ..Default::default() }; + assert_eq!( + extract_actual_weight(&Ok(Some(1250).into()), &pre), + Weight::from_ref_time(1000) + ); + assert_eq!( + extract_actual_weight( + &Err(DispatchError::BadOrigin.with_weight(Weight::from_ref_time(1300))), + &pre + ), + Weight::from_ref_time(1000), + ); + } + + #[test] + fn extract_actual_pays_fee_works() { + let pre = DispatchInfo { weight: Weight::from_ref_time(1000), ..Default::default() }; + assert_eq!(extract_actual_pays_fee(&Ok(Some(7).into()), &pre), Pays::Yes); + assert_eq!(extract_actual_pays_fee(&Ok(Some(1000).into()), &pre), Pays::Yes); + assert_eq!(extract_actual_pays_fee(&Ok((Some(1000), Pays::Yes).into()), &pre), Pays::Yes); + assert_eq!(extract_actual_pays_fee(&Ok((Some(1000), Pays::No).into()), &pre), Pays::No); + assert_eq!( + extract_actual_pays_fee( + &Err(DispatchError::BadOrigin.with_weight(Weight::from_ref_time(9))), + &pre + ), + Pays::Yes + ); + assert_eq!( + extract_actual_pays_fee( + &Err(DispatchErrorWithPostInfo { + post_info: PostDispatchInfo { actual_weight: None, pays_fee: Pays::No }, + error: DispatchError::BadOrigin, + }), + &pre + ), + Pays::No + ); + + let pre = DispatchInfo { + weight: Weight::from_ref_time(1000), + pays_fee: Pays::No, + ..Default::default() + }; + assert_eq!(extract_actual_pays_fee(&Ok(Some(7).into()), &pre), Pays::No); + assert_eq!(extract_actual_pays_fee(&Ok(Some(1000).into()), &pre), Pays::No); + assert_eq!(extract_actual_pays_fee(&Ok((Some(1000), Pays::Yes).into()), &pre), Pays::No); + } +} diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 315d856e5d0f8..8dcc51226923b 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -1376,7 +1376,10 @@ pub mod pallet_prelude { #[cfg(feature = "std")] pub use crate::traits::GenesisBuild; pub use crate::{ - dispatch::{DispatchError, DispatchResult, DispatchResultWithPostInfo, Parameter}, + dispatch::{ + DispatchClass, DispatchError, DispatchResult, DispatchResultWithPostInfo, Parameter, + Pays, + }, ensure, inherent::{InherentData, InherentIdentifier, ProvideInherent}, storage, @@ -1391,7 +1394,6 @@ pub mod pallet_prelude { ConstU32, EnsureOrigin, Get, GetDefault, GetStorageVersion, Hooks, IsType, PalletInfoAccess, StorageInfoTrait, StorageVersion, TypedGet, }, - weights::{DispatchClass, Pays, Weight}, Blake2_128, Blake2_128Concat, Blake2_256, CloneNoBound, DebugNoBound, EqNoBound, Identity, PartialEqNoBound, RuntimeDebug, RuntimeDebugNoBound, Twox128, Twox256, Twox64Concat, }; @@ -1407,6 +1409,7 @@ pub mod pallet_prelude { MAX_MODULE_ERROR_ENCODED_SIZE, }; pub use sp_std::marker::PhantomData; + pub use sp_weights::Weight; } /// `pallet` attribute macro allows to define a pallet to be used in `construct_runtime!`. diff --git a/frame/support/src/traits/misc.rs b/frame/support/src/traits/misc.rs index 59d5ec067baed..7fc4a6fb08a5a 100644 --- a/frame/support/src/traits/misc.rs +++ b/frame/support/src/traits/misc.rs @@ -711,13 +711,13 @@ pub trait EstimateCallFee { /// /// The dispatch info and the length is deduced from the call. The post info can optionally be /// provided. - fn estimate_call_fee(call: &Call, post_info: crate::weights::PostDispatchInfo) -> Balance; + fn estimate_call_fee(call: &Call, post_info: crate::dispatch::PostDispatchInfo) -> Balance; } // Useful for building mocks. #[cfg(feature = "std")] impl, const T: u32> EstimateCallFee for ConstU32 { - fn estimate_call_fee(_: &Call, _: crate::weights::PostDispatchInfo) -> Balance { + fn estimate_call_fee(_: &Call, _: crate::dispatch::PostDispatchInfo) -> Balance { T.into() } } diff --git a/frame/support/src/weights.rs b/frame/support/src/weights.rs index b4fd896e865dc..9ff49b97bf21f 100644 --- a/frame/support/src/weights.rs +++ b/frame/support/src/weights.rs @@ -15,102 +15,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! # Primitives for transaction weighting. -//! -//! Every dispatchable function is responsible for providing `#[weight = $x]` attribute. In this -//! snipped, `$x` can be any user provided struct that implements the following traits: -//! -//! - [`WeighData`]: the weight amount. -//! - [`ClassifyDispatch`]: class of the dispatch. -//! - [`PaysFee`]: whether this weight should be translated to fee and deducted upon dispatch. -//! -//! Substrate then bundles the output information of the three traits into [`DispatchInfo`] struct -//! and provides it by implementing the [`GetDispatchInfo`] for all `Call` both inner and outer call -//! types. -//! -//! Substrate provides two pre-defined ways to annotate weight: -//! -//! ### 1. Fixed values -//! -//! This can only be used when all 3 traits can be resolved statically. You have 3 degrees of -//! configuration: -//! -//! 1. Define only weight, **in which case `ClassifyDispatch` will be `Normal` and `PaysFee` will be -//! `Yes`**. -//! -//! ``` -//! # use frame_system::Config; -//! frame_support::decl_module! { -//! pub struct Module for enum Call where origin: T::Origin { -//! #[weight = 1000] -//! fn dispatching(origin) { unimplemented!() } -//! } -//! } -//! # fn main() {} -//! ``` -//! -//! 2.1 Define weight and class, **in which case `PaysFee` would be `Yes`**. -//! -//! ``` -//! # use frame_system::Config; -//! # use frame_support::weights::DispatchClass; -//! frame_support::decl_module! { -//! pub struct Module for enum Call where origin: T::Origin { -//! #[weight = (1000, DispatchClass::Operational)] -//! fn dispatching(origin) { unimplemented!() } -//! } -//! } -//! # fn main() {} -//! ``` -//! -//! 2.2 Define weight and `PaysFee`, **in which case `ClassifyDispatch` would be `Normal`**. -//! -//! ``` -//! # use frame_system::Config; -//! # use frame_support::weights::Pays; -//! frame_support::decl_module! { -//! pub struct Module for enum Call where origin: T::Origin { -//! #[weight = (1000, Pays::No)] -//! fn dispatching(origin) { unimplemented!() } -//! } -//! } -//! # fn main() {} -//! ``` -//! -//! 3. Define all 3 parameters. -//! -//! ``` -//! # use frame_system::Config; -//! # use frame_support::weights::{DispatchClass, Pays}; -//! frame_support::decl_module! { -//! pub struct Module for enum Call where origin: T::Origin { -//! #[weight = (1000, DispatchClass::Operational, Pays::No)] -//! fn dispatching(origin) { unimplemented!() } -//! } -//! } -//! # fn main() {} -//! ``` -//! -//! ### 2. Define weights as a function of input arguments. -//! -//! The arguments of the dispatch are available in the weight expressions as a borrowed value. -//! -//! ``` -//! # use frame_system::Config; -//! # use frame_support::weights::{DispatchClass, Pays}; -//! frame_support::decl_module! { -//! pub struct Module for enum Call where origin: T::Origin { -//! #[weight = ( -//! *a as u64 + *b, -//! DispatchClass::Operational, -//! if *a > 1000 { Pays::Yes } else { Pays::No } -//! )] -//! fn dispatching(origin, a: u32, b: u64) { unimplemented!() } -//! } -//! } -//! # fn main() {} -//! ``` -//! FRAME assumes a weight of `1_000_000_000_000` equals 1 second of compute on a standard machine. +//! Re-exports `sp-weights` public API, and contains benchmarked weight constants specific to +//! FRAME. //! //! Latest machine specification used to benchmark are: //! - Digital Ocean: ubuntu-s-2vcpu-4gb-ams3-01 @@ -123,41 +29,14 @@ mod block_weights; mod extrinsic_weights; mod paritydb_weights; mod rocksdb_weights; -mod weight_v2; - -use crate::{ - dispatch::{DispatchError, DispatchErrorWithPostInfo, DispatchResultWithPostInfo}, - traits::Get, -}; -use codec::{Decode, Encode, MaxEncodedLen}; -use scale_info::TypeInfo; -#[cfg(feature = "std")] -use serde::{Deserialize, Serialize}; -use smallvec::SmallVec; -use sp_arithmetic::{ - traits::{BaseArithmetic, Saturating, Unsigned}, - Perbill, -}; -use sp_runtime::{ - generic::{CheckedExtrinsic, UncheckedExtrinsic}, - traits::{SaturatedConversion, SignedExtension}, - RuntimeDebug, -}; - -/// Re-export priority as type -pub use sp_runtime::transaction_validity::TransactionPriority; -pub use weight_v2::*; +use crate::dispatch; +pub use sp_weights::*; /// These constants are specific to FRAME, and the current implementation of its various components. /// For example: FRAME System, FRAME Executive, our FRAME support libraries, etc... pub mod constants { - use super::Weight; - - pub const WEIGHT_PER_SECOND: Weight = Weight::from_ref_time(1_000_000_000_000); - pub const WEIGHT_PER_MILLIS: Weight = Weight::from_ref_time(1_000_000_000); - pub const WEIGHT_PER_MICROS: Weight = Weight::from_ref_time(1_000_000); - pub const WEIGHT_PER_NANOS: Weight = Weight::from_ref_time(1_000); + pub use sp_weights::constants::*; // Expose the Block and Extrinsic base weights. pub use super::{block_weights::BlockExecutionWeight, extrinsic_weights::ExtrinsicBaseWeight}; @@ -168,786 +47,69 @@ pub mod constants { }; } -/// Means of weighing some particular kind of data (`T`). -pub trait WeighData { - /// Weigh the data `T` given by `target`. When implementing this for a dispatchable, `T` will be - /// a tuple of all arguments given to the function (except origin). - fn weigh_data(&self, target: T) -> Weight; -} - -/// Means of classifying a dispatchable function. -pub trait ClassifyDispatch { - /// Classify the dispatch function based on input data `target` of type `T`. When implementing - /// this for a dispatchable, `T` will be a tuple of all arguments given to the function (except - /// origin). - fn classify_dispatch(&self, target: T) -> DispatchClass; -} - -/// Indicates if dispatch function should pay fees or not. -/// If set to `Pays::No`, the block resource limits are applied, yet no fee is deducted. -pub trait PaysFee { - fn pays_fee(&self, _target: T) -> Pays; -} - -/// Explicit enum to denote if a transaction pays fee or not. -#[derive(Clone, Copy, Eq, PartialEq, RuntimeDebug, Encode, Decode, TypeInfo)] -pub enum Pays { - /// Transactor will pay related fees. - Yes, - /// Transactor will NOT pay related fees. - No, -} - -impl Default for Pays { - fn default() -> Self { - Self::Yes - } -} - -impl From for PostDispatchInfo { - fn from(pays_fee: Pays) -> Self { - Self { actual_weight: None, pays_fee } - } -} - -/// A generalized group of dispatch types. -/// -/// NOTE whenever upgrading the enum make sure to also update -/// [DispatchClass::all] and [DispatchClass::non_mandatory] helper functions. -#[cfg_attr(feature = "std", derive(Serialize, Deserialize))] -#[cfg_attr(feature = "std", serde(rename_all = "camelCase"))] -#[derive(PartialEq, Eq, Clone, Copy, Encode, Decode, RuntimeDebug, TypeInfo)] -pub enum DispatchClass { - /// A normal dispatch. - Normal, - /// An operational dispatch. - Operational, - /// A mandatory dispatch. These kinds of dispatch are always included regardless of their - /// weight, therefore it is critical that they are separately validated to ensure that a - /// malicious validator cannot craft a valid but impossibly heavy block. Usually this just - /// means ensuring that the extrinsic can only be included once and that it is always very - /// light. - /// - /// Do *NOT* use it for extrinsics that can be heavy. - /// - /// The only real use case for this is inherent extrinsics that are required to execute in a - /// block for the block to be valid, and it solves the issue in the case that the block - /// initialization is sufficiently heavy to mean that those inherents do not fit into the - /// block. Essentially, we assume that in these exceptional circumstances, it is better to - /// allow an overweight block to be created than to not allow any block at all to be created. - Mandatory, -} - -impl Default for DispatchClass { - fn default() -> Self { - Self::Normal - } -} - -impl DispatchClass { - /// Returns an array containing all dispatch classes. - pub fn all() -> &'static [DispatchClass] { - &[DispatchClass::Normal, DispatchClass::Operational, DispatchClass::Mandatory] - } - - /// Returns an array of all dispatch classes except `Mandatory`. - pub fn non_mandatory() -> &'static [DispatchClass] { - &[DispatchClass::Normal, DispatchClass::Operational] - } -} - -/// A trait that represents one or many values of given type. -/// -/// Useful to accept as parameter type to let the caller pass either a single value directly -/// or an iterator. -pub trait OneOrMany { - /// The iterator type. - type Iter: Iterator; - /// Convert this item into an iterator. - fn into_iter(self) -> Self::Iter; -} - -impl OneOrMany for DispatchClass { - type Iter = sp_std::iter::Once; - fn into_iter(self) -> Self::Iter { - sp_std::iter::once(self) - } -} - -impl<'a> OneOrMany for &'a [DispatchClass] { - type Iter = sp_std::iter::Cloned>; - fn into_iter(self) -> Self::Iter { - self.iter().cloned() - } -} - -/// A bundle of static information collected from the `#[weight = $x]` attributes. -#[derive(Clone, Copy, Eq, PartialEq, Default, RuntimeDebug, Encode, Decode, TypeInfo)] -pub struct DispatchInfo { - /// Weight of this transaction. - pub weight: Weight, - /// Class of this transaction. - pub class: DispatchClass, - /// Does this transaction pay fees. - pub pays_fee: Pays, -} - -/// A `Dispatchable` function (aka transaction) that can carry some static information along with -/// it, using the `#[weight]` attribute. -pub trait GetDispatchInfo { - /// Return a `DispatchInfo`, containing relevant information of this dispatch. - /// - /// This is done independently of its encoded size. - fn get_dispatch_info(&self) -> DispatchInfo; -} - -impl GetDispatchInfo for () { - fn get_dispatch_info(&self) -> DispatchInfo { - DispatchInfo::default() - } -} - -/// Weight information that is only available post dispatch. -/// NOTE: This can only be used to reduce the weight or fee, not increase it. -#[derive(Clone, Copy, Eq, PartialEq, Default, RuntimeDebug, Encode, Decode, TypeInfo)] -pub struct PostDispatchInfo { - /// Actual weight consumed by a call or `None` which stands for the worst case static weight. - pub actual_weight: Option, - /// Whether this transaction should pay fees when all is said and done. - pub pays_fee: Pays, -} - -impl PostDispatchInfo { - /// Calculate how much (if any) weight was not used by the `Dispatchable`. - pub fn calc_unspent(&self, info: &DispatchInfo) -> Weight { - info.weight - self.calc_actual_weight(info) - } - - /// Calculate how much weight was actually spent by the `Dispatchable`. - pub fn calc_actual_weight(&self, info: &DispatchInfo) -> Weight { - if let Some(actual_weight) = self.actual_weight { - actual_weight.min(info.weight) - } else { - info.weight - } - } - - /// Determine if user should actually pay fees at the end of the dispatch. - pub fn pays_fee(&self, info: &DispatchInfo) -> Pays { - // If they originally were not paying fees, or the post dispatch info - // says they should not pay fees, then they don't pay fees. - // This is because the pre dispatch information must contain the - // worst case for weight and fees paid. - if info.pays_fee == Pays::No || self.pays_fee == Pays::No { - Pays::No - } else { - // Otherwise they pay. - Pays::Yes - } - } -} - -/// Extract the actual weight from a dispatch result if any or fall back to the default weight. -pub fn extract_actual_weight(result: &DispatchResultWithPostInfo, info: &DispatchInfo) -> Weight { - match result { - Ok(post_info) => post_info, - Err(err) => &err.post_info, - } - .calc_actual_weight(info) -} - -/// Extract the actual pays_fee from a dispatch result if any or fall back to the default weight. -pub fn extract_actual_pays_fee(result: &DispatchResultWithPostInfo, info: &DispatchInfo) -> Pays { - match result { - Ok(post_info) => post_info, - Err(err) => &err.post_info, - } - .pays_fee(info) -} - -impl From<()> for PostDispatchInfo { - fn from(_: ()) -> Self { - Self { actual_weight: None, pays_fee: Default::default() } - } -} - -impl sp_runtime::traits::Printable for PostDispatchInfo { - fn print(&self) { - "actual_weight=".print(); - match self.actual_weight { - Some(weight) => weight.print(), - None => "max-weight".print(), - }; - "pays_fee=".print(); - match self.pays_fee { - Pays::Yes => "Yes".print(), - Pays::No => "No".print(), - } - } -} - -/// Allows easy conversion from `DispatchError` to `DispatchErrorWithPostInfo` for dispatchables -/// that want to return a custom a posterior weight on error. -pub trait WithPostDispatchInfo { - /// Call this on your modules custom errors type in order to return a custom weight on error. - /// - /// # Example - /// - /// ```ignore - /// let who = ensure_signed(origin).map_err(|e| e.with_weight(Weight::from_ref_time(100)))?; - /// ensure!(who == me, Error::::NotMe.with_weight(200_000)); - /// ``` - fn with_weight(self, actual_weight: Weight) -> DispatchErrorWithPostInfo; -} - -impl WithPostDispatchInfo for T -where - T: Into, -{ - fn with_weight(self, actual_weight: Weight) -> DispatchErrorWithPostInfo { - DispatchErrorWithPostInfo { - post_info: PostDispatchInfo { - actual_weight: Some(actual_weight), - pays_fee: Default::default(), - }, - error: self.into(), - } - } -} - -/// Implementation for unchecked extrinsic. -impl GetDispatchInfo - for UncheckedExtrinsic -where - Call: GetDispatchInfo, - Extra: SignedExtension, -{ - fn get_dispatch_info(&self) -> DispatchInfo { - self.function.get_dispatch_info() - } -} - -/// Implementation for checked extrinsic. -impl GetDispatchInfo for CheckedExtrinsic -where - Call: GetDispatchInfo, -{ - fn get_dispatch_info(&self) -> DispatchInfo { - self.function.get_dispatch_info() - } -} - -/// Implementation for test extrinsic. -#[cfg(feature = "std")] -impl GetDispatchInfo for sp_runtime::testing::TestXt { - fn get_dispatch_info(&self) -> DispatchInfo { - // for testing: weight == size. - DispatchInfo { - weight: Weight::from_ref_time(self.encode().len() as _), - pays_fee: Pays::Yes, - ..Default::default() - } - } -} - -/// The weight of database operations that the runtime can invoke. -/// -/// NOTE: This is currently only measured in computational time, and will probably -/// be updated all together once proof size is accounted for. -#[derive(Clone, Copy, Eq, PartialEq, Default, RuntimeDebug, Encode, Decode, TypeInfo)] -pub struct RuntimeDbWeight { - pub read: u64, - pub write: u64, -} - -impl RuntimeDbWeight { - pub fn reads(self, r: u64) -> Weight { - Weight::from_ref_time(self.read.saturating_mul(r)) - } - - pub fn writes(self, w: u64) -> Weight { - Weight::from_ref_time(self.write.saturating_mul(w)) - } - - pub fn reads_writes(self, r: u64, w: u64) -> Weight { - let read_weight = self.read.saturating_mul(r); - let write_weight = self.write.saturating_mul(w); - Weight::from_ref_time(read_weight.saturating_add(write_weight)) - } -} - -/// One coefficient and its position in the `WeightToFee`. -/// -/// One term of polynomial is calculated as: -/// -/// ```ignore -/// coeff_integer * x^(degree) + coeff_frac * x^(degree) -/// ``` -/// -/// The `negative` value encodes whether the term is added or substracted from the -/// overall polynomial result. -#[derive(Clone, Encode, Decode, TypeInfo)] -pub struct WeightToFeeCoefficient { - /// The integral part of the coefficient. - pub coeff_integer: Balance, - /// The fractional part of the coefficient. - pub coeff_frac: Perbill, - /// True iff the coefficient should be interpreted as negative. - pub negative: bool, - /// Degree/exponent of the term. - pub degree: u8, -} - -/// A list of coefficients that represent one polynomial. -pub type WeightToFeeCoefficients = SmallVec<[WeightToFeeCoefficient; 4]>; - -/// A trait that describes the weight to fee calculation. -pub trait WeightToFee { - /// The type that is returned as result from calculation. - type Balance: BaseArithmetic + From + Copy + Unsigned; - - /// Calculates the fee from the passed `weight`. - fn weight_to_fee(weight: &Weight) -> Self::Balance; -} - -/// A trait that describes the weight to fee calculation as polynomial. -/// -/// An implementor should only implement the `polynomial` function. -pub trait WeightToFeePolynomial { - /// The type that is returned as result from polynomial evaluation. - type Balance: BaseArithmetic + From + Copy + Unsigned; - - /// Returns a polynomial that describes the weight to fee conversion. - /// - /// This is the only function that should be manually implemented. Please note - /// that all calculation is done in the probably unsigned `Balance` type. This means - /// that the order of coefficients is important as putting the negative coefficients - /// first will most likely saturate the result to zero mid evaluation. - fn polynomial() -> WeightToFeeCoefficients; -} - -impl WeightToFee for T -where - T: WeightToFeePolynomial, -{ - type Balance = ::Balance; - - /// Calculates the fee from the passed `weight` according to the `polynomial`. - /// - /// This should not be overridden in most circumstances. Calculation is done in the - /// `Balance` type and never overflows. All evaluation is saturating. - fn weight_to_fee(weight: &Weight) -> Self::Balance { - Self::polynomial() - .iter() - .fold(Self::Balance::saturated_from(0u32), |mut acc, args| { - let w = Self::Balance::saturated_from(weight.ref_time()) - .saturating_pow(args.degree.into()); - - // The sum could get negative. Therefore we only sum with the accumulator. - // The Perbill Mul implementation is non overflowing. - let frac = args.coeff_frac * w; - let integer = args.coeff_integer.saturating_mul(w); - - if args.negative { - acc = acc.saturating_sub(frac); - acc = acc.saturating_sub(integer); - } else { - acc = acc.saturating_add(frac); - acc = acc.saturating_add(integer); - } - - acc - }) - } -} - -/// Implementor of `WeightToFee` that maps one unit of weight to one unit of fee. -pub struct IdentityFee(sp_std::marker::PhantomData); - -impl WeightToFee for IdentityFee -where - T: BaseArithmetic + From + Copy + Unsigned, -{ - type Balance = T; - - fn weight_to_fee(weight: &Weight) -> Self::Balance { - Self::Balance::saturated_from(weight.ref_time()) - } -} - -/// Implementor of [`WeightToFee`] that uses a constant multiplier. -/// # Example -/// -/// ``` -/// # use frame_support::traits::ConstU128; -/// # use frame_support::weights::ConstantMultiplier; -/// // Results in a multiplier of 10 for each unit of weight (or length) -/// type LengthToFee = ConstantMultiplier::>; -/// ``` -pub struct ConstantMultiplier(sp_std::marker::PhantomData<(T, M)>); - -impl WeightToFee for ConstantMultiplier -where - T: BaseArithmetic + From + Copy + Unsigned, - M: Get, -{ - type Balance = T; - - fn weight_to_fee(weight: &Weight) -> Self::Balance { - Self::Balance::saturated_from(weight.ref_time()).saturating_mul(M::get()) - } -} - -/// A struct holding value for each `DispatchClass`. -#[derive(Clone, Eq, PartialEq, Default, RuntimeDebug, Encode, Decode, TypeInfo, MaxEncodedLen)] -pub struct PerDispatchClass { - /// Value for `Normal` extrinsics. - normal: T, - /// Value for `Operational` extrinsics. - operational: T, - /// Value for `Mandatory` extrinsics. - mandatory: T, -} - -impl PerDispatchClass { - /// Create new `PerDispatchClass` with the same value for every class. - pub fn new(val: impl Fn(DispatchClass) -> T) -> Self { - Self { - normal: val(DispatchClass::Normal), - operational: val(DispatchClass::Operational), - mandatory: val(DispatchClass::Mandatory), - } - } - - /// Get a mutable reference to current value of given class. - pub fn get_mut(&mut self, class: DispatchClass) -> &mut T { - match class { - DispatchClass::Operational => &mut self.operational, - DispatchClass::Normal => &mut self.normal, - DispatchClass::Mandatory => &mut self.mandatory, - } - } - - /// Get current value for given class. - pub fn get(&self, class: DispatchClass) -> &T { - match class { - DispatchClass::Normal => &self.normal, - DispatchClass::Operational => &self.operational, - DispatchClass::Mandatory => &self.mandatory, - } - } -} - -impl PerDispatchClass { - /// Set the value of given class. - pub fn set(&mut self, new: T, class: impl OneOrMany) { - for class in class.into_iter() { - *self.get_mut(class) = new.clone(); - } - } -} - -impl PerDispatchClass { - /// Returns the total weight consumed by all extrinsics in the block. - pub fn total(&self) -> Weight { - let mut sum = Weight::zero(); - for class in DispatchClass::all() { - sum = sum.saturating_add(*self.get(*class)); - } - sum - } - - /// Add some weight of a specific dispatch class, saturating at the numeric bounds of `Weight`. - pub fn add(&mut self, weight: Weight, class: DispatchClass) { - let value = self.get_mut(class); - *value = value.saturating_add(weight); - } - - /// Try to add some weight of a specific dispatch class, returning Err(()) if overflow would - /// occur. - pub fn checked_add(&mut self, weight: Weight, class: DispatchClass) -> Result<(), ()> { - let value = self.get_mut(class); - *value = value.checked_add(&weight).ok_or(())?; - Ok(()) - } - - /// Subtract some weight of a specific dispatch class, saturating at the numeric bounds of - /// `Weight`. - pub fn sub(&mut self, weight: Weight, class: DispatchClass) { - let value = self.get_mut(class); - *value = value.saturating_sub(weight); - } -} - -#[cfg(test)] -#[allow(dead_code)] -mod tests { - use super::*; - use crate::{decl_module, parameter_types, traits::Get}; - use smallvec::smallvec; - - pub trait Config: 'static { - type Origin; - type Balance; - type BlockNumber; - type DbWeight: Get; - type PalletInfo: crate::traits::PalletInfo; - } - - pub struct TraitImpl {} - - parameter_types! { - pub const DbWeight: RuntimeDbWeight = RuntimeDbWeight { - read: 100, - write: 1000, - }; - } - - impl Config for TraitImpl { - type Origin = u32; - type BlockNumber = u32; - type Balance = u32; - type DbWeight = DbWeight; - type PalletInfo = crate::tests::PanicPalletInfo; - } - - decl_module! { - pub struct Module for enum Call where origin: T::Origin, system=self { - // no arguments, fixed weight - #[weight = 1000] - fn f00(_origin) { unimplemented!(); } - - #[weight = (1000, DispatchClass::Mandatory)] - fn f01(_origin) { unimplemented!(); } - - #[weight = (1000, Pays::No)] - fn f02(_origin) { unimplemented!(); } - - #[weight = (1000, DispatchClass::Operational, Pays::No)] - fn f03(_origin) { unimplemented!(); } - - // weight = a x 10 + b - #[weight = ((_a * 10 + _eb * 1) as u64, DispatchClass::Normal, Pays::Yes)] - fn f11(_origin, _a: u32, _eb: u32) { unimplemented!(); } - - #[weight = (0, DispatchClass::Operational, Pays::Yes)] - fn f12(_origin, _a: u32, _eb: u32) { unimplemented!(); } - - #[weight = T::DbWeight::get().reads(3) + T::DbWeight::get().writes(2) + Weight::from_ref_time(10_000)] - fn f20(_origin) { unimplemented!(); } - - #[weight = T::DbWeight::get().reads_writes(6, 5) + Weight::from_ref_time(40_000)] - fn f21(_origin) { unimplemented!(); } - - } - } - - #[test] - fn weights_are_correct() { - // #[weight = 1000] - let info = Call::::f00 {}.get_dispatch_info(); - assert_eq!(info.weight, Weight::from_ref_time(1000)); - assert_eq!(info.class, DispatchClass::Normal); - assert_eq!(info.pays_fee, Pays::Yes); - - // #[weight = (1000, DispatchClass::Mandatory)] - let info = Call::::f01 {}.get_dispatch_info(); - assert_eq!(info.weight, Weight::from_ref_time(1000)); - assert_eq!(info.class, DispatchClass::Mandatory); - assert_eq!(info.pays_fee, Pays::Yes); - - // #[weight = (1000, Pays::No)] - let info = Call::::f02 {}.get_dispatch_info(); - assert_eq!(info.weight, Weight::from_ref_time(1000)); - assert_eq!(info.class, DispatchClass::Normal); - assert_eq!(info.pays_fee, Pays::No); - - // #[weight = (1000, DispatchClass::Operational, Pays::No)] - let info = Call::::f03 {}.get_dispatch_info(); - assert_eq!(info.weight, Weight::from_ref_time(1000)); - assert_eq!(info.class, DispatchClass::Operational); - assert_eq!(info.pays_fee, Pays::No); - - // #[weight = ((_a * 10 + _eb * 1) as Weight, DispatchClass::Normal, Pays::Yes)] - let info = Call::::f11 { _a: 13, _eb: 20 }.get_dispatch_info(); - assert_eq!(info.weight, Weight::from_ref_time(150)); // 13*10 + 20 - assert_eq!(info.class, DispatchClass::Normal); - assert_eq!(info.pays_fee, Pays::Yes); - - // #[weight = (0, DispatchClass::Operational, Pays::Yes)] - let info = Call::::f12 { _a: 10, _eb: 20 }.get_dispatch_info(); - assert_eq!(info.weight, Weight::from_ref_time(0)); - assert_eq!(info.class, DispatchClass::Operational); - assert_eq!(info.pays_fee, Pays::Yes); - - // #[weight = T::DbWeight::get().reads(3) + T::DbWeight::get().writes(2) + 10_000] - let info = Call::::f20 {}.get_dispatch_info(); - assert_eq!(info.weight, Weight::from_ref_time(12300)); // 100*3 + 1000*2 + 10_1000 - assert_eq!(info.class, DispatchClass::Normal); - assert_eq!(info.pays_fee, Pays::Yes); - - // #[weight = T::DbWeight::get().reads_writes(6, 5) + 40_000] - let info = Call::::f21 {}.get_dispatch_info(); - assert_eq!(info.weight, Weight::from_ref_time(45600)); // 100*6 + 1000*5 + 40_1000 - assert_eq!(info.class, DispatchClass::Normal); - assert_eq!(info.pays_fee, Pays::Yes); - } - - #[test] - fn extract_actual_weight_works() { - let pre = DispatchInfo { weight: Weight::from_ref_time(1000), ..Default::default() }; - assert_eq!(extract_actual_weight(&Ok(Some(7).into()), &pre), Weight::from_ref_time(7)); - assert_eq!( - extract_actual_weight(&Ok(Some(1000).into()), &pre), - Weight::from_ref_time(1000) - ); - assert_eq!( - extract_actual_weight( - &Err(DispatchError::BadOrigin.with_weight(Weight::from_ref_time(9))), - &pre - ), - Weight::from_ref_time(9) - ); - } - - #[test] - fn extract_actual_weight_caps_at_pre_weight() { - let pre = DispatchInfo { weight: Weight::from_ref_time(1000), ..Default::default() }; - assert_eq!( - extract_actual_weight(&Ok(Some(1250).into()), &pre), - Weight::from_ref_time(1000) - ); - assert_eq!( - extract_actual_weight( - &Err(DispatchError::BadOrigin.with_weight(Weight::from_ref_time(1300))), - &pre - ), - Weight::from_ref_time(1000), - ); - } - - #[test] - fn extract_actual_pays_fee_works() { - let pre = DispatchInfo { weight: Weight::from_ref_time(1000), ..Default::default() }; - assert_eq!(extract_actual_pays_fee(&Ok(Some(7).into()), &pre), Pays::Yes); - assert_eq!(extract_actual_pays_fee(&Ok(Some(1000).into()), &pre), Pays::Yes); - assert_eq!(extract_actual_pays_fee(&Ok((Some(1000), Pays::Yes).into()), &pre), Pays::Yes); - assert_eq!(extract_actual_pays_fee(&Ok((Some(1000), Pays::No).into()), &pre), Pays::No); - assert_eq!( - extract_actual_pays_fee( - &Err(DispatchError::BadOrigin.with_weight(Weight::from_ref_time(9))), - &pre - ), - Pays::Yes - ); - assert_eq!( - extract_actual_pays_fee( - &Err(DispatchErrorWithPostInfo { - post_info: PostDispatchInfo { actual_weight: None, pays_fee: Pays::No }, - error: DispatchError::BadOrigin, - }), - &pre - ), - Pays::No - ); - - let pre = DispatchInfo { - weight: Weight::from_ref_time(1000), - pays_fee: Pays::No, - ..Default::default() - }; - assert_eq!(extract_actual_pays_fee(&Ok(Some(7).into()), &pre), Pays::No); - assert_eq!(extract_actual_pays_fee(&Ok(Some(1000).into()), &pre), Pays::No); - assert_eq!(extract_actual_pays_fee(&Ok((Some(1000), Pays::Yes).into()), &pre), Pays::No); - } - - type Balance = u64; - - // 0.5x^3 + 2.333x^2 + 7x - 10_000 - struct Poly; - impl WeightToFeePolynomial for Poly { - type Balance = Balance; - - fn polynomial() -> WeightToFeeCoefficients { - smallvec![ - WeightToFeeCoefficient { - coeff_integer: 0, - coeff_frac: Perbill::from_float(0.5), - negative: false, - degree: 3 - }, - WeightToFeeCoefficient { - coeff_integer: 2, - coeff_frac: Perbill::from_rational(1u32, 3u32), - negative: false, - degree: 2 - }, - WeightToFeeCoefficient { - coeff_integer: 7, - coeff_frac: Perbill::zero(), - negative: false, - degree: 1 - }, - WeightToFeeCoefficient { - coeff_integer: 10_000, - coeff_frac: Perbill::zero(), - negative: true, - degree: 0 - }, - ] - } - } - - #[test] - fn polynomial_works() { - // 100^3/2=500000 100^2*(2+1/3)=23333 700 -10000 - assert_eq!(Poly::weight_to_fee(&Weight::from_ref_time(100)), 514033); - // 10123^3/2=518677865433 10123^2*(2+1/3)=239108634 70861 -10000 - assert_eq!(Poly::weight_to_fee(&Weight::from_ref_time(10_123)), 518917034928); - } - - #[test] - fn polynomial_does_not_underflow() { - assert_eq!(Poly::weight_to_fee(&Weight::zero()), 0); - assert_eq!(Poly::weight_to_fee(&Weight::from_ref_time(10)), 0); - } - - #[test] - fn polynomial_does_not_overflow() { - assert_eq!(Poly::weight_to_fee(&Weight::MAX), Balance::max_value() - 10_000); - } - - #[test] - fn identity_fee_works() { - assert_eq!(IdentityFee::::weight_to_fee(&Weight::zero()), 0); - assert_eq!(IdentityFee::::weight_to_fee(&Weight::from_ref_time(50)), 50); - assert_eq!(IdentityFee::::weight_to_fee(&Weight::MAX), Balance::max_value()); - } - - #[test] - fn constant_fee_works() { - use crate::traits::ConstU128; - assert_eq!( - ConstantMultiplier::>::weight_to_fee(&Weight::zero()), - 0 - ); - assert_eq!( - ConstantMultiplier::>::weight_to_fee(&Weight::from_ref_time( - 50 - )), - 500 - ); - assert_eq!( - ConstantMultiplier::>::weight_to_fee(&Weight::from_ref_time( - 16 - )), - 16384 - ); - assert_eq!( - ConstantMultiplier::>::weight_to_fee( - &Weight::from_ref_time(2) - ), - u128::MAX - ); +#[deprecated = "Function has moved to `frame_support::dispatch`"] +pub fn extract_actual_pays_fee( + res: &dispatch::DispatchResultWithPostInfo, + info: &dispatch::DispatchInfo, +) -> dispatch::Pays { + dispatch::extract_actual_pays_fee(res, info) +} +#[deprecated = "Function has moved to `frame_support::dispatch`"] +pub fn extract_actual_weight( + res: &dispatch::DispatchResultWithPostInfo, + info: &dispatch::DispatchInfo, +) -> Weight { + dispatch::extract_actual_weight(res, info) +} +#[deprecated = "Trait has moved to `frame_support::dispatch`"] +pub trait ClassifyDispatch: dispatch::ClassifyDispatch { + fn classify_dispatch(&self, target: T) -> dispatch::DispatchClass { + >::classify_dispatch(self, target) + } +} +#[deprecated = "Enum has moved to `frame_support::dispatch`"] +pub type DispatchClass = dispatch::DispatchClass; +#[deprecated = "Struct has moved to `frame_support::dispatch`"] +pub type DispatchInfo = dispatch::DispatchInfo; +#[deprecated = "Trait has moved to `frame_support::dispatch`"] +pub trait GetDispatchInfo: dispatch::GetDispatchInfo { + fn get_dispatch_info(&self) -> dispatch::DispatchInfo { + ::get_dispatch_info(self) + } +} +#[deprecated = "Trait has moved to `frame_support::dispatch`"] +pub trait OneOrMany: dispatch::OneOrMany { + fn into_iter(self) -> Self::Iter + where + Self: Sized, + { + >::into_iter(self) + } +} +#[deprecated = "Enum has moved to `frame_support::dispatch`"] +pub type Pays = dispatch::Pays; +#[deprecated = "Trait has moved to `frame_support::dispatch`"] +pub trait PaysFee: dispatch::PaysFee { + fn pays_fee(&self, target: T) -> dispatch::Pays { + >::pays_fee(self, target) + } +} +#[deprecated = "Struct has moved to `frame_support::dispatch`"] +pub type PerDispatchClass = dispatch::PerDispatchClass; +#[deprecated = "Struct has moved to `frame_support::dispatch`"] +pub type PostDispatchInfo = dispatch::PostDispatchInfo; +#[deprecated = "Trait has moved to `frame_support::dispatch`"] +pub trait WeighData: dispatch::WeighData { + fn weigh_data(&self, target: T) -> Weight { + >::weigh_data(self, target) + } +} +#[deprecated = "Trait has moved to `frame_support::dispatch`"] +pub trait WithPostDispatchInfo: dispatch::WithPostDispatchInfo { + fn with_weight(self, actual_weight: Weight) -> dispatch::DispatchErrorWithPostInfo + where + Self: Sized, + { + ::with_weight(self, actual_weight) } } diff --git a/frame/support/src/weights/block_weights.rs b/frame/support/src/weights/block_weights.rs index 57978f60a4ecb..240b80460aa09 100644 --- a/frame/support/src/weights/block_weights.rs +++ b/frame/support/src/weights/block_weights.rs @@ -34,10 +34,8 @@ // --warmup=10 // --repeat=100 -use frame_support::{ - parameter_types, - weights::{constants::WEIGHT_PER_NANOS, Weight}, -}; +use sp_core::parameter_types; +use sp_weights::{constants::WEIGHT_PER_NANOS, Weight}; parameter_types! { /// Time to execute an empty block. @@ -58,7 +56,7 @@ parameter_types! { #[cfg(test)] mod test_weights { - use frame_support::weights::constants; + use sp_weights::constants; /// Checks that the weight exists and is sane. // NOTE: If this test fails but you are sure that the generated values are fine, diff --git a/frame/support/src/weights/extrinsic_weights.rs b/frame/support/src/weights/extrinsic_weights.rs index 54f70a063cdec..913dbc4e91fc1 100644 --- a/frame/support/src/weights/extrinsic_weights.rs +++ b/frame/support/src/weights/extrinsic_weights.rs @@ -34,10 +34,8 @@ // --warmup=10 // --repeat=100 -use frame_support::{ - parameter_types, - weights::{constants::WEIGHT_PER_NANOS, Weight}, -}; +use sp_core::parameter_types; +use sp_weights::{constants::WEIGHT_PER_NANOS, Weight}; parameter_types! { /// Time to execute a NO-OP extrinsic, for example `System::remark`. @@ -58,7 +56,7 @@ parameter_types! { #[cfg(test)] mod test_weights { - use frame_support::weights::constants; + use sp_weights::constants; /// Checks that the weight exists and is sane. // NOTE: If this test fails but you are sure that the generated values are fine, diff --git a/frame/support/src/weights/paritydb_weights.rs b/frame/support/src/weights/paritydb_weights.rs index 9f6ba82dbbff6..344e6cf0ddb6e 100644 --- a/frame/support/src/weights/paritydb_weights.rs +++ b/frame/support/src/weights/paritydb_weights.rs @@ -16,10 +16,9 @@ // limitations under the License. pub mod constants { - use frame_support::{ - parameter_types, - weights::{constants, RuntimeDbWeight}, - }; + use frame_support::weights::constants; + use sp_core::parameter_types; + use sp_weights::RuntimeDbWeight; parameter_types! { /// ParityDB can be enabled with a feature flag, but is still experimental. These weights @@ -33,7 +32,7 @@ pub mod constants { #[cfg(test)] mod test_db_weights { use super::constants::ParityDbWeight as W; - use frame_support::weights::constants; + use sp_weights::constants; /// Checks that all weights exist and have sane values. // NOTE: If this test fails but you are sure that the generated values are fine, diff --git a/frame/support/src/weights/rocksdb_weights.rs b/frame/support/src/weights/rocksdb_weights.rs index cab983de0ad0a..4dec2d8c877ea 100644 --- a/frame/support/src/weights/rocksdb_weights.rs +++ b/frame/support/src/weights/rocksdb_weights.rs @@ -16,10 +16,9 @@ // limitations under the License. pub mod constants { - use frame_support::{ - parameter_types, - weights::{constants, RuntimeDbWeight}, - }; + use frame_support::weights::constants; + use sp_core::parameter_types; + use sp_weights::RuntimeDbWeight; parameter_types! { /// By default, Substrate uses RocksDB, so this will be the weight used throughout @@ -33,7 +32,7 @@ pub mod constants { #[cfg(test)] mod test_db_weights { use super::constants::RocksDbWeight as W; - use frame_support::weights::constants; + use sp_weights::constants; /// Checks that all weights exist and have sane values. // NOTE: If this test fails but you are sure that the generated values are fine, diff --git a/frame/support/test/tests/construct_runtime.rs b/frame/support/test/tests/construct_runtime.rs index 4f3c783252e00..4306314b4c005 100644 --- a/frame/support/test/tests/construct_runtime.rs +++ b/frame/support/test/tests/construct_runtime.rs @@ -200,7 +200,7 @@ pub mod module3 { } #[weight = 3] fn aux_4(_origin) -> frame_support::dispatch::DispatchResult { unreachable!() } - #[weight = (5, frame_support::weights::DispatchClass::Operational)] + #[weight = (5, frame_support::dispatch::DispatchClass::Operational)] fn operational(_origin) { unreachable!() } } } @@ -504,8 +504,8 @@ fn call_encode_is_correct_and_decode_works() { #[test] fn call_weight_should_attach_to_call_enum() { use frame_support::{ - dispatch::{DispatchInfo, GetDispatchInfo}, - weights::{DispatchClass, Pays, Weight}, + dispatch::{DispatchClass, DispatchInfo, GetDispatchInfo, Pays}, + weights::Weight, }; // operational. assert_eq!( diff --git a/frame/support/test/tests/origin.rs b/frame/support/test/tests/origin.rs index 53124059e6353..620f64ec3cfc2 100644 --- a/frame/support/test/tests/origin.rs +++ b/frame/support/test/tests/origin.rs @@ -100,7 +100,7 @@ pub mod module { } #[weight = 3] fn aux_4(_origin) -> frame_support::dispatch::DispatchResult { unreachable!() } - #[weight = (5, frame_support::weights::DispatchClass::Operational)] + #[weight = (5, frame_support::dispatch::DispatchClass::Operational)] fn operational(_origin) { unreachable!() } } } diff --git a/frame/support/test/tests/pallet.rs b/frame/support/test/tests/pallet.rs index 420617a9e4f24..4304aa1533a8b 100644 --- a/frame/support/test/tests/pallet.rs +++ b/frame/support/test/tests/pallet.rs @@ -16,14 +16,16 @@ // limitations under the License. use frame_support::{ - dispatch::{Parameter, UnfilteredDispatchable}, + dispatch::{ + DispatchClass, DispatchInfo, GetDispatchInfo, Parameter, Pays, UnfilteredDispatchable, + }, pallet_prelude::ValueQuery, storage::unhashed, traits::{ ConstU32, GetCallName, GetStorageVersion, OnFinalize, OnGenesis, OnInitialize, OnRuntimeUpgrade, PalletError, PalletInfoAccess, StorageVersion, }, - weights::{DispatchClass, DispatchInfo, GetDispatchInfo, Pays, RuntimeDbWeight, Weight}, + weights::{RuntimeDbWeight, Weight}, }; use scale_info::{meta_type, TypeInfo}; use sp_io::{ diff --git a/frame/support/test/tests/pallet_instance.rs b/frame/support/test/tests/pallet_instance.rs index f01ce17d71bf9..3bbbc559ff9b8 100644 --- a/frame/support/test/tests/pallet_instance.rs +++ b/frame/support/test/tests/pallet_instance.rs @@ -16,11 +16,10 @@ // limitations under the License. use frame_support::{ - dispatch::UnfilteredDispatchable, + dispatch::{DispatchClass, DispatchInfo, GetDispatchInfo, Pays, UnfilteredDispatchable}, pallet_prelude::ValueQuery, storage::unhashed, traits::{ConstU32, GetCallName, OnFinalize, OnGenesis, OnInitialize, OnRuntimeUpgrade}, - weights::{DispatchClass, DispatchInfo, GetDispatchInfo, Pays}, }; use sp_io::{ hashing::{blake2_128, twox_128, twox_64}, diff --git a/frame/system/Cargo.toml b/frame/system/Cargo.toml index 3429c6546c7fd..dd3a5d606bad5 100644 --- a/frame/system/Cargo.toml +++ b/frame/system/Cargo.toml @@ -23,6 +23,7 @@ sp-io = { version = "6.0.0", default-features = false, path = "../../primitives/ sp-runtime = { version = "6.0.0", default-features = false, path = "../../primitives/runtime" } sp-std = { version = "4.0.0", default-features = false, path = "../../primitives/std" } sp-version = { version = "5.0.0", default-features = false, path = "../../primitives/version" } +sp-weights = { version = "4.0.0", default-features = false, path = "../../primitives/weights" } [dev-dependencies] criterion = "0.3.3" @@ -42,6 +43,7 @@ std = [ "sp-runtime/std", "sp-std/std", "sp-version/std", + "sp-weights/std", ] runtime-benchmarks = [ "frame-support/runtime-benchmarks", diff --git a/frame/system/benchmarking/src/lib.rs b/frame/system/benchmarking/src/lib.rs index 367e6c73c4134..dbe38da5775a7 100644 --- a/frame/system/benchmarking/src/lib.rs +++ b/frame/system/benchmarking/src/lib.rs @@ -21,7 +21,7 @@ use codec::Encode; use frame_benchmarking::{benchmarks, whitelisted_caller}; -use frame_support::{storage, traits::Get, weights::DispatchClass}; +use frame_support::{dispatch::DispatchClass, storage, traits::Get}; use frame_system::{Call, Pallet as System, RawOrigin}; use sp_core::storage::well_known_keys; use sp_runtime::traits::Hash; diff --git a/frame/system/src/extensions/check_mortality.rs b/frame/system/src/extensions/check_mortality.rs index f515f1ae0fa27..635ab4ef1d9a9 100644 --- a/frame/system/src/extensions/check_mortality.rs +++ b/frame/system/src/extensions/check_mortality.rs @@ -101,7 +101,10 @@ impl SignedExtension for CheckMortality { mod tests { use super::*; use crate::mock::{new_test_ext, System, Test, CALL}; - use frame_support::weights::{DispatchClass, DispatchInfo, Pays, Weight}; + use frame_support::{ + dispatch::{DispatchClass, DispatchInfo, Pays}, + weights::Weight, + }; use sp_core::H256; #[test] diff --git a/frame/system/src/extensions/check_non_zero_sender.rs b/frame/system/src/extensions/check_non_zero_sender.rs index 093424999aab3..036f70c2fdd48 100644 --- a/frame/system/src/extensions/check_non_zero_sender.rs +++ b/frame/system/src/extensions/check_non_zero_sender.rs @@ -17,7 +17,7 @@ use crate::Config; use codec::{Decode, Encode}; -use frame_support::weights::DispatchInfo; +use frame_support::dispatch::DispatchInfo; use scale_info::TypeInfo; use sp_runtime::{ traits::{DispatchInfoOf, Dispatchable, SignedExtension}, diff --git a/frame/system/src/extensions/check_nonce.rs b/frame/system/src/extensions/check_nonce.rs index 7259b80013ae5..1616a2d8a119e 100644 --- a/frame/system/src/extensions/check_nonce.rs +++ b/frame/system/src/extensions/check_nonce.rs @@ -17,7 +17,7 @@ use crate::Config; use codec::{Decode, Encode}; -use frame_support::weights::DispatchInfo; +use frame_support::dispatch::DispatchInfo; use scale_info::TypeInfo; use sp_runtime::{ traits::{DispatchInfoOf, Dispatchable, One, SignedExtension}, diff --git a/frame/system/src/extensions/check_weight.rs b/frame/system/src/extensions/check_weight.rs index 9af79e480b394..466231b8455ec 100644 --- a/frame/system/src/extensions/check_weight.rs +++ b/frame/system/src/extensions/check_weight.rs @@ -18,8 +18,8 @@ use crate::{limits::BlockWeights, Config, Pallet}; use codec::{Decode, Encode}; use frame_support::{ + dispatch::{DispatchClass, DispatchInfo, PostDispatchInfo}, traits::Get, - weights::{DispatchClass, DispatchInfo, PostDispatchInfo, Weight}, }; use scale_info::TypeInfo; use sp_runtime::{ @@ -27,6 +27,7 @@ use sp_runtime::{ transaction_validity::{InvalidTransaction, TransactionValidity, TransactionValidityError}, DispatchResult, }; +use sp_weights::Weight; /// Block resource (weight) limit check. /// @@ -269,10 +270,7 @@ mod tests { mock::{new_test_ext, System, Test, CALL}, AllExtrinsicsLen, BlockWeight, }; - use frame_support::{ - assert_err, assert_ok, - weights::{Pays, Weight}, - }; + use frame_support::{assert_err, assert_ok, dispatch::Pays, weights::Weight}; use sp_std::marker::PhantomData; fn block_weights() -> crate::limits::BlockWeights { diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index 550e3939f995a..739dcadde9ac1 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -84,20 +84,20 @@ use sp_version::RuntimeVersion; use codec::{Decode, Encode, EncodeLike, FullCodec, MaxEncodedLen}; use frame_support::{ - dispatch::{DispatchResult, DispatchResultWithPostInfo}, + dispatch::{ + extract_actual_pays_fee, extract_actual_weight, DispatchClass, DispatchInfo, + DispatchResult, DispatchResultWithPostInfo, PerDispatchClass, + }, storage, traits::{ ConstU32, Contains, EnsureOrigin, Get, HandleLifetime, OnKilledAccount, OnNewAccount, OriginTrait, PalletInfo, SortedMembers, StoredMap, TypedGet, }, - weights::{ - extract_actual_pays_fee, extract_actual_weight, DispatchClass, DispatchInfo, - PerDispatchClass, RuntimeDbWeight, Weight, - }, Parameter, }; use scale_info::TypeInfo; use sp_core::storage::well_known_keys; +use sp_weights::{RuntimeDbWeight, Weight}; #[cfg(feature = "std")] use frame_support::traits::GenesisBuild; diff --git a/frame/system/src/limits.rs b/frame/system/src/limits.rs index ba73d29806bba..e182eb626424d 100644 --- a/frame/system/src/limits.rs +++ b/frame/system/src/limits.rs @@ -25,7 +25,10 @@ //! `DispatchClass`. This module contains configuration object for both resources, //! which should be passed to `frame_system` configuration when runtime is being set up. -use frame_support::weights::{constants, DispatchClass, OneOrMany, PerDispatchClass, Weight}; +use frame_support::{ + dispatch::{DispatchClass, OneOrMany, PerDispatchClass}, + weights::{constants, Weight}, +}; use scale_info::TypeInfo; use sp_runtime::{traits::Bounded, Perbill, RuntimeDebug}; diff --git a/frame/system/src/tests.rs b/frame/system/src/tests.rs index 641cda13b07a4..b6fdda8d9cbe2 100644 --- a/frame/system/src/tests.rs +++ b/frame/system/src/tests.rs @@ -18,8 +18,7 @@ use crate::*; use frame_support::{ assert_noop, assert_ok, - dispatch::PostDispatchInfo, - weights::{Pays, WithPostDispatchInfo}, + dispatch::{Pays, PostDispatchInfo, WithPostDispatchInfo}, }; use mock::{Origin, *}; use sp_core::H256; diff --git a/frame/transaction-payment/asset-tx-payment/src/lib.rs b/frame/transaction-payment/asset-tx-payment/src/lib.rs index c14c7ba11b546..e136da8b0bb75 100644 --- a/frame/transaction-payment/asset-tx-payment/src/lib.rs +++ b/frame/transaction-payment/asset-tx-payment/src/lib.rs @@ -39,7 +39,7 @@ use sp_std::prelude::*; use codec::{Decode, Encode}; use frame_support::{ - dispatch::DispatchResult, + dispatch::{DispatchInfo, DispatchResult, PostDispatchInfo}, traits::{ tokens::{ fungibles::{Balanced, CreditOf, Inspect}, @@ -47,7 +47,6 @@ use frame_support::{ }, IsType, }, - weights::{DispatchInfo, PostDispatchInfo}, DefaultNoBound, }; use pallet_transaction_payment::OnChargeTransaction; diff --git a/frame/transaction-payment/asset-tx-payment/src/tests.rs b/frame/transaction-payment/asset-tx-payment/src/tests.rs index dfce28fda43d2..1f6113aabdddd 100644 --- a/frame/transaction-payment/asset-tx-payment/src/tests.rs +++ b/frame/transaction-payment/asset-tx-payment/src/tests.rs @@ -18,10 +18,11 @@ use crate as pallet_asset_tx_payment; use frame_support::{ assert_ok, + dispatch::{DispatchClass, DispatchInfo, PostDispatchInfo}, pallet_prelude::*, parameter_types, traits::{fungibles::Mutate, ConstU32, ConstU64, ConstU8, FindAuthor}, - weights::{DispatchClass, DispatchInfo, PostDispatchInfo, Weight, WeightToFee as WeightToFeeT}, + weights::{Weight, WeightToFee as WeightToFeeT}, ConsensusEngineId, }; use frame_system as system; diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index 5f1f8321456be..0447ea6bfd99d 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -63,11 +63,11 @@ use sp_runtime::{ use sp_std::prelude::*; use frame_support::{ - dispatch::DispatchResult, - traits::{EstimateCallFee, Get}, - weights::{ - DispatchClass, DispatchInfo, GetDispatchInfo, Pays, PostDispatchInfo, Weight, WeightToFee, + dispatch::{ + DispatchClass, DispatchInfo, DispatchResult, GetDispatchInfo, Pays, PostDispatchInfo, }, + traits::{EstimateCallFee, Get}, + weights::{Weight, WeightToFee}, }; mod payment; @@ -849,12 +849,11 @@ mod tests { }; use frame_support::{ - assert_noop, assert_ok, parameter_types, + assert_noop, assert_ok, + dispatch::{DispatchClass, DispatchInfo, GetDispatchInfo, PostDispatchInfo}, + parameter_types, traits::{ConstU32, ConstU64, Currency, GenesisBuild, Imbalance, OnUnbalanced}, - weights::{ - DispatchClass, DispatchInfo, GetDispatchInfo, PostDispatchInfo, Weight, - WeightToFee as WeightToFeeT, - }, + weights::{Weight, WeightToFee as WeightToFeeT}, }; use frame_system as system; use pallet_balances::Call as BalancesCall; diff --git a/frame/transaction-payment/src/types.rs b/frame/transaction-payment/src/types.rs index 5e915d62a26d4..1f41ba7b0b72e 100644 --- a/frame/transaction-payment/src/types.rs +++ b/frame/transaction-payment/src/types.rs @@ -24,7 +24,7 @@ use serde::{Deserialize, Serialize}; use sp_runtime::traits::{AtLeast32BitUnsigned, Zero}; use sp_std::prelude::*; -use frame_support::weights::{DispatchClass, Weight}; +use frame_support::{dispatch::DispatchClass, weights::Weight}; /// The base fee and adjusted weight and length fees constitute the _inclusion fee_. #[derive(Encode, Decode, Clone, Eq, PartialEq)] diff --git a/frame/utility/src/lib.rs b/frame/utility/src/lib.rs index b5bd686b59f00..08fb34ed9e3c1 100644 --- a/frame/utility/src/lib.rs +++ b/frame/utility/src/lib.rs @@ -58,9 +58,8 @@ pub mod weights; use codec::{Decode, Encode}; use frame_support::{ - dispatch::PostDispatchInfo, + dispatch::{extract_actual_weight, GetDispatchInfo, PostDispatchInfo}, traits::{IsSubType, OriginTrait, UnfilteredDispatchable}, - weights::{extract_actual_weight, GetDispatchInfo}, }; use sp_core::TypeId; use sp_io::hashing::blake2_256; diff --git a/frame/utility/src/tests.rs b/frame/utility/src/tests.rs index 1985cc3022114..93b80672f55e2 100644 --- a/frame/utility/src/tests.rs +++ b/frame/utility/src/tests.rs @@ -24,10 +24,10 @@ use super::*; use crate as utility; use frame_support::{ assert_err_ignore_postinfo, assert_noop, assert_ok, - dispatch::{DispatchError, DispatchErrorWithPostInfo, Dispatchable}, + dispatch::{DispatchError, DispatchErrorWithPostInfo, Dispatchable, Pays}, parameter_types, storage, traits::{ConstU32, ConstU64, Contains}, - weights::{Pays, Weight}, + weights::Weight, }; use sp_core::H256; use sp_runtime::{ diff --git a/frame/whitelist/src/lib.rs b/frame/whitelist/src/lib.rs index 19e53f2491614..c5621a239f3a6 100644 --- a/frame/whitelist/src/lib.rs +++ b/frame/whitelist/src/lib.rs @@ -42,9 +42,10 @@ pub use weights::WeightInfo; use codec::{DecodeLimit, Encode, FullCodec}; use frame_support::{ + dispatch::{GetDispatchInfo, PostDispatchInfo}, ensure, traits::{PreimageProvider, PreimageRecipient}, - weights::{GetDispatchInfo, PostDispatchInfo, Weight}, + weights::Weight, }; use scale_info::TypeInfo; use sp_runtime::traits::{Dispatchable, Hash}; diff --git a/primitives/runtime/Cargo.toml b/primitives/runtime/Cargo.toml index 9e4b36c584e91..030c44c284593 100644 --- a/primitives/runtime/Cargo.toml +++ b/primitives/runtime/Cargo.toml @@ -29,6 +29,7 @@ sp-arithmetic = { version = "5.0.0", default-features = false, path = "../arithm sp-core = { version = "6.0.0", default-features = false, path = "../core" } sp-io = { version = "6.0.0", default-features = false, path = "../io" } sp-std = { version = "4.0.0", default-features = false, path = "../std" } +sp-weights = { version = "4.0.0", default-features = false, path = "../weights" } [dev-dependencies] rand = "0.7.2" diff --git a/primitives/runtime/src/traits.rs b/primitives/runtime/src/traits.rs index 2cf98b4e638d3..495dfa66c5462 100644 --- a/primitives/runtime/src/traits.rs +++ b/primitives/runtime/src/traits.rs @@ -1820,6 +1820,12 @@ impl Printable for bool { } } +impl Printable for sp_weights::Weight { + fn print(&self) { + self.ref_time().print() + } +} + impl Printable for () { fn print(&self) { "()".print() diff --git a/primitives/weights/Cargo.toml b/primitives/weights/Cargo.toml new file mode 100644 index 0000000000000..8c0302ff5d1b2 --- /dev/null +++ b/primitives/weights/Cargo.toml @@ -0,0 +1,40 @@ +[package] +name = "sp-weights" +version = "4.0.0" +authors = ["Parity Technologies "] +edition = "2021" +license = "Apache-2.0" +homepage = "https://substrate.io" +repository = "https://github.com/paritytech/substrate/" +description = "Types and traits for interfacing between the host and the wasm runtime." +documentation = "https://docs.rs/sp-wasm-interface" +readme = "README.md" + +[package.metadata.docs.rs] +targets = ["x86_64-unknown-linux-gnu"] + +[dependencies] +codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = ["derive"] } +impl-trait-for-tuples = "0.2.2" +scale-info = { version = "2.1.1", default-features = false, features = ["derive"] } +serde = { version = "1.0.136", optional = true, features = ["derive"] } +smallvec = "1.8.0" +sp-arithmetic = { version = "5.0.0", default-features = false, path = "../arithmetic" } +sp-core = { version = "6.0.0", default-features = false, path = "../core" } +sp-debug-derive = { version = "4.0.0", default-features = false, path = "../debug-derive" } +sp-std = { version = "4.0.0", default-features = false, path = "../std" } + +[features] +default = [ "std" ] +std = [ + "codec/std", + "scale-info/std", + "serde", + "sp-arithmetic/std", + "sp-core/std", + "sp-debug-derive/std", + "sp-std/std" +] +# By default some types have documentation, `full-metadata-docs` allows to add documentation to +# more types in the metadata. +full-metadata-docs = ["scale-info/docs"] diff --git a/primitives/weights/src/lib.rs b/primitives/weights/src/lib.rs new file mode 100644 index 0000000000000..d260f73d41268 --- /dev/null +++ b/primitives/weights/src/lib.rs @@ -0,0 +1,299 @@ +// This file is part of Substrate. + +// Copyright (C) 2019-2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! # Primitives for transaction weighting. +//! +//! Latest machine specification used to benchmark are: +//! - Digital Ocean: ubuntu-s-2vcpu-4gb-ams3-01 +//! - 2x Intel(R) Xeon(R) CPU E5-2650 v4 @ 2.20GHz +//! - 4GB RAM +//! - Ubuntu 19.10 (GNU/Linux 5.3.0-18-generic x86_64) +//! - rustc 1.42.0 (b8cedc004 2020-03-09) + +#![cfg_attr(not(feature = "std"), no_std)] + +extern crate self as sp_weights; + +mod weight_v2; + +use codec::{Decode, Encode}; +use scale_info::TypeInfo; +#[cfg(feature = "std")] +use serde::{Deserialize, Serialize}; +use smallvec::SmallVec; +use sp_arithmetic::{ + traits::{BaseArithmetic, SaturatedConversion, Saturating, Unsigned}, + Perbill, +}; +use sp_core::Get; +use sp_debug_derive::RuntimeDebug; + +pub use weight_v2::*; + +pub mod constants { + use super::Weight; + + pub const WEIGHT_PER_SECOND: Weight = Weight::from_ref_time(1_000_000_000_000); + pub const WEIGHT_PER_MILLIS: Weight = Weight::from_ref_time(1_000_000_000); + pub const WEIGHT_PER_MICROS: Weight = Weight::from_ref_time(1_000_000); + pub const WEIGHT_PER_NANOS: Weight = Weight::from_ref_time(1_000); +} + +/// The weight of database operations that the runtime can invoke. +/// +/// NOTE: This is currently only measured in computational time, and will probably +/// be updated all together once proof size is accounted for. +#[derive(Clone, Copy, Eq, PartialEq, Default, RuntimeDebug, Encode, Decode, TypeInfo)] +pub struct RuntimeDbWeight { + pub read: u64, + pub write: u64, +} + +impl RuntimeDbWeight { + pub fn reads(self, r: u64) -> Weight { + Weight::from_ref_time(self.read.saturating_mul(r)) + } + + pub fn writes(self, w: u64) -> Weight { + Weight::from_ref_time(self.write.saturating_mul(w)) + } + + pub fn reads_writes(self, r: u64, w: u64) -> Weight { + let read_weight = self.read.saturating_mul(r); + let write_weight = self.write.saturating_mul(w); + Weight::from_ref_time(read_weight.saturating_add(write_weight)) + } +} + +/// One coefficient and its position in the `WeightToFee`. +/// +/// One term of polynomial is calculated as: +/// +/// ```ignore +/// coeff_integer * x^(degree) + coeff_frac * x^(degree) +/// ``` +/// +/// The `negative` value encodes whether the term is added or substracted from the +/// overall polynomial result. +#[derive(Clone, Encode, Decode, TypeInfo)] +pub struct WeightToFeeCoefficient { + /// The integral part of the coefficient. + pub coeff_integer: Balance, + /// The fractional part of the coefficient. + pub coeff_frac: Perbill, + /// True iff the coefficient should be interpreted as negative. + pub negative: bool, + /// Degree/exponent of the term. + pub degree: u8, +} + +/// A list of coefficients that represent one polynomial. +pub type WeightToFeeCoefficients = SmallVec<[WeightToFeeCoefficient; 4]>; + +/// A trait that describes the weight to fee calculation. +pub trait WeightToFee { + /// The type that is returned as result from calculation. + type Balance: BaseArithmetic + From + Copy + Unsigned; + + /// Calculates the fee from the passed `weight`. + fn weight_to_fee(weight: &Weight) -> Self::Balance; +} + +/// A trait that describes the weight to fee calculation as polynomial. +/// +/// An implementor should only implement the `polynomial` function. +pub trait WeightToFeePolynomial { + /// The type that is returned as result from polynomial evaluation. + type Balance: BaseArithmetic + From + Copy + Unsigned; + + /// Returns a polynomial that describes the weight to fee conversion. + /// + /// This is the only function that should be manually implemented. Please note + /// that all calculation is done in the probably unsigned `Balance` type. This means + /// that the order of coefficients is important as putting the negative coefficients + /// first will most likely saturate the result to zero mid evaluation. + fn polynomial() -> WeightToFeeCoefficients; +} + +impl WeightToFee for T +where + T: WeightToFeePolynomial, +{ + type Balance = ::Balance; + + /// Calculates the fee from the passed `weight` according to the `polynomial`. + /// + /// This should not be overridden in most circumstances. Calculation is done in the + /// `Balance` type and never overflows. All evaluation is saturating. + fn weight_to_fee(weight: &Weight) -> Self::Balance { + Self::polynomial() + .iter() + .fold(Self::Balance::saturated_from(0u32), |mut acc, args| { + let w = Self::Balance::saturated_from(weight.ref_time()) + .saturating_pow(args.degree.into()); + + // The sum could get negative. Therefore we only sum with the accumulator. + // The Perbill Mul implementation is non overflowing. + let frac = args.coeff_frac * w; + let integer = args.coeff_integer.saturating_mul(w); + + if args.negative { + acc = acc.saturating_sub(frac); + acc = acc.saturating_sub(integer); + } else { + acc = acc.saturating_add(frac); + acc = acc.saturating_add(integer); + } + + acc + }) + } +} + +/// Implementor of `WeightToFee` that maps one unit of weight to one unit of fee. +pub struct IdentityFee(sp_std::marker::PhantomData); + +impl WeightToFee for IdentityFee +where + T: BaseArithmetic + From + Copy + Unsigned, +{ + type Balance = T; + + fn weight_to_fee(weight: &Weight) -> Self::Balance { + Self::Balance::saturated_from(weight.ref_time()) + } +} + +/// Implementor of [`WeightToFee`] that uses a constant multiplier. +/// # Example +/// +/// ``` +/// # use sp_core::ConstU128; +/// # use sp_weights::ConstantMultiplier; +/// // Results in a multiplier of 10 for each unit of weight (or length) +/// type LengthToFee = ConstantMultiplier::>; +/// ``` +pub struct ConstantMultiplier(sp_std::marker::PhantomData<(T, M)>); + +impl WeightToFee for ConstantMultiplier +where + T: BaseArithmetic + From + Copy + Unsigned, + M: Get, +{ + type Balance = T; + + fn weight_to_fee(weight: &Weight) -> Self::Balance { + Self::Balance::saturated_from(weight.ref_time()).saturating_mul(M::get()) + } +} + +#[cfg(test)] +#[allow(dead_code)] +mod tests { + use super::*; + use smallvec::smallvec; + + type Balance = u64; + + // 0.5x^3 + 2.333x^2 + 7x - 10_000 + struct Poly; + impl WeightToFeePolynomial for Poly { + type Balance = Balance; + + fn polynomial() -> WeightToFeeCoefficients { + smallvec![ + WeightToFeeCoefficient { + coeff_integer: 0, + coeff_frac: Perbill::from_float(0.5), + negative: false, + degree: 3 + }, + WeightToFeeCoefficient { + coeff_integer: 2, + coeff_frac: Perbill::from_rational(1u32, 3u32), + negative: false, + degree: 2 + }, + WeightToFeeCoefficient { + coeff_integer: 7, + coeff_frac: Perbill::zero(), + negative: false, + degree: 1 + }, + WeightToFeeCoefficient { + coeff_integer: 10_000, + coeff_frac: Perbill::zero(), + negative: true, + degree: 0 + }, + ] + } + } + + #[test] + fn polynomial_works() { + // 100^3/2=500000 100^2*(2+1/3)=23333 700 -10000 + assert_eq!(Poly::weight_to_fee(&Weight::from_ref_time(100)), 514033); + // 10123^3/2=518677865433 10123^2*(2+1/3)=239108634 70861 -10000 + assert_eq!(Poly::weight_to_fee(&Weight::from_ref_time(10_123)), 518917034928); + } + + #[test] + fn polynomial_does_not_underflow() { + assert_eq!(Poly::weight_to_fee(&Weight::zero()), 0); + assert_eq!(Poly::weight_to_fee(&Weight::from_ref_time(10)), 0); + } + + #[test] + fn polynomial_does_not_overflow() { + assert_eq!(Poly::weight_to_fee(&Weight::MAX), Balance::max_value() - 10_000); + } + + #[test] + fn identity_fee_works() { + assert_eq!(IdentityFee::::weight_to_fee(&Weight::zero()), 0); + assert_eq!(IdentityFee::::weight_to_fee(&Weight::from_ref_time(50)), 50); + assert_eq!(IdentityFee::::weight_to_fee(&Weight::MAX), Balance::max_value()); + } + + #[test] + fn constant_fee_works() { + use sp_core::ConstU128; + assert_eq!( + ConstantMultiplier::>::weight_to_fee(&Weight::zero()), + 0 + ); + assert_eq!( + ConstantMultiplier::>::weight_to_fee(&Weight::from_ref_time( + 50 + )), + 500 + ); + assert_eq!( + ConstantMultiplier::>::weight_to_fee(&Weight::from_ref_time( + 16 + )), + 16384 + ); + assert_eq!( + ConstantMultiplier::>::weight_to_fee( + &Weight::from_ref_time(2) + ), + u128::MAX + ); + } +} diff --git a/frame/support/src/weights/weight_v2.rs b/primitives/weights/src/weight_v2.rs similarity index 68% rename from frame/support/src/weights/weight_v2.rs rename to primitives/weights/src/weight_v2.rs index 2d50f07add596..a4e4da4f8a7b3 100644 --- a/frame/support/src/weights/weight_v2.rs +++ b/primitives/weights/src/weight_v2.rs @@ -17,10 +17,8 @@ use codec::{CompactAs, Decode, Encode, MaxEncodedLen}; use core::ops::{Add, AddAssign, Div, Mul, Sub, SubAssign}; -use sp_runtime::{ - traits::{Bounded, CheckedAdd, CheckedSub, Zero}, - RuntimeDebug, -}; +use sp_arithmetic::traits::{Bounded, CheckedAdd, CheckedSub, Zero}; +use sp_debug_derive::RuntimeDebug; use super::*; @@ -266,11 +264,11 @@ macro_rules! weight_mul_per_impl { } } weight_mul_per_impl!( - sp_runtime::Percent, - sp_runtime::PerU16, - sp_runtime::Permill, - sp_runtime::Perbill, - sp_runtime::Perquintill, + sp_arithmetic::Percent, + sp_arithmetic::PerU16, + sp_arithmetic::Permill, + sp_arithmetic::Perbill, + sp_arithmetic::Perquintill, ); macro_rules! weight_mul_primitive_impl { @@ -310,91 +308,6 @@ impl CheckedSub for Weight { } } -impl PaysFee for (Weight, DispatchClass, Pays) { - fn pays_fee(&self, _: T) -> Pays { - self.2 - } -} - -impl WeighData for (Weight, DispatchClass) { - fn weigh_data(&self, args: T) -> Weight { - return self.0.weigh_data(args) - } -} - -impl WeighData for (Weight, DispatchClass, Pays) { - fn weigh_data(&self, args: T) -> Weight { - return self.0.weigh_data(args) - } -} - -impl ClassifyDispatch for (Weight, DispatchClass) { - fn classify_dispatch(&self, _: T) -> DispatchClass { - self.1 - } -} - -impl PaysFee for (Weight, DispatchClass) { - fn pays_fee(&self, _: T) -> Pays { - Pays::Yes - } -} - -impl WeighData for (Weight, Pays) { - fn weigh_data(&self, args: T) -> Weight { - return self.0.weigh_data(args) - } -} - -impl ClassifyDispatch for (Weight, Pays) { - fn classify_dispatch(&self, _: T) -> DispatchClass { - DispatchClass::Normal - } -} - -impl PaysFee for (Weight, Pays) { - fn pays_fee(&self, _: T) -> Pays { - self.1 - } -} - -impl From<(Option, Pays)> for PostDispatchInfo { - fn from(post_weight_info: (Option, Pays)) -> Self { - let (actual_weight, pays_fee) = post_weight_info; - Self { actual_weight, pays_fee } - } -} - -impl From> for PostDispatchInfo { - fn from(actual_weight: Option) -> Self { - Self { actual_weight, pays_fee: Default::default() } - } -} - -impl WeighData for Weight { - fn weigh_data(&self, _: T) -> Weight { - return *self - } -} - -impl ClassifyDispatch for Weight { - fn classify_dispatch(&self, _: T) -> DispatchClass { - DispatchClass::Normal - } -} - -impl PaysFee for Weight { - fn pays_fee(&self, _: T) -> Pays { - Pays::Yes - } -} - -impl ClassifyDispatch for (Weight, DispatchClass, Pays) { - fn classify_dispatch(&self, _: T) -> DispatchClass { - self.1 - } -} - impl core::fmt::Display for Weight { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!(f, "Weight(ref_time: {})", self.ref_time) @@ -421,106 +334,3 @@ impl SubAssign for Weight { *self = Self { ref_time: self.ref_time - other.ref_time }; } } - -impl sp_runtime::traits::Printable for Weight { - fn print(&self) { - self.ref_time().print() - } -} - -// TODO: Eventually remove these - -impl From> for PostDispatchInfo { - fn from(maybe_actual_computation: Option) -> Self { - let actual_weight = match maybe_actual_computation { - Some(actual_computation) => Some(Weight::zero().set_ref_time(actual_computation)), - None => None, - }; - Self { actual_weight, pays_fee: Default::default() } - } -} - -impl From<(Option, Pays)> for PostDispatchInfo { - fn from(post_weight_info: (Option, Pays)) -> Self { - let (maybe_actual_time, pays_fee) = post_weight_info; - let actual_weight = match maybe_actual_time { - Some(actual_time) => Some(Weight::zero().set_ref_time(actual_time)), - None => None, - }; - Self { actual_weight, pays_fee } - } -} - -impl WeighData for u64 { - fn weigh_data(&self, _: T) -> Weight { - return Weight::zero().set_ref_time(*self) - } -} - -impl ClassifyDispatch for u64 { - fn classify_dispatch(&self, _: T) -> DispatchClass { - DispatchClass::Normal - } -} - -impl PaysFee for u64 { - fn pays_fee(&self, _: T) -> Pays { - Pays::Yes - } -} - -impl WeighData for (u64, DispatchClass, Pays) { - fn weigh_data(&self, args: T) -> Weight { - return self.0.weigh_data(args) - } -} - -impl ClassifyDispatch for (u64, DispatchClass, Pays) { - fn classify_dispatch(&self, _: T) -> DispatchClass { - self.1 - } -} - -impl PaysFee for (u64, DispatchClass, Pays) { - fn pays_fee(&self, _: T) -> Pays { - self.2 - } -} - -impl WeighData for (u64, DispatchClass) { - fn weigh_data(&self, args: T) -> Weight { - return self.0.weigh_data(args) - } -} - -impl ClassifyDispatch for (u64, DispatchClass) { - fn classify_dispatch(&self, _: T) -> DispatchClass { - self.1 - } -} - -impl PaysFee for (u64, DispatchClass) { - fn pays_fee(&self, _: T) -> Pays { - Pays::Yes - } -} - -impl WeighData for (u64, Pays) { - fn weigh_data(&self, args: T) -> Weight { - return self.0.weigh_data(args) - } -} - -impl ClassifyDispatch for (u64, Pays) { - fn classify_dispatch(&self, _: T) -> DispatchClass { - DispatchClass::Normal - } -} - -impl PaysFee for (u64, Pays) { - fn pays_fee(&self, _: T) -> Pays { - self.1 - } -} - -// END TODO diff --git a/utils/frame/benchmarking-cli/src/overhead/weights.hbs b/utils/frame/benchmarking-cli/src/overhead/weights.hbs index 0a4100953401f..86b2c0b0e0cdb 100644 --- a/utils/frame/benchmarking-cli/src/overhead/weights.hbs +++ b/utils/frame/benchmarking-cli/src/overhead/weights.hbs @@ -13,10 +13,8 @@ // {{arg}} {{/each}} -use frame_support::{ - parameter_types, - weights::{constants::WEIGHT_PER_NANOS, Weight}, -}; +use sp_core::parameter_types; +use sp_weights::{constants::WEIGHT_PER_NANOS, Weight}; parameter_types! { {{#if (eq short_name "block")}} @@ -42,7 +40,7 @@ parameter_types! { #[cfg(test)] mod test_weights { use super::*; - use frame_support::weights::constants; + use sp_weights::constants; /// Checks that the weight exists and is sane. // NOTE: If this test fails but you are sure that the generated values are fine, diff --git a/utils/frame/benchmarking-cli/src/storage/weights.hbs b/utils/frame/benchmarking-cli/src/storage/weights.hbs index 95f42c411d1fd..82e581cf990c8 100644 --- a/utils/frame/benchmarking-cli/src/storage/weights.hbs +++ b/utils/frame/benchmarking-cli/src/storage/weights.hbs @@ -17,10 +17,9 @@ /// Storage DB weights for the `{{runtime_name}}` runtime and `{{db_name}}`. pub mod constants { - use frame_support::{ - parameter_types, - weights::{constants, RuntimeDbWeight}, - }; + use frame_support::weights::constants; + use sp_core::parameter_types; + use sp_weights::RuntimeDbWeight; parameter_types! { {{#if (eq db_name "ParityDb")}} @@ -66,7 +65,7 @@ pub mod constants { #[cfg(test)] mod test_db_weights { use super::constants::{{db_name}}Weight as W; - use frame_support::weights::constants; + use sp_weights::constants; /// Checks that all weights exist and have sane values. // NOTE: If this test fails but you are sure that the generated values are fine, From 0ecae2cef735dcb43fe43416f85206d341ae8d6f Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Wed, 14 Sep 2022 13:32:45 +0300 Subject: [PATCH 17/17] Fix handshake message initialization --- client/network/src/service.rs | 7 ++----- client/service/src/builder.rs | 2 -- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/client/network/src/service.rs b/client/network/src/service.rs index d369459b25b27..6017543e528e7 100644 --- a/client/network/src/service.rs +++ b/client/network/src/service.rs @@ -225,11 +225,8 @@ where params.protocol_id.clone(), ¶ms.fork_id, ¶ms.network_config, - iter::once(Vec::new()) - .chain( - (0..params.network_config.extra_sets.len().saturating_sub(1)) - .map(|_| default_notif_handshake_message.clone()), - ) + (0..params.network_config.extra_sets.len()) + .map(|_| default_notif_handshake_message.clone()) .collect(), params.metrics_registry.as_ref(), params.chain_sync, diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index 8fc57f8c04b28..e21c9c6e6d727 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -892,8 +892,6 @@ where .extra_sets .insert(0, transactions_handler_proto.set_config()); - info!("extra sets", network_params .network_config .extra_sets); - let has_bootnodes = !network_params.network_config.boot_nodes.is_empty(); let network_mut = sc_network::NetworkWorker::new(network_params)?; let network = network_mut.service().clone();