-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
R4R: allow multiple simultaneous redelegations/ubds between same delegator/validator(s) addresses #3243
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #3243 +/- ##
===========================================
- Coverage 55.65% 55.64% -0.01%
===========================================
Files 134 134
Lines 9716 9746 +30
===========================================
+ Hits 5407 5423 +16
- Misses 3965 3980 +15
+ Partials 344 343 -1 |
x/stake/keeper/slash.go
Outdated
|
||
if unbondingDelegation.MinTime.Before(now) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note this subtle change of this line to instead use of entry.IsMature(now)
- previously a delegation would have been slashed if it was at MinTime
(which is a bit of an ambiguous name supposed to represent "min time to completion"). Under the updates the delegation will not be slashed if now
==completionTime
(rename from MinTime
) as this would be considered a mature bond.
x/stake/keeper/slash.go
Outdated
return sdk.ZeroInt() | ||
} | ||
|
||
if redelegation.MinTime.Before(now) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same subtle change as previously mentioned
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly LGTM, tests look fine, one minor comment. We can delete ErrConflictingRedelegation
in x/stake/types/errors.go
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few minor comments, but the changes look reasonable to me :)
Co-Authored-By: rigelrozanski <[email protected]>
…smos-sdk into rigel/delegation-index
closes #1402
follow up ref issue: #3270
docs/
) - more major doc updates to come during spec upgradePENDING.md
with issue #Files changed
in the github PR explorerFor Admin Use: