Configurable compounding staking reward

kianenigma commented 3 years ago

The RewardDestination::Staked is essentially compounding staking rewards, which is pretty cool. An additional feature could be configurability about what percentage of the rewards to be compounded and which percentage not. To do that, you need to specify a pair of current staking rewards, and a single perbill value (that ratio of the second will be the leftover).

// Probably anyone can come up with a better name than this, but anyhow. 
enum SplitOrUnifiedRewardDestination { 
     /// What we have now
    Unified(RewardDestination),
    /// Split the reward between two of the above strategies
    Split(RewardDestination, Perbill, RewardDestination),
} 

impl From<RewardDestination> for SplitOrUnifiedRewardDestination { ... }

This is useful for strategies (that I think are common) where you want x% of your rewards to be compounded and (1-x)% to be saved, or cashed out etc, and preferably you don't want to use your stash key every time you want to make such a split.

I am happy to work on this myself or mentor. Feedback on the idea itself would also be welcome (@wpank @shawntabrizi).

bkchr commented 3 years ago

Nice!

I thought about exactly this some time ago :D

shawntabrizi commented 3 years ago

I wonder who is really asking for this?

In general, it seems that we should only support 1 account to be paid in such a manner, or else we will double the base cost of all reward payouts since it will potentially update 2 accounts instead of 1.

4meta5 commented 3 years ago

What if we just add a variant to RewardDestination like

pub enum RewardDestination {
    ...
    Split(Perbill),
}

such that this splits between Stash and Staked by default such that Perbill * amount goes to Stash and (1-Perbill) * amount goes to Staked. There might be more sensible defaults, but I think this way doesn't hurt the worst case because it doesn't have potentially two dead accounts. With this in mind, we could add more variants as long as they only have one AccountId because that is the worst case scenario.

pub enum RewardDestination {
    ...
    SplitStash(AccountId,Perbill),
    SplitStake(AccountId,Perbill),
}

EDIT: still updates two accounts so still might not be the right change

shawntabrizi commented 3 years ago

@4meta5 yeah, unfortunately that doesnt satisfy my concern. The solution would be to update free balance and locked balance of the stash account, and then if they wanted to move funds out of the stash account, they could do that manually, but may not be very friendly... so i go back to my question, who was asking for this? lol

kianenigma commented 3 years ago

they could do that manually, but may not be very friendly... so i go back to my question, who was asking for this?

Exactly, this is not friendly at all since you need to your stash key every time.

so i go back to my question, who was asking for this?

tbh I wanted something like this personally and thought it would be something useful, and the rest is public here. I don't think anyone has asked for it, but I believe once it exists, people will use it.

I would personally not drop this issue just because it complicates weights. Many things do, and we should consider them further before being discarded. Also, it doesn't seem to be that the weight situation is much hard to solve. Something as simple as: "payout transaction will have to mention the number of compound payout targets (where we assume the rest are singular)" will solve it, to the best of my knowledge.

shawntabrizi commented 3 years ago

So I am a validator, and i have 100 nominators. 30 of those nominators have a 2 account payout scheme like designed in this thread.

So I would need to call payout_stakers(validator_id, era, 30) knowing that 30 people would have 2 sources of payouts...?

and to know that, I would need to query all 100 nominators for each era to know their staking preferences?

And then what if in the future someone asks to support 3 payment locations? Should we do it?

This all sounds like you are imposing your specific needs onto an otherwise pretty clean API.

At this point, it would make more sense to me to just double the weight of payouts. But that will make batching and whatever else slower, which really isnt a big deal. But i think making the API worse for this feature is a no-go to me.

kianenigma commented 3 years ago

This all sounds like you are imposing your specific needs onto an otherwise pretty clean API.

That's a fair comment if no one other than me would have need this. I am not sure how to ask around about it, but my gut feeling is still that many people will use this if it existed. In which case then making the payout default weight 2x is fine.

wpank commented 3 years ago

I can see this being useful, but I don't think it would be widely used. An example use case would be some validator that wants to pay for their infrastructure costs via DOT/KSM - they can specify a percentage to not get re-staked that they can then cash out and pay for server costs with. I don't think it would be widely used though, and if it complicates things a lot, maybe not worth implementing at the moment as the amount of validators / nominators is still growing (given the previous issues of weights with election solutions).

rphmeier commented 3 years ago

It's also not too hard to manually compound just by issuing a transaction or two on-chain to bond extra. bond_extra requires stash keys so it's less "set it and forget it" and probably not something you'd want online and automated.

Maybe we want an alternative mechanism where arbitrary accounts can contribute to the bond of some stash. Open question about how important permission to do so is. I suspect we can allow stashes or controllers to submit a permission value which is one of {Nobody, Vec<AccountId>, Everybody}. In more detail:

A hot account X invokes bond_to(C, Y)
This transfers C tokens from X's free balance directly to the bond of stash Y. Only if Y is already in the staking system. Only if X is permitted to do so by Y.

Then it should be relatively simple to handle more complicated compounding with a bot that just invokes bond_to from a hot payee address.

ruseinov commented 1 year ago

One other idea is to do this:

pub enum RewardDestination<AccountId> {
    Compound(RewardDestination, Perbill, RewardDestination)
...
}

And then Compound(Compound(Dest, Perbill, Dest), Perbill, Compound(Dest, Perbill, Dest)) is possible. The level of recursion could be limited to avoid going too deep.

rossbulat commented 1 year ago

Moonbeam now supports auto compounding a percentage of rewards. Simple but effective split of rewards as they come in.

I can see this being useful if I am a nominator and I want to split my daily rewards simply to cash out a percentage for personal expenses, or diversify my portfolio from an investment perspective. I may wish to take 50% of my rewards and convert it into another token, or send it to a pool that I want to bolster.

I don't have to pay additional fees to re-bond every day if I have RewardDestination::Stash
I don't have to wait 28 days to get free balance if I have RewardDestination::Staked

If the split mechanism is in place, we could build upon this in various ways, such as sending the free balance chunk to smart contracts that can do various things with those rewards.

In any case, I think a simple split to free balance, to the same account, is worthwhile doing. I like @ruseinov's suggestion, but do we need an additional variant? We could simply migrate:

RewardDestination::Staked

to

RewardDestination::Compound(Option<Perbill>) 

or

RewardDestination::Compound((Option<Perbill>, Option<AccountId>))

Where Option<Perbill> is the percentage to be compounded, if any, and Option<AccountId>, if any, is an external account to transfer the reward to. Perbill could be 100% by default, and be configurable via set_payee. We could even remove RewardDestination::Stash altogether by having a None, None value inside this variant.

In regards to weights, the split would just require both a deposit into the stash and a read + write operation of ledger. So would be the combined weight of the current RewardDestination::Stash and RewardDestination::Staked. Having a Perbill of 100% would be the same weight as RewardDestination::Staked is now, and a value of None would be the same weight as RewardDestination::Stash is now.

RewardDestination::Compound((Option<Perbill>, Option<AccountId>)): compound a percentage of my payouts & give the rest as free balance to myself (None) or an external account.
RewardDestination::Withdraw((Perbill, AccountId)): withdraw a percentage to my account & send the rest to another account. This account could be a smart contract that could then do other interesting things with the payout

Polkadot-Forum commented 1 year ago

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/1

paritytech / polkadot-sdk

Configurable compounding staking reward #466