[DO NOT MERGE] Nomination Pool Commission

.gitlab-ci.yml

@@ -48,23 +48,39 @@ variables:
   CARGO_INCREMENTAL:               0
   DOCKER_OS:                       "debian:stretch"
   ARCH:                            "x86_64"
-  CI_IMAGE:                        "paritytech/ci-linux:production"
+  CI_IMAGE:                        "paritytech/ci-linux@sha256:9140bc3c843a8b12a3bcf6f5886346536092795bbadfd7f1836362cb28dfcc71"
   BUILDAH_IMAGE:                   "quay.io/buildah/stable:v1.27"
   RUSTY_CACHIER_SINGLE_BRANCH:     master
   RUSTY_CACHIER_DONT_OPERATE_ON_MAIN_BRANCH: "true"
   RUSTY_CACHIER_COMPRESSION_METHOD: zstd
+  NEXTEST_FAILURE_OUTPUT:          immediate-final
+  NEXTEST_SUCCESS_OUTPUT:          final
   ZOMBIENET_IMAGE:                 "docker.io/paritytech/zombienet:v1.3.22"
 
-default:
+.shared-default:                   &shared-default
   retry:
     max: 2
     when:
       - runner_system_failure
       - unknown_failure
       - api_failure
-  interruptible:                   true
   cache:                           {}
 
+.default-pipeline-definitions:
+  default:
+    <<:                            *shared-default
+    interruptible:                 true
+
+.crate-publishing-pipeline-definitions:
+  default:
+    <<:                            *shared-default
+    # The crate-publishing pipeline defaults to `interruptible: false` so that we'll be able to
+    # reach and run the publishing jobs despite the "Auto-cancel redundant pipelines" CI setting.
+    # The setting is relevant because the crate-publishing pipeline runs on `master`, thus future
+    # pipelines on `master` (e.g. created for new commits or other schedules) might unintendedly
+    # cancel the publishing jobs or its dependencies before we get to actually publish the crates.
+    interruptible:                 false
+
 .collect-artifacts:
   artifacts:
     name:                          "${CI_JOB_NAME}_${CI_COMMIT_REF_NAME}"
@@ -213,9 +229,15 @@ default:
     # this job runs only on nightly pipeline with the mentioned variable, against `master` branch
     - if: $CI_COMMIT_REF_NAME == "master" && $CI_PIPELINE_SOURCE == "schedule" && $PIPELINE == "nightly"
 
+.crate-publishing-pipeline:
+  rules:
+    - if: $CI_COMMIT_REF_NAME != "master"
+      when: never
+
 .scheduled-crate-publishing-pipeline:
   rules:
-    - if: $CI_COMMIT_REF_NAME == "master" && $CI_PIPELINE_SOURCE == "schedule" && $PIPELINE == "automatic-crate-publishing"
+    - !reference [.crate-publishing-pipeline, rules]
+    - if: $CI_PIPELINE_SOURCE == "schedule" && $PIPELINE == "automatic-crate-publishing"
 
 .crates-publishing-template:
   stage:                           test
@@ -263,6 +285,22 @@ include:
   - scripts/ci/gitlab/pipeline/publish.yml
   # zombienet jobs
   - scripts/ci/gitlab/pipeline/zombienet.yml
+  # The crate-publishing pipeline requires a customized `interruptible` configuration. Unfortunately
+  # `interruptible` can't currently be dynamically set based on variables as per:
+  # - https://gitlab.com/gitlab-org/gitlab/-/issues/38349
+  # - https://gitlab.com/gitlab-org/gitlab/-/issues/194023
+  # Thus we work around that limitation by using conditional includes.
+  # For crate-publishing pipelines: run it with defaults + `interruptible: false`. The WHOLE
+  # pipeline is made uninterruptible to ensure that test jobs also get a chance to run to
+  # completion, because the publishing jobs depends on them AS INTENDED: crates should not be
+  # published before their source code is checked.
+  - local: scripts/ci/gitlab/crate-publishing-pipeline.yml
+    rules:
+      - if: $PIPELINE == "automatic-crate-publishing"
+  # For normal pipelines: run it with defaults + `interruptible: true`
+  - local: scripts/ci/gitlab/default-pipeline.yml
+    rules:
+      - if: $PIPELINE != "automatic-crate-publishing"
 
 #### stage:                        deploy
 

client/state-db/src/noncanonical.rs

@@ -323,7 +323,6 @@ impl<BlockHash: Hash, Key: Hash> NonCanonicalOverlay<BlockHash, Key> {
 		&self,
 		level_index: usize,
 		discarded_journals: &mut Vec<Vec<u8>>,
-		discarded_blocks: &mut Vec<BlockHash>,
 		hash: &BlockHash,
 	) {
 		if let Some(level) = self.levels.get(level_index) {
@@ -335,13 +334,7 @@ impl<BlockHash: Hash, Key: Hash> NonCanonicalOverlay<BlockHash, Key> {
 					.clone();
 				if parent == *hash {
 					discarded_journals.push(overlay.journal_key.clone());
-					discarded_blocks.push(overlay.hash.clone());
-					self.discard_journals(
-						level_index + 1,
-						discarded_journals,
-						discarded_blocks,
-						&overlay.hash,
-					);
+					self.discard_journals(level_index + 1, discarded_journals, &overlay.hash);
 				}
 			});
 		}
@@ -393,7 +386,6 @@ impl<BlockHash: Hash, Key: Hash> NonCanonicalOverlay<BlockHash, Key> {
 		self.pinned_canonincalized.push(hash.clone());
 
 		let mut discarded_journals = Vec::new();
-		let mut discarded_blocks = Vec::new();
 		for (i, overlay) in level.blocks.into_iter().enumerate() {
 			let mut pinned_children = 0;
 			// That's the one we need to canonicalize
@@ -411,12 +403,7 @@ impl<BlockHash: Hash, Key: Hash> NonCanonicalOverlay<BlockHash, Key> {
 				commit.data.deleted.extend(overlay.deleted.clone());
 			} else {
 				// Discard this overlay
-				self.discard_journals(
-					0,
-					&mut discarded_journals,
-					&mut discarded_blocks,
-					&overlay.hash,
-				);
+				self.discard_journals(0, &mut discarded_journals, &overlay.hash);
 				pinned_children = discard_descendants(
 					&mut self.levels.as_mut_slices(),
 					&mut self.values,
@@ -437,7 +424,6 @@ impl<BlockHash: Hash, Key: Hash> NonCanonicalOverlay<BlockHash, Key> {
 				discard_values(&mut self.values, overlay.inserted);
 			}
 			discarded_journals.push(overlay.journal_key.clone());
-			discarded_blocks.push(overlay.hash.clone());
 		}
 		commit.meta.deleted.append(&mut discarded_journals);
 
@@ -548,9 +534,6 @@ impl<BlockHash: Hash, Key: Hash> NonCanonicalOverlay<BlockHash, Key> {
 							trace!(target: "state-db-pin", "Discarding unpinned non-canon block: {:?}", hash);
 							discard_values(&mut self.values, inserted);
 							self.parents.remove(&hash);
-							true
-						} else {
-							false
 						}
 					},
 					Entry::Vacant(_) => break,

frame/assets/src/lib.rs

@@ -175,6 +175,7 @@ pub use pallet::*;
 pub use weights::WeightInfo;
 
 type AccountIdLookupOf<T> = <<T as frame_system::Config>::Lookup as StaticLookup>::Source;
+const LOG_TARGET: &str = "runtime::assets";
 
 /// Trait with callbacks that are executed after successfull asset creation or destruction.
 pub trait AssetsCallback<AssetId, AccountId> {

frame/assets/src/migration.rs

@@ -75,10 +75,18 @@ pub mod v1 {
 					Some(old_value.migrate_to_v1())
 				});
 				current_version.put::<Pallet<T>>();
-				log::info!(target: "runtime::assets", "Upgraded {} pools, storage to version {:?}", translated, current_version);
+				log::info!(
+					target: LOG_TARGET,
+					"Upgraded {} pools, storage to version {:?}",
+					translated,
+					current_version
+				);
 				T::DbWeight::get().reads_writes(translated + 1, translated + 1)
 			} else {
-				log::info!(target: "runtime::assets",  "Migration did not execute. This probably should be removed");
+				log::info!(
+					target: LOG_TARGET,
+					"Migration did not execute. This probably should be removed"
+				);
 				T::DbWeight::get().reads(1)
 			}
 		}

frame/balances/src/lib.rs

@@ -193,6 +193,8 @@ pub use weights::WeightInfo;
 
 pub use pallet::*;
 
+const LOG_TARGET: &str = "runtime::balances";
+
 type AccountIdLookupOf<T> = <<T as frame_system::Config>::Lookup as StaticLookup>::Source;
 
 #[frame_support::pallet]
@@ -950,7 +952,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 
 		if locks.len() as u32 > T::MaxLocks::get() {
 			log::warn!(
-				target: "runtime::balances",
+				target: LOG_TARGET,
 				"Warning: A user has more currency locks than expected. \
 				A runtime configuration adjustment may be needed."
 			);
@@ -985,7 +987,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 				// since the funds that are under the lock will themselves be stored in the
 				// account and therefore will need a reference.
 				log::warn!(
-					target: "runtime::balances",
+					target: LOG_TARGET,
 					"Warning: Attempt to introduce lock consumer reference, yet no providers. \
 					This is unexpected but should be safe."
 				);

frame/balances/src/migration.rs

@@ -40,10 +40,13 @@ fn migrate_v0_to_v1<T: Config<I>, I: 'static>(accounts: &[T::AccountId]) -> Weig
 		// Set storage version to `1`.
 		StorageVersion::new(1).put::<Pallet<T, I>>();
 
-		log::info!(target: "runtime::balances", "Storage to version 1");
+		log::info!(target: LOG_TARGET, "Storage to version 1");
 		T::DbWeight::get().reads_writes(2 + accounts.len() as u64, 3)
 	} else {
-		log::info!(target: "runtime::balances",  "Migration did not execute. This probably should be removed");
+		log::info!(
+			target: LOG_TARGET,
+			"Migration did not execute. This probably should be removed"
+		);
 		T::DbWeight::get().reads(1)
 	}
 }
@@ -87,10 +90,13 @@ impl<T: Config<I>, I: 'static> OnRuntimeUpgrade for ResetInactive<T, I> {
 			// Set storage version to `0`.
 			StorageVersion::new(0).put::<Pallet<T, I>>();
 
-			log::info!(target: "runtime::balances", "Storage to version 0");
+			log::info!(target: LOG_TARGET, "Storage to version 0");
 			T::DbWeight::get().reads_writes(1, 2)
 		} else {
-			log::info!(target: "runtime::balances",  "Migration did not execute. This probably should be removed");
+			log::info!(
+				target: LOG_TARGET,
+				"Migration did not execute. This probably should be removed"
+			);
 			T::DbWeight::get().reads(1)
 		}
 	}

frame/collective/src/lib.rs

@@ -71,6 +71,8 @@ pub mod weights;
 pub use pallet::*;
 pub use weights::WeightInfo;
 
+const LOG_TARGET: &str = "runtime::collective";
+
 /// Simple index type for proposal counting.
 pub type ProposalIndex = u32;
 
