Update from master (#69)

* parachain-system: Do not take `self` for `last_relay_block_number` (paritytech#2720) FRAME DSL is working in a static context. * fix zombienet test (paritytech#2719) Remove old version for `cli_args`, since this was fixed in the latest version of zombienet and the `latest` version of polkadot introduce the new flag `--insecure-validator-i-know-what-i-do`. Fix jobs like https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/4726174 Thx! * [NPoS] Remove better solution threshold for unsigned submissions (paritytech#2694) closes https://github.com/paritytech-secops/srlabs_findings/issues/78. Removes `BetterUnsignedThreshold` from pallet EPM. This will essentially mean any solution submitted by the validator that is strictly better than the current queued solution would be accepted. The reason for having these thresholds is to limit number of solutions submitted on-chain. However for unsigned submissions, the number of solutions that could be submitted on average is limited even without thresholding (calculation shown in the corresponding issue). * `chain-spec-builder`: Improve output path example (paritytech#2693) Example currently broken. It writes something to the given path, but not the full chain spec. --------- Co-authored-by: Michal Kucharczyk <[email protected]> * update lock file --------- Co-authored-by: Bastian Köcher <[email protected]> Co-authored-by: Javier Viola <[email protected]> Co-authored-by: Ankan <[email protected]> Co-authored-by: Liam Aharon <[email protected]> Co-authored-by: Michal Kucharczyk <[email protected]> Co-authored-by: claravanstaden <Cats 4 life!>
Snowfork · Dec 16, 2023 · 00e8eb2 · 00e8eb2
1 parent 46056e6
commit 00e8eb2
Show file tree

Hide file tree

Showing 11 changed files with 115 additions and 55 deletions.
diff --git a/.gitlab/pipeline/zombienet/polkadot.yml b/.gitlab/pipeline/zombienet/polkadot.yml
@@ -209,7 +209,7 @@ zombienet-polkadot-misc-0002-upgrade-node:
     # Exit if the job is not merge queue
     # - if [[ $CI_COMMIT_REF_NAME != *"gh-readonly-queue"* ]]; then echo "I will run only in a merge queue"; exit 0; fi
     - export ZOMBIENET_INTEGRATION_TEST_IMAGE="docker.io/parity/polkadot:latest"
-    - echo "Overrided poladot image ${ZOMBIENET_INTEGRATION_TEST_IMAGE}"
+    - echo "Overrided polkadot image ${ZOMBIENET_INTEGRATION_TEST_IMAGE}"
     - export COL_IMAGE="${COLANDER_IMAGE}":${PIPELINE_IMAGE_TAG}
     - BUILD_LINUX_JOB_ID="$(cat ./artifacts/BUILD_LINUX_JOB_ID)"
     - export POLKADOT_PR_ARTIFACTS_URL="https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/${BUILD_LINUX_JOB_ID}/artifacts/raw/artifacts"

diff --git a/cumulus/pallets/parachain-system/src/lib.rs b/cumulus/pallets/parachain-system/src/lib.rs
@@ -1630,7 +1630,7 @@ impl<T: Config> Pallet<T> {
 
 	/// Get the relay chain block number which was used as an anchor for the last block in this
 	/// chain.
-	pub fn last_relay_block_number(&self) -> RelayChainBlockNumber {
+	pub fn last_relay_block_number() -> RelayChainBlockNumber {
 		LastRelayChainBlockNumber::<T>::get()
 	}
 }

diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs
@@ -531,7 +531,6 @@ parameter_types! {
 	pub const SignedDepositByte: Balance = deposit(0, 10) / 1024;
 	// Each good submission will get 1 WND as reward
 	pub SignedRewardBase: Balance = 1 * UNITS;
-	pub BetterUnsignedThreshold: Perbill = Perbill::from_rational(5u32, 10_000);
 
 	// 1 hour session, 15 minutes unsigned phase, 4 offchain executions.
 	pub OffchainRepeat: BlockNumber = UnsignedPhase::get() / 4;
@@ -608,7 +607,6 @@ impl pallet_election_provider_multi_phase::Config for Runtime {
 	type MinerConfig = Self;
 	type SlashHandler = (); // burn slashes
 	type RewardHandler = (); // nothing to do upon rewards
-	type BetterUnsignedThreshold = BetterUnsignedThreshold;
 	type BetterSignedThreshold = ();
 	type OffchainRepeat = OffchainRepeat;
 	type MinerTxPriority = NposSolutionPriority;

diff --git a/polkadot/zombienet_tests/misc/0002-upgrade-node.toml b/polkadot/zombienet_tests/misc/0002-upgrade-node.toml
@@ -9,12 +9,10 @@ chain = "rococo-local"
   [[relaychain.nodes]]
   name = "alice"
   args = [ "-lparachain=debug,runtime=debug", "--db paritydb" ]
-  substrate_cli_args_version = 1
 
   [[relaychain.nodes]]
   name = "bob"
   args = [ "-lparachain=debug,runtime=debug", "--db rocksdb" ]
-  substrate_cli_args_version = 1
 
   [[relaychain.nodes]]
   name = "charlie"

diff --git a/prdoc/pr_2694.prdoc b/prdoc/pr_2694.prdoc
@@ -0,0 +1,14 @@
+# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
+# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
+
+title: "pallet-election-provider-multi-phase: Removes `BetterUnsignedThreshold` from pallet config"
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      Removes thresholding for accepting solutions better than the last queued for unsigned phase. This is unnecessary
+      as even without thresholding, the number of solutions that can be submitted to on-chain which is better than the
+      previous one is limited.
+
+crates:
+  - name: "pallet-election-provider-multi-phase"
diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs
@@ -704,8 +704,6 @@ parameter_types! {
 	pub const SignedDepositIncreaseFactor: Percent = Percent::from_percent(10);
 	pub const SignedDepositByte: Balance = 1 * CENTS;
 
-	pub BetterUnsignedThreshold: Perbill = Perbill::from_rational(1u32, 10_000);
-
 	// miner configs
 	pub const MultiPhaseUnsignedPriority: TransactionPriority = StakingUnsignedPriority::get() - 1u64;
 	pub MinerMaxWeight: Weight = RuntimeBlockWeights::get()
@@ -822,7 +820,6 @@ impl pallet_election_provider_multi_phase::Config for Runtime {
 	type EstimateCallFee = TransactionPayment;
 	type SignedPhase = SignedPhase;
 	type UnsignedPhase = UnsignedPhase;
-	type BetterUnsignedThreshold = BetterUnsignedThreshold;
 	type BetterSignedThreshold = ();
 	type OffchainRepeat = OffchainRepeat;
 	type MinerTxPriority = MultiPhaseUnsignedPriority;

diff --git a/substrate/bin/utils/chain-spec-builder/src/lib.rs b/substrate/bin/utils/chain-spec-builder/src/lib.rs
@@ -30,46 +30,52 @@
 //! ## Typical use-cases.
 //! ##### Get default config from runtime.
 //!
-//!	Query the default genesis config from the provided `runtime.wasm` and use it in the chain
-//! spec. Tool can also store runtime's default genesis config in given file:
-//!	```text
-//! chain-spec-builder create -r runtime.wasm default /dev/stdout
+//! Query the default genesis config from the provided `runtime.wasm` and use it in the chain
+//! spec. The tool allows specifying where to write the chain spec, and optionally also where the
+//! write the default genesis state config (which is `/dev/stdout` in the following example):
+//! ```text
+//! chain-spec-builder --chain_spec_path ./my_chain_spec.json create -r runtime.wasm default /dev/stdout
 //! ```
-//! 
-//! _Note:_ [`GenesisBuilder::create_default_config`][sp-genesis-builder-create] runtime function is called.
+//!
+//! _Note:_ [`GenesisBuilder::create_default_config`][sp-genesis-builder-create] runtime function is
+//! called.
 //!
 //!
 //! ##### Generate raw storage chain spec using genesis config patch.
 //!
 //! Patch the runtime's default genesis config with provided `patch.json` and generate raw
 //! storage (`-s`) version of chain spec:
-//! ```text
+//!
+//! ```bash
 //! chain-spec-builder create -s -r runtime.wasm patch patch.json
 //! ```
-//! 
+//!
 //! _Note:_ [`GenesisBuilder::build_config`][sp-genesis-builder-build] runtime function is called.
 //!
 //! ##### Generate raw storage chain spec using full genesis config.
 //!
 //! Build the chain spec using provided full genesis config json file. No defaults will be used:
-//! ```text
+//!
+//! ```bash
 //! chain-spec-builder create -s -r runtime.wasm full full-genesis-config.json
 //! ```
-//! 
+//!
 //! _Note_: [`GenesisBuilder::build_config`][sp-genesis-builder-build] runtime function is called.
 //!
 //! ##### Generate human readable chain spec using provided genesis config patch.
-//! ```text
+//! ```bash
 //! chain-spec-builder create -r runtime.wasm patch patch.json
 //! ```
-//! 
+//!
 //! ##### Generate human readable chain spec using provided full genesis config.
-//! ```text
+//!
+//! ```bash
 //! chain-spec-builder create -r runtime.wasm full full-genesis-config.json
 //! ```
-//! 
+//!
 //! ##### Extra tools.
-//! The `chain-spec-builder` provides also some extra utilities: [`VerifyCmd`], [`ConvertToRawCmd`], [`UpdateCodeCmd`].
+//! The `chain-spec-builder` provides also some extra utilities: [`VerifyCmd`], [`ConvertToRawCmd`],
+//! [`UpdateCodeCmd`].
 //!
 //! [`sc-chain-spec`]: ../sc_chain_spec/index.html
 //! [`node-cli`]: ../node_cli/index.html

diff --git a/substrate/frame/election-provider-multi-phase/src/lib.rs b/substrate/frame/election-provider-multi-phase/src/lib.rs
@@ -101,9 +101,8 @@
 //! unsigned transaction, thus the name _unsigned_ phase. This unsigned transaction can never be
 //! valid if propagated, and it acts similar to an inherent.
 //!
-//! Validators will only submit solutions if the one that they have computed is sufficiently better
-//! than the best queued one (see [`pallet::Config::BetterUnsignedThreshold`]) and will limit the
-//! weight of the solution to [`MinerConfig::MaxWeight`].
+//! Validators will only submit solutions if the one that they have computed is strictly better than
+//! the best queued one and will limit the weight of the solution to [`MinerConfig::MaxWeight`].
 //!
 //! The unsigned phase can be made passive depending on how the previous signed phase went, by
 //! setting the first inner value of [`Phase`] to `false`. For now, the signed phase is always
@@ -598,11 +597,6 @@ pub mod pallet {
 		#[pallet::constant]
 		type BetterSignedThreshold: Get<Perbill>;
 
-		/// The minimum amount of improvement to the solution score that defines a solution as
-		/// "better" in the Unsigned phase.
-		#[pallet::constant]
-		type BetterUnsignedThreshold: Get<Perbill>;
-
 		/// The repeat threshold of the offchain worker.
 		///
 		/// For example, if it is 5, that means that at least 5 blocks will elapse between attempts

diff --git a/substrate/frame/election-provider-multi-phase/src/mock.rs b/substrate/frame/election-provider-multi-phase/src/mock.rs
@@ -301,7 +301,6 @@ parameter_types! {
 	pub static SignedMaxWeight: Weight = BlockWeights::get().max_block;
 	pub static MinerTxPriority: u64 = 100;
 	pub static BetterSignedThreshold: Perbill = Perbill::zero();
-	pub static BetterUnsignedThreshold: Perbill = Perbill::zero();
 	pub static OffchainRepeat: BlockNumber = 5;
 	pub static MinerMaxWeight: Weight = BlockWeights::get().max_block;
 	pub static MinerMaxLength: u32 = 256;
@@ -399,7 +398,6 @@ impl crate::Config for Runtime {
 	type EstimateCallFee = frame_support::traits::ConstU32<8>;
 	type SignedPhase = SignedPhase;
 	type UnsignedPhase = UnsignedPhase;
-	type BetterUnsignedThreshold = BetterUnsignedThreshold;
 	type BetterSignedThreshold = BetterSignedThreshold;
 	type OffchainRepeat = OffchainRepeat;
 	type MinerTxPriority = MinerTxPriority;
@@ -538,10 +536,7 @@ impl ExtBuilder {
 		<BetterSignedThreshold>::set(p);
 		self
 	}
-	pub fn better_unsigned_threshold(self, p: Perbill) -> Self {
-		<BetterUnsignedThreshold>::set(p);
-		self
-	}
+
 	pub fn phases(self, signed: BlockNumber, unsigned: BlockNumber) -> Self {
 		<SignedPhase>::set(signed);
 		<UnsignedPhase>::set(unsigned);

diff --git a/substrate/frame/election-provider-multi-phase/src/unsigned.rs b/substrate/frame/election-provider-multi-phase/src/unsigned.rs
@@ -384,9 +384,8 @@ impl<T: Config> Pallet<T> {
 
 		// ensure score is being improved. Panic henceforth.
 		ensure!(
-			Self::queued_solution().map_or(true, |q: ReadySolution<_, _>| raw_solution
-				.score
-				.strict_threshold_better(q.score, T::BetterUnsignedThreshold::get())),
+			Self::queued_solution()
+				.map_or(true, |q: ReadySolution<_, _>| raw_solution.score > q.score),
 			Error::<T>::PreDispatchWeakSubmission,
 		);
 
@@ -1025,7 +1024,7 @@ mod tests {
 		bounded_vec,
 		offchain::storage_lock::{BlockAndTime, StorageLock},
 		traits::{Dispatchable, ValidateUnsigned, Zero},
-		ModuleError, PerU16, Perbill,
+		ModuleError, PerU16,
 	};
 
 	type Assignment = crate::unsigned::Assignment<Runtime>;
@@ -1360,20 +1359,23 @@ mod tests {
 			.desired_targets(1)
 			.add_voter(7, 2, bounded_vec![10])
 			.add_voter(8, 5, bounded_vec![10])
-			.better_unsigned_threshold(Perbill::from_percent(50))
+			.add_voter(9, 1, bounded_vec![10])
 			.build_and_execute(|| {
 				roll_to_unsigned();
 				assert!(MultiPhase::current_phase().is_unsigned());
 				assert_eq!(MultiPhase::desired_targets().unwrap(), 1);
 
 				// an initial solution
 				let result = ElectionResult {
-					// note: This second element of backing stake is not important here.
-					winners: vec![(10, 10)],
-					assignments: vec![Assignment {
-						who: 10,
-						distribution: vec![(10, PerU16::one())],
-					}],
+					winners: vec![(10, 12)],
+					assignments: vec![
+						Assignment { who: 10, distribution: vec![(10, PerU16::one())] },
+						Assignment {
+							who: 7,
+							// note: this percent doesn't even matter, in solution it is 100%.
+							distribution: vec![(10, PerU16::one())],
+						},
+					],
 				};
 
 				let RoundSnapshot { voters, targets } = MultiPhase::snapshot().unwrap();
@@ -1394,9 +1396,35 @@ mod tests {
 					Box::new(solution),
 					witness
 				));
-				assert_eq!(MultiPhase::queued_solution().unwrap().score.minimal_stake, 10);
+				assert_eq!(MultiPhase::queued_solution().unwrap().score.minimal_stake, 12);
 
-				// trial 1: a solution who's score is only 2, i.e. 20% better in the first element.
+				// trial 1: a solution who's minimal stake is 10, i.e. worse than the first solution
+				// of 12.
+				let result = ElectionResult {
+					winners: vec![(10, 10)],
+					assignments: vec![Assignment {
+						who: 10,
+						distribution: vec![(10, PerU16::one())],
+					}],
+				};
+				let (raw, score, _, _) = Miner::<Runtime>::prepare_election_result_with_snapshot(
+					result,
+					voters.clone(),
+					targets.clone(),
+					desired_targets,
+				)
+				.unwrap();
+				let solution = RawSolution { solution: raw, score, round: MultiPhase::round() };
+				// 10 is not better than 12
+				assert_eq!(solution.score.minimal_stake, 10);
+				// submitting this will actually panic.
+				assert_noop!(
+					MultiPhase::unsigned_pre_dispatch_checks(&solution),
+					Error::<Runtime>::PreDispatchWeakSubmission,
+				);
+
+				// trial 2: try resubmitting another solution with same score (12) as the queued
+				// solution.
 				let result = ElectionResult {
 					winners: vec![(10, 12)],
 					assignments: vec![
@@ -1408,6 +1436,7 @@ mod tests {
 						},
 					],
 				};
+
 				let (raw, score, _, _) = Miner::<Runtime>::prepare_election_result_with_snapshot(
 					result,
 					voters.clone(),
@@ -1416,15 +1445,45 @@ mod tests {
 				)
 				.unwrap();
 				let solution = RawSolution { solution: raw, score, round: MultiPhase::round() };
-				// 12 is not 50% more than 10
+				// 12 is not better than 12. We need score of atleast 13 to be accepted.
 				assert_eq!(solution.score.minimal_stake, 12);
+				// submitting this will panic.
 				assert_noop!(
 					MultiPhase::unsigned_pre_dispatch_checks(&solution),
 					Error::<Runtime>::PreDispatchWeakSubmission,
 				);
-				// submitting this will actually panic.
 
-				// trial 2: a solution who's score is only 7, i.e. 70% better in the first element.
+				// trial 3: a solution who's minimal stake is 13, i.e. 1 better than the queued
+				// solution of 12.
+				let result = ElectionResult {
+					winners: vec![(10, 12)],
+					assignments: vec![
+						Assignment { who: 10, distribution: vec![(10, PerU16::one())] },
+						Assignment { who: 7, distribution: vec![(10, PerU16::one())] },
+						Assignment { who: 9, distribution: vec![(10, PerU16::one())] },
+					],
+				};
+				let (raw, score, witness, _) =
+					Miner::<Runtime>::prepare_election_result_with_snapshot(
+						result,
+						voters.clone(),
+						targets.clone(),
+						desired_targets,
+					)
+					.unwrap();
+				let solution = RawSolution { solution: raw, score, round: MultiPhase::round() };
+				assert_eq!(solution.score.minimal_stake, 13);
+
+				// this should work
+				assert_ok!(MultiPhase::unsigned_pre_dispatch_checks(&solution));
+				assert_ok!(MultiPhase::submit_unsigned(
+					RuntimeOrigin::none(),
+					Box::new(solution),
+					witness
+				));
+
+				// trial 4: a solution who's minimal stake is 17, i.e. 4 better than the last
+				// soluton.
 				let result = ElectionResult {
 					winners: vec![(10, 12)],
 					assignments: vec![

diff --git a/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs b/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs
@@ -190,7 +190,6 @@ impl pallet_election_provider_multi_phase::Config for Runtime {
 	type SignedPhase = SignedPhase;
 	type UnsignedPhase = UnsignedPhase;
 	type BetterSignedThreshold = ();
-	type BetterUnsignedThreshold = ();
 	type OffchainRepeat = OffchainRepeat;
 	type MinerTxPriority = TransactionPriority;
 	type MinerConfig = Self;