Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Add Beneficiary Account for Rewards #3544

Closed
joepetrowski opened this issue Sep 4, 2019 · 13 comments · Fixed by #6832
Closed

Add Beneficiary Account for Rewards #3544

joepetrowski opened this issue Sep 4, 2019 · 13 comments · Fixed by #6832
Labels
J0-enhancement An additional feature request.
Milestone

Comments

@joepetrowski
Copy link
Contributor

Right now, you can have rewards sent to one of three destinations:

  • Stash, staked
  • Stash, free
  • Controller

We should add the option for the stash to designate a beneficiary account.

Problem with sending to stash: The stash is supposed to be as cold as possible. Ideally, the only transactions it ever submits are bond and set_controller. However, validators will have to liquidate some of their rewards to pay operating costs. If rewards are sent to the stash, then they will have to make a monthly transfer, making the stash keys warm.

Why not send to the controller? Some staking service providers are operating the controller for their clients so that the client doesn't have to manage session keys. The staker wouldn't want to send rewards to the service company because it will add a delay to receiving them, and the service company probably doesn't want them from a tax/accounting perspective.

It would be best to have the option to designate a beneficiary account that the staker can send rewards to. From there they can do what they please, for example liquidate some portion and transfer the rest to a cold account for savings.

@joepetrowski joepetrowski added the J0-enhancement An additional feature request. label Sep 4, 2019
@kianenigma
Copy link
Contributor

Code-wise it is quite trivial (probably a good issue as mentoring/easy). Need to be discussed with the research team if it has any security implications.

@joepetrowski
Copy link
Contributor Author

Code-wise it is quite trivial (probably a good issue as mentoring/easy). Need to be discussed with the research team if it has any security implications.

I was thinking that even I could do it :). But yeah, needs some higher-level input.

@rphmeier
Copy link
Contributor

rphmeier commented Sep 5, 2019

Another question is whether the controller or the stash should be able to change the beneficiary. It would probably be safest to restrict controllers from changing it - it's unlikely to change beyond the initial bond transaction.

@joepetrowski
Copy link
Contributor Author

joepetrowski commented Sep 5, 2019

Another question is whether the controller or the stash should be able to change the beneficiary. It would probably be safest to restrict controllers from changing it - it's unlikely to change beyond the initial bond transaction.

Right, in my idea, the controller could not change it since that key is "warm" and more at risk of compromise.

Perhaps a more advanced feature that the controller could do is split the rewards, e.g.:

  • 20% to beneficiary account
  • 80% to stash, added to stake

The controller could set the 20/80 numbers, but only the stash could designate the beneficiary account.

@burdges
Copy link

burdges commented Sep 6, 2019

I think this sounds fine. A stash accounts specify both a controller account and a beneficiary account independently and without restrictions on the beneficiary, so repeating either the stash or controller works. Yes, only a stash account key can change its controller and beneficiary delegations.

As an aside..

Initially, I thought the stash account needed sole control over its own staking levels. I later relaxed thinking controllers could safely control stash account staking levels. I'd also expect controllers carry out governance voting for stash account keys.

I'm not sure either of these two functions really works perfectly with service providers operating controller keys for clients though. Should we say that operating controller keys for clients is officially discouraged, but not forbidden? We note eager to complicate delegation I think.

@burdges
Copy link

burdges commented Sep 6, 2019

Actually what key does a controller nominate? I'd envisioned a controller nominating another controller, but not sure if that's exactly what we do.

If so, there might be some leeway to provide an avenue for adding some complexity without forcing it onto everyone, i.e. everyone has their own controller, but we permit the staking service provider to go through a couple layers of controller that delegate. This makes the setup for our election scheme more complex though, which sounds bad.

@joepetrowski
Copy link
Contributor Author

I'd also expect controllers carry out governance voting for stash account keys.

Actually, vote proxies are handled in democracy, so controller has no voting power for stash. Although you could delegate the same account as both controller and proxy.

It would be nice if you actually delegated a controller that is controlled by a service provider for staking and a proxy that is controlled by yourself for voting, which is totally possible right now, we just need to communicate this better.

Should we say that operating controller keys for clients is officially discouraged, but not forbidden?

Yeah, I mean I don't think we can really forbid it anyway.

@burdges
Copy link

burdges commented Sep 6, 2019

I'm happy with a separate proxy account of course, so normally the beneficiary would be one of stash, controller, or proxy.

There is still a question about who controls the stash's staked vs free balances, but maybe best leave that to the stash itself if people are delegating to other people's controller keys.

@kianenigma
Copy link
Contributor

Yes, only a stash account key can change its controller and beneficiary delegations.

Well, at least right now a controller can change the delegation (even though there are only 3 options, ctrl, stash, staked).

About the details of what this issue is aiming to solve, this matter should be resolved. Because the same set_payee should be used an extra field must be added here:

/// A destination account for payment.
#[derive(PartialEq, Eq, Copy, Clone, Encode, Decode)]
#[cfg_attr(feature = "std", derive(Debug))]
- pub enum RewardDestination {
+ pub enum RewardDestination<AccountId> {
	/// Pay into the stash account, increasing the amount at stake accordingly.
	Staked,
	/// Pay into the stash account, not increasing the amount at stake.
	Stash,
	/// Pay into the controller account.
	Controller,
+      /// Some beneficiary account. Can not be the stash or the controller itself.
+ .    Benefeciary(AccountId),
}

which can be set.

Looking at it now, I would say set_payee is wrong from the beginning and only stash can set this (regardless of the variants of the RewardDestination)

@burdges
Copy link

burdges commented Sep 6, 2019

It sounds more likely the payee could change the controller really, but probably just leave all that to the stash.

@gavofyork
Copy link
Member

This stuff is, I think, post-1.0. I don't want to hold up or complicate NPoS auditing (and certainly not risk introducing subtle bugs) for stuff that can be worked around just with one or two daily transactions from the controller account.

@wpank
Copy link

wpank commented Aug 4, 2020

I think this is a very useful feature that many have suggested wanting, so just wanted to bump this issue.

@Polkadot-Forum
Copy link

This issue has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/revision-of-rewarddestination-to-account-for-split-and-controller-removal/3360/13

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
J0-enhancement An additional feature request.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants