[ValSet-Pref] Added Proto Definitions and spec

[stackman27]

Part of : #2579

This PR definies the proto messages to build validator-set-preference. There are 3 tx messages;

1. CreateValidatorSetPreference (creates a new {valAddr, Weight} set)
2. StakeToValidatorSet (gets the tokens and stakes to the existing validator set)
3. UnStakeFromoValidatorSet (unstakes the coins from an existing validator set)

Brief Changelog

n/a

Testing and Verifying

n/a

Documentation and Release Note

• Does this pull request introduce a new feature or user-facing behavior changes? (yes / no)
• Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? (yes / no)
• How is the feature or change documented? (not applicable / specification (x/<module>/spec/) / Osmosis docs repo / not documented)

[p0mvn]

What if I stake with set A, then I change my preferences to different proportions (set B) and try to unstake? Is there some kind of history of preferences to make sure that we don't attempt to unstake more or fewer tokens based on the changed preference list?

[stackman27]

hmm thats interesting. Are you recommending something like this approach?

UserA -> {ValA: 0.5, ValB: 0.2, ValC: 0.3}
User A stakes: 100osmo {ValA-> 50osmo, ValB-> 20osmo, ValC -> 30osmo} 
** Store the history of {weight, amount} ** 

UserA modifies the weight -> {ValA: 0.1, ValB: 0.1, ValC: 0.8}
** Internally check if this modification is possible **  
** Only allow weight modification if the newAmountToStake < oldAmountStaked which can be calculated by weight*TotalAmount ** 

UserA unstakes 50osmo -> {ValA -> 5osmo, ValB -> 10osmo, ValC -> 40osmo} 
**Result in error because ValC only has 30tokens staked** 

[p0mvn]

On another look, I think I just misinterpreted what is happening here.

I guess when userA changes their preferences list, the unstaking from the old set and restaking to a new set is going to happen behind the scenes. That way, the problem I originally mentioned wouldn't happen. It might be worth specifying in the doc that changing the preferences list implies that we unstake from the old + restake to a new set.

[stackman27]

aah i see. Yea i believe this is a much cleaner approach. I will mention it :))

[stackman27]

I'm going to leave this unresolved, so that we can get more feedback from other reviewers

[ValarDragon]

So this spec doesn't discuss the case of what is done when a user does some 'direct' undelegation.

So I set valset { A: .5, B: .2, C: .3 }, and suppose I stake 100 to it, to get:

A: 50, B: 20, C: 30

Then I directly undelegate 20 from B. (So I'm at A:50, C:30)

If I try to valset undelegate 80, what should happen. Options that I think are fine:

• valset Undelegate fails
• valset Undelegate Takes {arbitrary distribution across valset validators if theres been direct modification that is unsatisfiable}

[ValarDragon]

I find this generally not that concerning, since theres multiple direct user actions to fix this

[stackman27]

hmm i didn't really think about "direct unstake" and "val-set undelegate" colliding. Should i look into updating "validator-set" state based on the txs from "direct undelegate" and vice versa?

[mattverse]

Nice work! The implementation seems pretty straight foward!
Left some comments, please take a look 🌮

[stackman27]

Thank you @p0mvn and @mattverse for the feedback! Resolved pretty much all the suggestions

(:<. . 🌮

[ValarDragon]

Suggested change

	message MsgCreateValidatorSetPreference {
	message MsgSetValidatorSetPreference {

[ValarDragon]

I think we just make 1 message, SetValidatorSetPreference wdyt?

(As opposed to create + update)

[alexanderbez]

I agree with @ValarDragon here

[ValarDragon]

Why is this in the struct? We should just be keying using the delegator address

[mattverse]

Changes LGTM Once @ValarDragon's suggestion of changing message name get's changed!

[alexanderbez]

This LGTM. I think folks left some good feedback that looks like it's being addressed, so approving 👍

[alexanderbez]

I agree with @ValarDragon here

[stackman27]

Hi @ValarDragon, all comments resolved!