@@ -390,7 +392,7 @@ pub mod pallet {
 			ensure_root(origin)?;
 			if new_members.len() > T::MaxMembers::get() as usize {
 				log::error!(
-					target: "runtime::collective",
+					target: LOG_TARGET,
 					"New members count ({}) exceeds maximum amount of members expected ({}).",
 					new_members.len(),
 					T::MaxMembers::get(),
@@ -400,7 +402,7 @@ pub mod pallet {
 			let old = Members::<T, I>::get();
 			if old.len() > old_count as usize {
 				log::warn!(
-					target: "runtime::collective",
+					target: LOG_TARGET,
 					"Wrong count used to estimate set_members weight. expected ({}) vs actual ({})",
 					old_count,
 					old.len(),
@@ -1040,7 +1042,7 @@ impl<T: Config<I>, I: 'static> ChangeMembers<T::AccountId> for Pallet<T, I> {
 	) {
 		if new.len() > T::MaxMembers::get() as usize {
 			log::error!(
-				target: "runtime::collective",
+				target: LOG_TARGET,
 				"New members count ({}) exceeds maximum amount of members expected ({}).",
 				new.len(),
 				T::MaxMembers::get(),

frame/collective/src/migrations/v4.rs

@@ -17,6 +17,7 @@
 
 use sp_io::hashing::twox_128;
 
+use super::super::LOG_TARGET;
 use frame_support::{
 	traits::{
 		Get, GetStorageVersion, PalletInfoAccess, StorageVersion,
@@ -42,15 +43,15 @@ pub fn migrate<T: frame_system::Config, P: GetStorageVersion + PalletInfoAccess,
 
 	if new_pallet_name == old_pallet_name {
 		log::info!(
-			target: "runtime::collective",
+			target: LOG_TARGET,
 			"New pallet name is equal to the old pallet name. No migration needs to be done.",
 		);
 		return Weight::zero()
 	}
 
 	let on_chain_storage_version = <P as GetStorageVersion>::on_chain_storage_version();
 	log::info!(
-		target: "runtime::collective",
+		target: LOG_TARGET,
 		"Running migration to v4 for collective with storage version {:?}",
 		on_chain_storage_version,
 	);
@@ -66,7 +67,7 @@ pub fn migrate<T: frame_system::Config, P: GetStorageVersion + PalletInfoAccess,
 		<T as frame_system::Config>::BlockWeights::get().max_block
 	} else {
 		log::warn!(
-			target: "runtime::collective",
+			target: LOG_TARGET,
 			"Attempted to apply migration to v4 but failed because storage version is {:?}",
 			on_chain_storage_version,
 		);
@@ -138,7 +139,7 @@ pub fn post_migrate<P: GetStorageVersion + PalletInfoAccess, N: AsRef<str>>(old_
 
 fn log_migration(stage: &str, old_pallet_name: &str, new_pallet_name: &str) {
 	log::info!(
-		target: "runtime::collective",
+		target: LOG_TARGET,
 		"{}, prefix: '{}' ==> '{}'",
 		stage,
 		old_pallet_name,

frame/election-provider-multi-phase/src/benchmarking.rs

@@ -200,7 +200,7 @@ frame_benchmarking::benchmarks! {
 		assert!(<MultiPhase<T>>::snapshot().is_none());
 		assert!(<MultiPhase<T>>::current_phase().is_off());
 	}: {
-		<MultiPhase<T>>::on_initialize_open_signed();
+		<MultiPhase<T>>::phase_transition(Phase::Signed);
 	} verify {
 		assert!(<MultiPhase<T>>::snapshot().is_none());
 		assert!(<MultiPhase<T>>::current_phase().is_signed());
@@ -210,7 +210,8 @@ frame_benchmarking::benchmarks! {
 		assert!(<MultiPhase<T>>::snapshot().is_none());
 		assert!(<MultiPhase<T>>::current_phase().is_off());
 	}: {
-		<MultiPhase<T>>::on_initialize_open_unsigned(true, 1u32.into())
+		let now = frame_system::Pallet::<T>::block_number();
+		<MultiPhase<T>>::phase_transition(Phase::Unsigned((true, now)));
 	} verify {
 		assert!(<MultiPhase<T>>::snapshot().is_none());
 		assert!(<MultiPhase<T>>::current_phase().is_unsigned());
@@ -318,7 +319,7 @@ frame_benchmarking::benchmarks! {
 	submit {
 		// the queue is full and the solution is only better than the worse.
 		<MultiPhase<T>>::create_snapshot().map_err(<&str>::from)?;
-		MultiPhase::<T>::on_initialize_open_signed();
+		<MultiPhase<T>>::phase_transition(Phase::Signed);
 		<Round<T>>::put(1);
 
 		let mut signed_submissions = SignedSubmissions::<T>::get();