Skip to content

Commit

Permalink
Merge pull request #2205 from subspace/remove_auto_staking
Browse files Browse the repository at this point in the history 
Remove auto staking on farmer rewards
  • Loading branch information
@vedhavyas
vedhavyas authored Nov 8, 2023
2 parents 2e10e36 + cc051c5 commit 51d3c29
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 202 deletions.
25 changes: 0 additions & 25 deletions crates/pallet-domains/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -419,31 +419,6 @@ mod benchmarks {
);
}

#[benchmark]
fn auto_stake_block_rewards() {
let nominator = account("nominator", 1, SEED);
let minimum_nominator_stake = T::Currency::minimum_balance();
T::Currency::set_balance(
&nominator,
minimum_nominator_stake + T::Currency::minimum_balance(),
);

let domain_id = register_domain::<T>();
let (_, operator_id) = register_helper_operator::<T>(domain_id, minimum_nominator_stake);
assert_ok!(Domains::<T>::nominate_operator(
RawOrigin::Signed(nominator.clone()).into(),
operator_id,
minimum_nominator_stake,
));
do_finalize_domain_staking::<T>(domain_id, 1u32.into())
.expect("finalize domain staking should success");

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

assert_eq!(PreferredOperator::<T>::get(nominator), Some(operator_id));
}

