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] Simulator Fix for RedelegateValSet & UndelegateFromValset #4073

Merged
merged 12 commits into from
Jan 22, 2023

Conversation

stackman27
Copy link
Contributor

Part of : #2579

What is the purpose of the change

Simulator Fix
Logs:

❯  make test-sim-app
 "valsetpref": {
  "MsgDelegateToValidatorSet": {
   "failure": 86,
   "ok": 19
  },
  "MsgRedelegateValSet": {
   "failure": 100,
   "ok": 25
  },
  "MsgUndelegateFromValidatorSet": {
   "ok": 111
  },
  "SetValidatorSetPreference": {
   "failure": 4,
   "ok": 106
  }

❯  make test-sim-determinism 
 "valsetpref": {
  "MsgDelegateToValidatorSet": {
   "failure": 7,
   "ok": 3
  },
  "MsgRedelegateValSet": {
   "failure": 2,
   "ok": 2
  },
  "MsgUndelegateFromValidatorSet": {
   "ok": 8
  },
  "SetValidatorSetPreference": {
   "ok": 5
  }
 }

❯  make test-sim-determinism --seed=5577006791947779410
 "valsetpref": {
  "MsgDelegateToValidatorSet": {
   "failure": 11
  },
  "MsgRedelegateValSet": {
   "failure": 7
  },
  "MsgUndelegateFromValidatorSet": {
   "ok": 7
  },
  "SetValidatorSetPreference": {
   "failure": 2
  }
 }

Brief Changelog

n/a

Testing and Verifying

Sim test

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 C:app-wiring Changes to the app folder C:simulator Edits simulator or simulations labels Jan 20, 2023
@stackman27 stackman27 added the V:state/breaking State machine breaking PR label Jan 20, 2023
@sunnya97
Copy link
Collaborator

Task linked: CU-8669c0uzk Simulator Fix

x/valset-pref/simulation/sim_msgs.go Outdated Show resolved Hide resolved
x/valset-pref/simulation/sim_msgs.go Outdated Show resolved Hide resolved
@stackman27 stackman27 requested a review from czarcas7ic January 21, 2023 07:43
Copy link
Member

@mattverse mattverse left a comment

Choose a reason for hiding this comment

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

Left some minor comments! Otherwise main logic seems smooth

}, nil
}
if sim.StakingKeeper().HasReceivingRedelegation(ctx, delAddr, val) {
return nil, fmt.Errorf("receveing redelegation is not allowed for source validators")
Copy link
Member

Choose a reason for hiding this comment

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

what does it mean here by "source validators"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For redelegation, we need validators to redelegate to(target validators) and validators to redelegate from(source validators).

Comment on lines +143 to +145
if sim.StakingKeeper().HasReceivingRedelegation(ctx, delAddr, val) {
return nil, fmt.Errorf("receveing redelegation is not allowed for target validators")
}
Copy link
Member

Choose a reason for hiding this comment

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

Why are they not allowed to receive redelegations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because this is the clause in redelegation. For ex:
ValA --- redelegation --> ValB
Val --redelegation --> ValC (ERROR: because ValB is receiving redelegation)

x/valset-pref/simulation/sim_msgs.go Show resolved Hide resolved
x/valset-pref/simulation/sim_msgs.go Outdated Show resolved Hide resolved
@stackman27
Copy link
Contributor Author

Addressed your comments @mattverse @czarcas7ic. Ready for rereview!

@stackman27 stackman27 requested a review from mattverse January 22, 2023 00:44
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.

LGTM!

@czarcas7ic czarcas7ic merged commit 5daf0eb into main Jan 22, 2023
@czarcas7ic czarcas7ic deleted the sis/valset-sim-fix branch January 22, 2023 02:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:app-wiring Changes to the app folder C:simulator Edits simulator or simulations V:state/breaking State machine breaking PR
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants