Skip to content

Commit

Permalink
Implements a variable deposit base calculation for EPM signed submiss…
Browse files Browse the repository at this point in the history
…ions (#1547)

**Note**: This is a lift-and-shift PR from the old substrate and
polkadot repos, both PRs have been reviewed and audited
(paritytech/substrate#13983,
paritytech/polkadot#7140)

---

This PR implements a generic `BaseDeposit` calculation for signed
submissions, based on the size of the submission queue.

It adds a new associated type to EPM's config, `type SignedDepositBase`,
that implements `Convert<usize, BalanceOf<T>>`, which is used to
calculate the base deposit for signed submissions based on the size of
the signed submissions queue.

`struct GeometricDepositBase<Balance, Fixed, Inc>` implements the
convert trait so that the deposit value increases as a geometric
progression. The deposit base is calculated by `deposit_base =
fixed_deposit_base * (1 + increase_factor)^n`, where `n` is the term of
the progression (i.e. the number of signed submissions in the queue).
`Fixed` and `Inc` generic params are getters for `Balance` and
`IncreaseFactor` to compute the geometric progression. If
`IncreaseFactor = 0`, then the signed deposit is constant and equal to
`Fixed` regardless of the size of the queue.

### Runtime configs

In Kusama, the progression with 10% increase without changing the
current signed fixed deposit is: (term == size of the queue)

Term 1: `1,333,333,332,000`
Term 2: `1,333,333,332,000 * 1.10 = 1,466,666,665,200`
Term 3: `1,333,333,332,000 * 1.10^2 = 1,613,333,331,200`
Term 4: `1,333,333,332,000 * 1.10^3 = 1,774,666,664,320`
Term 5: `1,333,333,332,000 * 1.10^4 = 1,952,133,330,752`
Term 6: `1,333,333,332,000 * 1.10^5 = 2,147,346,663,827.20`
Term 7: `1,333,333,332,000 * 1.10^6 = 2,362,081,330,210.92`
Term 8: `1,333,333,332,000 * 1.10^7 = 2,598,289,463,231.01`
Term 9: `1,333,333,332,000 * 1.10^8 = 2,858,118,409,554.11`
Term 10: `1,333,333,332,000 * 1.10^9 = 3,143,930,250,509.52`

Westend:

Term 1: `2,000,000,000,000`
Term 2: `2,000,000,000,000 * 1.10 = 2,200,000,000,000`
Term 3: `2,000,000,000,000 * 1.10^2 = 2,420,000,000,000`
Term 4: `2,000,000,000,000 * 1.10^3 = 2,662,000,000,000`
Term 5: `2,000,000,000,000 * 1.10^4 = 2,928,200,000,000`
Term 6: `2,000,000,000,000 * 1.10^5 = 3,221,020,000,000`
Term 7: `2,000,000,000,000 * 1.10^6 = 3,543,122,000,000`
Term 8: `2,000,000,000,000 * 1.10^7 = 3,897,434,200,000`
Term 9: `2,000,000,000,000 * 1.10^8 = 4,287,177,620,000`
Term 10: `2,000,000,000,000 * 1.10^9 = 4,715,895,382,000`

and in Polkadot, the deposit increase is disabled in the current state
of the PR, as the increase factor is 0% -- so nothing changes from the
current behaviour.

Closes paritytech-secops/srlabs_findings#189
  • Loading branch information
gpestana authored Sep 18, 2023
1 parent d569e72 commit 614aa31
Show file tree
Hide file tree
Showing 8 changed files with 150 additions and 34 deletions.
10 changes: 6 additions & 4 deletions polkadot/runtime/kusama/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ use xcm::latest::Junction;

pub use frame_system::Call as SystemCall;
pub use pallet_balances::Call as BalancesCall;
pub use pallet_election_provider_multi_phase::Call as EPMCall;
pub use pallet_election_provider_multi_phase::{Call as EPMCall, GeometricDepositBase};
#[cfg(feature = "std")]
pub use pallet_staking::StakerStatus;
use pallet_staking::UseValidatorsMap;
Expand Down Expand Up @@ -511,7 +511,8 @@ parameter_types! {
// signed config
pub const SignedMaxSubmissions: u32 = 16;
pub const SignedMaxRefunds: u32 = 16 / 4;
pub const SignedDepositBase: Balance = deposit(2, 0);
pub const SignedFixedDeposit: Balance = deposit(2, 0);
pub const SignedDepositIncreaseFactor: Percent = Percent::from_percent(10);
pub const SignedDepositByte: Balance = deposit(0, 10) / 1024;
// Each good submission will get 1/10 KSM as reward
pub SignedRewardBase: Balance = UNITS / 10;
Expand Down Expand Up @@ -584,7 +585,8 @@ impl pallet_election_provider_multi_phase::Config for Runtime {
type SignedMaxSubmissions = SignedMaxSubmissions;
type SignedMaxRefunds = SignedMaxRefunds;
type SignedRewardBase = SignedRewardBase;
type SignedDepositBase = SignedDepositBase;
type SignedDepositBase =
GeometricDepositBase<Balance, SignedFixedDeposit, SignedDepositIncreaseFactor>;
type SignedDepositByte = SignedDepositByte;
type SignedDepositWeight = ();
type SignedMaxWeight =
Expand Down Expand Up @@ -2484,7 +2486,7 @@ mod fees_tests {
fn signed_deposit_is_sensible() {
// ensure this number does not change, or that it is checked after each change.
// a 1 MB solution should need around 0.16 KSM deposit
let deposit = SignedDepositBase::get() + (SignedDepositByte::get() * 1024 * 1024);
let deposit = SignedFixedDeposit::get() + (SignedDepositByte::get() * 1024 * 1024);
assert_eq_error_rate!(deposit, UNITS * 167 / 100, UNITS / 100);
}
}
Expand Down
9 changes: 6 additions & 3 deletions polkadot/runtime/polkadot/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ use xcm::latest::Junction;

pub use frame_system::Call as SystemCall;
pub use pallet_balances::Call as BalancesCall;
pub use pallet_election_provider_multi_phase::Call as EPMCall;
pub use pallet_election_provider_multi_phase::{Call as EPMCall, GeometricDepositBase};
#[cfg(feature = "std")]
pub use pallet_staking::StakerStatus;
use pallet_staking::UseValidatorsMap;
Expand Down Expand Up @@ -382,6 +382,8 @@ parameter_types! {
// signed config
pub const SignedMaxSubmissions: u32 = 16;
pub const SignedMaxRefunds: u32 = 16 / 4;
pub const SignedFixedDeposit: Balance = deposit(2, 0);
pub const SignedDepositIncreaseFactor: Percent = Percent::from_percent(10);
// 40 DOTs fixed deposit..
pub const SignedDepositBase: Balance = deposit(2, 0);
// 0.01 DOT per KB of solution data.
Expand Down Expand Up @@ -456,7 +458,8 @@ impl pallet_election_provider_multi_phase::Config for Runtime {
type SignedMaxSubmissions = SignedMaxSubmissions;
type SignedMaxRefunds = SignedMaxRefunds;
type SignedRewardBase = SignedRewardBase;
type SignedDepositBase = SignedDepositBase;
type SignedDepositBase =
GeometricDepositBase<Balance, SignedFixedDeposit, SignedDepositIncreaseFactor>;
type SignedDepositByte = SignedDepositByte;
type SignedDepositWeight = ();
type SignedMaxWeight =
Expand Down Expand Up @@ -2352,7 +2355,7 @@ mod test_fees {
fn signed_deposit_is_sensible() {
// ensure this number does not change, or that it is checked after each change.
// a 1 MB solution should take (40 + 10) DOTs of deposit.
let deposit = SignedDepositBase::get() + (SignedDepositByte::get() * 1024 * 1024);
let deposit = SignedFixedDeposit::get() + (SignedDepositByte::get() * 1024 * 1024);
assert_eq_error_rate!(deposit, 50 * DOLLARS, DOLLARS);
}
}
Expand Down
10 changes: 6 additions & 4 deletions polkadot/runtime/westend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ use sp_runtime::{
Keccak256, OpaqueKeys, SaturatedConversion, Verify,
},
transaction_validity::{TransactionPriority, TransactionSource, TransactionValidity},
ApplyExtrinsicResult, FixedU128, KeyTypeId, Perbill,
ApplyExtrinsicResult, FixedU128, KeyTypeId, Perbill, Percent,
};
use sp_staking::SessionIndex;
use sp_std::{collections::btree_map::BTreeMap, prelude::*};
Expand All @@ -90,7 +90,7 @@ use xcm::latest::Junction;

pub use frame_system::Call as SystemCall;
pub use pallet_balances::Call as BalancesCall;
pub use pallet_election_provider_multi_phase::Call as EPMCall;
pub use pallet_election_provider_multi_phase::{Call as EPMCall, GeometricDepositBase};
#[cfg(feature = "std")]
pub use pallet_staking::StakerStatus;
use pallet_staking::UseValidatorsMap;
Expand Down Expand Up @@ -481,7 +481,8 @@ parameter_types! {
// signed config
pub const SignedMaxSubmissions: u32 = 128;
pub const SignedMaxRefunds: u32 = 128 / 4;
pub const SignedDepositBase: Balance = deposit(2, 0);
pub const SignedFixedDeposit: Balance = deposit(2, 0);
pub const SignedDepositIncreaseFactor: Percent = Percent::from_percent(10);
pub const SignedDepositByte: Balance = deposit(0, 10) / 1024;
// Each good submission will get 1 WND as reward
pub SignedRewardBase: Balance = 1 * UNITS;
Expand Down Expand Up @@ -553,7 +554,8 @@ impl pallet_election_provider_multi_phase::Config for Runtime {
type SignedMaxSubmissions = SignedMaxSubmissions;
type SignedMaxRefunds = SignedMaxRefunds;
type SignedRewardBase = SignedRewardBase;
type SignedDepositBase = SignedDepositBase;
type SignedDepositBase =
GeometricDepositBase<Balance, SignedFixedDeposit, SignedDepositIncreaseFactor>;
type SignedDepositByte = SignedDepositByte;
type SignedDepositWeight = ();
type SignedMaxWeight =
Expand Down
8 changes: 5 additions & 3 deletions substrate/bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub use node_primitives::{AccountId, Signature};
use node_primitives::{AccountIndex, Balance, BlockNumber, Hash, Moment, Nonce};
use pallet_asset_conversion::{NativeOrAssetId, NativeOrAssetIdConverter};
use pallet_broker::{CoreAssignment, CoreIndex, CoretimeInterface, PartsOf57600};
use pallet_election_provider_multi_phase::SolutionAccuracyOf;
use pallet_election_provider_multi_phase::{GeometricDepositBase, SolutionAccuracyOf};
use pallet_im_online::sr25519::AuthorityId as ImOnlineId;
use pallet_nfts::PalletFeatures;
use pallet_nis::WithMaximumOf;
Expand Down Expand Up @@ -694,7 +694,8 @@ parameter_types! {

// signed config
pub const SignedRewardBase: Balance = 1 * DOLLARS;
pub const SignedDepositBase: Balance = 1 * DOLLARS;
pub const SignedFixedDeposit: Balance = 1 * DOLLARS;
pub const SignedDepositIncreaseFactor: Percent = Percent::from_percent(10);
pub const SignedDepositByte: Balance = 1 * CENTS;

pub BetterUnsignedThreshold: Perbill = Perbill::from_rational(1u32, 10_000);
Expand Down Expand Up @@ -822,7 +823,8 @@ impl pallet_election_provider_multi_phase::Config for Runtime {
type MinerConfig = Self;
type SignedMaxSubmissions = ConstU32<10>;
type SignedRewardBase = SignedRewardBase;
type SignedDepositBase = SignedDepositBase;
type SignedDepositBase =
GeometricDepositBase<Balance, SignedFixedDeposit, SignedDepositIncreaseFactor>;
type SignedDepositByte = SignedDepositByte;
type SignedMaxRefunds = ConstU32<3>;
type SignedDepositWeight = ();
Expand Down
13 changes: 7 additions & 6 deletions substrate/frame/election-provider-multi-phase/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,8 +279,8 @@ use unsigned::VoterOf;
pub use weights::WeightInfo;

pub use signed::{
BalanceOf, NegativeImbalanceOf, PositiveImbalanceOf, SignedSubmission, SignedSubmissionOf,
SignedSubmissions, SubmissionIndicesOf,
BalanceOf, GeometricDepositBase, NegativeImbalanceOf, PositiveImbalanceOf, SignedSubmission,
SignedSubmissionOf, SignedSubmissions, SubmissionIndicesOf,
};
pub use unsigned::{Miner, MinerConfig};

Expand Down Expand Up @@ -572,6 +572,7 @@ pub mod pallet {
use frame_election_provider_support::{InstantElectionProvider, NposSolver};
use frame_support::{pallet_prelude::*, traits::EstimateCallFee};
use frame_system::pallet_prelude::*;
use sp_runtime::traits::Convert;

#[pallet::config]
pub trait Config: frame_system::Config + SendTransactionTypes<Call<Self>> {
Expand Down Expand Up @@ -649,10 +650,6 @@ pub mod pallet {
#[pallet::constant]
type SignedRewardBase: Get<BalanceOf<Self>>;

/// Base deposit for a signed solution.
#[pallet::constant]
type SignedDepositBase: Get<BalanceOf<Self>>;

/// Per-byte deposit for a signed solution.
#[pallet::constant]
type SignedDepositByte: Get<BalanceOf<Self>>;
Expand All @@ -668,6 +665,10 @@ pub mod pallet {
#[pallet::constant]
type MaxWinners: Get<u32>;

/// Something that calculates the signed deposit base based on the signed submissions queue
/// size.
type SignedDepositBase: Convert<usize, BalanceOf<Self>>;

/// The maximum number of electing voters and electable targets to put in the snapshot.
/// At the moment, snapshots are only over a single block, but once multi-block elections
/// are introduced they will take place over multiple blocks.
Expand Down
34 changes: 28 additions & 6 deletions substrate/frame/election-provider-multi-phase/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
// limitations under the License.

use super::*;
use crate::{self as multi_phase, unsigned::MinerConfig};
use crate::{self as multi_phase, signed::GeometricDepositBase, unsigned::MinerConfig};
use frame_election_provider_support::{
bounds::{DataProviderBounds, ElectionBounds},
data_provider, onchain, ElectionDataProvider, NposSolution, SequentialPhragmen,
Expand Down Expand Up @@ -44,8 +44,8 @@ use sp_npos_elections::{
use sp_runtime::{
bounded_vec,
testing::Header,
traits::{BlakeTwo256, IdentityLookup},
BuildStorage, PerU16,
traits::{BlakeTwo256, Convert, IdentityLookup},
BuildStorage, PerU16, Percent,
};
use std::sync::Arc;

Expand Down Expand Up @@ -283,7 +283,11 @@ parameter_types! {
pub static UnsignedPhase: BlockNumber = 5;
pub static SignedMaxSubmissions: u32 = 5;
pub static SignedMaxRefunds: u32 = 1;
pub static SignedDepositBase: Balance = 5;
// for tests only. if `EnableVariableDepositBase` is true, the deposit base will be calculated
// by `Multiphase::DepositBase`. Otherwise the deposit base is `SignedFixedDeposit`.
pub static EnableVariableDepositBase: bool = false;
pub static SignedFixedDeposit: Balance = 5;
pub static SignedDepositIncreaseFactor: Percent = Percent::from_percent(10);
pub static SignedDepositByte: Balance = 0;
pub static SignedDepositWeight: Balance = 0;
pub static SignedRewardBase: Balance = 7;
Expand Down Expand Up @@ -393,7 +397,7 @@ impl crate::Config for Runtime {
type OffchainRepeat = OffchainRepeat;
type MinerTxPriority = MinerTxPriority;
type SignedRewardBase = SignedRewardBase;
type SignedDepositBase = SignedDepositBase;
type SignedDepositBase = Self;
type SignedDepositByte = ();
type SignedDepositWeight = ();
type SignedMaxWeight = SignedMaxWeight;
Expand All @@ -414,6 +418,18 @@ impl crate::Config for Runtime {
type ElectionBounds = ElectionsBounds;
}

impl Convert<usize, BalanceOf<Runtime>> for Runtime {
/// returns the geometric increase deposit fee if `EnableVariableDepositBase` is set, otherwise
/// the fee is `SignedFixedDeposit`.
fn convert(queue_len: usize) -> Balance {
if !EnableVariableDepositBase::get() {
SignedFixedDeposit::get()
} else {
GeometricDepositBase::<Balance, SignedFixedDeposit, SignedDepositIncreaseFactor>::convert(queue_len)
}
}
}

impl<LocalCall> frame_system::offchain::SendTransactionTypes<LocalCall> for Runtime
where
RuntimeCall: From<LocalCall>,
Expand Down Expand Up @@ -553,8 +569,14 @@ impl ExtBuilder {
<SignedMaxSubmissions>::set(count);
self
}
pub fn signed_base_deposit(self, base: u64, variable: bool, increase: Percent) -> Self {
<EnableVariableDepositBase>::set(variable);
<SignedFixedDeposit>::set(base);
<SignedDepositIncreaseFactor>::set(increase);
self
}
pub fn signed_deposit(self, base: u64, byte: u64, weight: u64) -> Self {
<SignedDepositBase>::set(base);
<SignedFixedDeposit>::set(base);
<SignedDepositByte>::set(byte);
<SignedDepositWeight>::set(weight);
self
Expand Down
89 changes: 84 additions & 5 deletions substrate/frame/election-provider-multi-phase/src/signed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

//! The signed phase implementation.

use core::marker::PhantomData;

use crate::{
unsigned::MinerConfig, Config, ElectionCompute, Pallet, QueuedSolution, RawSolution,
ReadySolution, SignedSubmissionIndices, SignedSubmissionNextIndex, SignedSubmissionsMap,
Expand All @@ -32,8 +34,8 @@ use sp_arithmetic::traits::SaturatedConversion;
use sp_core::bounded::BoundedVec;
use sp_npos_elections::ElectionScore;
use sp_runtime::{
traits::{Saturating, Zero},
RuntimeDebug,
traits::{Convert, Saturating, Zero},
FixedPointNumber, FixedPointOperand, FixedU128, Percent, RuntimeDebug,
};
use sp_std::{
cmp::Ordering,
Expand Down Expand Up @@ -348,6 +350,32 @@ impl<T: Config> SignedSubmissions<T> {
}
}

/// Type that can be used to calculate the deposit base for signed submissions.
///
/// The deposit base is calculated as a geometric progression based on the number of signed
/// submissions in the queue. The size of the queue represents the progression term.
pub struct GeometricDepositBase<Balance, Fixed, Inc> {
_marker: (PhantomData<Balance>, PhantomData<Fixed>, PhantomData<Inc>),
}

impl<Balance, Fixed, Inc> Convert<usize, Balance> for GeometricDepositBase<Balance, Fixed, Inc>
where
Balance: FixedPointOperand,
Fixed: Get<Balance>,
Inc: Get<Percent>,
{
// Calculates the base deposit as a geometric progression based on the number of signed
// submissions.
//
// The nth term is obtained by calculating `base * (1 + increase_factor)^nth`. Example: factor
// 5, with initial deposit of 1000 and 10% of increase factor is 1000 * (1 + 0.1)^5.
fn convert(queue_len: usize) -> Balance {
let increase_factor: FixedU128 = FixedU128::from_u32(1) + Inc::get().into();

increase_factor.saturating_pow(queue_len).saturating_mul_int(Fixed::get())
}
}

impl<T: Config> Pallet<T> {
/// `Self` accessor for `SignedSubmission<T>`.
pub fn signed_submissions() -> SignedSubmissions<T> {
Expand Down Expand Up @@ -520,14 +548,14 @@ impl<T: Config> Pallet<T> {
size: SolutionOrSnapshotSize,
) -> BalanceOf<T> {
let encoded_len: u32 = raw_solution.encoded_size().saturated_into();
let encoded_len: BalanceOf<T> = encoded_len.into();
let encoded_len_balance: BalanceOf<T> = encoded_len.into();
let feasibility_weight = Self::solution_weight_of(raw_solution, size);

let len_deposit = T::SignedDepositByte::get().saturating_mul(encoded_len);
let len_deposit = T::SignedDepositByte::get().saturating_mul(encoded_len_balance);
let weight_deposit = T::SignedDepositWeight::get()
.saturating_mul(feasibility_weight.ref_time().saturated_into());

T::SignedDepositBase::get()
T::SignedDepositBase::convert(Self::signed_submissions().len())
.saturating_add(len_deposit)
.saturating_add(weight_deposit)
}
Expand All @@ -541,6 +569,7 @@ mod tests {
Phase,
};
use frame_support::{assert_noop, assert_ok, assert_storage_noop};
use sp_runtime::Percent;

#[test]
fn cannot_submit_too_early() {
Expand Down Expand Up @@ -779,6 +808,56 @@ mod tests {
})
}

#[test]
fn geometric_deposit_queue_size_works() {
let constant = vec![1000; 10];
// geometric progression with 10% increase in each iteration for 10 terms.
let progression_10 = vec![1000, 1100, 1210, 1331, 1464, 1610, 1771, 1948, 2143, 2357];
let progression_40 = vec![1000, 1400, 1960, 2744, 3841, 5378, 7529, 10541, 14757, 20661];

let check_progressive_base_fee = |expected: &Vec<u64>| {
for s in 0..SignedMaxSubmissions::get() {
let account = 99 + s as u64;
Balances::make_free_balance_be(&account, 10000000);
let mut solution = raw_solution();
solution.score.minimal_stake -= s as u128;

assert_ok!(MultiPhase::submit(RuntimeOrigin::signed(account), Box::new(solution)));
assert_eq!(balances(&account).1, expected[s as usize])
}
};

ExtBuilder::default()
.signed_max_submission(10)
.signed_base_deposit(1000, true, Percent::from_percent(0))
.build_and_execute(|| {
roll_to_signed();
assert!(MultiPhase::current_phase().is_signed());

check_progressive_base_fee(&constant);
});

ExtBuilder::default()
.signed_max_submission(10)
.signed_base_deposit(1000, true, Percent::from_percent(10))
.build_and_execute(|| {
roll_to_signed();
assert!(MultiPhase::current_phase().is_signed());

check_progressive_base_fee(&progression_10);
});

ExtBuilder::default()
.signed_max_submission(10)
.signed_base_deposit(1000, true, Percent::from_percent(40))
.build_and_execute(|| {
roll_to_signed();
assert!(MultiPhase::current_phase().is_signed());

check_progressive_base_fee(&progression_40);
});
}

#[test]
fn call_fee_refund_is_limited_by_signed_max_refunds() {
ExtBuilder::default().build_and_execute(|| {
Expand Down
Loading

0 comments on commit 614aa31

Please sign in to comment.