fn register_runtime<T: Config>() -> RuntimeId {
let runtime_blob =
include_bytes!("../res/evm_domain_test_runtime.compact.compressed.wasm").to_vec();
Expand Down
50 changes: 5 additions & 45 deletions crates/pallet-domains/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub mod weights;
extern crate alloc;

use crate::block_tree::verify_execution_receipt;
use crate::staking::{do_nominate_operator, OperatorStatus};
use crate::staking::OperatorStatus;
use codec::{Decode, Encode};
use frame_support::ensure;
use frame_support::traits::fungible::{Inspect, InspectHold};
Expand Down Expand Up @@ -128,9 +128,9 @@ mod pallet {
#[cfg(not(feature = "runtime-benchmarks"))]
use crate::staking::do_reward_operators;
use crate::staking::{
do_auto_stake_block_rewards, do_deregister_operator, do_nominate_operator,
do_register_operator, do_slash_operators, do_switch_operator_domain, do_withdraw_stake,
Error as StakingError, Nominator, Operator, OperatorConfig, StakingSummary, Withdraw,
do_deregister_operator, do_nominate_operator, do_register_operator, do_slash_operators,
do_switch_operator_domain, do_withdraw_stake, Error as StakingError, Nominator, Operator,
OperatorConfig, StakingSummary, Withdraw,
};
#[cfg(not(feature = "runtime-benchmarks"))]
use crate::staking_epoch::do_unlock_pending_withdrawals;
Expand Down Expand Up @@ -574,12 +574,6 @@ mod pallet {
pub(super) type LastEpochStakingDistribution<T: Config> =
StorageMap<_, Identity, DomainId, ElectionVerificationParams<BalanceOf<T>>, OptionQuery>;

/// A preferred Operator for a given Farmer, enabling automatic staking of block rewards.
/// For the auto-staking to succeed, the Farmer must also be a Nominator of the preferred Operator.
#[pallet::storage]
pub(super) type PreferredOperator<T: Config> =
StorageMap<_, Identity, NominatorId<T>, OperatorId, OptionQuery>;

#[derive(TypeInfo, Encode, Decode, PalletError, Debug, PartialEq)]
pub enum BundleError {
/// Can not find the operator for given operator id.
Expand Down Expand Up @@ -1149,32 +1143,14 @@ mod pallet {
Ok(())
}

#[pallet::call_index(10)]
#[pallet::weight(T::WeightInfo::auto_stake_block_rewards())]
pub fn auto_stake_block_rewards(
origin: OriginFor<T>,
operator_id: OperatorId,
) -> DispatchResult {
let who = ensure_signed(origin)?;

do_auto_stake_block_rewards::<T>(who.clone(), operator_id).map_err(Error::<T>::from)?;

Self::deposit_event(Event::PreferredOperator {
operator_id,
nominator_id: who,
});

Ok(())
}

/// Extrinsic to update domain's operator allow list.
/// Note:
/// - If the previous allowed list is set to specific operators and new allow list is set
/// to `Anyone`, then domain will become permissioned to open for all operators.
/// - If the previous allowed list is set to `Anyone` or specific operators and the new
/// allow list is set to specific operators, then all the registered not allowed operators
/// will continue to operate until they de-register themselves.
#[pallet::call_index(12)]
#[pallet::call_index(10)]
#[pallet::weight(Weight::from_all(10_000))]
pub fn update_domain_operator_allow_list(
origin: OriginFor<T>,
Expand Down Expand Up @@ -1784,22 +1760,6 @@ impl<T: Config> Pallet<T> {
})
}

/// Increase the nomination stake by `reward` to the preferred operator of `who`.
/// Preference is removed if the nomination fails.
pub fn on_block_reward(who: NominatorId<T>, reward: BalanceOf<T>) {
PreferredOperator::<T>::mutate_exists(who.clone(), |maybe_preferred_operator_id| {
if let Some(operator_id) = maybe_preferred_operator_id {
if let Err(err) = do_nominate_operator::<T>(*operator_id, who, reward) {
log::trace!(
target: "runtime::domains",
"Failed to stake the reward amount to preferred operator: {err:?}. Removing preference."
);
maybe_preferred_operator_id.take();
}
}
});
}

/// Returns if there are any ERs in the challenge period that have non empty extrinsics.
/// Note that Genesis ER is also considered special and hence non empty
pub fn non_empty_er_exists(domain_id: DomainId) -> bool {
Expand Down
90 changes: 2 additions & 88 deletions crates/pallet-domains/src/staking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::pallet::{
DomainRegistry, DomainStakingSummary, NextOperatorId, NominatorCount, Nominators,
OperatorIdOwner, Operators, PendingDeposits, PendingNominatorUnlocks,
PendingOperatorDeregistrations, PendingOperatorSwitches, PendingOperatorUnlocks,
PendingSlashes, PendingStakingOperationCount, PendingWithdrawals, PreferredOperator,
PendingSlashes, PendingStakingOperationCount, PendingWithdrawals,
};
use crate::staking_epoch::{mint_funds, PendingNominatorUnlock, PendingOperatorSlashInfo};
use crate::{BalanceOf, Config, Event, HoldIdentifier, NominatorId, Pallet};
Expand Down Expand Up @@ -529,29 +529,6 @@ pub(crate) fn do_reward_operators<T: Config>(
})
}

/// Sets Operator as the preferred one to auto stake the block rewards.
/// Caller must be nominator of the Operator.
pub(crate) fn do_auto_stake_block_rewards<T: Config>(
nominator_id: NominatorId<T>,
operator_id: OperatorId,
) -> Result<(), Error> {
// must be a nominator of this operator
ensure!(
Nominators::<T>::contains_key(operator_id, nominator_id.clone()),
Error::UnknownNominator
);

let operator = Operators::<T>::get(operator_id).ok_or(Error::UnknownOperator)?;

ensure!(
operator.status == OperatorStatus::Registered,
Error::OperatorNotRegistered
);

PreferredOperator::<T>::insert(nominator_id, operator_id);
Ok(())
}

/// Freezes the slashed operators and moves the operator to be removed once the domain they are
/// operating finishes the epoch.
pub(crate) fn do_slash_operators<T: Config, Iter: Iterator<Item = OperatorId>>(
Expand Down Expand Up @@ -636,7 +613,7 @@ pub(crate) mod tests {
Config, DomainRegistry, DomainStakingSummary, NextOperatorId, NominatorCount,
OperatorIdOwner, Operators, PendingDeposits, PendingNominatorUnlocks,
PendingOperatorDeregistrations, PendingOperatorSwitches, PendingSlashes,
PendingStakingOperationCount, PendingUnlocks, PendingWithdrawals, PreferredOperator,
PendingStakingOperationCount, PendingUnlocks, PendingWithdrawals,
};
use crate::staking::{
do_nominate_operator, do_reward_operators, do_slash_operators, do_withdraw_stake,
Expand Down Expand Up @@ -1677,69 +1654,6 @@ pub(crate) mod tests {
});
}

#[test]
fn auto_stake_block_rewards() {
let domain_id = DomainId::new(0);
let operator_account = 1;
let operator_free_balance = 1500 * SSC;
let operator_stake = 1000 * SSC;
let pair = OperatorPair::from_seed(&U256::from(0u32).into());

let nominator_account = 2;
let nominator_free_balance = 150 * SSC;
let nominator_stake = 100 * SSC;
let nominators = BTreeMap::from_iter(vec![(
nominator_account,
(nominator_free_balance, nominator_stake),
)]);

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(),
nominators,
);

// Finalize pending deposit
do_finalize_domain_current_epoch::<Test>(domain_id, 0).unwrap();
assert!(!PreferredOperator::<Test>::contains_key(nominator_account));

let res = Domains::auto_stake_block_rewards(
RuntimeOrigin::signed(nominator_account),
operator_id,
);
assert_ok!(res);

assert_eq!(
operator_id,
PreferredOperator::<Test>::get(nominator_account).unwrap()
);

// should auto deposit
Domains::on_block_reward(nominator_account, 10 * SSC);
let deposit = PendingDeposits::<Test>::get(operator_id, nominator_account).unwrap();
assert_eq!(deposit, 10 * SSC);

// an issues with nominator will lead to removal of preference
Operators::<Test>::mutate(operator_id, |maybe_operator| {
let operator = maybe_operator.as_mut().unwrap();
operator.status = OperatorStatus::Deregistered;
});
Domains::on_block_reward(nominator_account, 10 * SSC);

// deposit is still 10 SSC
let deposit = PendingDeposits::<Test>::get(operator_id, nominator_account).unwrap();
assert_eq!(deposit, 10 * SSC);
// no preference
assert!(!PreferredOperator::<Test>::contains_key(nominator_account));
});
}

