diff --git a/substrate/frame/delegated-staking/src/lib.rs b/substrate/frame/delegated-staking/src/lib.rs index 61809dcb54eea..8203f75133057 100644 --- a/substrate/frame/delegated-staking/src/lib.rs +++ b/substrate/frame/delegated-staking/src/lib.rs @@ -411,6 +411,11 @@ pub mod pallet { Delegation::::can_delegate(&delegator, &agent), Error::::InvalidDelegation ); + + // Implementation note: Staking uses deprecated locks (similar to freeze) which are not + // mutually exclusive of holds. This means, if we allow delegating for existing stakers, + // already staked funds might be reused for delegation. We avoid that by just blocking + // this. ensure!(!Self::is_direct_staker(&delegator), Error::::AlreadyStaking); // ensure agent is sane. @@ -505,7 +510,7 @@ impl Pallet { Preservation::Expendable, )?; - T::CoreStaking::update_payee(who, reward_account)?; + T::CoreStaking::set_payee(who, reward_account)?; // delegate all transferred funds back to agent. Self::do_delegate(proxy_delegator, Agent::from(who.clone()), amount_to_transfer)?; diff --git a/substrate/frame/delegated-staking/src/tests.rs b/substrate/frame/delegated-staking/src/tests.rs index 385bb17ddadbd..ade0872dd3900 100644 --- a/substrate/frame/delegated-staking/src/tests.rs +++ b/substrate/frame/delegated-staking/src/tests.rs @@ -573,14 +573,14 @@ mod staking_integration { 100 )); - // update_payee to self fails. + // set_payee to self fails. assert_noop!( - ::update_payee(&200, &200), + ::set_payee(&200, &200), StakingError::::RewardDestinationRestricted ); // passing correct reward destination works - assert_ok!(::update_payee(&200, &201)); + assert_ok!(::set_payee(&200, &201)); // amount is staked correctly assert!(eq_stake(200, 100, 100)); diff --git a/substrate/frame/nomination-pools/src/lib.rs b/substrate/frame/nomination-pools/src/lib.rs index 472f0affcc735..70ad06e2a4da3 100644 --- a/substrate/frame/nomination-pools/src/lib.rs +++ b/substrate/frame/nomination-pools/src/lib.rs @@ -1985,8 +1985,13 @@ pub mod pallet { #[pallet::call] impl Pallet { - /// Stake funds with a pool. The amount to bond is transferred from the member to the - /// pools account and immediately increases the pools bond. + /// Stake funds with a pool. The amount to bond is transferred from the member to the pool + /// account and immediately increases the pools bond. + /// + /// The method of transferring the amount to the pool account is determined by + /// [`adapter::StakeStrategyType`]. If the pool is configured to use + /// [`adapter::StakeStrategyType::Delegate`], the funds remain in the account of + /// the `origin`, while the pool gains the right to use these funds for staking. /// /// # Note /// diff --git a/substrate/frame/nomination-pools/src/mock.rs b/substrate/frame/nomination-pools/src/mock.rs index 6c0082073f682..cc942039760c0 100644 --- a/substrate/frame/nomination-pools/src/mock.rs +++ b/substrate/frame/nomination-pools/src/mock.rs @@ -136,7 +136,7 @@ impl sp_staking::StakingInterface for StakingMock { Ok(()) } - fn update_payee(_stash: &Self::AccountId, _reward_acc: &Self::AccountId) -> DispatchResult { + fn set_payee(_stash: &Self::AccountId, _reward_acc: &Self::AccountId) -> DispatchResult { unimplemented!("method currently not used in testing") } diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs index b19a127d13c43..2df3bc084eb0b 100644 --- a/substrate/frame/staking/src/pallet/impls.rs +++ b/substrate/frame/staking/src/pallet/impls.rs @@ -1779,7 +1779,7 @@ impl StakingInterface for Pallet { .map(|_| ()) } - fn update_payee(stash: &Self::AccountId, reward_acc: &Self::AccountId) -> DispatchResult { + fn set_payee(stash: &Self::AccountId, reward_acc: &Self::AccountId) -> DispatchResult { // Since virtual stakers are not allowed to compound their rewards as this pallet does not // manage their locks, we do not allow reward account to be set same as stash. For // external pallets that manage the virtual bond, they can claim rewards and re-bond them. @@ -1788,12 +1788,12 @@ impl StakingInterface for Pallet { Error::::RewardDestinationRestricted ); - // since controller is deprecated and this function is never used for old ledgers with - // distinct controllers, we can safely assume that stash is the controller. - Self::set_payee( - RawOrigin::Signed(stash.clone()).into(), - RewardDestination::Account(reward_acc.clone()), - ) + let ledger = Self::ledger(Stash(stash.clone()))?; + let _ = ledger + .set_payee(RewardDestination::Account(reward_acc.clone())) + .defensive_proof("ledger was retrieved from storage, thus its bonded; qed.")?; + + Ok(()) } fn chill(who: &Self::AccountId) -> DispatchResult { diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index c35e5e8a06c6f..0b6aad4a1b082 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -7157,7 +7157,7 @@ mod staking_unchecked { // cannot set via set_payee as well. assert_noop!( - ::update_payee(&10, &10), + ::set_payee(&10, &10), Error::::RewardDestinationRestricted ); }); @@ -7219,7 +7219,7 @@ mod staking_unchecked { // migrate them to virtual staker ::migrate_to_virtual_staker(&200); // payee needs to be updated to a non-stash account. - assert_ok!(::update_payee(&200, &201)); + assert_ok!(::set_payee(&200, &201)); // ensure the balance is not locked anymore assert_eq!(Balances::balance_locked(crate::STAKING_ID, &200), 0); @@ -7246,7 +7246,7 @@ mod staking_unchecked { // make 101 a virtual nominator ::migrate_to_virtual_staker(&101); // set payee different to self. - assert_ok!(::update_payee(&101, &102)); + assert_ok!(::set_payee(&101, &102)); // cache values let nominator_stake = Staking::ledger(101.into()).unwrap().active; @@ -7321,7 +7321,7 @@ mod staking_unchecked { // make 101 a virtual nominator ::migrate_to_virtual_staker(&101); // set payee different to self. - assert_ok!(::update_payee(&101, &102)); + assert_ok!(::set_payee(&101, &102)); // cache values let validator_balance = Balances::free_balance(&11); diff --git a/substrate/primitives/staking/src/lib.rs b/substrate/primitives/staking/src/lib.rs index c1cf7f2778ff6..03c1af50240bb 100644 --- a/substrate/primitives/staking/src/lib.rs +++ b/substrate/primitives/staking/src/lib.rs @@ -254,8 +254,8 @@ pub trait StakingInterface { /// schedules have reached their unlocking era should allow more calls to this function. fn unbond(stash: &Self::AccountId, value: Self::Balance) -> DispatchResult; - /// Update the reward destination for the ledger associated with the stash. - fn update_payee(stash: &Self::AccountId, reward_acc: &Self::AccountId) -> DispatchResult; + /// Set the reward destination for the ledger associated with the stash. + fn set_payee(stash: &Self::AccountId, reward_acc: &Self::AccountId) -> DispatchResult; /// Unlock any funds schedule to unlock before or at the current era. ///