Skip to content

Commit

Permalink
Remove MaxNominators congig type from domains and update benchmarks
Browse files Browse the repository at this point in the history
  • Loading branch information
@vedhavyas
vedhavyas committed May 23, 2024
1 parent 82e952c commit 0fc99c1
Show file tree
Hide file tree
Showing 8 changed files with 356 additions and 418 deletions.
71 changes: 37 additions & 34 deletions crates/pallet-domains/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,14 @@ use sp_core::crypto::{Ss58Codec, UncheckedFrom};
use sp_core::ByteArray;
use sp_domains::{
dummy_opaque_bundle, ConfirmedDomainBlock, DomainId, ExecutionReceipt, OperatorAllowList,
OperatorId, OperatorPublicKey, OperatorSignature, RuntimeType,
OperatorId, OperatorPublicKey, OperatorSignature, PermissionedActionAllowedBy, RuntimeType,
};
use sp_domains_fraud_proof::fraud_proof::FraudProof;
use sp_runtime::traits::{CheckedAdd, One, Zero};
use sp_std::collections::btree_set::BTreeSet;

const SEED: u32 = 0;
const MAX_NOMINATORS_TO_SLASH_WITHOUT_OPERATOR: u32 = MAX_NOMINATORS_TO_SLASH - 1;

