diff --git a/Cargo.lock b/Cargo.lock index 29d15b9e015cb..09da4278f69f4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6018,6 +6018,7 @@ dependencies = [ "parity-scale-codec", "scale-info", "serde", + "sp-arithmetic", "sp-core", "sp-io", "sp-runtime", @@ -9500,6 +9501,7 @@ dependencies = [ "rand 0.7.3", "scale-info", "serde", + "sp-core", "sp-debug-derive", "sp-std", "static_assertions", diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index c98bb5cc13f85..9b5bc0273ab33 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -31,7 +31,7 @@ use frame_support::{ pallet_prelude::Get, parameter_types, traits::{ - AsEnsureOriginWithArg, ConstU128, ConstU16, ConstU32, Currency, EnsureOneOf, + AsEnsureOriginWithArg, ConstU128, ConstU16, ConstU32, Currency, EitherOfDiverse, EqualPrivilegeOnly, Everything, Imbalance, InstanceFilter, KeyOwnerProofSystem, LockIdentifier, Nothing, OnUnbalanced, U128CurrencyToVote, }, @@ -550,7 +550,7 @@ impl pallet_staking::Config for Runtime { type BondingDuration = BondingDuration; type SlashDeferDuration = SlashDeferDuration; /// A super-majority of the council can cancel the slash. - type SlashCancelOrigin = EnsureOneOf< + type SlashCancelOrigin = EitherOfDiverse< EnsureRoot, pallet_collective::EnsureProportionAtLeast, >; @@ -794,12 +794,14 @@ impl pallet_referenda::TracksInfo for TracksInfo { confirm_period: 2, min_enactment_period: 4, min_approval: pallet_referenda::Curve::LinearDecreasing { - begin: Perbill::from_percent(100), - delta: Perbill::from_percent(50), + length: Perbill::from_percent(100), + floor: Perbill::from_percent(50), + ceil: Perbill::from_percent(100), }, - min_turnout: pallet_referenda::Curve::LinearDecreasing { - begin: Perbill::from_percent(100), - delta: Perbill::from_percent(100), + min_support: pallet_referenda::Curve::LinearDecreasing { + length: Perbill::from_percent(100), + floor: Perbill::from_percent(0), + ceil: Perbill::from_percent(100), }, }, )]; @@ -882,7 +884,7 @@ impl pallet_democracy::Config for Runtime { pallet_collective::EnsureProportionAtLeast; // To cancel a proposal before it has been passed, the technical committee must be unanimous or // Root must agree. - type CancelProposalOrigin = EnsureOneOf< + type CancelProposalOrigin = EitherOfDiverse< EnsureRoot, pallet_collective::EnsureProportionAtLeast, >; @@ -972,7 +974,7 @@ impl pallet_collective::Config for Runtime { type WeightInfo = pallet_collective::weights::SubstrateWeight; } -type EnsureRootOrHalfCouncil = EnsureOneOf< +type EnsureRootOrHalfCouncil = EitherOfDiverse< EnsureRoot, pallet_collective::EnsureProportionMoreThan, >; @@ -1006,11 +1008,11 @@ parameter_types! { impl pallet_treasury::Config for Runtime { type PalletId = TreasuryPalletId; type Currency = Balances; - type ApproveOrigin = EnsureOneOf< + type ApproveOrigin = EitherOfDiverse< EnsureRoot, pallet_collective::EnsureProportionAtLeast, >; - type RejectOrigin = EnsureOneOf< + type RejectOrigin = EitherOfDiverse< EnsureRoot, pallet_collective::EnsureProportionMoreThan, >; @@ -1025,6 +1027,7 @@ impl pallet_treasury::Config for Runtime { type SpendFunds = Bounties; type WeightInfo = pallet_treasury::weights::SubstrateWeight; type MaxApprovals = MaxApprovals; + type SpendOrigin = frame_support::traits::NeverEnsureOrigin; } parameter_types! { diff --git a/docs/Upgrading-2.0-to-3.0.md b/docs/Upgrading-2.0-to-3.0.md index 017467ede2d7e..f750c6dd5865b 100644 --- a/docs/Upgrading-2.0-to-3.0.md +++ b/docs/Upgrading-2.0-to-3.0.md @@ -290,7 +290,7 @@ Democracy brings three new settings with this release, all to allow for better i type CancellationOrigin = pallet_collective::EnsureProportionAtLeast<_2, _3, AccountId, CouncilCollective>; + // To cancel a proposal before it has been passed, the technical committee must be unanimous or + // Root must agree. -+ type CancelProposalOrigin = EnsureOneOf< ++ type CancelProposalOrigin = EitherOfDiverse< + AccountId, + EnsureRoot, + pallet_collective::EnsureProportionAtLeast<_1, _1, AccountId, TechnicalCollective>, diff --git a/frame/bounties/src/tests.rs b/frame/bounties/src/tests.rs index 9a84bd687abc1..0904e3a2901bb 100644 --- a/frame/bounties/src/tests.rs +++ b/frame/bounties/src/tests.rs @@ -109,7 +109,6 @@ parameter_types! { pub const TreasuryPalletId: PalletId = PalletId(*b"py/trsry"); } -// impl pallet_treasury::Config for Test { impl pallet_treasury::Config for Test { type PalletId = TreasuryPalletId; type Currency = pallet_balances::Pallet; @@ -126,6 +125,7 @@ impl pallet_treasury::Config for Test { type WeightInfo = (); type SpendFunds = Bounties; type MaxApprovals = ConstU32<100>; + type SpendOrigin = frame_support::traits::NeverEnsureOrigin; } parameter_types! { diff --git a/frame/child-bounties/src/tests.rs b/frame/child-bounties/src/tests.rs index 61545561a26c3..2584445071471 100644 --- a/frame/child-bounties/src/tests.rs +++ b/frame/child-bounties/src/tests.rs @@ -130,6 +130,7 @@ impl pallet_treasury::Config for Test { type WeightInfo = (); type SpendFunds = Bounties; type MaxApprovals = ConstU32<100>; + type SpendOrigin = frame_support::traits::NeverEnsureOrigin; } parameter_types! { // This will be 50% of the bounty fee. diff --git a/frame/conviction-voting/src/tests.rs b/frame/conviction-voting/src/tests.rs index 6a8bad5d8944e..9eb7f679efca3 100644 --- a/frame/conviction-voting/src/tests.rs +++ b/frame/conviction-voting/src/tests.rs @@ -21,7 +21,7 @@ use std::collections::BTreeMap; use frame_support::{ assert_noop, assert_ok, parameter_types, - traits::{ConstU32, ConstU64, Contains, Polling}, + traits::{ConstU32, ConstU64, Contains, Polling, VoteTally}, }; use sp_core::H256; use sp_runtime::{ @@ -166,7 +166,7 @@ impl Polling> for TestPolls { fn create_ongoing(class: Self::Class) -> Result { let mut polls = Polls::get(); let i = polls.keys().rev().next().map_or(0, |x| x + 1); - polls.insert(i, Ongoing(Tally::default(), class)); + polls.insert(i, Ongoing(Tally::new(0), class)); Polls::set(polls); Ok(i) } @@ -271,19 +271,19 @@ fn basic_voting_works() { assert_ok!(Voting::vote(Origin::signed(1), 3, aye(2, 5))); assert_eq!(tally(3), Tally::from_parts(10, 0, 2)); assert_ok!(Voting::vote(Origin::signed(1), 3, nay(2, 5))); - assert_eq!(tally(3), Tally::from_parts(0, 10, 2)); + assert_eq!(tally(3), Tally::from_parts(0, 10, 0)); assert_eq!(Balances::usable_balance(1), 8); assert_ok!(Voting::vote(Origin::signed(1), 3, aye(5, 1))); assert_eq!(tally(3), Tally::from_parts(5, 0, 5)); assert_ok!(Voting::vote(Origin::signed(1), 3, nay(5, 1))); - assert_eq!(tally(3), Tally::from_parts(0, 5, 5)); + assert_eq!(tally(3), Tally::from_parts(0, 5, 0)); assert_eq!(Balances::usable_balance(1), 5); assert_ok!(Voting::vote(Origin::signed(1), 3, aye(10, 0))); assert_eq!(tally(3), Tally::from_parts(1, 0, 10)); assert_ok!(Voting::vote(Origin::signed(1), 3, nay(10, 0))); - assert_eq!(tally(3), Tally::from_parts(0, 1, 10)); + assert_eq!(tally(3), Tally::from_parts(0, 1, 0)); assert_eq!(Balances::usable_balance(1), 0); assert_ok!(Voting::remove_vote(Origin::signed(1), None, 3)); @@ -300,19 +300,19 @@ fn voting_balance_gets_locked() { assert_ok!(Voting::vote(Origin::signed(1), 3, aye(2, 5))); assert_eq!(tally(3), Tally::from_parts(10, 0, 2)); assert_ok!(Voting::vote(Origin::signed(1), 3, nay(2, 5))); - assert_eq!(tally(3), Tally::from_parts(0, 10, 2)); + assert_eq!(tally(3), Tally::from_parts(0, 10, 0)); assert_eq!(Balances::usable_balance(1), 8); assert_ok!(Voting::vote(Origin::signed(1), 3, aye(5, 1))); assert_eq!(tally(3), Tally::from_parts(5, 0, 5)); assert_ok!(Voting::vote(Origin::signed(1), 3, nay(5, 1))); - assert_eq!(tally(3), Tally::from_parts(0, 5, 5)); + assert_eq!(tally(3), Tally::from_parts(0, 5, 0)); assert_eq!(Balances::usable_balance(1), 5); assert_ok!(Voting::vote(Origin::signed(1), 3, aye(10, 0))); assert_eq!(tally(3), Tally::from_parts(1, 0, 10)); assert_ok!(Voting::vote(Origin::signed(1), 3, nay(10, 0))); - assert_eq!(tally(3), Tally::from_parts(0, 1, 10)); + assert_eq!(tally(3), Tally::from_parts(0, 1, 0)); assert_eq!(Balances::usable_balance(1), 0); assert_ok!(Voting::remove_vote(Origin::signed(1), None, 3)); @@ -376,10 +376,10 @@ fn classwise_delegation_works() { new_test_ext().execute_with(|| { Polls::set( vec![ - (0, Ongoing(Tally::default(), 0)), - (1, Ongoing(Tally::default(), 1)), - (2, Ongoing(Tally::default(), 2)), - (3, Ongoing(Tally::default(), 2)), + (0, Ongoing(Tally::new(0), 0)), + (1, Ongoing(Tally::new(0), 1)), + (2, Ongoing(Tally::new(0), 2)), + (3, Ongoing(Tally::new(0), 2)), ] .into_iter() .collect(), @@ -403,9 +403,9 @@ fn classwise_delegation_works() { assert_eq!( Polls::get(), vec![ - (0, Ongoing(Tally::from_parts(6, 2, 35), 0)), - (1, Ongoing(Tally::from_parts(6, 2, 35), 1)), - (2, Ongoing(Tally::from_parts(6, 2, 35), 2)), + (0, Ongoing(Tally::from_parts(6, 2, 15), 0)), + (1, Ongoing(Tally::from_parts(6, 2, 15), 1)), + (2, Ongoing(Tally::from_parts(6, 2, 15), 2)), (3, Ongoing(Tally::from_parts(0, 0, 0), 2)), ] .into_iter() @@ -417,10 +417,10 @@ fn classwise_delegation_works() { assert_eq!( Polls::get(), vec![ - (0, Ongoing(Tally::from_parts(6, 2, 35), 0)), - (1, Ongoing(Tally::from_parts(6, 2, 35), 1)), - (2, Ongoing(Tally::from_parts(6, 2, 35), 2)), - (3, Ongoing(Tally::from_parts(0, 6, 15), 2)), + (0, Ongoing(Tally::from_parts(6, 2, 15), 0)), + (1, Ongoing(Tally::from_parts(6, 2, 15), 1)), + (2, Ongoing(Tally::from_parts(6, 2, 15), 2)), + (3, Ongoing(Tally::from_parts(0, 6, 0), 2)), ] .into_iter() .collect() @@ -432,10 +432,10 @@ fn classwise_delegation_works() { assert_eq!( Polls::get(), vec![ - (0, Ongoing(Tally::from_parts(6, 2, 35), 0)), - (1, Ongoing(Tally::from_parts(6, 2, 35), 1)), - (2, Ongoing(Tally::from_parts(1, 7, 35), 2)), - (3, Ongoing(Tally::from_parts(0, 1, 10), 2)), + (0, Ongoing(Tally::from_parts(6, 2, 15), 0)), + (1, Ongoing(Tally::from_parts(6, 2, 15), 1)), + (2, Ongoing(Tally::from_parts(1, 7, 10), 2)), + (3, Ongoing(Tally::from_parts(0, 1, 0), 2)), ] .into_iter() .collect() @@ -451,10 +451,10 @@ fn classwise_delegation_works() { assert_eq!( Polls::get(), vec![ - (0, Ongoing(Tally::from_parts(4, 2, 33), 0)), - (1, Ongoing(Tally::from_parts(4, 2, 33), 1)), - (2, Ongoing(Tally::from_parts(4, 2, 33), 2)), - (3, Ongoing(Tally::from_parts(0, 4, 13), 2)), + (0, Ongoing(Tally::from_parts(4, 2, 13), 0)), + (1, Ongoing(Tally::from_parts(4, 2, 13), 1)), + (2, Ongoing(Tally::from_parts(4, 2, 13), 2)), + (3, Ongoing(Tally::from_parts(0, 4, 0), 2)), ] .into_iter() .collect() @@ -483,10 +483,10 @@ fn classwise_delegation_works() { assert_eq!( Polls::get(), vec![ - (0, Ongoing(Tally::from_parts(7, 2, 36), 0)), - (1, Ongoing(Tally::from_parts(8, 2, 37), 1)), - (2, Ongoing(Tally::from_parts(9, 2, 38), 2)), - (3, Ongoing(Tally::from_parts(0, 9, 18), 2)), + (0, Ongoing(Tally::from_parts(7, 2, 16), 0)), + (1, Ongoing(Tally::from_parts(8, 2, 17), 1)), + (2, Ongoing(Tally::from_parts(9, 2, 18), 2)), + (3, Ongoing(Tally::from_parts(0, 9, 0), 2)), ] .into_iter() .collect() @@ -497,7 +497,7 @@ fn classwise_delegation_works() { #[test] fn redelegation_after_vote_ending_should_keep_lock() { new_test_ext().execute_with(|| { - Polls::set(vec![(0, Ongoing(Tally::default(), 0))].into_iter().collect()); + Polls::set(vec![(0, Ongoing(Tally::new(0), 0))].into_iter().collect()); assert_ok!(Voting::delegate(Origin::signed(1), 0, 2, Conviction::Locked1x, 5)); assert_ok!(Voting::vote(Origin::signed(2), 0, aye(10, 1))); Polls::set(vec![(0, Completed(1, true))].into_iter().collect()); @@ -515,9 +515,9 @@ fn lock_amalgamation_valid_with_multiple_removed_votes() { new_test_ext().execute_with(|| { Polls::set( vec![ - (0, Ongoing(Tally::default(), 0)), - (1, Ongoing(Tally::default(), 0)), - (2, Ongoing(Tally::default(), 0)), + (0, Ongoing(Tally::new(0), 0)), + (1, Ongoing(Tally::new(0), 0)), + (2, Ongoing(Tally::new(0), 0)), ] .into_iter() .collect(), @@ -587,7 +587,7 @@ fn lock_amalgamation_valid_with_multiple_delegations() { #[test] fn lock_amalgamation_valid_with_move_roundtrip_to_delegation() { new_test_ext().execute_with(|| { - Polls::set(vec![(0, Ongoing(Tally::default(), 0))].into_iter().collect()); + Polls::set(vec![(0, Ongoing(Tally::new(0), 0))].into_iter().collect()); assert_ok!(Voting::vote(Origin::signed(1), 0, aye(5, 1))); Polls::set(vec![(0, Completed(1, true))].into_iter().collect()); assert_ok!(Voting::remove_vote(Origin::signed(1), Some(0), 0)); @@ -599,7 +599,7 @@ fn lock_amalgamation_valid_with_move_roundtrip_to_delegation() { assert_ok!(Voting::unlock(Origin::signed(1), 0, 1)); assert_eq!(Balances::usable_balance(1), 0); - Polls::set(vec![(1, Ongoing(Tally::default(), 0))].into_iter().collect()); + Polls::set(vec![(1, Ongoing(Tally::new(0), 0))].into_iter().collect()); assert_ok!(Voting::vote(Origin::signed(1), 1, aye(5, 2))); Polls::set(vec![(1, Completed(1, true))].into_iter().collect()); assert_ok!(Voting::remove_vote(Origin::signed(1), Some(0), 1)); @@ -627,7 +627,7 @@ fn lock_amalgamation_valid_with_move_roundtrip_to_casting() { assert_ok!(Voting::unlock(Origin::signed(1), 0, 1)); assert_eq!(Balances::usable_balance(1), 5); - Polls::set(vec![(0, Ongoing(Tally::default(), 0))].into_iter().collect()); + Polls::set(vec![(0, Ongoing(Tally::new(0), 0))].into_iter().collect()); assert_ok!(Voting::vote(Origin::signed(1), 0, aye(10, 1))); Polls::set(vec![(0, Completed(1, true))].into_iter().collect()); assert_ok!(Voting::remove_vote(Origin::signed(1), Some(0), 0)); @@ -688,9 +688,9 @@ fn lock_aggregation_over_different_classes_with_casting_works() { new_test_ext().execute_with(|| { Polls::set( vec![ - (0, Ongoing(Tally::default(), 0)), - (1, Ongoing(Tally::default(), 1)), - (2, Ongoing(Tally::default(), 2)), + (0, Ongoing(Tally::new(0), 0)), + (1, Ongoing(Tally::new(0), 1)), + (2, Ongoing(Tally::new(0), 2)), ] .into_iter() .collect(), @@ -747,10 +747,10 @@ fn errors_with_vote_work() { assert_ok!(Voting::undelegate(Origin::signed(1), 0)); Polls::set( vec![ - (0, Ongoing(Tally::default(), 0)), - (1, Ongoing(Tally::default(), 0)), - (2, Ongoing(Tally::default(), 0)), - (3, Ongoing(Tally::default(), 0)), + (0, Ongoing(Tally::new(0), 0)), + (1, Ongoing(Tally::new(0), 0)), + (2, Ongoing(Tally::new(0), 0)), + (3, Ongoing(Tally::new(0), 0)), ] .into_iter() .collect(), diff --git a/frame/conviction-voting/src/types.rs b/frame/conviction-voting/src/types.rs index e2b5844ddd5df..2469009855c5f 100644 --- a/frame/conviction-voting/src/types.rs +++ b/frame/conviction-voting/src/types.rs @@ -17,18 +17,16 @@ //! Miscellaneous additional datatypes. -use sp_std::marker::PhantomData; - use codec::{Codec, Decode, Encode, MaxEncodedLen}; use frame_support::{ - traits::VoteTally, CloneNoBound, DefaultNoBound, EqNoBound, PartialEqNoBound, - RuntimeDebugNoBound, + traits::VoteTally, CloneNoBound, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound, }; use scale_info::TypeInfo; use sp_runtime::{ traits::{Saturating, Zero}, RuntimeDebug, }; +use sp_std::{fmt::Debug, marker::PhantomData}; use super::*; use crate::{AccountVote, Conviction, Vote}; @@ -36,7 +34,6 @@ use crate::{AccountVote, Conviction, Vote}; /// Info regarding an ongoing referendum. #[derive( CloneNoBound, - DefaultNoBound, PartialEqNoBound, EqNoBound, RuntimeDebugNoBound, @@ -46,84 +43,84 @@ use crate::{AccountVote, Conviction, Vote}; MaxEncodedLen, )] #[scale_info(skip_type_params(Total))] -pub struct Tally< - Votes: Clone + Default + PartialEq + Eq + sp_std::fmt::Debug + TypeInfo + Codec, - Total, -> { +pub struct Tally { /// The number of aye votes, expressed in terms of post-conviction lock-vote. pub ayes: Votes, /// The number of nay votes, expressed in terms of post-conviction lock-vote. pub nays: Votes, - /// The amount of funds currently expressing its opinion. Pre-conviction. - pub turnout: Votes, + /// The basic number of aye votes, expressed pre-conviction. + pub support: Votes, /// Dummy. dummy: PhantomData, } impl< - Votes: Clone - + Default - + PartialEq - + Eq - + sp_std::fmt::Debug - + Copy - + AtLeast32BitUnsigned - + TypeInfo - + Codec, + Votes: Clone + Default + PartialEq + Eq + Debug + Copy + AtLeast32BitUnsigned + TypeInfo + Codec, Total: Get, - > VoteTally for Tally + Class, + > VoteTally for Tally { - fn ayes(&self) -> Votes { + fn new(_: Class) -> Self { + Self { ayes: Zero::zero(), nays: Zero::zero(), support: Zero::zero(), dummy: PhantomData } + } + + fn ayes(&self, _: Class) -> Votes { self.ayes } - fn turnout(&self) -> Perbill { - Perbill::from_rational(self.turnout, Total::get()) + fn support(&self, _: Class) -> Perbill { + Perbill::from_rational(self.support, Total::get()) } - fn approval(&self) -> Perbill { + fn approval(&self, _: Class) -> Perbill { Perbill::from_rational(self.ayes, self.ayes.saturating_add(self.nays)) } #[cfg(feature = "runtime-benchmarks")] - fn unanimity() -> Self { - Self { ayes: Total::get(), nays: Zero::zero(), turnout: Total::get(), dummy: PhantomData } + fn unanimity(_: Class) -> Self { + Self { ayes: Total::get(), nays: Zero::zero(), support: Total::get(), dummy: PhantomData } + } + + #[cfg(feature = "runtime-benchmarks")] + fn rejection(_: Class) -> Self { + Self { ayes: Zero::zero(), nays: Total::get(), support: Total::get(), dummy: PhantomData } } #[cfg(feature = "runtime-benchmarks")] - fn from_requirements(turnout: Perbill, approval: Perbill) -> Self { - let turnout = turnout.mul_ceil(Total::get()); - let ayes = approval.mul_ceil(turnout); - Self { ayes, nays: turnout - ayes, turnout, dummy: PhantomData } + fn from_requirements(support: Perbill, approval: Perbill, _: Class) -> Self { + let support = support.mul_ceil(Total::get()); + let ayes = approval.mul_ceil(support); + Self { ayes, nays: support - ayes, support, dummy: PhantomData } } } impl< - Votes: Clone - + Default - + PartialEq - + Eq - + sp_std::fmt::Debug - + Copy - + AtLeast32BitUnsigned - + TypeInfo - + Codec, + Votes: Clone + Default + PartialEq + Eq + Debug + Copy + AtLeast32BitUnsigned + TypeInfo + Codec, Total: Get, > Tally { /// Create a new tally. - pub fn new(vote: Vote, balance: Votes) -> Self { + pub fn from_vote(vote: Vote, balance: Votes) -> Self { let Delegations { votes, capital } = vote.conviction.votes(balance); Self { ayes: if vote.aye { votes } else { Zero::zero() }, nays: if vote.aye { Zero::zero() } else { votes }, - turnout: capital, + support: capital, dummy: PhantomData, } } - pub fn from_parts(ayes: Votes, nays: Votes, turnout: Votes) -> Self { - Self { ayes, nays, turnout, dummy: PhantomData } + pub fn from_parts( + ayes_with_conviction: Votes, + nays_with_conviction: Votes, + ayes: Votes, + ) -> Self { + Self { + ayes: ayes_with_conviction, + nays: nays_with_conviction, + support: ayes, + dummy: PhantomData, + } } /// Add an account's vote into the tally. @@ -131,16 +128,18 @@ impl< match vote { AccountVote::Standard { vote, balance } => { let Delegations { votes, capital } = vote.conviction.votes(balance); - self.turnout = self.turnout.checked_add(&capital)?; match vote.aye { - true => self.ayes = self.ayes.checked_add(&votes)?, + true => { + self.support = self.support.checked_add(&capital)?; + self.ayes = self.ayes.checked_add(&votes)? + }, false => self.nays = self.nays.checked_add(&votes)?, } }, AccountVote::Split { aye, nay } => { let aye = Conviction::None.votes(aye); let nay = Conviction::None.votes(nay); - self.turnout = self.turnout.checked_add(&aye.capital)?.checked_add(&nay.capital)?; + self.support = self.support.checked_add(&aye.capital)?; self.ayes = self.ayes.checked_add(&aye.votes)?; self.nays = self.nays.checked_add(&nay.votes)?; }, @@ -153,16 +152,18 @@ impl< match vote { AccountVote::Standard { vote, balance } => { let Delegations { votes, capital } = vote.conviction.votes(balance); - self.turnout = self.turnout.checked_sub(&capital)?; match vote.aye { - true => self.ayes = self.ayes.checked_sub(&votes)?, + true => { + self.support = self.support.checked_sub(&capital)?; + self.ayes = self.ayes.checked_sub(&votes)? + }, false => self.nays = self.nays.checked_sub(&votes)?, } }, AccountVote::Split { aye, nay } => { let aye = Conviction::None.votes(aye); let nay = Conviction::None.votes(nay); - self.turnout = self.turnout.checked_sub(&aye.capital)?.checked_sub(&nay.capital)?; + self.support = self.support.checked_sub(&aye.capital)?; self.ayes = self.ayes.checked_sub(&aye.votes)?; self.nays = self.nays.checked_sub(&nay.votes)?; }, @@ -172,18 +173,22 @@ impl< /// Increment some amount of votes. pub fn increase(&mut self, approve: bool, delegations: Delegations) { - self.turnout = self.turnout.saturating_add(delegations.capital); match approve { - true => self.ayes = self.ayes.saturating_add(delegations.votes), + true => { + self.support = self.support.saturating_add(delegations.capital); + self.ayes = self.ayes.saturating_add(delegations.votes); + }, false => self.nays = self.nays.saturating_add(delegations.votes), } } /// Decrement some amount of votes. pub fn reduce(&mut self, approve: bool, delegations: Delegations) { - self.turnout = self.turnout.saturating_sub(delegations.capital); match approve { - true => self.ayes = self.ayes.saturating_sub(delegations.votes), + true => { + self.support = self.support.saturating_sub(delegations.capital); + self.ayes = self.ayes.saturating_sub(delegations.votes); + }, false => self.nays = self.nays.saturating_sub(delegations.votes), } } @@ -196,7 +201,7 @@ impl< pub struct Delegations { /// The number of votes (this is post-conviction). pub votes: Balance, - /// The amount of raw capital, used for the turnout. + /// The amount of raw capital, used for the support. pub capital: Balance, } diff --git a/frame/identity/src/tests.rs b/frame/identity/src/tests.rs index 8cb0563ebeaa1..6066f176a6106 100644 --- a/frame/identity/src/tests.rs +++ b/frame/identity/src/tests.rs @@ -23,7 +23,7 @@ use crate as pallet_identity; use codec::{Decode, Encode}; use frame_support::{ assert_noop, assert_ok, ord_parameter_types, parameter_types, - traits::{ConstU32, ConstU64, EnsureOneOf}, + traits::{ConstU32, ConstU64, EitherOfDiverse}, BoundedVec, }; use frame_system::{EnsureRoot, EnsureSignedBy}; @@ -100,8 +100,8 @@ ord_parameter_types! { pub const One: u64 = 1; pub const Two: u64 = 2; } -type EnsureOneOrRoot = EnsureOneOf, EnsureSignedBy>; -type EnsureTwoOrRoot = EnsureOneOf, EnsureSignedBy>; +type EnsureOneOrRoot = EitherOfDiverse, EnsureSignedBy>; +type EnsureTwoOrRoot = EitherOfDiverse, EnsureSignedBy>; impl pallet_identity::Config for Test { type Event = Event; type Currency = Balances; diff --git a/frame/referenda/Cargo.toml b/frame/referenda/Cargo.toml index 75888c6d14f3d..ef3d5fe5a8e06 100644 --- a/frame/referenda/Cargo.toml +++ b/frame/referenda/Cargo.toml @@ -19,6 +19,7 @@ codec = { package = "parity-scale-codec", version = "3.0.0", default-features = ] } scale-info = { version = "2.1.1", default-features = false, features = ["derive"] } serde = { version = "1.0.136", features = ["derive"], optional = true } +sp-arithmetic = { version = "5.0.0", default-features = false, path = "../../primitives/arithmetic" } frame-benchmarking = { version = "4.0.0-dev", default-features = false, optional = true, path = "../benchmarking" } frame-support = { version = "4.0.0-dev", default-features = false, path = "../support" } frame-system = { version = "4.0.0-dev", default-features = false, path = "../system" } @@ -39,6 +40,8 @@ std = [ "codec/std", "frame-benchmarking/std", "frame-support/std", + "sp-runtime/std", + "sp-arithmetic/std", "frame-system/std", "scale-info/std", "serde", diff --git a/frame/referenda/src/benchmarking.rs b/frame/referenda/src/benchmarking.rs index 87b13868db9d9..e4cbf2ca52e7c 100644 --- a/frame/referenda/src/benchmarking.rs +++ b/frame/referenda/src/benchmarking.rs @@ -101,27 +101,27 @@ fn info(index: ReferendumIndex) -> &'static TrackInfoOf { } fn make_passing_after(index: ReferendumIndex, period_portion: Perbill) { - let turnout = info::(index).min_turnout.threshold(period_portion); + let support = info::(index).min_support.threshold(period_portion); let approval = info::(index).min_approval.threshold(period_portion); Referenda::::access_poll(index, |status| { - if let PollStatus::Ongoing(tally, ..) = status { - *tally = T::Tally::from_requirements(turnout, approval); + if let PollStatus::Ongoing(tally, class) = status { + *tally = T::Tally::from_requirements(support, approval, class); } }); } fn make_passing(index: ReferendumIndex) { Referenda::::access_poll(index, |status| { - if let PollStatus::Ongoing(tally, ..) = status { - *tally = T::Tally::unanimity(); + if let PollStatus::Ongoing(tally, class) = status { + *tally = T::Tally::unanimity(class); } }); } fn make_failing(index: ReferendumIndex) { Referenda::::access_poll(index, |status| { - if let PollStatus::Ongoing(tally, ..) = status { - *tally = T::Tally::default(); + if let PollStatus::Ongoing(tally, class) = status { + *tally = T::Tally::rejection(class); } }); } @@ -501,6 +501,7 @@ benchmarks! { let (_caller, index) = create_referendum::(); place_deposit::(index); skip_prepare_period::(index); + make_failing::(index); nudge::(index); skip_decision_period::(index); }: nudge_referendum(RawOrigin::Root, index) diff --git a/frame/referenda/src/lib.rs b/frame/referenda/src/lib.rs index b53be191d3525..e3f31bc411328 100644 --- a/frame/referenda/src/lib.rs +++ b/frame/referenda/src/lib.rs @@ -41,7 +41,7 @@ //! In order to become concluded, one of three things must happen: //! - The referendum should remain in an unbroken _Passing_ state for a period of time. This //! is known as the _Confirmation Period_ and is determined by the track. A referendum is considered -//! _Passing_ when there is a sufficiently high turnout and approval, given the amount of time it +//! _Passing_ when there is a sufficiently high support and approval, given the amount of time it //! has been being decided. Generally the threshold for what counts as being "sufficiently high" //! will reduce over time. The curves setting these thresholds are determined by the track. In this //! case, the referendum is considered _Approved_ and the proposal is scheduled for dispatch. @@ -54,6 +54,10 @@ //! //! Once a referendum is concluded, the decision deposit may be refunded. //! +//! ## Terms +//! - *Support*: The number of aye-votes, pre-conviction, as a proportion of the total number of +//! pre-conviction votes able to be cast in the population. +//! //! - [`Config`] //! - [`Call`] @@ -148,7 +152,12 @@ pub mod pallet { /// The counting type for votes. Usually just balance. type Votes: AtLeast32BitUnsigned + Copy + Parameter + Member; /// The tallying type. - type Tally: VoteTally + Default + Clone + Codec + Eq + Debug + TypeInfo; + type Tally: VoteTally> + + Clone + + Codec + + Eq + + Debug + + TypeInfo; // Constants /// The minimum amount to be used as a deposit for a public referendum proposal. @@ -369,7 +378,7 @@ pub mod pallet { submission_deposit, decision_deposit: None, deciding: None, - tally: Default::default(), + tally: TallyOf::::new(track), in_queue: false, alarm: Self::set_alarm(nudge_call, now.saturating_add(T::UndecidingTimeout::get())), }; @@ -613,7 +622,7 @@ impl, I: 'static> Polling for Pallet { submission_deposit: Deposit { who: dummy_account_id, amount: Zero::zero() }, decision_deposit: None, deciding: None, - tally: Default::default(), + tally: TallyOf::::new(class), in_queue: false, alarm: None, }; @@ -723,8 +732,9 @@ impl, I: 'static> Pallet { &status.tally, Zero::zero(), track.decision_period, - &track.min_turnout, + &track.min_support, &track.min_approval, + status.track, ); status.in_queue = false; Self::deposit_event(Event::::DecisionStarted { @@ -740,7 +750,7 @@ impl, I: 'static> Pallet { None }; let deciding_status = DecidingStatus { since: now, confirming }; - let alarm = Self::decision_time(&deciding_status, &status.tally, track); + let alarm = Self::decision_time(&deciding_status, &status.tally, status.track, track); status.deciding = Some(deciding_status); let branch = if is_passing { BeginDecidingBranch::Passing } else { BeginDecidingBranch::Failing }; @@ -765,7 +775,7 @@ impl, I: 'static> Pallet { (r.0, r.1.into()) } else { // Add to queue. - let item = (index, status.tally.ayes()); + let item = (index, status.tally.ayes(status.track)); status.in_queue = true; TrackQueue::::mutate(status.track, |q| q.insert_sorted_by_key(item, |x| x.1)); (None, ServiceBranch::Queued) @@ -872,7 +882,7 @@ impl, I: 'static> Pallet { // Are we already queued for deciding? if status.in_queue { // Does our position in the queue need updating? - let ayes = status.tally.ayes(); + let ayes = status.tally.ayes(status.track); let mut queue = TrackQueue::::get(status.track); let maybe_old_pos = queue.iter().position(|(x, _)| *x == index); let new_pos = queue.binary_search_by_key(&ayes, |x| x.1).unwrap_or_else(|x| x); @@ -930,8 +940,9 @@ impl, I: 'static> Pallet { &status.tally, now.saturating_sub(deciding.since), track.decision_period, - &track.min_turnout, + &track.min_support, &track.min_approval, + status.track, ); branch = if is_passing { match deciding.confirming { @@ -996,7 +1007,7 @@ impl, I: 'static> Pallet { ServiceBranch::ContinueNotConfirming } }; - alarm = Self::decision_time(deciding, &status.tally, track); + alarm = Self::decision_time(deciding, &status.tally, status.track, track); }, } @@ -1009,15 +1020,16 @@ impl, I: 'static> Pallet { fn decision_time( deciding: &DecidingStatusOf, tally: &T::Tally, + track_id: TrackIdOf, track: &TrackInfoOf, ) -> T::BlockNumber { deciding.confirming.unwrap_or_else(|| { // Set alarm to the point where the current voting would make it pass. - let approval = tally.approval(); - let turnout = tally.turnout(); + let approval = tally.approval(track_id); + let support = tally.support(track_id); let until_approval = track.min_approval.delay(approval); - let until_turnout = track.min_turnout.delay(turnout); - let offset = until_turnout.max(until_approval); + let until_support = track.min_support.delay(support); + let offset = until_support.max(until_approval); deciding.since.saturating_add(offset * track.decision_period) }) } @@ -1062,16 +1074,18 @@ impl, I: 'static> Pallet { } /// Determine whether the given `tally` would result in a referendum passing at `elapsed` blocks - /// into a total decision `period`, given the two curves for `turnout_needed` and + /// into a total decision `period`, given the two curves for `support_needed` and /// `approval_needed`. fn is_passing( tally: &T::Tally, elapsed: T::BlockNumber, period: T::BlockNumber, - turnout_needed: &Curve, + support_needed: &Curve, approval_needed: &Curve, + id: TrackIdOf, ) -> bool { let x = Perbill::from_rational(elapsed.min(period), period); - turnout_needed.passing(x, tally.turnout()) && approval_needed.passing(x, tally.approval()) + support_needed.passing(x, tally.support(id)) && + approval_needed.passing(x, tally.approval(id)) } } diff --git a/frame/referenda/src/mock.rs b/frame/referenda/src/mock.rs index 1b0bbba24bbe6..a3026ce78e986 100644 --- a/frame/referenda/src/mock.rs +++ b/frame/referenda/src/mock.rs @@ -160,12 +160,14 @@ impl TracksInfo for TestTracksInfo { confirm_period: 2, min_enactment_period: 4, min_approval: Curve::LinearDecreasing { - begin: Perbill::from_percent(100), - delta: Perbill::from_percent(50), + length: Perbill::from_percent(100), + floor: Perbill::from_percent(50), + ceil: Perbill::from_percent(100), }, - min_turnout: Curve::LinearDecreasing { - begin: Perbill::from_percent(100), - delta: Perbill::from_percent(100), + min_support: Curve::LinearDecreasing { + length: Perbill::from_percent(100), + floor: Perbill::from_percent(0), + ceil: Perbill::from_percent(100), }, }, ), @@ -180,12 +182,14 @@ impl TracksInfo for TestTracksInfo { confirm_period: 1, min_enactment_period: 2, min_approval: Curve::LinearDecreasing { - begin: Perbill::from_percent(55), - delta: Perbill::from_percent(5), + length: Perbill::from_percent(100), + floor: Perbill::from_percent(95), + ceil: Perbill::from_percent(100), }, - min_turnout: Curve::LinearDecreasing { - begin: Perbill::from_percent(10), - delta: Perbill::from_percent(10), + min_support: Curve::LinearDecreasing { + length: Perbill::from_percent(100), + floor: Perbill::from_percent(90), + ceil: Perbill::from_percent(100), }, }, ), @@ -241,35 +245,48 @@ pub fn new_test_ext_execute_with_cond(execute: impl FnOnce(bool) -> () + Clone) new_test_ext().execute_with(|| execute(true)); } -#[derive(Encode, Debug, Decode, TypeInfo, Eq, PartialEq, Clone, Default, MaxEncodedLen)] +#[derive(Encode, Debug, Decode, TypeInfo, Eq, PartialEq, Clone, MaxEncodedLen)] pub struct Tally { pub ayes: u32, pub nays: u32, } -impl VoteTally for Tally { - fn ayes(&self) -> u32 { +impl VoteTally for Tally { + fn new(_: Class) -> Self { + Self { ayes: 0, nays: 0 } + } + + fn ayes(&self, _: Class) -> u32 { self.ayes } - fn turnout(&self) -> Perbill { - Perbill::from_percent(self.ayes + self.nays) + fn support(&self, _: Class) -> Perbill { + Perbill::from_percent(self.ayes) } - fn approval(&self) -> Perbill { - Perbill::from_rational(self.ayes, self.ayes + self.nays) + fn approval(&self, _: Class) -> Perbill { + if self.ayes + self.nays > 0 { + Perbill::from_rational(self.ayes, self.ayes + self.nays) + } else { + Perbill::zero() + } } #[cfg(feature = "runtime-benchmarks")] - fn unanimity() -> Self { + fn unanimity(_: Class) -> Self { Self { ayes: 100, nays: 0 } } #[cfg(feature = "runtime-benchmarks")] - fn from_requirements(turnout: Perbill, approval: Perbill) -> Self { - let turnout = turnout.mul_ceil(100u32); - let ayes = approval.mul_ceil(turnout); - Self { ayes, nays: turnout - ayes } + fn rejection(_: Class) -> Self { + Self { ayes: 0, nays: 100 } + } + + #[cfg(feature = "runtime-benchmarks")] + fn from_requirements(support: Perbill, approval: Perbill, _: Class) -> Self { + let ayes = support.mul_ceil(100u32); + let nays = ((ayes as u64) * 1_000_000_000u64 / approval.deconstruct() as u64) as u32 - ayes; + Self { ayes, nays } } } diff --git a/frame/referenda/src/tests.rs b/frame/referenda/src/tests.rs index 96edd4ce879ce..8134e024dda39 100644 --- a/frame/referenda/src/tests.rs +++ b/frame/referenda/src/tests.rs @@ -504,12 +504,14 @@ fn set_balance_proposal_is_correctly_filtered_out() { #[test] fn curve_handles_all_inputs() { - let test_curve = Curve::LinearDecreasing { begin: Perbill::zero(), delta: Perbill::zero() }; + let test_curve = Curve::LinearDecreasing { + length: Perbill::one(), + floor: Perbill::zero(), + ceil: Perbill::from_percent(100), + }; let delay = test_curve.delay(Perbill::zero()); - assert_eq!(delay, Perbill::zero()); - - let test_curve = Curve::LinearDecreasing { begin: Perbill::zero(), delta: Perbill::one() }; + assert_eq!(delay, Perbill::one()); let threshold = test_curve.threshold(Perbill::one()); assert_eq!(threshold, Perbill::zero()); diff --git a/frame/referenda/src/types.rs b/frame/referenda/src/types.rs index 5e0361c8fe160..27b3c6a5a9d59 100644 --- a/frame/referenda/src/types.rs +++ b/frame/referenda/src/types.rs @@ -21,7 +21,8 @@ use super::*; use codec::{Decode, Encode, EncodeLike, MaxEncodedLen}; use frame_support::{traits::schedule::Anon, Parameter}; use scale_info::TypeInfo; -use sp_runtime::RuntimeDebug; +use sp_arithmetic::{Rounding::*, SignedRounding::*}; +use sp_runtime::{FixedI64, PerThing, RuntimeDebug}; use sp_std::fmt::Debug; pub type BalanceOf = @@ -91,40 +92,6 @@ impl> InsertSorted for BoundedVec { } } -#[cfg(test)] -mod tests { - use super::*; - use frame_support::traits::ConstU32; - - #[test] - fn insert_sorted_works() { - let mut b: BoundedVec> = vec![20, 30, 40].try_into().unwrap(); - assert!(b.insert_sorted_by_key(10, |&x| x)); - assert_eq!(&b[..], &[10, 20, 30, 40][..]); - - assert!(b.insert_sorted_by_key(60, |&x| x)); - assert_eq!(&b[..], &[10, 20, 30, 40, 60][..]); - - assert!(b.insert_sorted_by_key(50, |&x| x)); - assert_eq!(&b[..], &[10, 20, 30, 40, 50, 60][..]); - - assert!(!b.insert_sorted_by_key(9, |&x| x)); - assert_eq!(&b[..], &[10, 20, 30, 40, 50, 60][..]); - - assert!(b.insert_sorted_by_key(11, |&x| x)); - assert_eq!(&b[..], &[11, 20, 30, 40, 50, 60][..]); - - assert!(b.insert_sorted_by_key(21, |&x| x)); - assert_eq!(&b[..], &[20, 21, 30, 40, 50, 60][..]); - - assert!(b.insert_sorted_by_key(61, |&x| x)); - assert_eq!(&b[..], &[21, 30, 40, 50, 60, 61][..]); - - assert!(b.insert_sorted_by_key(51, |&x| x)); - assert_eq!(&b[..], &[30, 40, 50, 51, 60, 61][..]); - } -} - #[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo, MaxEncodedLen)] pub struct DecidingStatus { /// When this referendum began being "decided". If confirming, then the @@ -143,7 +110,7 @@ pub struct Deposit { #[derive(Clone, Encode, TypeInfo)] pub struct TrackInfo { - /// Name of this track. TODO was &'static str + /// Name of this track. pub name: &'static str, /// A limit for the number of referenda on this track that can be being decided at once. /// For Root origin this should generally be just one. @@ -161,9 +128,9 @@ pub struct TrackInfo { /// Minimum aye votes as percentage of overall conviction-weighted votes needed for /// approval as a function of time into decision period. pub min_approval: Curve, - /// Minimum turnout as percentage of overall population that is needed for - /// approval as a function of time into decision period. - pub min_turnout: Curve, + /// Minimum pre-conviction aye-votes ("support") as percentage of overall population that is + /// needed for approval as a function of time into decision period. + pub min_support: Curve, } /// Information on the voting tracks. @@ -282,21 +249,186 @@ impl< #[derive(Clone, Eq, PartialEq, Encode, Decode, TypeInfo, MaxEncodedLen)] #[cfg_attr(not(feature = "std"), derive(RuntimeDebug))] pub enum Curve { - /// Linear curve starting at `(0, begin)`, ending at `(period, begin - delta)`. - LinearDecreasing { begin: Perbill, delta: Perbill }, + /// Linear curve starting at `(0, ceil)`, proceeding linearly to `(length, floor)`, then + /// remaining at `floor` until the end of the period. + LinearDecreasing { length: Perbill, floor: Perbill, ceil: Perbill }, + /// Stepped curve, beginning at `(0, begin)`, then remaining constant for `period`, at which + /// point it steps down to `(period, begin - step)`. It then remains constant for another + /// `period` before stepping down to `(period * 2, begin - step * 2)`. This pattern continues + /// but the `y` component has a lower limit of `end`. + SteppedDecreasing { begin: Perbill, end: Perbill, step: Perbill, period: Perbill }, + /// A recipocal (`K/(x+S)-T`) curve: `factor` is `K` and `x_offset` is `S`, `y_offset` is `T`. + Reciprocal { factor: FixedI64, x_offset: FixedI64, y_offset: FixedI64 }, +} + +/// Calculate the quadratic solution for the given curve. +/// +/// WARNING: This is a `const` function designed for convenient use at build time and +/// will panic on overflow. Ensure that any inputs are sensible. +const fn pos_quad_solution(a: FixedI64, b: FixedI64, c: FixedI64) -> FixedI64 { + const TWO: FixedI64 = FixedI64::from_u32(2); + const FOUR: FixedI64 = FixedI64::from_u32(4); + b.neg().add(b.mul(b).sub(FOUR.mul(a).mul(c)).sqrt()).div(TWO.mul(a)) } impl Curve { + /// Create a `Curve::Linear` instance from a high-level description. + /// + /// WARNING: This is a `const` function designed for convenient use at build time and + /// will panic on overflow. Ensure that any inputs are sensible. + pub const fn make_linear(length: u128, period: u128, floor: FixedI64, ceil: FixedI64) -> Curve { + let length = FixedI64::from_rational(length, period).into_perbill(); + let floor = floor.into_perbill(); + let ceil = ceil.into_perbill(); + Curve::LinearDecreasing { length, floor, ceil } + } + + /// Create a `Curve::Reciprocal` instance from a high-level description. + /// + /// WARNING: This is a `const` function designed for convenient use at build time and + /// will panic on overflow. Ensure that any inputs are sensible. + pub const fn make_reciprocal( + delay: u128, + period: u128, + level: FixedI64, + floor: FixedI64, + ceil: FixedI64, + ) -> Curve { + let delay = FixedI64::from_rational(delay, period).into_perbill(); + let mut bounds = ( + ( + FixedI64::from_u32(0), + Self::reciprocal_from_parts(FixedI64::from_u32(0), floor, ceil), + FixedI64::from_inner(i64::max_value()), + ), + ( + FixedI64::from_u32(1), + Self::reciprocal_from_parts(FixedI64::from_u32(1), floor, ceil), + FixedI64::from_inner(i64::max_value()), + ), + ); + const TWO: FixedI64 = FixedI64::from_u32(2); + while (bounds.1).0.sub((bounds.0).0).into_inner() > 1 { + let factor = (bounds.0).0.add((bounds.1).0).div(TWO); + let curve = Self::reciprocal_from_parts(factor, floor, ceil); + let curve_level = FixedI64::from_perbill(curve.const_threshold(delay)); + if curve_level.into_inner() > level.into_inner() { + bounds = (bounds.0, (factor, curve, curve_level.sub(level))); + } else { + bounds = ((factor, curve, level.sub(curve_level)), bounds.1); + } + } + if (bounds.0).2.into_inner() < (bounds.1).2.into_inner() { + (bounds.0).1 + } else { + (bounds.1).1 + } + } + + /// Create a `Curve::Reciprocal` instance from basic parameters. + /// + /// WARNING: This is a `const` function designed for convenient use at build time and + /// will panic on overflow. Ensure that any inputs are sensible. + const fn reciprocal_from_parts(factor: FixedI64, floor: FixedI64, ceil: FixedI64) -> Self { + let delta = ceil.sub(floor); + let x_offset = pos_quad_solution(delta, delta, factor.neg()); + let y_offset = floor.sub(factor.div(FixedI64::from_u32(1).add(x_offset))); + Curve::Reciprocal { factor, x_offset, y_offset } + } + + /// Print some info on the curve. + #[cfg(feature = "std")] + pub fn info(&self, days: u32, name: impl std::fmt::Display) { + let hours = days * 24; + println!("Curve {} := {:?}:", name, self); + println!(" t + 0h: {:?}", self.threshold(Perbill::zero())); + println!(" t + 1h: {:?}", self.threshold(Perbill::from_rational(1, hours))); + println!(" t + 2h: {:?}", self.threshold(Perbill::from_rational(2, hours))); + println!(" t + 3h: {:?}", self.threshold(Perbill::from_rational(3, hours))); + println!(" t + 6h: {:?}", self.threshold(Perbill::from_rational(6, hours))); + println!(" t + 12h: {:?}", self.threshold(Perbill::from_rational(12, hours))); + println!(" t + 24h: {:?}", self.threshold(Perbill::from_rational(24, hours))); + let mut l = 0; + for &(n, d) in [(1, 12), (1, 8), (1, 4), (1, 2), (3, 4), (1, 1)].iter() { + let t = days * n / d; + if t != l { + println!(" t + {}d: {:?}", t, self.threshold(Perbill::from_rational(t, days))); + l = t; + } + } + let t = |p: Perbill| -> std::string::String { + if p.is_one() { + "never".into() + } else { + let minutes = p * (hours * 60); + if minutes < 60 { + format!("{} minutes", minutes) + } else if minutes < 8 * 60 && minutes % 60 != 0 { + format!("{} hours {} minutes", minutes / 60, minutes % 60) + } else if minutes < 72 * 60 { + format!("{} hours", minutes / 60) + } else if minutes / 60 % 24 == 0 { + format!("{} days", minutes / 60 / 24) + } else { + format!("{} days {} hours", minutes / 60 / 24, minutes / 60 % 24) + } + } + }; + if self.delay(Perbill::from_percent(49)) < Perbill::one() { + println!(" 30% threshold: {}", t(self.delay(Perbill::from_percent(30)))); + println!(" 10% threshold: {}", t(self.delay(Perbill::from_percent(10)))); + println!(" 3% threshold: {}", t(self.delay(Perbill::from_percent(3)))); + println!(" 1% threshold: {}", t(self.delay(Perbill::from_percent(1)))); + println!(" 0.1% threshold: {}", t(self.delay(Perbill::from_rational(1u32, 1_000)))); + println!(" 0.01% threshold: {}", t(self.delay(Perbill::from_rational(1u32, 10_000)))); + } else { + println!( + " 99.9% threshold: {}", + t(self.delay(Perbill::from_rational(999u32, 1_000))) + ); + println!(" 99% threshold: {}", t(self.delay(Perbill::from_percent(99)))); + println!(" 95% threshold: {}", t(self.delay(Perbill::from_percent(95)))); + println!(" 90% threshold: {}", t(self.delay(Perbill::from_percent(90)))); + println!(" 75% threshold: {}", t(self.delay(Perbill::from_percent(75)))); + println!(" 60% threshold: {}", t(self.delay(Perbill::from_percent(60)))); + } + } + /// Determine the `y` value for the given `x` value. pub(crate) fn threshold(&self, x: Perbill) -> Perbill { match self { - Self::LinearDecreasing { begin, delta } => *begin - (*delta * x).min(*begin), + Self::LinearDecreasing { length, floor, ceil } => + *ceil - (x.min(*length).saturating_div(*length, Down) * (*ceil - *floor)), + Self::SteppedDecreasing { begin, end, step, period } => + (*begin - (step.int_mul(x.int_div(*period))).min(*begin)).max(*end), + Self::Reciprocal { factor, x_offset, y_offset } => factor + .checked_rounding_div(FixedI64::from(x) + *x_offset, Low) + .map(|yp| (yp + *y_offset).into_clamped_perthing()) + .unwrap_or_else(Perbill::one), + } + } + + /// Determine the `y` value for the given `x` value. + /// + /// This is a partial implementation designed only for use in const functions. + const fn const_threshold(&self, x: Perbill) -> Perbill { + match self { + Self::Reciprocal { factor, x_offset, y_offset } => { + match factor.checked_rounding_div(FixedI64::from_perbill(x).add(*x_offset), Low) { + Some(yp) => (yp.add(*y_offset)).into_perbill(), + None => Perbill::one(), + } + }, + _ => panic!("const_threshold cannot be used on this curve"), } } /// Determine the smallest `x` value such that `passing` returns `true` when passed along with /// the given `y` value. /// + /// If `passing` never returns `true` for any value of `x` when paired with `y`, then + /// `Perbill::one` may be returned. + /// /// ```nocompile /// let c = Curve::LinearDecreasing { begin: Perbill::one(), delta: Perbill::one() }; /// // ^^^ Can be any curve. @@ -307,12 +439,27 @@ impl Curve { /// ``` pub fn delay(&self, y: Perbill) -> Perbill { match self { - Self::LinearDecreasing { begin, delta } => - if delta.is_zero() { - *delta + Self::LinearDecreasing { length, floor, ceil } => + if y < *floor { + Perbill::one() + } else if y > *ceil { + Perbill::zero() + } else { + (*ceil - y).saturating_div(*ceil - *floor, Up).saturating_mul(*length) + }, + Self::SteppedDecreasing { begin, end, step, period } => + if y < *end { + Perbill::one() } else { - (*begin - y.min(*begin)).min(*delta) / *delta + period.int_mul((*begin - y.min(*begin) + step.less_epsilon()).int_div(*step)) }, + Self::Reciprocal { factor, x_offset, y_offset } => { + let y = FixedI64::from(y); + let maybe_term = factor.checked_rounding_div(y - *y_offset, High); + maybe_term + .and_then(|term| (term - *x_offset).try_into_perthing().ok()) + .unwrap_or_else(Perbill::one) + }, } } @@ -326,14 +473,176 @@ impl Curve { impl Debug for Curve { fn fmt(&self, f: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result { match self { - Self::LinearDecreasing { begin, delta } => { + Self::LinearDecreasing { length, floor, ceil } => { + write!( + f, + "Linear[(0%, {:?}) -> ({:?}, {:?}) -> (100%, {:?})]", + ceil, length, floor, floor, + ) + }, + Self::SteppedDecreasing { begin, end, step, period } => { + write!( + f, + "Stepped[(0%, {:?}) -> (100%, {:?}) by ({:?}, {:?})]", + begin, end, period, step, + ) + }, + Self::Reciprocal { factor, x_offset, y_offset } => { write!( f, - "Linear[(0%, {}%) -> (100%, {}%)]", - *begin * 100u32, - (*begin - *delta) * 100u32, + "Reciprocal[factor of {:?}, x_offset of {:?}, y_offset of {:?}]", + factor, x_offset, y_offset, ) }, } } } + +#[cfg(test)] +mod tests { + use super::*; + use frame_support::traits::ConstU32; + use sp_runtime::PerThing; + + const fn percent(x: u128) -> FixedI64 { + FixedI64::from_rational(x, 100) + } + + const TIP_APP: Curve = Curve::make_linear(10, 28, percent(50), percent(100)); + const TIP_SUP: Curve = Curve::make_reciprocal(1, 28, percent(4), percent(0), percent(50)); + const ROOT_APP: Curve = Curve::make_reciprocal(4, 28, percent(80), percent(50), percent(100)); + const ROOT_SUP: Curve = Curve::make_linear(28, 28, percent(0), percent(50)); + const WHITE_APP: Curve = + Curve::make_reciprocal(16, 28 * 24, percent(96), percent(50), percent(100)); + const WHITE_SUP: Curve = Curve::make_reciprocal(1, 28, percent(20), percent(10), percent(50)); + const SMALL_APP: Curve = Curve::make_linear(10, 28, percent(50), percent(100)); + const SMALL_SUP: Curve = Curve::make_reciprocal(8, 28, percent(1), percent(0), percent(50)); + const MID_APP: Curve = Curve::make_linear(17, 28, percent(50), percent(100)); + const MID_SUP: Curve = Curve::make_reciprocal(12, 28, percent(1), percent(0), percent(50)); + const BIG_APP: Curve = Curve::make_linear(23, 28, percent(50), percent(100)); + const BIG_SUP: Curve = Curve::make_reciprocal(16, 28, percent(1), percent(0), percent(50)); + const HUGE_APP: Curve = Curve::make_linear(28, 28, percent(50), percent(100)); + const HUGE_SUP: Curve = Curve::make_reciprocal(20, 28, percent(1), percent(0), percent(50)); + const PARAM_APP: Curve = Curve::make_reciprocal(4, 28, percent(80), percent(50), percent(100)); + const PARAM_SUP: Curve = Curve::make_reciprocal(7, 28, percent(10), percent(0), percent(50)); + const ADMIN_APP: Curve = Curve::make_linear(17, 28, percent(50), percent(100)); + const ADMIN_SUP: Curve = Curve::make_reciprocal(12, 28, percent(1), percent(0), percent(50)); + + // TODO: ceil for linear. + + #[test] + #[should_panic] + fn check_curves() { + TIP_APP.info(28u32, "Tip Approval"); + TIP_SUP.info(28u32, "Tip Support"); + ROOT_APP.info(28u32, "Root Approval"); + ROOT_SUP.info(28u32, "Root Support"); + WHITE_APP.info(28u32, "Whitelist Approval"); + WHITE_SUP.info(28u32, "Whitelist Support"); + SMALL_APP.info(28u32, "Small Spend Approval"); + SMALL_SUP.info(28u32, "Small Spend Support"); + MID_APP.info(28u32, "Mid Spend Approval"); + MID_SUP.info(28u32, "Mid Spend Support"); + BIG_APP.info(28u32, "Big Spend Approval"); + BIG_SUP.info(28u32, "Big Spend Support"); + HUGE_APP.info(28u32, "Huge Spend Approval"); + HUGE_SUP.info(28u32, "Huge Spend Support"); + PARAM_APP.info(28u32, "Mid-tier Parameter Change Approval"); + PARAM_SUP.info(28u32, "Mid-tier Parameter Change Support"); + ADMIN_APP.info(28u32, "Admin (e.g. Cancel Slash) Approval"); + ADMIN_SUP.info(28u32, "Admin (e.g. Cancel Slash) Support"); + assert!(false); + } + + #[test] + fn insert_sorted_works() { + let mut b: BoundedVec> = vec![20, 30, 40].try_into().unwrap(); + assert!(b.insert_sorted_by_key(10, |&x| x)); + assert_eq!(&b[..], &[10, 20, 30, 40][..]); + + assert!(b.insert_sorted_by_key(60, |&x| x)); + assert_eq!(&b[..], &[10, 20, 30, 40, 60][..]); + + assert!(b.insert_sorted_by_key(50, |&x| x)); + assert_eq!(&b[..], &[10, 20, 30, 40, 50, 60][..]); + + assert!(!b.insert_sorted_by_key(9, |&x| x)); + assert_eq!(&b[..], &[10, 20, 30, 40, 50, 60][..]); + + assert!(b.insert_sorted_by_key(11, |&x| x)); + assert_eq!(&b[..], &[11, 20, 30, 40, 50, 60][..]); + + assert!(b.insert_sorted_by_key(21, |&x| x)); + assert_eq!(&b[..], &[20, 21, 30, 40, 50, 60][..]); + + assert!(b.insert_sorted_by_key(61, |&x| x)); + assert_eq!(&b[..], &[21, 30, 40, 50, 60, 61][..]); + + assert!(b.insert_sorted_by_key(51, |&x| x)); + assert_eq!(&b[..], &[30, 40, 50, 51, 60, 61][..]); + } + + #[test] + fn translated_reciprocal_works() { + let c: Curve = Curve::Reciprocal { + factor: FixedI64::from_float(0.03125), + x_offset: FixedI64::from_float(0.0363306838226), + y_offset: FixedI64::from_float(0.139845532427), + }; + c.info(28u32, "Test"); + + for i in 0..9_696_969u32 { + let query = Perbill::from_rational(i, 9_696_969); + // Determine the nearest point in time when the query will be above threshold. + let delay_needed = c.delay(query); + // Ensure that it actually does pass at that time, or that it will never pass. + assert!(delay_needed.is_one() || c.passing(delay_needed, query)); + } + } + + #[test] + fn stepped_decreasing_works() { + fn pc(x: u32) -> Perbill { + Perbill::from_percent(x) + } + + let c = + Curve::SteppedDecreasing { begin: pc(80), end: pc(30), step: pc(10), period: pc(15) }; + + for i in 0..9_696_969u32 { + let query = Perbill::from_rational(i, 9_696_969); + // Determine the nearest point in time when the query will be above threshold. + let delay_needed = c.delay(query); + // Ensure that it actually does pass at that time, or that it will never pass. + assert!(delay_needed.is_one() || c.passing(delay_needed, query)); + } + + assert_eq!(c.threshold(pc(0)), pc(80)); + assert_eq!(c.threshold(pc(15).less_epsilon()), pc(80)); + assert_eq!(c.threshold(pc(15)), pc(70)); + assert_eq!(c.threshold(pc(30).less_epsilon()), pc(70)); + assert_eq!(c.threshold(pc(30)), pc(60)); + assert_eq!(c.threshold(pc(45).less_epsilon()), pc(60)); + assert_eq!(c.threshold(pc(45)), pc(50)); + assert_eq!(c.threshold(pc(60).less_epsilon()), pc(50)); + assert_eq!(c.threshold(pc(60)), pc(40)); + assert_eq!(c.threshold(pc(75).less_epsilon()), pc(40)); + assert_eq!(c.threshold(pc(75)), pc(30)); + assert_eq!(c.threshold(pc(100)), pc(30)); + + assert_eq!(c.delay(pc(100)), pc(0)); + assert_eq!(c.delay(pc(80)), pc(0)); + assert_eq!(c.delay(pc(80).less_epsilon()), pc(15)); + assert_eq!(c.delay(pc(70)), pc(15)); + assert_eq!(c.delay(pc(70).less_epsilon()), pc(30)); + assert_eq!(c.delay(pc(60)), pc(30)); + assert_eq!(c.delay(pc(60).less_epsilon()), pc(45)); + assert_eq!(c.delay(pc(50)), pc(45)); + assert_eq!(c.delay(pc(50).less_epsilon()), pc(60)); + assert_eq!(c.delay(pc(40)), pc(60)); + assert_eq!(c.delay(pc(40).less_epsilon()), pc(75)); + assert_eq!(c.delay(pc(30)), pc(75)); + assert_eq!(c.delay(pc(30).less_epsilon()), pc(100)); + assert_eq!(c.delay(pc(0)), pc(100)); + } +} diff --git a/frame/scheduler/src/mock.rs b/frame/scheduler/src/mock.rs index ecd04c3e48b52..008105dc737ea 100644 --- a/frame/scheduler/src/mock.rs +++ b/frame/scheduler/src/mock.rs @@ -23,7 +23,7 @@ use crate as scheduler; use frame_support::{ ord_parameter_types, parameter_types, traits::{ - ConstU32, ConstU64, Contains, EnsureOneOf, EqualPrivilegeOnly, OnFinalize, OnInitialize, + ConstU32, ConstU64, Contains, EitherOfDiverse, EqualPrivilegeOnly, OnFinalize, OnInitialize, }, weights::constants::RocksDbWeight, }; @@ -174,7 +174,7 @@ impl Config for Test { type PalletsOrigin = OriginCaller; type Call = Call; type MaximumWeight = MaximumSchedulerWeight; - type ScheduleOrigin = EnsureOneOf, EnsureSignedBy>; + type ScheduleOrigin = EitherOfDiverse, EnsureSignedBy>; type MaxScheduledPerBlock = ConstU32<10>; type WeightInfo = (); type OriginPrivilegeCmp = EqualPrivilegeOnly; diff --git a/frame/staking/src/benchmarking.rs b/frame/staking/src/benchmarking.rs index eb9129ac4b436..12de0ff9cc665 100644 --- a/frame/staking/src/benchmarking.rs +++ b/frame/staking/src/benchmarking.rs @@ -990,7 +990,7 @@ mod tests { let (validator_stash, nominators) = create_validator_with_nominators::( n, - ::MaxNominatorRewardedPerValidator::get(), + <::MaxNominatorRewardedPerValidator as Get<_>>::get(), false, RewardDestination::Staked, ) @@ -1015,7 +1015,7 @@ mod tests { let (validator_stash, _nominators) = create_validator_with_nominators::( n, - ::MaxNominatorRewardedPerValidator::get(), + <::MaxNominatorRewardedPerValidator as Get<_>>::get(), false, RewardDestination::Staked, ) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index ccd9558c5c21d..d0ebef27b4ef6 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -3388,7 +3388,7 @@ fn six_session_delay() { #[test] fn test_max_nominator_rewarded_per_validator_and_cant_steal_someone_else_reward() { ExtBuilder::default().build_and_execute(|| { - for i in 0..=::MaxNominatorRewardedPerValidator::get() { + for i in 0..=<::MaxNominatorRewardedPerValidator as Get<_>>::get() { let stash = 10_000 + i as AccountId; let controller = 20_000 + i as AccountId; let balance = 10_000 + i as Balance; @@ -3411,7 +3411,7 @@ fn test_max_nominator_rewarded_per_validator_and_cant_steal_someone_else_reward( mock::make_all_reward_payment(1); // Assert only nominators from 1 to Max are rewarded - for i in 0..=::MaxNominatorRewardedPerValidator::get() { + for i in 0..=<::MaxNominatorRewardedPerValidator as Get<_>>::get() { let stash = 10_000 + i as AccountId; let balance = 10_000 + i as Balance; if stash == 10_000 { @@ -3649,7 +3649,8 @@ fn payout_stakers_handles_weight_refund() { // Note: this test relies on the assumption that `payout_stakers_alive_staked` is solely used by // `payout_stakers` to calculate the weight of each payout op. ExtBuilder::default().has_stakers(false).build_and_execute(|| { - let max_nom_rewarded = ::MaxNominatorRewardedPerValidator::get(); + let max_nom_rewarded = + <::MaxNominatorRewardedPerValidator as Get<_>>::get(); // Make sure the configured value is meaningful for our use. assert!(max_nom_rewarded >= 4); let half_max_nom_rewarded = max_nom_rewarded / 2; diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 178fddb61b0a2..fa37fa5cda22b 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -350,6 +350,13 @@ macro_rules! parameter_types { I::from(Self::get()) } } + + impl $crate::traits::TypedGet for $name { + type Type = $type; + fn get() -> $type { + Self::get() + } + } }; (IMPL $name:ident, $type:ty, $value:expr) => { impl $name { @@ -364,6 +371,13 @@ macro_rules! parameter_types { I::from(Self::get()) } } + + impl $crate::traits::TypedGet for $name { + type Type = $type; + fn get() -> $type { + Self::get() + } + } }; (IMPL_STORAGE $name:ident, $type:ty, $value:expr) => { impl $name { @@ -397,6 +411,13 @@ macro_rules! parameter_types { I::from(Self::get()) } } + + impl $crate::traits::TypedGet for $name { + type Type = $type; + fn get() -> $type { + Self::get() + } + } }; ( $( #[ $attr:meta ] )* @@ -805,7 +826,7 @@ pub mod tests { }; use codec::{Codec, EncodeLike}; use frame_support::traits::CrateVersion; - use sp_io::TestExternalities; + use sp_io::{MultiRemovalResults, TestExternalities}; use sp_std::result; /// A PalletInfo implementation which just panics. @@ -1065,16 +1086,61 @@ pub mod tests { }); } + #[test] + fn double_map_basic_insert_remove_remove_prefix_with_commit_should_work() { + let key1 = 17u32; + let key2 = 18u32; + type DoubleMap = DataDM; + let mut e = new_test_ext(); + e.execute_with(|| { + // initialized during genesis + assert_eq!(DoubleMap::get(&15u32, &16u32), 42u64); + + // get / insert / take + assert_eq!(DoubleMap::get(&key1, &key2), 0u64); + DoubleMap::insert(&key1, &key2, &4u64); + assert_eq!(DoubleMap::get(&key1, &key2), 4u64); + assert_eq!(DoubleMap::take(&key1, &key2), 4u64); + assert_eq!(DoubleMap::get(&key1, &key2), 0u64); + + // mutate + DoubleMap::mutate(&key1, &key2, |val| *val = 15); + assert_eq!(DoubleMap::get(&key1, &key2), 15u64); + + // remove + DoubleMap::remove(&key1, &key2); + assert_eq!(DoubleMap::get(&key1, &key2), 0u64); + + // remove prefix + DoubleMap::insert(&key1, &key2, &4u64); + DoubleMap::insert(&key1, &(key2 + 1), &4u64); + DoubleMap::insert(&(key1 + 1), &key2, &4u64); + DoubleMap::insert(&(key1 + 1), &(key2 + 1), &4u64); + }); + e.commit_all().unwrap(); + e.execute_with(|| { + assert!(matches!( + DoubleMap::clear_prefix(&key1, u32::max_value(), None), + MultiRemovalResults { maybe_cursor: None, backend: 2, unique: 2, loops: 2 } + )); + assert_eq!(DoubleMap::get(&key1, &key2), 0u64); + assert_eq!(DoubleMap::get(&key1, &(key2 + 1)), 0u64); + assert_eq!(DoubleMap::get(&(key1 + 1), &key2), 4u64); + assert_eq!(DoubleMap::get(&(key1 + 1), &(key2 + 1)), 4u64); + }); + } + #[test] fn double_map_basic_insert_remove_remove_prefix_should_work() { new_test_ext().execute_with(|| { + let key1 = 17u32; + let key2 = 18u32; type DoubleMap = DataDM; + // initialized during genesis assert_eq!(DoubleMap::get(&15u32, &16u32), 42u64); // get / insert / take - let key1 = 17u32; - let key2 = 18u32; assert_eq!(DoubleMap::get(&key1, &key2), 0u64); DoubleMap::insert(&key1, &key2, &4u64); assert_eq!(DoubleMap::get(&key1, &key2), 4u64); @@ -1082,9 +1148,7 @@ pub mod tests { assert_eq!(DoubleMap::get(&key1, &key2), 0u64); // mutate - DoubleMap::mutate(&key1, &key2, |val| { - *val = 15; - }); + DoubleMap::mutate(&key1, &key2, |val| *val = 15); assert_eq!(DoubleMap::get(&key1, &key2), 15u64); // remove @@ -1096,13 +1160,18 @@ pub mod tests { DoubleMap::insert(&key1, &(key2 + 1), &4u64); DoubleMap::insert(&(key1 + 1), &key2, &4u64); DoubleMap::insert(&(key1 + 1), &(key2 + 1), &4u64); + // all in overlay + assert!(matches!( + DoubleMap::clear_prefix(&key1, u32::max_value(), None), + MultiRemovalResults { maybe_cursor: None, backend: 0, unique: 0, loops: 0 } + )); + // Note this is the incorrect answer (for now), since we are using v2 of + // `clear_prefix`. + // When we switch to v3, then this will become: + // MultiRemovalResults:: { maybe_cursor: None, backend: 0, unique: 2, loops: 2 }, assert!(matches!( DoubleMap::clear_prefix(&key1, u32::max_value(), None), - // Note this is the incorrect answer (for now), since we are using v2 of - // `clear_prefix`. - // When we switch to v3, then this will become: - // sp_io::MultiRemovalResults::NoneLeft { db: 0, total: 2 }, - sp_io::MultiRemovalResults { maybe_cursor: None, backend: 0, unique: 0, loops: 0 }, + MultiRemovalResults { maybe_cursor: None, backend: 0, unique: 0, loops: 0 } )); assert_eq!(DoubleMap::get(&key1, &key2), 0u64); assert_eq!(DoubleMap::get(&key1, &(key2 + 1)), 0u64); @@ -1321,7 +1390,7 @@ pub mod pallet_prelude { }, traits::{ ConstU32, EnsureOrigin, Get, GetDefault, GetStorageVersion, Hooks, IsType, - PalletInfoAccess, StorageInfoTrait, StorageVersion, + PalletInfoAccess, StorageInfoTrait, StorageVersion, TypedGet, }, weights::{DispatchClass, Pays, Weight}, Blake2_128, Blake2_128Concat, Blake2_256, CloneNoBound, DebugNoBound, EqNoBound, Identity, diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index a6fc88dfb5e76..d1872544e024d 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -59,7 +59,7 @@ pub use misc::{ ConstU32, ConstU64, ConstU8, DefensiveSaturating, EnsureInherentsAreFirst, EqualPrivilegeOnly, EstimateCallFee, ExecuteBlock, ExtrinsicCall, Get, GetBacking, GetDefault, HandleLifetime, IsSubType, IsType, Len, OffchainWorker, OnKilledAccount, OnNewAccount, PreimageProvider, - PreimageRecipient, PrivilegeCmp, SameOrOther, Time, TryCollect, TryDrop, UnixTime, + PreimageRecipient, PrivilegeCmp, SameOrOther, Time, TryCollect, TryDrop, TypedGet, UnixTime, WrapperKeepOpaque, WrapperOpaque, }; #[doc(hidden)] @@ -93,9 +93,11 @@ pub use storage::{ }; mod dispatch; +#[allow(deprecated)] +pub use dispatch::EnsureOneOf; pub use dispatch::{ - AsEnsureOriginWithArg, DispatchableWithStorageLayer, EnsureOneOf, EnsureOrigin, - EnsureOriginWithArg, OriginTrait, UnfilteredDispatchable, + AsEnsureOriginWithArg, DispatchableWithStorageLayer, EitherOf, EitherOfDiverse, EnsureOrigin, + EnsureOriginWithArg, NeverEnsureOrigin, OriginTrait, UnfilteredDispatchable, }; mod voting; diff --git a/frame/support/src/traits/dispatch.rs b/frame/support/src/traits/dispatch.rs index 1cec6cf837aba..b1bd52ca960da 100644 --- a/frame/support/src/traits/dispatch.rs +++ b/frame/support/src/traits/dispatch.rs @@ -43,6 +43,19 @@ pub trait EnsureOrigin { fn successful_origin() -> OuterOrigin; } +/// `EnsureOrigin` implementation that always fails. +pub struct NeverEnsureOrigin(sp_std::marker::PhantomData); +impl EnsureOrigin for NeverEnsureOrigin { + type Success = Success; + fn try_origin(o: OO) -> Result { + Err(o) + } + #[cfg(feature = "runtime-benchmarks")] + fn successful_origin() -> OO { + panic!("No `successful_origin` possible for `NeverEnsureOrigin`") + } +} + /// Some sort of check on the origin is performed by this object. pub trait EnsureOriginWithArg { /// A return type. @@ -163,13 +176,16 @@ pub trait OriginTrait: Sized { fn signed(by: Self::AccountId) -> Self; } -/// The "OR gate" implementation of `EnsureOrigin`. +/// "OR gate" implementation of `EnsureOrigin` allowing for different `Success` types for `L` +/// and `R`, with them combined using an `Either` type. /// /// Origin check will pass if `L` or `R` origin check passes. `L` is tested first. -pub struct EnsureOneOf(sp_std::marker::PhantomData<(L, R)>); +/// +/// Successful origin is derived from the left side. +pub struct EitherOfDiverse(sp_std::marker::PhantomData<(L, R)>); impl, R: EnsureOrigin> - EnsureOrigin for EnsureOneOf + EnsureOrigin for EitherOfDiverse { type Success = Either; fn try_origin(o: OuterOrigin) -> Result { @@ -183,17 +199,53 @@ impl, R: EnsureOrigin> } } +/// "OR gate" implementation of `EnsureOrigin` allowing for different `Success` types for `L` +/// and `R`, with them combined using an `Either` type. +/// +/// Origin check will pass if `L` or `R` origin check passes. `L` is tested first. +/// +/// Successful origin is derived from the left side. +#[deprecated = "Use `EitherOfDiverse` instead"] +pub type EnsureOneOf = EitherOfDiverse; + +/// "OR gate" implementation of `EnsureOrigin`, `Success` type for both `L` and `R` must +/// be equal. +/// +/// Origin check will pass if `L` or `R` origin check passes. `L` is tested first. +/// +/// Successful origin is derived from the left side. +pub struct EitherOf(sp_std::marker::PhantomData<(L, R)>); + +impl< + OuterOrigin, + L: EnsureOrigin, + R: EnsureOrigin, + > EnsureOrigin for EitherOf +{ + type Success = L::Success; + fn try_origin(o: OuterOrigin) -> Result { + L::try_origin(o).or_else(|o| R::try_origin(o)) + } + + #[cfg(feature = "runtime-benchmarks")] + fn successful_origin() -> OuterOrigin { + L::successful_origin() + } +} + #[cfg(test)] mod tests { use super::*; + use crate::traits::{ConstBool, ConstU8, TypedGet}; + use std::marker::PhantomData; - struct EnsureSuccess; - struct EnsureFail; + struct EnsureSuccess(PhantomData); + struct EnsureFail(PhantomData); - impl EnsureOrigin<()> for EnsureSuccess { - type Success = (); + impl EnsureOrigin<()> for EnsureSuccess { + type Success = V::Type; fn try_origin(_: ()) -> Result { - Ok(()) + Ok(V::get()) } #[cfg(feature = "runtime-benchmarks")] fn successful_origin() -> () { @@ -201,8 +253,8 @@ mod tests { } } - impl EnsureOrigin<()> for EnsureFail { - type Success = (); + impl EnsureOrigin<()> for EnsureFail { + type Success = T; fn try_origin(_: ()) -> Result { Err(()) } @@ -213,10 +265,46 @@ mod tests { } #[test] - fn ensure_one_of_test() { - assert!(>::try_origin(()).is_ok()); - assert!(>::try_origin(()).is_ok()); - assert!(>::try_origin(()).is_ok()); - assert!(>::try_origin(()).is_err()); + fn either_of_diverse_works() { + assert_eq!( + EitherOfDiverse::< + EnsureSuccess>, + EnsureSuccess>, + >::try_origin(()).unwrap().left(), + Some(true) + ); + assert_eq!( + EitherOfDiverse::>, EnsureFail>::try_origin(()) + .unwrap() + .left(), + Some(true) + ); + assert_eq!( + EitherOfDiverse::, EnsureSuccess>>::try_origin(()) + .unwrap() + .right(), + Some(0u8) + ); + assert!(EitherOfDiverse::, EnsureFail>::try_origin(()).is_err()); + } + + #[test] + fn either_of_works() { + assert_eq!( + EitherOf::< + EnsureSuccess>, + EnsureSuccess>, + >::try_origin(()).unwrap(), + true + ); + assert_eq!( + EitherOf::>, EnsureFail>::try_origin(()).unwrap(), + true + ); + assert_eq!( + EitherOf::, EnsureSuccess>>::try_origin(()).unwrap(), + false + ); + assert!(EitherOf::, EnsureFail>::try_origin(()).is_err()); } } diff --git a/frame/support/src/traits/misc.rs b/frame/support/src/traits/misc.rs index 03420f64dd55b..bea4e2a394411 100644 --- a/frame/support/src/traits/misc.rs +++ b/frame/support/src/traits/misc.rs @@ -387,6 +387,16 @@ where } } +/// A trait for querying a single value from a type defined in the trait. +/// +/// It is not required that the value is constant. +pub trait TypedGet { + /// The type which is returned. + type Type; + /// Return the current value. + fn get() -> Self::Type; +} + /// A trait for querying a single value from a type. /// /// It is not required that the value is constant. @@ -423,6 +433,12 @@ macro_rules! impl_const_get { Some(T) } } + impl TypedGet for $name { + type Type = $t; + fn get() -> $t { + T + } + } }; } diff --git a/frame/support/src/traits/voting.rs b/frame/support/src/traits/voting.rs index 978c5ce4f6a01..6c802a6112246 100644 --- a/frame/support/src/traits/voting.rs +++ b/frame/support/src/traits/voting.rs @@ -95,16 +95,18 @@ impl + UniqueSaturatedFrom> CurrencyToVote } } -pub trait VoteTally { - fn ayes(&self) -> Votes; - fn turnout(&self) -> Perbill; - fn approval(&self) -> Perbill; +pub trait VoteTally { + fn new(_: Class) -> Self; + fn ayes(&self, class: Class) -> Votes; + fn support(&self, class: Class) -> Perbill; + fn approval(&self, class: Class) -> Perbill; #[cfg(feature = "runtime-benchmarks")] - fn unanimity() -> Self; + fn unanimity(class: Class) -> Self; #[cfg(feature = "runtime-benchmarks")] - fn from_requirements(turnout: Perbill, approval: Perbill) -> Self; + fn rejection(class: Class) -> Self; + #[cfg(feature = "runtime-benchmarks")] + fn from_requirements(support: Perbill, approval: Perbill, class: Class) -> Self; } - pub enum PollStatus { None, Ongoing(Tally, Class), diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index f2999f945c5c2..bc28cc7e8118e 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -86,7 +86,7 @@ use frame_support::{ storage, traits::{ ConstU32, Contains, EnsureOrigin, Get, HandleLifetime, OnKilledAccount, OnNewAccount, - OriginTrait, PalletInfo, SortedMembers, StoredMap, + OriginTrait, PalletInfo, SortedMembers, StoredMap, TypedGet, }, weights::{ extract_actual_weight, DispatchClass, DispatchInfo, PerDispatchClass, RuntimeDbWeight, @@ -787,6 +787,29 @@ impl, O>> + From>, Acco } } +pub struct EnsureRootWithSuccess( + sp_std::marker::PhantomData<(AccountId, Success)>, +); +impl< + O: Into, O>> + From>, + AccountId, + Success: TypedGet, + > EnsureOrigin for EnsureRootWithSuccess +{ + type Success = Success::Type; + fn try_origin(o: O) -> Result { + o.into().and_then(|o| match o { + RawOrigin::Root => Ok(Success::get()), + r => Err(O::from(r)), + }) + } + + #[cfg(feature = "runtime-benchmarks")] + fn successful_origin() -> O { + O::from(RawOrigin::Root) + } +} + pub struct EnsureSigned(sp_std::marker::PhantomData); impl, O>> + From>, AccountId: Decode> EnsureOrigin for EnsureSigned diff --git a/frame/tips/src/tests.rs b/frame/tips/src/tests.rs index 27cce5576a3b9..235952fd1092c 100644 --- a/frame/tips/src/tests.rs +++ b/frame/tips/src/tests.rs @@ -144,6 +144,7 @@ impl pallet_treasury::Config for Test { type WeightInfo = (); type SpendFunds = (); type MaxApprovals = ConstU32<100>; + type SpendOrigin = frame_support::traits::NeverEnsureOrigin; } parameter_types! { pub const TipFindersFee: Percent = Percent::from_percent(20); diff --git a/frame/treasury/src/benchmarking.rs b/frame/treasury/src/benchmarking.rs index 47bc1b9ea99de..4f17189d79b39 100644 --- a/frame/treasury/src/benchmarking.rs +++ b/frame/treasury/src/benchmarking.rs @@ -22,7 +22,11 @@ use super::{Pallet as Treasury, *}; use frame_benchmarking::{account, benchmarks_instance_pallet}; -use frame_support::{ensure, traits::OnInitialize}; +use frame_support::{ + dispatch::UnfilteredDispatchable, + ensure, + traits::{EnsureOrigin, OnInitialize}, +}; use frame_system::RawOrigin; const SEED: u32 = 0; @@ -57,7 +61,21 @@ fn setup_pot_account, I: 'static>() { let _ = T::Currency::make_free_balance_be(&pot_account, value); } +fn assert_last_event, I: 'static>(generic_event: >::Event) { + frame_system::Pallet::::assert_last_event(generic_event.into()); +} + benchmarks_instance_pallet! { + spend { + let (_, value, beneficiary_lookup) = setup_proposal::(SEED); + let origin = T::SpendOrigin::successful_origin(); + let beneficiary = T::Lookup::lookup(beneficiary_lookup.clone()).unwrap(); + let call = Call::::spend { amount: value, beneficiary: beneficiary_lookup }; + }: { call.dispatch_bypass_filter(origin)? } + verify { + assert_last_event::(Event::SpendApproved { proposal_index: 0, amount: value, beneficiary }.into()) + } + propose_spend { let (caller, value, beneficiary_lookup) = setup_proposal::(SEED); // Whitelist caller account from further DB operations. diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 419970ed18afa..6730f985b16e0 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -198,6 +198,11 @@ pub mod pallet { /// NOTE: This parameter is also used within the Bounties Pallet extension if enabled. #[pallet::constant] type MaxApprovals: Get; + + /// The origin required for approving spends from the treasury outside of the proposal + /// process. The `Success` value is the maximum amount that this origin is allowed to + /// spend at a time. + type SpendOrigin: EnsureOrigin>; } /// Number of proposals that have been made. @@ -275,6 +280,12 @@ pub mod pallet { Rollover { rollover_balance: BalanceOf }, /// Some funds have been deposited. Deposit { value: BalanceOf }, + /// A new spend proposal has been approved. + SpendApproved { + proposal_index: ProposalIndex, + amount: BalanceOf, + beneficiary: T::AccountId, + }, } /// Error for the treasury pallet. @@ -286,6 +297,9 @@ pub mod pallet { InvalidIndex, /// Too many approvals in the queue. TooManyApprovals, + /// The spend origin is valid but the amount it is allowed to spend is lower than the + /// amount to be spent. + InsufficientPermission, /// Proposal has not been approved. ProposalNotApproved, } @@ -393,6 +407,40 @@ pub mod pallet { Ok(()) } + /// Propose and approve a spend of treasury funds. + /// + /// - `origin`: Must be `SpendOrigin` with the `Success` value being at least `amount`. + /// - `amount`: The amount to be transferred from the treasury to the `beneficiary`. + /// - `beneficiary`: The destination account for the transfer. + /// + /// NOTE: For record-keeping purposes, the proposer is deemed to be equivalent to the + /// beneficiary. + #[pallet::weight(T::WeightInfo::spend())] + pub fn spend( + origin: OriginFor, + #[pallet::compact] amount: BalanceOf, + beneficiary: ::Source, + ) -> DispatchResult { + let max_amount = T::SpendOrigin::ensure_origin(origin)?; + let beneficiary = T::Lookup::lookup(beneficiary)?; + + ensure!(amount <= max_amount, Error::::InsufficientPermission); + let proposal_index = Self::proposal_count(); + Approvals::::try_append(proposal_index) + .map_err(|_| Error::::TooManyApprovals)?; + let proposal = Proposal { + proposer: beneficiary.clone(), + value: amount, + beneficiary: beneficiary.clone(), + bond: Default::default(), + }; + Proposals::::insert(proposal_index, proposal); + ProposalCount::::put(proposal_index + 1); + + Self::deposit_event(Event::SpendApproved { proposal_index, amount, beneficiary }); + Ok(()) + } + /// Force a previously approved proposal to be removed from the approval queue. /// The original deposit will no longer be returned. /// diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index b755db29682aa..a21296d1b39ec 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -24,7 +24,7 @@ use std::cell::RefCell; use sp_core::H256; use sp_runtime::{ testing::Header, - traits::{BlakeTwo256, IdentityLookup}, + traits::{BadOrigin, BlakeTwo256, IdentityLookup}, }; use frame_support::{ @@ -101,8 +101,26 @@ parameter_types! { pub const ProposalBond: Permill = Permill::from_percent(5); pub const Burn: Permill = Permill::from_percent(50); pub const TreasuryPalletId: PalletId = PalletId(*b"py/trsry"); - pub const MaxApprovals: u32 = 100; } +pub struct TestSpendOrigin; +impl frame_support::traits::EnsureOrigin for TestSpendOrigin { + type Success = u64; + fn try_origin(o: Origin) -> Result { + Result::, Origin>::from(o).and_then(|o| match o { + frame_system::RawOrigin::Root => Ok(u64::max_value()), + frame_system::RawOrigin::Signed(10) => Ok(5), + frame_system::RawOrigin::Signed(11) => Ok(10), + frame_system::RawOrigin::Signed(12) => Ok(20), + frame_system::RawOrigin::Signed(13) => Ok(50), + r => Err(Origin::from(r)), + }) + } + #[cfg(feature = "runtime-benchmarks")] + fn successful_origin() -> Origin { + Origin::root() + } +} + impl Config for Test { type PalletId = TreasuryPalletId; type Currency = pallet_balances::Pallet; @@ -119,6 +137,7 @@ impl Config for Test { type WeightInfo = (); type SpendFunds = (); type MaxApprovals = ConstU32<100>; + type SpendOrigin = TestSpendOrigin; } pub fn new_test_ext() -> sp_io::TestExternalities { @@ -141,6 +160,51 @@ fn genesis_config_works() { }); } +#[test] +fn spend_origin_permissioning_works() { + new_test_ext().execute_with(|| { + assert_noop!(Treasury::spend(Origin::signed(1), 1, 1), BadOrigin); + assert_noop!( + Treasury::spend(Origin::signed(10), 6, 1), + Error::::InsufficientPermission + ); + assert_noop!( + Treasury::spend(Origin::signed(11), 11, 1), + Error::::InsufficientPermission + ); + assert_noop!( + Treasury::spend(Origin::signed(12), 21, 1), + Error::::InsufficientPermission + ); + assert_noop!( + Treasury::spend(Origin::signed(13), 51, 1), + Error::::InsufficientPermission + ); + }); +} + +#[test] +fn spend_origin_works() { + new_test_ext().execute_with(|| { + // Check that accumulate works when we have Some value in Dummy already. + Balances::make_free_balance_be(&Treasury::account_id(), 101); + assert_ok!(Treasury::spend(Origin::signed(10), 5, 6)); + assert_ok!(Treasury::spend(Origin::signed(10), 5, 6)); + assert_ok!(Treasury::spend(Origin::signed(10), 5, 6)); + assert_ok!(Treasury::spend(Origin::signed(10), 5, 6)); + assert_ok!(Treasury::spend(Origin::signed(11), 10, 6)); + assert_ok!(Treasury::spend(Origin::signed(12), 20, 6)); + assert_ok!(Treasury::spend(Origin::signed(13), 50, 6)); + + >::on_initialize(1); + assert_eq!(Balances::free_balance(6), 0); + + >::on_initialize(2); + assert_eq!(Balances::free_balance(6), 100); + assert_eq!(Treasury::pot(), 0); + }); +} + #[test] fn minting_works() { new_test_ext().execute_with(|| { @@ -372,7 +436,7 @@ fn max_approvals_limited() { Balances::make_free_balance_be(&Treasury::account_id(), u64::MAX); Balances::make_free_balance_be(&0, u64::MAX); - for _ in 0..MaxApprovals::get() { + for _ in 0..::MaxApprovals::get() { assert_ok!(Treasury::propose_spend(Origin::signed(0), 100, 3)); assert_ok!(Treasury::approve_proposal(Origin::root(), 0)); } diff --git a/frame/treasury/src/weights.rs b/frame/treasury/src/weights.rs index f1667bea35480..f6b5414a05652 100644 --- a/frame/treasury/src/weights.rs +++ b/frame/treasury/src/weights.rs @@ -44,6 +44,7 @@ use sp_std::marker::PhantomData; /// Weight functions needed for pallet_treasury. pub trait WeightInfo { + fn spend() -> Weight; fn propose_spend() -> Weight; fn reject_proposal() -> Weight; fn approve_proposal(p: u32, ) -> Weight; @@ -54,6 +55,13 @@ pub trait WeightInfo { /// Weights for pallet_treasury using the Substrate node and recommended hardware. pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { + // Storage: Treasury ProposalCount (r:1 w:1) + // Storage: Treasury Proposals (r:0 w:1) + fn spend() -> Weight { + (22_063_000 as Weight) + .saturating_add(T::DbWeight::get().reads(1 as Weight)) + .saturating_add(T::DbWeight::get().writes(2 as Weight)) + } // Storage: Treasury ProposalCount (r:1 w:1) // Storage: Treasury Proposals (r:0 w:1) fn propose_spend() -> Weight { @@ -100,6 +108,13 @@ impl WeightInfo for SubstrateWeight { // For backwards compatibility and tests impl WeightInfo for () { + // Storage: Treasury ProposalCount (r:1 w:1) + // Storage: Treasury Proposals (r:0 w:1) + fn spend() -> Weight { + (22_063_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(1 as Weight)) + .saturating_add(RocksDbWeight::get().writes(2 as Weight)) + } // Storage: Treasury ProposalCount (r:1 w:1) // Storage: Treasury Proposals (r:0 w:1) fn propose_spend() -> Weight { diff --git a/primitives/arithmetic/Cargo.toml b/primitives/arithmetic/Cargo.toml index 4fdf983943c41..d7046b3254699 100644 --- a/primitives/arithmetic/Cargo.toml +++ b/primitives/arithmetic/Cargo.toml @@ -29,6 +29,7 @@ sp-std = { version = "4.0.0", default-features = false, path = "../std" } [dev-dependencies] criterion = "0.3" primitive-types = "0.11.1" +sp-core = { version = "6.0.0", features = ["full_crypto"], path = "../core" } rand = "0.7.2" [features] diff --git a/primitives/arithmetic/src/fixed_point.rs b/primitives/arithmetic/src/fixed_point.rs index 7ce17bb72611f..f4ed70ee8b5dc 100644 --- a/primitives/arithmetic/src/fixed_point.rs +++ b/primitives/arithmetic/src/fixed_point.rs @@ -18,12 +18,12 @@ //! Decimal Fixed Point implementations for Substrate runtime. use crate::{ - helpers_128bit::multiply_by_rational, + helpers_128bit::{multiply_by_rational, multiply_by_rational_with_rounding, sqrt}, traits::{ Bounded, CheckedAdd, CheckedDiv, CheckedMul, CheckedNeg, CheckedSub, One, SaturatedConversion, Saturating, UniqueSaturatedInto, Zero, }, - PerThing, + PerThing, Perbill, Rounding, SignedRounding, }; use codec::{CompactAs, Decode, Encode}; use sp_std::{ @@ -406,20 +406,326 @@ macro_rules! implement_fixed { } impl $name { - /// const version of `FixedPointNumber::from_inner`. + /// Create a new instance from the given `inner` value. + /// + /// `const` version of `FixedPointNumber::from_inner`. pub const fn from_inner(inner: $inner_type) -> Self { Self(inner) } + /// Return the instance's inner value. + /// + /// `const` version of `FixedPointNumber::into_inner`. + pub const fn into_inner(self) -> $inner_type { + self.0 + } + + /// Creates self from a `u32`. + /// + /// WARNING: This is a `const` function designed for convenient use at build time and + /// will panic on overflow. Ensure that any inputs are sensible. + pub const fn from_u32(n: u32) -> Self { + Self::from_inner((n as $inner_type) * $div) + } + + /// Convert from a `float` value. #[cfg(any(feature = "std", test))] pub fn from_float(x: f64) -> Self { Self((x * (::DIV as f64)) as $inner_type) } + /// Convert from a `Perbill` value. + pub const fn from_perbill(n: Perbill) -> Self { + Self::from_rational(n.deconstruct() as u128, 1_000_000_000) + } + + /// Convert into a `Perbill` value. Will saturate if above one or below zero. + pub const fn into_perbill(self) -> Perbill { + if self.0 <= 0 { + Perbill::zero() + } else if self.0 >= $div { + Perbill::one() + } else { + match multiply_by_rational_with_rounding( + self.0 as u128, + 1_000_000_000, + Self::DIV as u128, + Rounding::NearestPrefDown, + ) { + Some(value) => { + if value > (u32::max_value() as u128) { + panic!( + "prior logic ensures 0 Perbill::zero(), + } + } + } + + /// Convert into a `float` value. #[cfg(any(feature = "std", test))] pub fn to_float(self) -> f64 { self.0 as f64 / ::DIV as f64 } + + /// Attempt to convert into a `PerThing`. This will succeed iff `self` is at least zero + /// and at most one. If it is out of bounds, it will result in an error returning the + /// clamped value. + pub fn try_into_perthing(self) -> Result { + if self < Self::zero() { + Err(P::zero()) + } else if self > Self::one() { + Err(P::one()) + } else { + Ok(P::from_rational(self.0 as u128, $div)) + } + } + + /// Attempt to convert into a `PerThing`. This will always succeed resulting in a + /// clamped value if `self` is less than zero or greater than one. + pub fn into_clamped_perthing(self) -> P { + if self < Self::zero() { + P::zero() + } else if self > Self::one() { + P::one() + } else { + P::from_rational(self.0 as u128, $div) + } + } + + /// Negate the value. + /// + /// WARNING: This is a `const` function designed for convenient use at build time and + /// will panic on overflow. Ensure that any inputs are sensible. + pub const fn neg(self) -> Self { + Self(0 - self.0) + } + + /// Take the square root of a positive value. + /// + /// WARNING: This is a `const` function designed for convenient use at build time and + /// will panic on overflow. Ensure that any inputs are sensible. + pub const fn sqrt(self) -> Self { + match self.try_sqrt() { + Some(v) => v, + None => panic!("sqrt overflow or negative input"), + } + } + + /// Compute the square root, rounding as desired. If it overflows or is negative, then + /// `None` is returned. + pub const fn try_sqrt(self) -> Option { + if self.0 == 0 { + return Some(Self(0)) + } + if self.0 < 1 { + return None + } + let v = self.0 as u128; + + // Want x' = sqrt(x) where x = n/D and x' = n'/D (D is fixed) + // Our prefered way is: + // sqrt(n/D) = sqrt(nD / D^2) = sqrt(nD)/sqrt(D^2) = sqrt(nD)/D + // ergo n' = sqrt(nD) + // but this requires nD to fit into our type. + // if nD doesn't fit then we can fall back on: + // sqrt(nD) = sqrt(n)*sqrt(D) + // computing them individually and taking the product at the end. we will lose some + // precision though. + let maybe_vd = u128::checked_mul(v, $div); + let r = if let Some(vd) = maybe_vd { sqrt(vd) } else { sqrt(v) * sqrt($div) }; + Some(Self(r as $inner_type)) + } + + /// Add a value and return the result. + /// + /// WARNING: This is a `const` function designed for convenient use at build time and + /// will panic on overflow. Ensure that any inputs are sensible. + pub const fn add(self, rhs: Self) -> Self { + Self(self.0 + rhs.0) + } + + /// Subtract a value and return the result. + /// + /// WARNING: This is a `const` function designed for convenient use at build time and + /// will panic on overflow. Ensure that any inputs are sensible. + pub const fn sub(self, rhs: Self) -> Self { + Self(self.0 - rhs.0) + } + + /// Multiply by a value and return the result. + /// + /// Result will be rounded to the nearest representable value, rounding down if it is + /// equidistant between two neighbours. + /// + /// WARNING: This is a `const` function designed for convenient use at build time and + /// will panic on overflow. Ensure that any inputs are sensible. + pub const fn mul(self, rhs: Self) -> Self { + match $name::const_checked_mul(self, rhs) { + Some(v) => v, + None => panic!("attempt to multiply with overflow"), + } + } + + /// Divide by a value and return the result. + /// + /// Result will be rounded to the nearest representable value, rounding down if it is + /// equidistant between two neighbours. + /// + /// WARNING: This is a `const` function designed for convenient use at build time and + /// will panic on overflow. Ensure that any inputs are sensible. + pub const fn div(self, rhs: Self) -> Self { + match $name::const_checked_div(self, rhs) { + Some(v) => v, + None => panic!("attempt to divide with overflow or NaN"), + } + } + + /// Convert into an `I129` format value. + /// + /// WARNING: This is a `const` function designed for convenient use at build time and + /// will panic on overflow. Ensure that any inputs are sensible. + const fn into_i129(self) -> I129 { + #[allow(unused_comparisons)] + if self.0 < 0 { + let value = match self.0.checked_neg() { + Some(n) => n as u128, + None => u128::saturating_add(<$inner_type>::max_value() as u128, 1), + }; + I129 { value, negative: true } + } else { + I129 { value: self.0 as u128, negative: false } + } + } + + /// Convert from an `I129` format value. + /// + /// WARNING: This is a `const` function designed for convenient use at build time and + /// will panic on overflow. Ensure that any inputs are sensible. + const fn from_i129(n: I129) -> Option { + let max_plus_one = u128::saturating_add(<$inner_type>::max_value() as u128, 1); + #[allow(unused_comparisons)] + let inner = if n.negative && <$inner_type>::min_value() < 0 && n.value == max_plus_one { + <$inner_type>::min_value() + } else { + let unsigned_inner = n.value as $inner_type; + if unsigned_inner as u128 != n.value || (unsigned_inner > 0) != (n.value > 0) { + return None + }; + if n.negative { + match unsigned_inner.checked_neg() { + Some(v) => v, + None => return None, + } + } else { + unsigned_inner + } + }; + Some(Self(inner)) + } + + /// Calculate an approximation of a rational. + /// + /// Result will be rounded to the nearest representable value, rounding down if it is + /// equidistant between two neighbours. + /// + /// WARNING: This is a `const` function designed for convenient use at build time and + /// will panic on overflow. Ensure that any inputs are sensible. + pub const fn from_rational(a: u128, b: u128) -> Self { + Self::from_rational_with_rounding(a, b, Rounding::NearestPrefDown) + } + + /// Calculate an approximation of a rational with custom rounding. + /// + /// WARNING: This is a `const` function designed for convenient use at build time and + /// will panic on overflow. Ensure that any inputs are sensible. + pub const fn from_rational_with_rounding(a: u128, b: u128, rounding: Rounding) -> Self { + if b == 0 { + panic!("attempt to divide by zero in from_rational") + } + match multiply_by_rational_with_rounding(Self::DIV as u128, a, b, rounding) { + Some(value) => match Self::from_i129(I129 { value, negative: false }) { + Some(x) => x, + None => panic!("overflow in from_rational"), + }, + None => panic!("overflow in from_rational"), + } + } + + /// Multiply by another value, returning `None` in the case of an error. + /// + /// Result will be rounded to the nearest representable value, rounding down if it is + /// equidistant between two neighbours. + pub const fn const_checked_mul(self, other: Self) -> Option { + self.const_checked_mul_with_rounding(other, SignedRounding::NearestPrefLow) + } + + /// Multiply by another value with custom rounding, returning `None` in the case of an + /// error. + /// + /// Result will be rounded to the nearest representable value, rounding down if it is + /// equidistant between two neighbours. + pub const fn const_checked_mul_with_rounding( + self, + other: Self, + rounding: SignedRounding, + ) -> Option { + let lhs = self.into_i129(); + let rhs = other.into_i129(); + let negative = lhs.negative != rhs.negative; + + match multiply_by_rational_with_rounding( + lhs.value, + rhs.value, + Self::DIV as u128, + Rounding::from_signed(rounding, negative), + ) { + Some(value) => Self::from_i129(I129 { value, negative }), + None => None, + } + } + + /// Divide by another value, returning `None` in the case of an error. + /// + /// Result will be rounded to the nearest representable value, rounding down if it is + /// equidistant between two neighbours. + pub const fn const_checked_div(self, other: Self) -> Option { + self.checked_rounding_div(other, SignedRounding::NearestPrefLow) + } + + /// Divide by another value with custom rounding, returning `None` in the case of an + /// error. + /// + /// Result will be rounded to the nearest representable value, rounding down if it is + /// equidistant between two neighbours. + pub const fn checked_rounding_div( + self, + other: Self, + rounding: SignedRounding, + ) -> Option { + if other.0 == 0 { + return None + } + + let lhs = self.into_i129(); + let rhs = other.into_i129(); + let negative = lhs.negative != rhs.negative; + + match multiply_by_rational_with_rounding( + lhs.value, + Self::DIV as u128, + rhs.value, + Rounding::from_signed(rounding, negative), + ) { + Some(value) => Self::from_i129(I129 { value, negative }), + None => None, + } + } } impl Saturating for $name { @@ -522,6 +828,10 @@ macro_rules! implement_fixed { let rhs: I129 = other.0.into(); let negative = lhs.negative != rhs.negative; + // Note that this uses the old (well-tested) code with sign-ignorant rounding. This + // is equivalent to the `SignedRounding::NearestPrefMinor`. This means it is + // expected to give exactly the same result as `const_checked_div` when the result + // is positive and a result up to one epsilon greater when it is negative. multiply_by_rational(lhs.value, Self::DIV as u128, rhs.value) .ok() .and_then(|value| from_i129(I129 { value, negative })) @@ -851,6 +1161,16 @@ macro_rules! implement_fixed { } } + #[test] + fn op_sqrt_works() { + for i in 1..1_000i64 { + let x = $name::saturating_from_rational(i, 1_000i64); + assert_eq!((x * x).try_sqrt(), Some(x)); + let x = $name::saturating_from_rational(i, 1i64); + assert_eq!((x * x).try_sqrt(), Some(x)); + } + } + #[test] fn op_div_works() { let a = $name::saturating_from_integer(42); @@ -1133,6 +1453,41 @@ macro_rules! implement_fixed { assert_eq!(a.into_inner(), 0); } + #[test] + fn from_rational_works() { + let inner_max: u128 = <$name as FixedPointNumber>::Inner::max_value() as u128; + let inner_min: u128 = 0; + let accuracy: u128 = $name::accuracy() as u128; + + // Max - 1. + let a = $name::from_rational(inner_max - 1, accuracy); + assert_eq!(a.into_inner() as u128, inner_max - 1); + + // Min + 1. + let a = $name::from_rational(inner_min + 1, accuracy); + assert_eq!(a.into_inner() as u128, inner_min + 1); + + // Max. + let a = $name::from_rational(inner_max, accuracy); + assert_eq!(a.into_inner() as u128, inner_max); + + // Min. + let a = $name::from_rational(inner_min, accuracy); + assert_eq!(a.into_inner() as u128, inner_min); + + let a = $name::from_rational(inner_max, 3 * accuracy); + assert_eq!(a.into_inner() as u128, inner_max / 3); + + let a = $name::from_rational(1, accuracy); + assert_eq!(a.into_inner() as u128, 1); + + let a = $name::from_rational(1, accuracy + 1); + assert_eq!(a.into_inner() as u128, 1); + + let a = $name::from_rational_with_rounding(1, accuracy + 1, Rounding::Down); + assert_eq!(a.into_inner() as u128, 0); + } + #[test] fn checked_mul_int_works() { let a = $name::saturating_from_integer(2); @@ -1272,6 +1627,76 @@ macro_rules! implement_fixed { ); } + #[test] + fn const_checked_mul_works() { + let inner_max = <$name as FixedPointNumber>::Inner::max_value(); + let inner_min = <$name as FixedPointNumber>::Inner::min_value(); + + let a = $name::saturating_from_integer(2u32); + + // Max - 1. + let b = $name::from_inner(inner_max - 1); + assert_eq!(a.const_checked_mul((b / 2.into())), Some(b)); + + // Max. + let c = $name::from_inner(inner_max); + assert_eq!(a.const_checked_mul((c / 2.into())), Some(b)); + + // Max + 1 => None. + let e = $name::from_inner(1); + assert_eq!(a.const_checked_mul((c / 2.into() + e)), None); + + if $name::SIGNED { + // Min + 1. + let b = $name::from_inner(inner_min + 1) / 2.into(); + let c = $name::from_inner(inner_min + 2); + assert_eq!(a.const_checked_mul(b), Some(c)); + + // Min. + let b = $name::from_inner(inner_min) / 2.into(); + let c = $name::from_inner(inner_min); + assert_eq!(a.const_checked_mul(b), Some(c)); + + // Min - 1 => None. + let b = $name::from_inner(inner_min) / 2.into() - $name::from_inner(1); + assert_eq!(a.const_checked_mul(b), None); + + let b = $name::saturating_from_rational(1i32, -2i32); + let c = $name::saturating_from_integer(-21i32); + let d = $name::saturating_from_integer(42); + + assert_eq!(b.const_checked_mul(d), Some(c)); + + let minus_two = $name::saturating_from_integer(-2i32); + assert_eq!( + b.const_checked_mul($name::max_value()), + $name::max_value().const_checked_div(minus_two) + ); + assert_eq!( + b.const_checked_mul($name::min_value()), + $name::min_value().const_checked_div(minus_two) + ); + + let c = $name::saturating_from_integer(255u32); + assert_eq!(c.const_checked_mul($name::min_value()), None); + } + + let a = $name::saturating_from_rational(1i32, 2i32); + let c = $name::saturating_from_integer(255i32); + + assert_eq!(a.const_checked_mul(42.into()), Some(21.into())); + assert_eq!(c.const_checked_mul(2.into()), Some(510.into())); + assert_eq!(c.const_checked_mul($name::max_value()), None); + assert_eq!( + a.const_checked_mul($name::max_value()), + $name::max_value().checked_div(&2.into()) + ); + assert_eq!( + a.const_checked_mul($name::min_value()), + $name::min_value().const_checked_div($name::saturating_from_integer(2)) + ); + } + #[test] fn checked_div_int_works() { let inner_max = <$name as FixedPointNumber>::Inner::max_value(); diff --git a/primitives/arithmetic/src/helpers_128bit.rs b/primitives/arithmetic/src/helpers_128bit.rs index 735b11287cbe4..260f90ed60cd0 100644 --- a/primitives/arithmetic/src/helpers_128bit.rs +++ b/primitives/arithmetic/src/helpers_128bit.rs @@ -1,6 +1,7 @@ // This file is part of Substrate. // Copyright (C) 2019-2022 Parity Technologies (UK) Ltd. +// Some code is based upon Derek Dreery's IntegerSquareRoot impl, used under license. // SPDX-License-Identifier: Apache-2.0 // Licensed under the Apache License, Version 2.0 (the "License"); @@ -20,10 +21,11 @@ //! assumptions of a bigger type (u128) being available, or simply create a per-thing and use the //! multiplication implementation provided there. -use crate::biguint; +use crate::{biguint, Rounding}; use num_traits::Zero; use sp_std::{ cmp::{max, min}, + convert::TryInto, mem, }; @@ -117,3 +119,254 @@ pub fn multiply_by_rational(mut a: u128, mut b: u128, mut c: u128) -> Result u128 { + a & ((1 << 64) - 1) + } + + /// Returns the most significant 64 bits of a + const fn high_64(a: u128) -> u128 { + a >> 64 + } + + /// Returns 2^128 - a (two's complement) + const fn neg128(a: u128) -> u128 { + (!a).wrapping_add(1) + } + + /// Returns 2^128 / a + const fn div128(a: u128) -> u128 { + (neg128(a) / a).wrapping_add(1) + } + + /// Returns 2^128 % a + const fn mod128(a: u128) -> u128 { + neg128(a) % a + } + + #[derive(Copy, Clone, Eq, PartialEq)] + pub struct Double128 { + high: u128, + low: u128, + } + + impl Double128 { + pub const fn try_into_u128(self) -> Result { + match self.high { + 0 => Ok(self.low), + _ => Err(()), + } + } + + pub const fn zero() -> Self { + Self { high: 0, low: 0 } + } + + /// Return a `Double128` value representing the `scaled_value << 64`. + /// + /// This means the lower half of the `high` component will be equal to the upper 64-bits of + /// `scaled_value` (in the lower positions) and the upper half of the `low` component will + /// be equal to the lower 64-bits of `scaled_value`. + pub const fn left_shift_64(scaled_value: u128) -> Self { + Self { high: scaled_value >> 64, low: scaled_value << 64 } + } + + /// Construct a value from the upper 128 bits only, with the lower being zeroed. + pub const fn from_low(low: u128) -> Self { + Self { high: 0, low } + } + + /// Returns the same value ignoring anything in the high 128-bits. + pub const fn low_part(self) -> Self { + Self { high: 0, ..self } + } + + /// Returns a*b (in 256 bits) + pub const fn product_of(a: u128, b: u128) -> Self { + // Split a and b into hi and lo 64-bit parts + let (a_low, a_high) = (low_64(a), high_64(a)); + let (b_low, b_high) = (low_64(b), high_64(b)); + // a = (a_low + a_high << 64); b = (b_low + b_high << 64); + // ergo a*b = (a_low + a_high << 64)(b_low + b_high << 64) + // = a_low * b_low + // + a_low * b_high << 64 + // + a_high << 64 * b_low + // + a_high << 64 * b_high << 64 + // assuming: + // f = a_low * b_low + // o = a_low * b_high + // i = a_high * b_low + // l = a_high * b_high + // then: + // a*b = (o+i) << 64 + f + l << 128 + let (f, o, i, l) = (a_low * b_low, a_low * b_high, a_high * b_low, a_high * b_high); + let fl = Self { high: l, low: f }; + let i = Self::left_shift_64(i); + let o = Self::left_shift_64(o); + fl.add(i).add(o) + } + + pub const fn add(self, b: Self) -> Self { + let (low, overflow) = self.low.overflowing_add(b.low); + let carry = overflow as u128; // 1 if true, 0 if false. + let high = self.high.wrapping_add(b.high).wrapping_add(carry as u128); + Double128 { high, low } + } + + pub const fn div(mut self, rhs: u128) -> (Self, u128) { + if rhs == 1 { + return (self, 0) + } + + // (self === a; rhs === b) + // Calculate a / b + // = (a_high << 128 + a_low) / b + // let (q, r) = (div128(b), mod128(b)); + // = (a_low * (q * b + r)) + a_high) / b + // = (a_low * q * b + a_low * r + a_high)/b + // = (a_low * r + a_high) / b + a_low * q + let (q, r) = (div128(rhs), mod128(rhs)); + + // x = current result + // a = next number + let mut x = Self::zero(); + while self.high != 0 { + // x += a.low * q + x = x.add(Self::product_of(self.high, q)); + // a = a.low * r + a.high + self = Self::product_of(self.high, r).add(self.low_part()); + } + + (x.add(Self::from_low(self.low / rhs)), self.low % rhs) + } + } +} + +/// Returns `a * b / c` and `(a * b) % c` (wrapping to 128 bits) or `None` in the case of +/// overflow. +pub const fn multiply_by_rational_with_rounding( + a: u128, + b: u128, + c: u128, + r: Rounding, +) -> Option { + use double128::Double128; + if c == 0 { + panic!("attempt to divide by zero") + } + let (result, remainder) = Double128::product_of(a, b).div(c); + let mut result: u128 = match result.try_into_u128() { + Ok(v) => v, + Err(_) => return None, + }; + if match r { + Rounding::Up => remainder > 0, + // cannot be `(c + 1) / 2` since `c` might be `max_value` and overflow. + Rounding::NearestPrefUp => remainder >= c / 2 + c % 2, + Rounding::NearestPrefDown => remainder > c / 2, + Rounding::Down => false, + } { + result = match result.checked_add(1) { + Some(v) => v, + None => return None, + }; + } + Some(result) +} + +pub const fn sqrt(mut n: u128) -> u128 { + // Modified from https://github.com/derekdreery/integer-sqrt-rs (Apache/MIT). + if n == 0 { + return 0 + } + + // Compute bit, the largest power of 4 <= n + let max_shift: u32 = 0u128.leading_zeros() - 1; + let shift: u32 = (max_shift - n.leading_zeros()) & !1; + let mut bit = 1u128 << shift; + + // Algorithm based on the implementation in: + // https://en.wikipedia.org/wiki/Methods_of_computing_square_roots#Binary_numeral_system_(base_2) + // Note that result/bit are logically unsigned (even if T is signed). + let mut result = 0u128; + while bit != 0 { + if n >= result + bit { + n -= result + bit; + result = (result >> 1) + bit; + } else { + result = result >> 1; + } + bit = bit >> 2; + } + result +} + +#[cfg(test)] +mod tests { + use super::*; + use codec::{Decode, Encode}; + use multiply_by_rational_with_rounding as mulrat; + use Rounding::*; + + const MAX: u128 = u128::max_value(); + + #[test] + fn rational_multiply_basic_rounding_works() { + assert_eq!(mulrat(1, 1, 1, Up), Some(1)); + assert_eq!(mulrat(3, 1, 3, Up), Some(1)); + assert_eq!(mulrat(1, 1, 3, Up), Some(1)); + assert_eq!(mulrat(1, 2, 3, Down), Some(0)); + assert_eq!(mulrat(1, 1, 3, NearestPrefDown), Some(0)); + assert_eq!(mulrat(1, 1, 2, NearestPrefDown), Some(0)); + assert_eq!(mulrat(1, 2, 3, NearestPrefDown), Some(1)); + assert_eq!(mulrat(1, 1, 3, NearestPrefUp), Some(0)); + assert_eq!(mulrat(1, 1, 2, NearestPrefUp), Some(1)); + assert_eq!(mulrat(1, 2, 3, NearestPrefUp), Some(1)); + } + + #[test] + fn rational_multiply_big_number_works() { + assert_eq!(mulrat(MAX, MAX - 1, MAX, Down), Some(MAX - 1)); + assert_eq!(mulrat(MAX, 1, MAX, Down), Some(1)); + assert_eq!(mulrat(MAX, MAX - 1, MAX, Up), Some(MAX - 1)); + assert_eq!(mulrat(MAX, 1, MAX, Up), Some(1)); + assert_eq!(mulrat(1, MAX - 1, MAX, Down), Some(0)); + assert_eq!(mulrat(1, 1, MAX, Up), Some(1)); + assert_eq!(mulrat(1, MAX / 2, MAX, NearestPrefDown), Some(0)); + assert_eq!(mulrat(1, MAX / 2 + 1, MAX, NearestPrefDown), Some(1)); + assert_eq!(mulrat(1, MAX / 2, MAX, NearestPrefUp), Some(0)); + assert_eq!(mulrat(1, MAX / 2 + 1, MAX, NearestPrefUp), Some(1)); + } + + #[test] + fn sqrt_works() { + for i in 0..100_000u32 { + let a = sqrt(random_u128(i)); + assert_eq!(sqrt(a * a), a); + } + } + + fn random_u128(seed: u32) -> u128 { + u128::decode(&mut &seed.using_encoded(sp_core::hashing::twox_128)[..]).unwrap_or(0) + } + + #[test] + fn op_checked_rounded_div_works() { + for i in 0..100_000u32 { + let a = random_u128(i); + let b = random_u128(i + 1 << 30); + let c = random_u128(i + 1 << 31); + let x = mulrat(a, b, c, NearestPrefDown); + let y = multiply_by_rational(a, b, c).ok(); + assert_eq!(x.is_some(), y.is_some()); + let x = x.unwrap_or(0); + let y = y.unwrap_or(0); + let d = x.max(y) - x.min(y); + assert_eq!(d, 0); + } + } +} diff --git a/primitives/arithmetic/src/lib.rs b/primitives/arithmetic/src/lib.rs index e43e42763575d..244242c0f7580 100644 --- a/primitives/arithmetic/src/lib.rs +++ b/primitives/arithmetic/src/lib.rs @@ -41,7 +41,10 @@ pub mod rational; pub mod traits; pub use fixed_point::{FixedI128, FixedI64, FixedPointNumber, FixedPointOperand, FixedU128}; -pub use per_things::{InnerOf, PerThing, PerU16, Perbill, Percent, Permill, Perquintill, UpperOf}; +pub use per_things::{ + InnerOf, PerThing, PerU16, Perbill, Percent, Permill, Perquintill, Rounding, SignedRounding, + UpperOf, +}; pub use rational::{Rational128, RationalInfinite}; use sp_std::{cmp::Ordering, fmt::Debug, prelude::*}; diff --git a/primitives/arithmetic/src/per_things.rs b/primitives/arithmetic/src/per_things.rs index 3851270b8d4db..7f39d35774190 100644 --- a/primitives/arithmetic/src/per_things.rs +++ b/primitives/arithmetic/src/per_things.rs @@ -26,7 +26,7 @@ use codec::{CompactAs, Encode}; use num_traits::{Pow, SaturatingAdd, SaturatingSub}; use sp_std::{ fmt, ops, - ops::{Add, Sub}, + ops::{Add, AddAssign, Div, Rem, Sub}, prelude::*, }; @@ -89,6 +89,40 @@ pub trait PerThing: self.deconstruct() == Self::ACCURACY } + /// Return the next lower value to `self` or `self` if it is already zero. + fn less_epsilon(self) -> Self { + if self.is_zero() { + return self + } + Self::from_parts(self.deconstruct() - One::one()) + } + + /// Return the next lower value to `self` or an error with the same value if `self` is already + /// zero. + fn try_less_epsilon(self) -> Result { + if self.is_zero() { + return Err(self) + } + Ok(Self::from_parts(self.deconstruct() - One::one())) + } + + /// Return the next higher value to `self` or `self` if it is already one. + fn plus_epsilon(self) -> Self { + if self.is_one() { + return self + } + Self::from_parts(self.deconstruct() + One::one()) + } + + /// Return the next higher value to `self` or an error with the same value if `self` is already + /// one. + fn try_plus_epsilon(self) -> Result { + if self.is_one() { + return Err(self) + } + Ok(Self::from_parts(self.deconstruct() + One::one())) + } + /// Build this type from a percent. Equivalent to `Self::from_parts(x * Self::ACCURACY / 100)` /// but more accurate and can cope with potential type overflows. fn from_percent(x: Self::Inner) -> Self { @@ -188,7 +222,7 @@ pub trait PerThing: + Unsigned, Self::Inner: Into, { - saturating_reciprocal_mul::(b, self.deconstruct(), Rounding::Nearest) + saturating_reciprocal_mul::(b, self.deconstruct(), Rounding::NearestPrefUp) } /// Saturating multiplication by the reciprocal of `self`. The result is rounded down to the @@ -275,9 +309,9 @@ pub trait PerThing: /// # fn main () { /// // 989/1000 is technically closer to 99%. /// assert_eq!( - /// Percent::from_rational(989u64, 1000), - /// Percent::from_parts(98), - /// ); + /// Percent::from_rational(989u64, 1000), + /// Percent::from_parts(98), + /// ); /// # } /// ``` fn from_rational(p: N, q: N) -> Self @@ -289,7 +323,82 @@ pub trait PerThing: + ops::Div + ops::Rem + ops::Add - + Unsigned, + + ops::AddAssign + + Unsigned + + Zero + + One, + Self::Inner: Into, + { + Self::from_rational_with_rounding(p, q, Rounding::Down).unwrap_or_else(|_| Self::one()) + } + + /// Approximate the fraction `p/q` into a per-thing fraction. + /// + /// The computation of this approximation is performed in the generic type `N`. Given + /// `M` as the data type that can hold the maximum value of this per-thing (e.g. `u32` for + /// `Perbill`), this can only work if `N == M` or `N: From + TryInto`. + /// + /// In the case of an overflow (or divide by zero), an `Err` is returned. + /// + /// Rounding is determined by the parameter `rounding`, i.e. + /// + /// ```rust + /// # use sp_arithmetic::{Percent, PerThing, Rounding::*}; + /// # fn main () { + /// // 989/100 is technically closer to 99%. + /// assert_eq!( + /// Percent::from_rational_with_rounding(989u64, 1000, Down).unwrap(), + /// Percent::from_parts(98), + /// ); + /// assert_eq!( + /// Percent::from_rational_with_rounding(984u64, 1000, NearestPrefUp).unwrap(), + /// Percent::from_parts(98), + /// ); + /// assert_eq!( + /// Percent::from_rational_with_rounding(985u64, 1000, NearestPrefDown).unwrap(), + /// Percent::from_parts(98), + /// ); + /// assert_eq!( + /// Percent::from_rational_with_rounding(985u64, 1000, NearestPrefUp).unwrap(), + /// Percent::from_parts(99), + /// ); + /// assert_eq!( + /// Percent::from_rational_with_rounding(986u64, 1000, NearestPrefDown).unwrap(), + /// Percent::from_parts(99), + /// ); + /// assert_eq!( + /// Percent::from_rational_with_rounding(981u64, 1000, Up).unwrap(), + /// Percent::from_parts(99), + /// ); + /// assert_eq!( + /// Percent::from_rational_with_rounding(1001u64, 1000, Up), + /// Err(()), + /// ); + /// # } + /// ``` + /// + /// ```rust + /// # use sp_arithmetic::{Percent, PerThing, Rounding::*}; + /// # fn main () { + /// assert_eq!( + /// Percent::from_rational_with_rounding(981u64, 1000, Up).unwrap(), + /// Percent::from_parts(99), + /// ); + /// # } + /// ``` + fn from_rational_with_rounding(p: N, q: N, rounding: Rounding) -> Result + where + N: Clone + + Ord + + TryInto + + TryInto + + ops::Div + + ops::Rem + + ops::Add + + ops::AddAssign + + Unsigned + + Zero + + One, Self::Inner: Into; /// Same as `Self::from_rational`. @@ -303,6 +412,7 @@ pub trait PerThing: + ops::Div + ops::Rem + ops::Add + + ops::AddAssign + Unsigned + Zero + One, @@ -312,14 +422,54 @@ pub trait PerThing: } } -/// The rounding method to use. -/// -/// `PerThing`s are unsigned so `Up` means towards infinity and `Down` means towards zero. -/// `Nearest` will round an exact half down. -enum Rounding { +/// The rounding method to use for unsigned quantities. +#[derive(Copy, Clone, sp_std::fmt::Debug)] +pub enum Rounding { + // Towards infinity. Up, + // Towards zero. Down, - Nearest, + // Nearest integer, rounding as `Up` when equidistant. + NearestPrefUp, + // Nearest integer, rounding as `Down` when equidistant. + NearestPrefDown, +} + +/// The rounding method to use. +#[derive(Copy, Clone, sp_std::fmt::Debug)] +pub enum SignedRounding { + // Towards positive infinity. + High, + // Towards negative infinity. + Low, + // Nearest integer, rounding as `High` when exactly equidistant. + NearestPrefHigh, + // Nearest integer, rounding as `Low` when exactly equidistant. + NearestPrefLow, + // Away from zero (up when positive, down when negative). When positive, equivalent to `High`. + Major, + // Towards zero (down when positive, up when negative). When positive, equivalent to `Low`. + Minor, + // Nearest integer, rounding as `Major` when exactly equidistant. + NearestPrefMajor, + // Nearest integer, rounding as `Minor` when exactly equidistant. + NearestPrefMinor, +} + +impl Rounding { + /// Returns the value for `Rounding` which would give the same result ignorant of the sign. + pub const fn from_signed(rounding: SignedRounding, negative: bool) -> Self { + use Rounding::*; + use SignedRounding::*; + match (rounding, negative) { + (Low, true) | (Major, _) | (High, false) => Up, + (High, true) | (Minor, _) | (Low, false) => Down, + (NearestPrefMajor, _) | (NearestPrefHigh, false) | (NearestPrefLow, true) => + NearestPrefUp, + (NearestPrefMinor, _) | (NearestPrefLow, false) | (NearestPrefHigh, true) => + NearestPrefDown, + } + } } /// Saturating reciprocal multiplication. Compute `x / self`, saturating at the numeric @@ -397,18 +547,53 @@ where rem_mul_div_inner += 1.into(); } }, - // Round up if the fractional part of the result is greater than a half. An exact half is - // rounded down. - Rounding::Nearest => { + Rounding::NearestPrefDown => if rem_mul_upper % denom_upper > denom_upper / 2.into() { // `rem * numer / denom` is less than `numer`, so this will not overflow. rem_mul_div_inner += 1.into(); - } - }, + }, + Rounding::NearestPrefUp => + if rem_mul_upper % denom_upper >= denom_upper / 2.into() + denom_upper % 2.into() { + // `rem * numer / denom` is less than `numer`, so this will not overflow. + rem_mul_div_inner += 1.into(); + }, } rem_mul_div_inner.into() } +/// Just a simple generic integer divide with custom rounding. +fn div_rounded(n: N, d: N, r: Rounding) -> N +where + N: Clone + + Eq + + Ord + + Zero + + One + + AddAssign + + Add + + Rem + + Div, +{ + let mut o = n.clone() / d.clone(); + use Rounding::*; + let two = || N::one() + N::one(); + if match r { + Up => !((n % d).is_zero()), + NearestPrefDown => { + let rem = n % d.clone(); + rem > d / two() + }, + NearestPrefUp => { + let rem = n % d.clone(); + rem >= d.clone() / two() + d % two() + }, + Down => false, + } { + o += N::one() + } + o +} + macro_rules! implement_per_thing { ( $name:ident, @@ -423,7 +608,7 @@ macro_rules! implement_per_thing { /// #[doc = $title] #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] - #[derive(Encode, Copy, Clone, PartialEq, Eq, codec::MaxEncodedLen, PartialOrd, Ord, sp_std::fmt::Debug, scale_info::TypeInfo)] + #[derive(Encode, Copy, Clone, PartialEq, Eq, codec::MaxEncodedLen, PartialOrd, Ord, scale_info::TypeInfo)] pub struct $name($type); /// Implementation makes any compact encoding of `PerThing::Inner` valid, @@ -445,6 +630,55 @@ macro_rules! implement_per_thing { } } + #[cfg(feature = "std")] + impl sp_std::fmt::Debug for $name { + fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result { + if $max == <$type>::max_value() { + // Not a power of ten: show as N/D and approx % + let pc = (self.0 as f64) / (self.0 as f64) * 100f64; + write!(fmt, "{:.2}% ({}/{})", pc, self.0, $max) + } else { + // A power of ten: calculate exact percent + let units = self.0 / ($max / 100); + let rest = self.0 % ($max / 100); + write!(fmt, "{}", units)?; + if rest > 0 { + write!(fmt, ".")?; + let mut m = $max / 100; + while rest % m > 0 { + m /= 10; + write!(fmt, "{:01}", rest / m % 10)?; + } + } + write!(fmt, "%") + } + } + } + + #[cfg(not(feature = "std"))] + impl sp_std::fmt::Debug for $name { + fn fmt(&self, fmt: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result { + if $max == <$type>::max_value() { + // Not a power of ten: show as N/D and approx % + write!(fmt, "{}/{}", self.0, $max) + } else { + // A power of ten: calculate exact percent + let units = self.0 / ($max / 100); + let rest = self.0 % ($max / 100); + write!(fmt, "{}", units)?; + if rest > 0 { + write!(fmt, ".")?; + let mut m = $max / 100; + while rest % m > 0 { + m /= 10; + write!(fmt, "{:01}", rest / m % 10)?; + } + } + write!(fmt, "%") + } + } + } + impl PerThing for $name { type Inner = $type; type Upper = $upper_type; @@ -463,53 +697,50 @@ macro_rules! implement_per_thing { Self::from_parts((x.max(0.).min(1.) * $max as f64) as Self::Inner) } - fn from_rational(p: N, q: N) -> Self + fn from_rational_with_rounding(p: N, q: N, r: Rounding) -> Result where - N: Clone + Ord + TryInto + TryInto - + ops::Div + ops::Rem + ops::Add + Unsigned - + Zero + One, - Self::Inner: Into, + N: Clone + + Ord + + TryInto + + TryInto + + ops::Div + + ops::Rem + + ops::Add + + ops::AddAssign + + Unsigned + + Zero + + One, + Self::Inner: Into { - let div_ceil = |x: N, f: N| -> N { - let mut o = x.clone() / f.clone(); - let r = x.rem(f.clone()); - if r > N::zero() { - o = o + N::one(); - } - o - }; - // q cannot be zero. - let q: N = q.max((1 as Self::Inner).into()); + if q.is_zero() { return Err(()) } // p should not be bigger than q. - let p: N = p.min(q.clone()); + if p > q { return Err(()) } - let factor: N = div_ceil(q.clone(), $max.into()).max((1 as Self::Inner).into()); + let factor = div_rounded::(q.clone(), $max.into(), Rounding::Up).max(One::one()); // q cannot overflow: (q / (q/$max)) < $max. p < q hence p also cannot overflow. - let q_reduce: $type = (q.clone() / factor.clone()) + let q_reduce: $type = div_rounded(q, factor.clone(), r) .try_into() .map_err(|_| "Failed to convert") .expect( - "q / ceil(q/$max) < $max. Macro prevents any type being created that \ + "`q / ceil(q/$max) < $max`; macro prevents any type being created that \ does not satisfy this; qed" ); - let p_reduce: $type = (p / factor) + let p_reduce: $type = div_rounded(p, factor, r) .try_into() .map_err(|_| "Failed to convert") .expect( - "q / ceil(q/$max) < $max. Macro prevents any type being created that \ + "`p / ceil(p/$max) < $max`; macro prevents any type being created that \ does not satisfy this; qed" ); - // `p_reduced` and `q_reduced` are withing Self::Inner. Mul by another $max will - // always fit in $upper_type. This is guaranteed by the macro tests. - let part = - p_reduce as $upper_type - * <$upper_type>::from($max) - / q_reduce as $upper_type; - - $name(part as Self::Inner) + // `p_reduced` and `q_reduced` are within `Self::Inner`. Multiplication by another + // `$max` will always fit in `$upper_type`. This is guaranteed by the macro tests. + let n = p_reduce as $upper_type * <$upper_type>::from($max); + let d = q_reduce as $upper_type; + let part = div_rounded(n, d, r); + Ok($name(part as Self::Inner)) } } @@ -570,24 +801,52 @@ macro_rules! implement_per_thing { /// See [`PerThing::from_rational`]. #[deprecated = "Use `PerThing::from_rational` instead"] pub fn from_rational_approximation(p: N, q: N) -> Self - where N: Clone + Ord + TryInto<$type> + - TryInto<$upper_type> + ops::Div + ops::Rem + - ops::Add + Unsigned, - $type: Into, + where + N: Clone + + Ord + + TryInto<$type> + + TryInto<$upper_type> + + ops::Div + + ops::Rem + + ops::Add + + ops::AddAssign + + Unsigned + + Zero + + One, + $type: Into { ::from_rational(p, q) } /// See [`PerThing::from_rational`]. pub fn from_rational(p: N, q: N) -> Self - where N: Clone + Ord + TryInto<$type> + - TryInto<$upper_type> + ops::Div + ops::Rem + - ops::Add + Unsigned, - $type: Into, + where + N: Clone + + Ord + + TryInto<$type> + + TryInto<$upper_type> + + ops::Div + + ops::Rem + + ops::Add + + ops::AddAssign + + Unsigned + + Zero + + One, + $type: Into { ::from_rational(p, q) } + /// Integer multiplication with another value, saturating at 1. + pub fn int_mul(self, b: $type) -> Self { + PerThing::from_parts(self.0.saturating_mul(b)) + } + + /// Integer division with another value, rounding down. + pub fn int_div(self, b: Self) -> $type { + self.0 / b.0 + } + /// See [`PerThing::mul_floor`]. pub fn mul_floor(self, b: N) -> N where @@ -643,6 +902,38 @@ macro_rules! implement_per_thing { { PerThing::saturating_reciprocal_mul_ceil(self, b) } + + /// Saturating division. Compute `self / rhs`, saturating at one if `rhs < self`. + /// + /// The `rounding` method must be specified. e.g.: + /// + /// ```rust + /// # use sp_arithmetic::{Percent, PerThing, Rounding::*}; + /// # fn main () { + /// let pc = |x| Percent::from_percent(x); + /// assert_eq!( + /// pc(2).saturating_div(pc(3), Down), + /// pc(66), + /// ); + /// assert_eq!( + /// pc(1).saturating_div(pc(3), NearestPrefUp), + /// pc(33), + /// ); + /// assert_eq!( + /// pc(2).saturating_div(pc(3), NearestPrefDown), + /// pc(67), + /// ); + /// assert_eq!( + /// pc(1).saturating_div(pc(3), Up), + /// pc(34), + /// ); + /// # } + /// ``` + pub fn saturating_div(self, rhs: Self, r: Rounding) -> Self { + let p = self.0; + let q = rhs.0; + Self::from_rational_with_rounding(p, q, r).unwrap_or_else(|_| Self::one()) + } } impl Saturating for $name { @@ -756,7 +1047,7 @@ macro_rules! implement_per_thing { { type Output = N; fn mul(self, b: N) -> Self::Output { - overflow_prune_mul::(b, self.deconstruct(), Rounding::Nearest) + overflow_prune_mul::(b, self.deconstruct(), Rounding::NearestPrefDown) } } @@ -903,6 +1194,11 @@ macro_rules! implement_per_thing { } } + #[test] + fn from_parts_cannot_overflow() { + assert_eq!(<$name>::from_parts($max.saturating_add(1)), <$name>::one()); + } + #[test] fn has_max_encoded_len() { struct AsMaxEncodedLen { @@ -1040,10 +1336,10 @@ macro_rules! implement_per_thing { #[test] fn per_thing_mul_rounds_to_nearest_number() { - assert_eq!($name::from_float(0.33) * 10u64, 3); - assert_eq!($name::from_float(0.34) * 10u64, 3); - assert_eq!($name::from_float(0.35) * 10u64, 3); - assert_eq!($name::from_float(0.36) * 10u64, 4); + assert_eq!($name::from_percent(33) * 10u64, 3); + assert_eq!($name::from_percent(34) * 10u64, 3); + assert_eq!($name::from_percent(35) * 10u64, 3); + assert_eq!($name::from_percent(36) * 10u64, 4); } #[test] @@ -1351,7 +1647,7 @@ macro_rules! implement_per_thing { <$type>::max_value(), <$type>::max_value(), <$type>::max_value(), - super::Rounding::Nearest, + super::Rounding::NearestPrefDown, ), 0, ); @@ -1360,7 +1656,7 @@ macro_rules! implement_per_thing { <$type>::max_value() - 1, <$type>::max_value(), <$type>::max_value(), - super::Rounding::Nearest, + super::Rounding::NearestPrefDown, ), <$type>::max_value() - 1, ); @@ -1369,7 +1665,7 @@ macro_rules! implement_per_thing { ((<$type>::max_value() - 1) as $upper_type).pow(2), <$type>::max_value(), <$type>::max_value(), - super::Rounding::Nearest, + super::Rounding::NearestPrefDown, ), 1, ); @@ -1379,7 +1675,7 @@ macro_rules! implement_per_thing { (<$type>::max_value() as $upper_type).pow(2) - 1, <$type>::max_value(), <$type>::max_value(), - super::Rounding::Nearest, + super::Rounding::NearestPrefDown, ), <$upper_type>::from((<$type>::max_value() - 1)), ); @@ -1389,7 +1685,7 @@ macro_rules! implement_per_thing { (<$type>::max_value() as $upper_type).pow(2), <$type>::max_value(), 2 as $type, - super::Rounding::Nearest, + super::Rounding::NearestPrefDown, ), <$type>::max_value() as $upper_type / 2, ); @@ -1399,7 +1695,7 @@ macro_rules! implement_per_thing { (<$type>::max_value() as $upper_type).pow(2) - 1, 2 as $type, <$type>::max_value(), - super::Rounding::Nearest, + super::Rounding::NearestPrefDown, ), 2, ); @@ -1586,6 +1882,33 @@ macro_rules! implement_per_thing_with_perthousand { } } +#[test] +fn from_rational_with_rounding_works_in_extreme_case() { + use Rounding::*; + for &r in [Down, NearestPrefDown, NearestPrefUp, Up].iter() { + Percent::from_rational_with_rounding(1, u64::max_value(), r).unwrap(); + Percent::from_rational_with_rounding(1, u32::max_value(), r).unwrap(); + Percent::from_rational_with_rounding(1, u16::max_value(), r).unwrap(); + Percent::from_rational_with_rounding(u64::max_value() - 1, u64::max_value(), r).unwrap(); + Percent::from_rational_with_rounding(u32::max_value() - 1, u32::max_value(), r).unwrap(); + Percent::from_rational_with_rounding(u16::max_value() - 1, u16::max_value(), r).unwrap(); + PerU16::from_rational_with_rounding(1, u64::max_value(), r).unwrap(); + PerU16::from_rational_with_rounding(1, u32::max_value(), r).unwrap(); + PerU16::from_rational_with_rounding(1, u16::max_value(), r).unwrap(); + PerU16::from_rational_with_rounding(u64::max_value() - 1, u64::max_value(), r).unwrap(); + PerU16::from_rational_with_rounding(u32::max_value() - 1, u32::max_value(), r).unwrap(); + PerU16::from_rational_with_rounding(u16::max_value() - 1, u16::max_value(), r).unwrap(); + Permill::from_rational_with_rounding(1, u64::max_value(), r).unwrap(); + Permill::from_rational_with_rounding(1, u32::max_value(), r).unwrap(); + Permill::from_rational_with_rounding(u64::max_value() - 1, u64::max_value(), r).unwrap(); + Permill::from_rational_with_rounding(u32::max_value() - 1, u32::max_value(), r).unwrap(); + Perbill::from_rational_with_rounding(1, u64::max_value(), r).unwrap(); + Perbill::from_rational_with_rounding(1, u32::max_value(), r).unwrap(); + Perbill::from_rational_with_rounding(u64::max_value() - 1, u64::max_value(), r).unwrap(); + Perbill::from_rational_with_rounding(u32::max_value() - 1, u32::max_value(), r).unwrap(); + } +} + implement_per_thing!(Percent, test_per_cent, [u32, u64, u128], 100u8, u8, u16, "_Percent_",); implement_per_thing_with_perthousand!( PerU16, diff --git a/primitives/arithmetic/src/traits.rs b/primitives/arithmetic/src/traits.rs index 748aaed2a7cf5..466d5696c7136 100644 --- a/primitives/arithmetic/src/traits.rs +++ b/primitives/arithmetic/src/traits.rs @@ -58,6 +58,7 @@ pub trait BaseArithmetic: + Bounded + HasCompact + Sized + + Clone + TryFrom + TryInto + TryFrom @@ -113,6 +114,7 @@ impl< + Bounded + HasCompact + Sized + + Clone + TryFrom + TryInto + TryFrom