fix(mstaking): bound redelegation slash by remaining amount#524
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughModified ChangesSlashRedelegation Double-Counting Fix
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
x/mstaking/keeper/slash_test.go (1)
103-107: 💤 Low valueConsider clarifying the test documentation.
The comment mixes describing the current behavior with the old buggy behavior ("used to be slashed"), which is a bit confusing. Consider rewording:
-// Test_SlashRedelegation_UndelegateAfterRedelegate is a test for SlashRedelegation -// when a delegator redelegates and then partially undelegates from the -// destination validator, the destination-side unbonding delegation is slashed -// AND the still-bonded delegation used to be slashed for the full `SharesDst * slashFactor`. +// Test_SlashRedelegation_UndelegateAfterRedelegate verifies that SlashRedelegation +// bounds the delegation-side slash by the remaining amount after the unbonding +// delegation absorbs part of the slash. Previously, the still-bonded delegation +// was slashed for the full `SharesDst * slashFactor`, causing double-counting.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@x/mstaking/keeper/slash_test.go` around lines 103 - 107, Update the test comment for Test_SlashRedelegation_UndelegateAfterRedelegate to remove ambiguous phrasing about past buggy behavior and clearly describe the expected behavior: state that when a delegator redelegates and then partially undelegates from the destination validator, the destination-side unbonding delegation is slashed and the remaining bonded delegation is slashed by SharesDst * slashFactor (i.e., describe current expected outcome rather than saying "used to be slashed"). Ensure the function name Test_SlashRedelegation_UndelegateAfterRedelegate is referenced in the comment for clarity.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@x/mstaking/keeper/slash_test.go`:
- Around line 103-107: Update the test comment for
Test_SlashRedelegation_UndelegateAfterRedelegate to remove ambiguous phrasing
about past buggy behavior and clearly describe the expected behavior: state that
when a delegator redelegates and then partially undelegates from the destination
validator, the destination-side unbonding delegation is slashed and the
remaining bonded delegation is slashed by SharesDst * slashFactor (i.e.,
describe current expected outcome rather than saying "used to be slashed").
Ensure the function name Test_SlashRedelegation_UndelegateAfterRedelegate is
referenced in the comment for clarity.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5394b7bf-498f-4e2b-85b3-0fa07b3bee20
📒 Files selected for processing (2)
x/mstaking/keeper/slash.gox/mstaking/keeper/slash_test.go
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #524 +/- ##
==========================================
+ Coverage 38.41% 38.44% +0.02%
==========================================
Files 327 327
Lines 30685 30693 +8
==========================================
+ Hits 11789 11801 +12
+ Misses 17006 16999 -7
- Partials 1890 1893 +3
🚀 New features to boost your workflow:
|
beer-1
suggested changes
Jun 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Closes: #XXXX
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...