#[test]
fn pending_staking_operation_limit() {
let domain_id = DomainId::new(0);
Expand Down
37 changes: 3 additions & 34 deletions crates/pallet-domains/src/staking_epoch.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::pallet::{
DomainStakingSummary, LastEpochStakingDistribution, Nominators, OperatorIdOwner, Operators,
PendingDeposits, PendingNominatorUnlocks, PendingOperatorDeregistrations,
PendingOperatorSwitches, PendingOperatorUnlocks, PendingSlashes, PendingStakingOperationCount,
PendingUnlocks, PendingWithdrawals, PreferredOperator,
PendingUnlocks, PendingWithdrawals,
};
use crate::staking::{Error as TransitionError, Nominator, OperatorStatus, Withdraw};
use crate::{
Expand Down Expand Up @@ -246,8 +246,6 @@ fn unlock_operator<T: Config>(operator_id: OperatorId) -> Result<(), Error> {
)
.map_err(|_| TransitionError::RemoveLock)?;

remove_preferred_operator::<T>(nominator_id, &operator_id);

// update pool's remaining shares and stake
total_shares = total_shares
.checked_sub(&nominator.shares)
Expand Down Expand Up @@ -457,17 +455,6 @@ pub(crate) fn mint_funds<T: Config>(
Ok(())
}

/// Remove the preference if the operator id matches
fn remove_preferred_operator<T: Config>(nominator_id: NominatorId<T>, operator_id: &OperatorId) {
PreferredOperator::<T>::mutate_exists(nominator_id, |maybe_preferred_operator| {
if let Some(preferred_operator_id) = maybe_preferred_operator {
if preferred_operator_id == operator_id {
maybe_preferred_operator.take();
}
}
});
}

#[allow(clippy::too_many_arguments)]
fn finalize_nominator_withdrawal<T: Config>(
domain_id: DomainId,
Expand Down Expand Up @@ -504,7 +491,6 @@ fn finalize_nominator_withdrawal<T: Config>(
)
.map_err(|_| TransitionError::RemoveLock)?;

remove_preferred_operator::<T>(nominator_id.clone(), &operator_id);
(nominator_staked_amount, nominator.shares)
}
Withdraw::Some(withdraw_amount) => {
Expand Down Expand Up @@ -693,8 +679,6 @@ pub(crate) fn do_finalize_slashed_operators<T: Config>(
.checked_sub(&locked_amount)
.ok_or(TransitionError::BalanceUnderflow)?;

remove_preferred_operator::<T>(nominator_id, &operator_id);

Ok(())
})?;

Expand Down Expand Up @@ -749,12 +733,11 @@ mod tests {
use crate::pallet::{
DomainRegistry, DomainStakingSummary, LastEpochStakingDistribution, NominatorCount,
Nominators, OperatorIdOwner, Operators, PendingDeposits, PendingOperatorSwitches,
PendingOperatorUnlocks, PendingUnlocks, PendingWithdrawals, PreferredOperator,
PendingOperatorUnlocks, PendingUnlocks, PendingWithdrawals,
};
use crate::staking::tests::register_operator;
use crate::staking::{
do_auto_stake_block_rewards, do_deregister_operator, do_nominate_operator,
do_reward_operators, StakingSummary,
do_deregister_operator, do_nominate_operator, do_reward_operators, StakingSummary,
};
use crate::staking_epoch::{
do_finalize_domain_current_epoch, do_finalize_operator_deregistrations,
Expand Down Expand Up @@ -897,16 +880,6 @@ mod tests {
.unwrap()
}

for nominator in &nominators {
if !nominator.1 .1.is_zero() {
do_auto_stake_block_rewards::<Test>(*nominator.0, operator_id).unwrap();
assert_eq!(
operator_id,
PreferredOperator::<Test>::get(nominator.0).unwrap()
)
}
}

// de-register operator
do_deregister_operator::<Test>(operator_account, operator_id).unwrap();

Expand Down Expand Up @@ -935,10 +908,6 @@ mod tests {
PendingWithdrawals::<Test>::get(operator_id, *nominator.0),
None
);

if !nominator.1 .0.is_zero() {
assert!(!PreferredOperator::<Test>::contains_key(nominator.0))
}
}

assert_eq!(Operators::<Test>::get(operator_id), None);
Expand Down
12 changes: 2 additions & 10 deletions crates/subspace-runtime/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
spec_version: 1,
impl_version: 0,
apis: RUNTIME_API_VERSIONS,
transaction_version: 0,
transaction_version: 1,
state_version: 0,
};

Expand Down Expand Up @@ -637,14 +637,6 @@ impl pallet_domains::Config for Runtime {
type SudoId = SudoId;
}

pub struct StakingOnReward;

impl pallet_rewards::OnReward<AccountId, Balance> for StakingOnReward {
fn on_reward(account: AccountId, reward: Balance) {
Domains::on_block_reward(account, reward);
}
}

parameter_types! {
pub const BlockReward: Balance = SSC / (ExpectedVotesPerBlock::get() as Balance + 1);
pub const VoteReward: Balance = SSC / (ExpectedVotesPerBlock::get() as Balance + 1);
Expand All @@ -658,7 +650,7 @@ impl pallet_rewards::Config for Runtime {
type FindBlockRewardAddress = Subspace;
type FindVotingRewardAddresses = Subspace;
type WeightInfo = ();
type OnReward = StakingOnReward;
type OnReward = ();
}

impl pallet_runtime_configs::Config for Runtime {
Expand Down

0 comments on commit 51d3c29

Please sign in to comment.