Staking Payouts Only Work for Existing Accounts

[shawntabrizi]

The make_payout function in the Staking Pallet intentionally only pays out to existing accounts:

	/// Actually make a payment to a staker. This uses the currency's reward function
	/// to pay the right payee for the given staker account.
	fn make_payout(stash: &T::AccountId, amount: BalanceOf<T>) -> Option<PositiveImbalanceOf<T>> {
		let dest = Self::payee(stash);
		match dest {
			RewardDestination::Controller => Self::bonded(stash)
				.and_then(|controller|
					T::Currency::deposit_into_existing(&controller, amount).ok()
				),
			RewardDestination::Stash =>
				T::Currency::deposit_into_existing(stash, amount).ok(),
			RewardDestination::Staked => Self::bonded(stash)
				.and_then(|c| Self::ledger(&c).map(|l| (c, l)))
				.and_then(|(controller, mut l)| {
					l.active += amount;
					l.total += amount;
					let r = T::Currency::deposit_into_existing(stash, amount).ok();
					Self::update_ledger(&controller, &l);
					r
				}),
		}
	}

Note all instances of payout use deposit_into_existing.

However, it is possible to set your controller to a dead account, and then payouts here will fail.

(Stash can't really be dead, but maybe some fringe situation it could?)

We probably need some combination of Runtime + UI assistance to prevent users from setting their controller to a dead account, and then requesting a payout.

[shawntabrizi]

Or we could allow deposit_creating for controller.

[xlc]

I will suggest require an alive controller and use inc_ref to ensure it cannot be dead. This also helps on Polkadot that people setting controller to new account (recommended by UI) and they can't transfer fund to new account to do anything.

The payout could be smaller than existential deposit so deposit_creating can still fail.

We have inc_ref for stash, but not controller

substrate/frame/staking/src/lib.rs

Line 1434 in 6d11c9d

system::Module::<T>::inc_ref(&stash);

[gavofyork]

stick to a UI "accomodation" for now, which i believe @jacogr is already working on anyway. i don't want to introduce deposit_creating as it both complicates weights and introduces a way to "transfer" funds, which should be specifically banned at this stage. i would accept a PR to require an alice controller and inc_ref.

[shawntabrizi]

I have created a PR ^^

[shawntabrizi]

@joepetrowski points out that this PR would break the use of sub accounts as controllers since they are mostly unfunded.

What if instead we fallback to paying the stash in the case that the controller cannot be paid?

[xlc]

In that case we can ensure the controller is alive only when reward destination is set to controller?

[shawntabrizi]

@xlc we can do it, but there would be kind of an awkward check in a bunch of places:

• set_controller
• bond
• set_payee
• maybe others?

It would increase the read count of all of these operations, which is also fine, but starting to make me feel less happy about the behavior/logic. Maybe I am being overly critical in my head, this is also a valid option technically speaking.