Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ValSet-Pref] Add RedelegateValidatorSet message #3599

Merged
merged 16 commits into from
Dec 16, 2022

Conversation

stackman27
Copy link
Contributor

@stackman27 stackman27 commented Dec 2, 2022

Part of : #2579

What is the purpose of the change

Introduce Redelegation logic to validator set preference. User now will be able to redelegate from a specific validator to a new set of validator. We are doing this by executing 2 messages;

  1. SetValidatorSetPreference: Updates the existing weights on chain
  2. RedelegateValidatorSet: Performs the redelegation logic using the streaming algorithm

Brief Changelog

n/a

Testing and Verifying

Added msg_server_test.go for TestRedelegateValidatorSet

example output:

EXISTING VAL:  osmovaloper1f99wh220vumlpaudckceqg66x9m6ggnlqzwynt 4000000.000000000000000000
EXISTING VAL:  osmovaloper1wrvu8dudkkgcafrx8dqq6jnzymxtdufjq9p2kp 6640000.000000000000000000
EXISTING VAL:  osmovaloper14sqedajp657t4tkque6d9ahw89wrtygz7qecu8 2400000.000000000000000000
EXISTING VAL:  osmovaloper1rfmmp7j5u7972rsl39dguypk5hwe4zwwdrkpd4 6960000.000000000000000000
NEW VAL:  osmovaloper1cwwh4t3sm06ax36gyaaltc3yc2n7tzwveaszhe 4000000.000000000000000000
NEW VAL:  osmovaloper1qpen3ey0pmaf9zyxe2ptp2sgnk63mfh5da6x3a 4000000.000000000000000000
NEW VAL:  osmovaloper1hap08c8nr7dm9nv92fvxjadz8hwzee9u4kem67 12000000.000000000000000000
Internal DIFF AMOUNT osmovaloper1f99wh220vumlpaudckceqg66x9m6ggnlqzwynt 4000000.000000000000000000
Internal DIFF AMOUNT osmovaloper1wrvu8dudkkgcafrx8dqq6jnzymxtdufjq9p2kp 6640000.000000000000000000
Internal DIFF AMOUNT osmovaloper14sqedajp657t4tkque6d9ahw89wrtygz7qecu8 2400000.000000000000000000
Internal DIFF AMOUNT osmovaloper1rfmmp7j5u7972rsl39dguypk5hwe4zwwdrkpd4 6960000.000000000000000000
Internal DIFF AMOUNT osmovaloper1cwwh4t3sm06ax36gyaaltc3yc2n7tzwveaszhe -4000000.000000000000000000
Internal DIFF AMOUNT osmovaloper1qpen3ey0pmaf9zyxe2ptp2sgnk63mfh5da6x3a -4000000.000000000000000000
Internal DIFF AMOUNT osmovaloper1hap08c8nr7dm9nv92fvxjadz8hwzee9u4kem67 -12000000.000000000000000000
Redelegate osmovaloper1f99wh220vumlpaudckceqg66x9m6ggnlqzwynt osmovaloper1hap08c8nr7dm9nv92fvxjadz8hwzee9u4kem67 4000000.000000000000000000
Redelegate osmovaloper1wrvu8dudkkgcafrx8dqq6jnzymxtdufjq9p2kp osmovaloper1hap08c8nr7dm9nv92fvxjadz8hwzee9u4kem67 6640000.000000000000000000
Redelegate osmovaloper14sqedajp657t4tkque6d9ahw89wrtygz7qecu8 osmovaloper1cwwh4t3sm06ax36gyaaltc3yc2n7tzwveaszhe 2400000.000000000000000000
Redelegate osmovaloper1rfmmp7j5u7972rsl39dguypk5hwe4zwwdrkpd4 osmovaloper1qpen3ey0pmaf9zyxe2ptp2sgnk63mfh5da6x3a 4000000.000000000000000000
Redelegate osmovaloper1rfmmp7j5u7972rsl39dguypk5hwe4zwwdrkpd4 osmovaloper1cwwh4t3sm06ax36gyaaltc3yc2n7tzwveaszhe 1600000.000000000000000000
Redelegate osmovaloper1rfmmp7j5u7972rsl39dguypk5hwe4zwwdrkpd4 osmovaloper1hap08c8nr7dm9nv92fvxjadz8hwzee9u4kem67 1360000.000000000000000000
************************************ FINAL VALIDATOR OUTPUT *********************************
Validators:  osmovaloper1f99wh220vumlpaudckceqg66x9m6ggnlqzwynt 100.000000000000000000
Validators:  osmovaloper1wrvu8dudkkgcafrx8dqq6jnzymxtdufjq9p2kp 100.000000000000000000
Validators:  osmovaloper14sqedajp657t4tkque6d9ahw89wrtygz7qecu8 100.000000000000000000
Validators:  osmovaloper1rfmmp7j5u7972rsl39dguypk5hwe4zwwdrkpd4 100.000000000000000000
Validators:  osmovaloper1cwwh4t3sm06ax36gyaaltc3yc2n7tzwveaszhe 4000100.000000000000000000
Validators:  osmovaloper1qpen3ey0pmaf9zyxe2ptp2sgnk63mfh5da6x3a 4000100.000000000000000000
Validators:  osmovaloper1hap08c8nr7dm9nv92fvxjadz8hwzee9u4kem67 12000100.000000000000000000

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)

