v1.7.6 upgradehandler #1809
v1.7.6 upgradehandler #1809
Conversation
…tdated unbonding records
joe-bowman
requested review from
faddat,
ajansari95 and
arhamchordia
as code owners
WalkthroughThe pull request introduces an upgrade handler ( Changes
Sequence DiagramsequenceDiagram
participant UH as Upgrade Handler
participant WR as Withdrawal Records
participant UR as Unbonding Records
UH->>WR: Identify stuck records
UH->>WR: Reset record properties
UH->>WR: Update record status
UH->>UR: Iterate through records
UH->>UR: Remove old/empty records
UH->>UR: Log deleted records
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/upgrades_test.go (1)
616-624: Consider adding more assertions.While the current assertions verify the deletion of records, consider adding checks for:
- The state of requeued withdrawal records
- The logging of deleted unbonding records
Add these assertions:
_, found = app.InterchainstakingKeeper.GetUnbondingRecord(ctx, "sommelier-3", "sommvaloper1y0few0kgxa7vtq8nskjsdwdtyqglj3k5pv2c4d", 243) s.True(found) + +// Check requeued withdrawal records +record, found := app.InterchainstakingKeeper.GetWithdrawalRecord(ctx, "regen-1", "ee0b5f5c423508c8dd6a501168a77a0b72d5a8aaf1702a64804e522334ff272b", icstypes.WithdrawStatusQueued) +s.True(found) +s.Equal(icstypes.WithdrawStatusQueued, record.Status) +s.Equal(int64(0), record.SendErrors) +s.True(record.CompletionTime.IsZero()) + +record, found = app.InterchainstakingKeeper.GetWithdrawalRecord(ctx, "sommelier-3", "a55f1f4deaa501ff5671ef96fbbb5b60e225d4b8db4825ae3706893bb94e052c", icstypes.WithdrawStatusQueued) +s.True(found) +s.Equal(icstypes.WithdrawStatusQueued, record.Status) +s.Equal(int64(0), record.SendErrors) +s.True(record.CompletionTime.IsZero())
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/upgrades/v1_7.go(1 hunks)app/upgrades_test.go(1 hunks)x/interchainstaking/keeper/abci.go(1 hunks)
🧰 Additional context used
🪛 GitHub Check: devskim
app/upgrades/v1_7.go
[failure] 176-176: A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
Do not store tokens or keys in source code.
[failure] 177-177: A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
Do not store tokens or keys in source code.
app/upgrades_test.go
[failure] 531-531: A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
Do not store tokens or keys in source code.
[failure] 552-552: A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
Do not store tokens or keys in source code.
[failure] 574-574: A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
Do not store tokens or keys in source code.
[failure] 575-575: A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
Do not store tokens or keys in source code.
[failure] 576-576: A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
Do not store tokens or keys in source code.
[failure] 577-577: A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
Do not store tokens or keys in source code.
[failure] 591-591: A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
Do not store tokens or keys in source code.
[failure] 602-602: A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
Do not store tokens or keys in source code.
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: test quicksilver (darwin, arm64)
- GitHub Check: test quicksilver (amd64, windows)
- GitHub Check: Analyze
- GitHub Check: test quicksilver (amd64, linux)
🔇 Additional comments (4)
x/interchainstaking/keeper/abci.go (1)
34-36: LGTM! Activated handling of matured unbondings.The uncommented line enables periodic processing of matured unbondings every 30 blocks, which aligns with the PR objectives.
app/upgrades/v1_7.go (1)
169-191: LGTM! Unblocking stuck unbondings for Regen and Sommelier networks.The implementation correctly resets and requeues the specified unbonding records by:
- Resetting error counts, amounts, and completion time
- Setting status back to queued
🧰 Tools
🪛 GitHub Check: devskim
[failure] 176-176: A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
Do not store tokens or keys in source code.
[failure] 177-177: A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
Do not store tokens or keys in source code.app/upgrades_test.go (2)
527-565: LGTM! Comprehensive test coverage for withdrawal records.The test correctly sets up withdrawal records for both chains with appropriate test data.
🧰 Tools
🪛 GitHub Check: devskim
[failure] 531-531: A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
Do not store tokens or keys in source code.
[failure] 552-552: A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
Do not store tokens or keys in source code.
566-610: LGTM! Good test coverage for different unbonding scenarios.The test covers all edge cases for unbonding records:
- Empty completion time
- Future completion time
- Past completion time (12 hours ago)
🧰 Tools
🪛 GitHub Check: devskim
[failure] 574-574: A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
Do not store tokens or keys in source code.
[failure] 575-575: A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
Do not store tokens or keys in source code.
[failure] 576-576: A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
Do not store tokens or keys in source code.
[failure] 577-577: A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
Do not store tokens or keys in source code.
[failure] 591-591: A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
Do not store tokens or keys in source code.
[failure] 602-602: A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
Do not store tokens or keys in source code.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1809 +/- ##
==========================================
+ Coverage 63.37% 63.43% +0.06%
==========================================
Files 172 172
Lines 15422 15449 +27
==========================================
+ Hits 9773 9800 +27
Misses 4838 4838
Partials 811 811
Flags with carried forward coverage won't be shown. Click here to find out more.
|
1. Summary
Upgrade handler for v1.7.6;
Enable periodic garbage collection on UBRs.
Add tests for v1.7.6.
Summary by CodeRabbit
New Features
Bug Fixes
Tests