#[benchmarks]
mod benchmarks {
Expand Down Expand Up @@ -356,27 +357,19 @@ mod benchmarks {
assert!(staking_summary.current_epoch_rewards.is_empty());
}

/// Benchmark `do_finalize_slashed_operators` based on the number of operator and the number of their
// nominator that has slashed in the current epoch
/// Benchmark `do_slash_operator` based on the number of their
// nominators
#[benchmark]
fn finalize_slashed_operators(
n: Linear<1, { MAX_BUNLDE_PER_BLOCK * T::MaxNominators::get() }>,
) {
fn slash_operator(n: Linear<0, MAX_NOMINATORS_TO_SLASH_WITHOUT_OPERATOR>) {
let minimum_nominator_stake = T::MinNominatorStake::get();
let domain_id = register_domain::<T>();

let (operator_count, nominator_per_operator) = if n <= MAX_BUNLDE_PER_BLOCK {
(n, 1)
} else {
(MAX_BUNLDE_PER_BLOCK, n.div_ceil(MAX_BUNLDE_PER_BLOCK))
};
let operator_count = 1;
let nominator_per_operator = n;

let (_, operator_id) =
register_operator_with_seed::<T>(domain_id, 1, minimum_nominator_stake);

let mut operator_ids = Vec::new();
for i in 0..operator_count {
let (_, operator_id) =
register_operator_with_seed::<T>(domain_id, i + 1, minimum_nominator_stake);
operator_ids.push(operator_id);
}
do_finalize_domain_current_epoch::<T>(domain_id)
.expect("finalize domain staking should success");

Expand All @@ -386,24 +379,21 @@ mod benchmarks {
T::Currency::minimum_balance() + 1u32.into(),
);

for (i, operator_id) in operator_ids.iter().enumerate() {
// Minus one since the operator owner is already a nominator
for j in 1..nominator_per_operator {
let nominator = account("nominator", i as u32, j);
T::Currency::set_balance(&nominator, minimum_nominator_stake * 2u32.into());
assert_ok!(Domains::<T>::nominate_operator(
RawOrigin::Signed(nominator).into(),
*operator_id,
minimum_nominator_stake,
));
}
do_finalize_domain_current_epoch::<T>(domain_id)
.expect("finalize domain staking should success");
for j in 0..nominator_per_operator {
let nominator = account("nominator", 0, j);
T::Currency::set_balance(&nominator, minimum_nominator_stake * 2u32.into());
assert_ok!(Domains::<T>::nominate_operator(
RawOrigin::Signed(nominator).into(),
operator_id,
minimum_nominator_stake,
));
}
do_finalize_domain_current_epoch::<T>(domain_id)
.expect("finalize domain staking should success");

// Slash operator
do_mark_operators_as_slashed::<T>(
operator_ids.into_iter(),
vec![operator_id].into_iter(),
SlashedReason::InvalidBundle(1u32.into()),
)
.expect("slash operator should success");
Expand Down Expand Up @@ -548,8 +538,13 @@ mod benchmarks {
initial_balances: Default::default(),
};

assert_ok!(Domains::<T>::set_permissioned_action_allowed_by(
RawOrigin::Root.into(),
PermissionedActionAllowedBy::Anyone,
));

#[extrinsic_call]
_(RawOrigin::Root, domain_config.clone());
_(RawOrigin::Signed(creator.clone()), domain_config.clone());

let domain_obj = DomainRegistry::<T>::get(domain_id).expect("domain object must exist");
assert_eq!(domain_obj.domain_config, domain_config);
Expand Down Expand Up @@ -659,6 +654,9 @@ mod benchmarks {
let (operator_owner, operator_id) =
register_helper_operator::<T>(domain_id, T::MinNominatorStake::get());

do_finalize_domain_epoch_staking::<T>(domain_id)
.expect("finalize domain staking should success");

#[extrinsic_call]
_(RawOrigin::Signed(operator_owner.clone()), operator_id);

Expand Down Expand Up @@ -778,7 +776,7 @@ mod benchmarks {
assert!(Deposits::<T>::get(operator_id, nominator).is_none());
}

/// Benchmark `unlock_nominator` extrinsic based on the number of nominator of the unlocked operator
/// Benchmark `unlock_nominator` extrinsic for a given de-registered operator
#[benchmark]
fn unlock_nominator() {
let domain_id = register_domain::<T>();
Expand Down Expand Up @@ -884,8 +882,13 @@ mod benchmarks {
initial_balances: Default::default(),
};

assert_ok!(Domains::<T>::instantiate_domain(
assert_ok!(Domains::<T>::set_permissioned_action_allowed_by(
RawOrigin::Root.into(),
PermissionedActionAllowedBy::Anyone,
));

assert_ok!(Domains::<T>::instantiate_domain(
RawOrigin::Signed(creator.clone()).into(),
domain_config.clone(),
));

Expand Down
54 changes: 29 additions & 25 deletions crates/pallet-domains/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,16 +191,18 @@ mod pallet {
do_unlock_nominator, do_withdraw_stake, Deposit, DomainEpoch, Error as StakingError,
Operator, OperatorConfig, SharePrice, StakingSummary, Withdrawal,
};
use crate::staking_epoch::{
do_finalize_domain_current_epoch, do_slash_operator, Error as StakingEpochError,
};
#[cfg(not(feature = "runtime-benchmarks"))]
use crate::staking_epoch::do_slash_operator;
use crate::staking_epoch::{do_finalize_domain_current_epoch, Error as StakingEpochError};
use crate::weights::WeightInfo;
#[cfg(not(feature = "runtime-benchmarks"))]
use crate::DomainHashingFor;
#[cfg(not(feature = "runtime-benchmarks"))]
use crate::MAX_NOMINATORS_TO_SLASH;
use crate::{
BalanceOf, BlockSlot, BlockTreeNodeFor, DomainBlockNumberFor, ElectionVerificationParams,
FraudProofFor, HoldIdentifier, NominatorId, OpaqueBundleOf, ReceiptHashFor, StateRootOf,
MAX_BUNLDE_PER_BLOCK, MAX_NOMINATORS_TO_SLASH, STORAGE_VERSION,
MAX_BUNLDE_PER_BLOCK, STORAGE_VERSION,
};
#[cfg(not(feature = "std"))]
use alloc::string::String;
Expand Down Expand Up @@ -372,10 +374,6 @@ mod pallet {
#[pallet::constant]
type MaxPendingStakingOperation: Get<u32>;

/// The maximum number of nominators for given operator.
#[pallet::constant]
type MaxNominators: Get<u32>;

/// Randomness source.
type Randomness: RandomnessT<Self::Hash, BlockNumberFor<Self>>;

Expand Down Expand Up @@ -1104,10 +1102,14 @@ mod pallet {
SuccessfulBundles::<T>::append(domain_id, bundle_hash);

// slash operator who are in pending slash
// TODO: include this for benchmarking
let _slashed_nominator_count =
do_slash_operator::<T>(domain_id, MAX_NOMINATORS_TO_SLASH)
.map_err(Error::<T>::from)?;
#[cfg(not(feature = "runtime-benchmarks"))]
{
let slashed_nominator_count =
do_slash_operator::<T>(domain_id, MAX_NOMINATORS_TO_SLASH)
.map_err(Error::<T>::from)?;
actual_weight = actual_weight
.saturating_add(Self::actual_slash_operator_weight(slashed_nominator_count));
}

Self::deposit_event(Event::BundleStored {
domain_id,
Expand Down Expand Up @@ -1374,7 +1376,7 @@ mod pallet {
/// Unlocks the nominator under given operator given the unlocking period is complete.
/// A nominator can initiate their unlock given operator is already deregistered.
#[pallet::call_index(11)]
#[pallet::weight(T::WeightInfo::unlock_operator(T::MaxNominators::get()))]
#[pallet::weight(T::WeightInfo::unlock_nominator())]
pub fn unlock_nominator(origin: OriginFor<T>, operator_id: OperatorId) -> DispatchResult {
let nominator = ensure_signed(origin)?;

Expand Down Expand Up @@ -2320,25 +2322,22 @@ impl<T: Config> Pallet<T> {
.saturating_add(
// NOTE: within `submit_bundle`, only one of (or none) `handle_bad_receipt` and
// `confirm_domain_block` can happen, thus we use the `max` of them
T::WeightInfo::handle_bad_receipt(T::MaxNominators::get()).max(

// We use `MAX_BUNLDE_PER_BLOCK` number to assume the number of slashed operators.
// We do not expect so many operators to be slashed but nontheless, if it did happen
// we will limit the weight to 100 operators.
T::WeightInfo::handle_bad_receipt(MAX_BUNLDE_PER_BLOCK).max(
T::WeightInfo::confirm_domain_block(MAX_BUNLDE_PER_BLOCK, MAX_BUNLDE_PER_BLOCK),
),
)
.saturating_add(Self::max_staking_epoch_transition())
.saturating_add(T::WeightInfo::slash_operator(MAX_NOMINATORS_TO_SLASH))
}

pub fn max_staking_epoch_transition() -> Weight {
T::WeightInfo::operator_reward_tax_and_restake(MAX_BUNLDE_PER_BLOCK)
.saturating_add(T::WeightInfo::finalize_slashed_operators(
// FIXME: the actual value should be `N * T::MaxNominators` where `N` is the number of
// submitter of the bad ER, which is probabilistically bounded by `bundle_slot_probability`
// we use `N = 1` here because `finalize_slashed_operators` is expensive and can consume
// more weight than the max block weight
T::MaxNominators::get(),
))
.saturating_add(T::WeightInfo::finalize_domain_epoch_staking(
T::MaxPendingStakingOperation::get(),
))
T::WeightInfo::operator_reward_tax_and_restake(MAX_BUNLDE_PER_BLOCK).saturating_add(
T::WeightInfo::finalize_domain_epoch_staking(T::MaxPendingStakingOperation::get()),
)
}

fn actual_epoch_transition_weight(epoch_transition_res: EpochTransitionResult) -> Weight {
Expand All @@ -2353,6 +2352,11 @@ impl<T: Config> Pallet<T> {
)
}

#[cfg(not(feature = "runtime-benchmarks"))]
fn actual_slash_operator_weight(slashed_nominators: u32) -> Weight {
T::WeightInfo::slash_operator(slashed_nominators)
}

pub fn storage_fund_account_balance(operator_id: OperatorId) -> BalanceOf<T> {
let storage_fund_acc = storage_fund_account::<T>(operator_id);
T::Currency::reducible_balance(&storage_fund_acc, Preservation::Preserve, Fortitude::Polite)
Expand Down
59 changes: 2 additions & 57 deletions crates/pallet-domains/src/staking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,6 @@ pub enum Error {
TooManyPendingStakingOperation,
OperatorNotAllowed,
InvalidOperatorSigningKey,
MaximumNominators,
DuplicateOperatorSigningKey,
MissingOperatorEpochSharePrice,
MissingWithdrawal,
Expand Down Expand Up @@ -636,7 +635,6 @@ pub(crate) fn do_nominate_operator<T: Config>(
if first_deposit_in_epoch {
NominatorCount::<T>::try_mutate(operator_id, |count| {
*count += 1;
ensure!(*count <= T::MaxNominators::get(), Error::MaximumNominators);
Ok(())
})?;
}
Expand Down Expand Up @@ -1322,7 +1320,7 @@ pub(crate) mod tests {
OperatorConfig, OperatorSigningKeyProofOfOwnershipData, OperatorStatus, StakingSummary,
};
use crate::staking_epoch::{do_finalize_domain_current_epoch, do_slash_operator};
use crate::tests::{new_test_ext, AccountId, ExistentialDeposit, RuntimeOrigin, Test};
use crate::tests::{new_test_ext, ExistentialDeposit, RuntimeOrigin, Test};
use crate::{
bundle_storage_fund, BalanceOf, Error, NominatorId, SlashedReason, MAX_NOMINATORS_TO_SLASH,
};
Expand All @@ -1341,7 +1339,7 @@ pub(crate) mod tests {
use sp_runtime::{PerThing, Perbill};
use std::collections::{BTreeMap, BTreeSet};
use std::vec;
use subspace_runtime_primitives::{Balance, SSC};
use subspace_runtime_primitives::SSC;

type Balances = pallet_balances::Pallet<Test>;
type Domains = crate::Pallet<Test>;
Expand Down Expand Up @@ -1716,59 +1714,6 @@ pub(crate) mod tests {
});
}

#[test]
fn nominate_operator_max_nominators() {
let domain_id = DomainId::new(0);
let operator_account = 0;
let operator_free_balance = 1500 * SSC;
let operator_stake = 1000 * SSC;
let pair = OperatorPair::from_seed(&U256::from(0u32).into());
let data = OperatorSigningKeyProofOfOwnershipData {
operator_owner: operator_account,
};
let signature = pair.sign(&data.encode());
let nominator_account = 26;
let nominator_free_balance = 150 * SSC;
let nominator_stake = 100 * SSC;

let nominator_accounts: Vec<AccountId> = (1..=25).collect();
let nominators: BTreeMap<AccountId, (Balance, Balance)> = nominator_accounts
.into_iter()
.map(|nominator_id| (nominator_id, (nominator_free_balance, nominator_stake)))
.collect();

let mut ext = new_test_ext();
ext.execute_with(|| {
let (operator_id, _) = register_operator(
domain_id,
operator_account,
operator_free_balance,
operator_stake,
10 * SSC,
pair.public(),
signature,
nominators,
);

Balances::set_balance(&nominator_account, nominator_free_balance);
let nominator_count = NominatorCount::<Test>::get(operator_id);

// nomination should fail since Max nominators number has reached.
let res = Domains::nominate_operator(
RuntimeOrigin::signed(nominator_account),
operator_id,
40 * SSC,
);
assert_err!(
res,
Error::<Test>::Staking(crate::staking::Error::MaximumNominators)
);

// should not update nominator count.
assert_eq!(nominator_count, NominatorCount::<Test>::get(operator_id));
});
}

#[test]
fn operator_deregistration() {
let domain_id = DomainId::new(0);
Expand Down
2 changes: 0 additions & 2 deletions crates/pallet-domains/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ parameter_types! {
pub TreasuryAccount: u128 = PalletId(*b"treasury").into_account_truncating();
pub const BlockReward: Balance = 10 * SSC;
pub const MaxPendingStakingOperation: u32 = 512;
pub const MaxNominators: u32 = 25;
pub const DomainsPalletId: PalletId = PalletId(*b"domains_");
pub const DomainChainByteFee: Balance = 1;
pub const MaxInitialDomainAccounts: u32 = 5;
Expand Down Expand Up @@ -266,7 +265,6 @@ impl pallet_domains::Config for Test {
type StakeEpochDuration = StakeEpochDuration;
type TreasuryAccount = TreasuryAccount;
type MaxPendingStakingOperation = MaxPendingStakingOperation;
type MaxNominators = MaxNominators;
type Randomness = MockRandomness;
type PalletId = DomainsPalletId;
type StorageFee = DummyStorageFee;
Expand Down
Loading

0 comments on commit 0fc99c1

Please sign in to comment.