@github-actions github-actions bot added the C:docs Improvements or additions to documentation label Dec 2, 2022
// A redelegation object is created every time a redelegation occurs. To prevent "redelegation hopping" redelegations may not occur under the situation that:
// 1. the (re)delegator already has another immature redelegation in progress with a destination to a validator (let's call it Validator X)
// 2. the (re)delegator is attempting to create a new redelegation where the source validator for this new redelegation is Validator X
func (k Keeper) PreformRedelegation(ctx sdk.Context, delegator sdk.AccAddress, existingSet types.ValidatorSetPreferences, newSet []types.ValidatorPreference) error {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notes for reviewers: Best way to follow with this algorithm would be to check README.MD where i have made sure the pseudocode comments matches with the code comments

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The algo looks correct to me, but I think in a follow-up PR we can simplify / break up the logic a bit.

e.g. make getExistingValset -> getExistingDelegations its own function

@stackman27 stackman27 added the V:state/breaking State machine breaking PR label Dec 2, 2022
x/valset-pref/README.md Outdated Show resolved Hide resolved
x/valset-pref/msg_server_test.go Outdated Show resolved Hide resolved
x/valset-pref/msg_server_test.go Outdated Show resolved Hide resolved
x/valset-pref/msg_server_test.go Outdated Show resolved Hide resolved
x/valset-pref/msg_server_test.go Outdated Show resolved Hide resolved
x/valset-pref/validator_set.go Show resolved Hide resolved
x/valset-pref/validator_set.go Outdated Show resolved Hide resolved
x/valset-pref/validator_set.go Outdated Show resolved Hide resolved
x/valset-pref/validator_set.go Outdated Show resolved Hide resolved
Copy link
Member

@czarcas7ic czarcas7ic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love the comments throughout the module!

x/valset-pref/types/keys.go Show resolved Hide resolved
x/valset-pref/validator_set.go Show resolved Hide resolved
proto/osmosis/valset-pref/v1beta1/tx.proto Show resolved Hide resolved
x/valset-pref/validator_set.go Outdated Show resolved Hide resolved
Copy link
Member

@czarcas7ic czarcas7ic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous comments were addressed, LGTM

Copy link
Member

@ValarDragon ValarDragon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Core redelegation loop LGTM, though I think we should simplify the code a bit in a subsequent PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:docs Improvements or additions to documentation V:state/breaking State machine breaking PR
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants