From 00e8eb2e04375260d728f143352e21cbf7a4436c Mon Sep 17 00:00:00 2001 From: Clara van Staden Date: Sat, 16 Dec 2023 19:53:57 +0200 Subject: [PATCH] Update from master (#69) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * parachain-system: Do not take `self` for `last_relay_block_number` (#2720) FRAME DSL is working in a static context. * fix zombienet test (#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 (#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 (#2693) Example currently broken. It writes something to the given path, but not the full chain spec. --------- Co-authored-by: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> * update lock file --------- Co-authored-by: Bastian Köcher Co-authored-by: Javier Viola Co-authored-by: Ankan <10196091+Ank4n@users.noreply.github.com> Co-authored-by: Liam Aharon Co-authored-by: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Co-authored-by: claravanstaden --- .gitlab/pipeline/zombienet/polkadot.yml | 2 +- cumulus/pallets/parachain-system/src/lib.rs | 2 +- polkadot/runtime/westend/src/lib.rs | 2 - .../misc/0002-upgrade-node.toml | 2 - prdoc/pr_2694.prdoc | 14 +++ substrate/bin/node/runtime/src/lib.rs | 3 - .../bin/utils/chain-spec-builder/src/lib.rs | 36 +++++--- .../election-provider-multi-phase/src/lib.rs | 10 +- .../election-provider-multi-phase/src/mock.rs | 7 +- .../src/unsigned.rs | 91 +++++++++++++++---- .../test-staking-e2e/src/mock.rs | 1 - 11 files changed, 115 insertions(+), 55 deletions(-) create mode 100644 prdoc/pr_2694.prdoc diff --git a/.gitlab/pipeline/zombienet/polkadot.yml b/.gitlab/pipeline/zombienet/polkadot.yml index 356abaa93cdd..6b89648c4e36 100644 --- 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 index bac1ee28a7ca..fc2c83627fb5 100644 --- a/cumulus/pallets/parachain-system/src/lib.rs +++ b/cumulus/pallets/parachain-system/src/lib.rs @@ -1630,7 +1630,7 @@ impl Pallet { /// 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::::get() } } diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index 0f068b093f5a..e958a660e6ec 100644 --- 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 index b6fff6c8cb83..1edb18abcece 100644 --- 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 new file mode 100644 index 000000000000..c393dcfeb9a8 --- /dev/null +++ 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 index 4d0b253413a8..76a5c2bf65f7 100644 --- 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 index 60ec0f1a656b..8c78030c8854 100644 --- 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 index 41284f6c78b1..04325a40d0ad 100644 --- 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; - /// The minimum amount of improvement to the solution score that defines a solution as - /// "better" in the Unsigned phase. - #[pallet::constant] - type BetterUnsignedThreshold: Get; - /// 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 index abad7db037e7..af126f08d510 100644 --- 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 { ::set(p); self } - pub fn better_unsigned_threshold(self, p: Perbill) -> Self { - ::set(p); - self - } + pub fn phases(self, signed: BlockNumber, unsigned: BlockNumber) -> Self { ::set(signed); ::set(unsigned); diff --git a/substrate/frame/election-provider-multi-phase/src/unsigned.rs b/substrate/frame/election-provider-multi-phase/src/unsigned.rs index e3d0ded97515..943481813340 100644 --- 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 Pallet { // 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::::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; @@ -1360,7 +1359,7 @@ 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()); @@ -1368,12 +1367,15 @@ mod tests { // 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::::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::::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::::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::::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::::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 index ecb2ae435b8c..04d218acf8fd 100644 --- 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;