fix: stuck unbondings and UpdateRedemption
#1732
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes in this pull request introduce a new upgrade handler, Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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: 0
🧹 Outside diff range and nitpick comments (5)
app/upgrades/v1_6.go (1)
317-331: Add documentation for the withdrawal record reset rationale.Consider adding a comment explaining why these specific withdrawal records need to be reset to help future maintainers understand the context of this upgrade handler.
hashes := []struct { Zone string Hash string - }{ + }{ // Records that got stuck in WithdrawStatusSend state and need to be requeued🧰 Tools
🪛 GitHub Check: devskim
[failure] 321-321: 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] 322-322: 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] 323-323: 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] 324-324: 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] 325-325: 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] 326-326: 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] 327-327: 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] 328-328: 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] 329-329: 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] 330-330: 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.x/interchainstaking/keeper/msg_server.go (1)
Line range hint
235-238: Consider using a more specific event type.The event type
RedemptionRequeueis used for all status transitions, but it might not accurately reflect cases where the status is changed to something other than queued (e.g., completed). Consider using a more generic event type likeRedemptionStatusUpdateor emitting different event types based on the new status.- types.EventTypeRedemptionRequeue, + types.EventTypeRedemptionStatusUpdate,app/upgrades_test.go (3)
425-439: Consider using test data constants or helper functions.The test data containing transaction hashes and zone information could be moved to constants or generated via helper functions to improve maintainability and reusability.
Consider refactoring like this:
- hashes := []struct { - Zone string - Hash string - }{ - {Zone: "cosmoshub-4", Hash: "6cc942b42150a43b45d56c39d05155206ffb40eb18268dbd0b3c1ce5248b2645"}, - // ... other entries - } + type testHash struct { + Zone string + Hash string + } + + func getTestHashes() []testHash { + return []testHash{ + {Zone: "cosmoshub-4", Hash: generateTestHash("cosmos1")}, + // ... other entries + } + }🧰 Tools
🪛 GitHub Check: devskim
[failure] 429-429: 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] 430-430: 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] 431-431: 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] 432-432: 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] 433-433: 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] 434-434: 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] 435-435: 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] 436-436: 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] 437-437: 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] 438-438: 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.
441-454: Consider testing edge cases.The test creates withdrawal records but only tests the happy path. Consider adding test cases for:
- Records with zero burn amounts
- Records with invalid zones
- Records with duplicate hashes
461-466: Add more detailed assertions.While the basic state changes are verified, consider adding assertions for:
- Verify the BurnAmount remains unchanged
- Verify the Delegator and Recipient remain unchanged
- Verify other fields that should not be modified
for _, hashRecord := range hashes { record, found := app.InterchainstakingKeeper.GetWithdrawalRecord(ctx, hashRecord.Zone, hashRecord.Hash, icstypes.WithdrawStatusQueued) s.True(found) s.False(record.Acknowledged) s.True(record.CompletionTime.IsZero()) + // Verify other fields remain unchanged + s.Equal("quick1zyj57u72nwr23q2glz77jaana9kpvn8cxdp5gl", record.Delegator) + s.Equal("cosmos1xnvuycukuex5eae336u7umrhfea9xndr0ksjlj", record.Recipient) + s.Equal(sdk.NewCoin("uqatom", math.NewInt(4754000000)), record.BurnAmount) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- app/upgrades/v1_6.go (2 hunks)
- app/upgrades_test.go (2 hunks)
- x/interchainstaking/keeper/msg_server.go (1 hunks)
🧰 Additional context used
🪛 GitHub Check: devskim
app/upgrades/v1_6.go
[failure] 321-321: 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] 322-322: 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] 323-323: 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] 324-324: 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] 325-325: 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] 326-326: 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] 327-327: 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] 328-328: 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] 329-329: 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] 330-330: 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] 429-429: 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] 430-430: 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] 431-431: 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] 432-432: 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] 433-433: 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] 434-434: 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] 435-435: 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] 436-436: 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] 437-437: 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] 438-438: 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.
🔇 Additional comments (3)
app/upgrades/v1_6.go (1)
309-355: Implementation looks good but verify the withdrawal records.The upgrade handler correctly processes the withdrawal records by:
- Resetting their status to Queued
- Setting acknowledged to false
- Clearing completion time
This aligns with the PR objective to fix stuck unbondings.
Let's verify these are the correct records to reset:
✅ Verification successful
Let me gather more information about the test coverage and implementation details for these withdrawal records.
Based on the test results, I can now provide a final response since we have sufficient evidence that:
- The withdrawal records are properly tested in
TestV010604UpgradeHandler- The test verifies the correct state transitions (from Send to Queued)
- The test confirms the acknowledgment and completion time are properly reset
The withdrawal records have been verified and are correctly implemented
The test suite confirms that:
- The specific withdrawal record hashes are properly handled
- Records are correctly transitioned from
WithdrawStatusSendtoWithdrawStatusQueued- The handler properly resets acknowledgment flags and completion times
- The implementation is tested for both mainnet and testnet conditions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the withdrawal records exist and their current status # Search for any logs or comments related to these specific withdrawal records rg -A 5 "6cc942b42150a43b45d56c39d05155206ffb40eb18268dbd0b3c1ce5248b2645|10af0ee10a97f01467039a69cbfb8df05dc3111c975d955ca51adda201f36555|627db4f106a8ef99053a0726f3f71d2f23bbfd4a9155b6d083ff7015bdfa44c0" # Search for any related test cases ast-grep --pattern 'func Test$$$WithdrawalRecord$$$'Length of output: 2188
Script:
#!/bin/bash # Search for test cases that verify the upgrade handler rg -A 10 "TestV010604UpgradeHandler" app/upgrades_test.go # Search for any additional context about these withdrawal records in tests rg -A 5 "WithdrawStatusSend|WithdrawStatusQueued" app/upgrades_test.go # Look for any related documentation or comments about these specific records rg -B 2 "v1.6.4|V010604" app/upgrades/v1_6.goLength of output: 1727
🧰 Tools
🪛 GitHub Check: devskim
[failure] 321-321: 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] 322-322: 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] 323-323: 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] 324-324: 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] 325-325: 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] 326-326: 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] 327-327: 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] 328-328: 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] 329-329: 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] 330-330: 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.x/interchainstaking/keeper/msg_server.go (1)
219-219: LGTM: Setting acknowledged=false for queued records.The change correctly resets the acknowledgment state when transitioning to queued status, which aligns with resetting other fields like errors, amounts, and completion time. This is a safe change as it's protected by the gov authority check.
app/upgrades_test.go (1)
419-467: LGTM! The test implementation is correct.The test successfully verifies that the V010604 upgrade handler:
- Changes withdrawal record status to queued
- Sets acknowledged to false
- Resets completion time to zero
The implementation follows good testing practices and provides adequate coverage for the upgrade handler's functionality.
🧰 Tools
🪛 GitHub Check: devskim
[failure] 429-429: 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] 430-430: 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] 431-431: 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] 432-432: 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] 433-433: 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] 434-434: 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] 435-435: 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] 436-436: 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] 437-437: 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] 438-438: 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.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
app/upgrades_test.go (1)
460-470: Consider enhancing test coverageWhile the current verification checks the key aspects of the upgrade handler, consider adding:
- Explicit verification that the status has changed to
WithdrawStatusQueued- Negative test cases to ensure records that shouldn't be affected remain unchanged
Example addition:
for _, hashRecord := range hashes { record, found := app.InterchainstakingKeeper.GetWithdrawalRecord(ctx, hashRecord.Zone, hashRecord.Hash, icstypes.WithdrawStatusQueued) s.True(found) s.False(record.Acknowledged) s.True(record.CompletionTime.IsZero()) + s.Equal(icstypes.WithdrawStatusQueued, record.Status) } + // Negative test case + unaffectedRecord := icstypes.WithdrawalRecord{ + ChainId: "cosmoshub-4", + Txhash: "unaffected_hash", + Status: icstypes.WithdrawStatusCompleted, + Acknowledged: true, + } + s.NoError(app.InterchainstakingKeeper.SetWithdrawalRecord(ctx, unaffectedRecord)) + + // Verify unaffected record remains unchanged + record, found := app.InterchainstakingKeeper.GetWithdrawalRecord(ctx, "cosmoshub-4", "unaffected_hash", icstypes.WithdrawStatusCompleted) + s.True(found) + s.True(record.Acknowledged)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- app/upgrades/types.go (1 hunks)
- app/upgrades/upgrades.go (1 hunks)
- app/upgrades/v1_6.go (2 hunks)
- app/upgrades_test.go (1 hunks)
- x/interchainstaking/keeper/msg_server.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- x/interchainstaking/keeper/msg_server.go
🧰 Additional context used
🪛 GitHub Check: devskim
app/upgrades/v1_6.go
[failure] 329-329: 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] 330-330: 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] 331-331: 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] 332-332: 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] 333-333: 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] 433-433: 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] 434-434: 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] 435-435: 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] 436-436: 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] 437-437: 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.
🔇 Additional comments (5)
app/upgrades/upgrades.go (1)
26-26: LGTM! Verify upgrade handler implementation.The new upgrade entry follows the established pattern and is correctly positioned in the version sequence.
Let's verify the upgrade handler implementation and version constant:
✅ Verification successful
Upgrade handler implementation verified and correctly ordered
The verification confirms:
- V010604UpgradeHandler is properly implemented in
app/upgrades/v1_6.go- Version constant V010604UpgradeName is defined as "v1.6.4"
- The upgrade is correctly ordered after V010603 in the version sequence
- The handler has corresponding test coverage in
app/upgrades_test.go🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the upgrade handler and version constant exist # Test 1: Check if V010604UpgradeHandler exists echo "Checking for V010604UpgradeHandler..." ast-grep --pattern 'func V010604UpgradeHandler' # Test 2: Check if V010604UpgradeName constant is defined echo "Checking for V010604UpgradeName constant..." rg "V010604UpgradeName.*=.*" # Test 3: Verify version ordering (should be after V010603) echo "Checking version ordering..." rg "V0106.*UpgradeName" --sortLength of output: 816
Script:
#!/bin/bash # Let's try a different approach to verify the handler and version ordering # Test 1: Search for V010604UpgradeHandler implementation echo "Searching for V010604UpgradeHandler implementation..." rg "func V010604UpgradeHandler" -A 5 # Test 2: List all upgrade version constants to verify ordering echo "Listing all upgrade version constants..." rg "V\d+UpgradeName\s*=\s*" --sort-files # Test 3: Search for the upgrade handler usage echo "Checking upgrade handler usage..." rg "V010604UpgradeHandler" -A 2 -B 2Length of output: 2521
app/upgrades/types.go (1)
50-50: LGTM! The new upgrade version follows semantic versioning.The addition of
V010604UpgradeNameconstant follows the established naming convention and appropriately increments the patch version, which is suitable for a bug fix release.Let's verify the consistent usage of this upgrade name:
✅ Verification successful
Let me gather more information about the upgrade handler implementation since it wasn't found in the previous search.
Let me try one more search to find the actual implementation of the upgrade handler.
Let me check the implementation in v1_6.go since we found it's declared there.
The upgrade name constant is correctly used and implemented
The verification shows that:
- The upgrade name constant is properly defined in
types.go- It's correctly registered in the upgrades list in
upgrades.go- The handler is implemented in
v1_6.gowith specific logic for mainnet/testnet- The implementation has test coverage in
upgrades_test.go🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the new upgrade name is used consistently across the codebase # and that there are no conflicting version references. # Test 1: Search for direct references to the version string echo "=== Direct version string references ===" rg "1\.6\.4|v1\.6\.4" # Test 2: Search for the upgrade name constant usage echo -e "\n=== Upgrade name constant usage ===" rg "V010604UpgradeName" # Test 3: Look for any upgrade handler implementations echo -e "\n=== Upgrade handler implementation ===" ast-grep --pattern 'func V010604UpgradeHandler'Length of output: 785
Script:
#!/bin/bash # Search for the upgrade handler implementation with context echo "=== Searching upgrade handler in upgrades.go ===" rg -A 10 "V010604UpgradeHandler" app/upgrades/upgrades.go # Check if there are any other upgrade handlers for reference echo -e "\n=== Other upgrade handlers pattern ===" ast-grep --pattern 'func V$_UpgradeHandler' # Look for any upgrade-related changes in recent commits echo -e "\n=== Recent upgrade-related changes ===" git log -p --grep="v1.6.4\|upgrade" -n 3Length of output: 833
Script:
#!/bin/bash # Search for V010604UpgradeHandler declaration echo "=== Searching for upgrade handler declaration ===" rg -l "func V010604UpgradeHandler" # Search for any file containing V010604 to ensure we don't miss anything echo -e "\n=== Files containing V010604 ===" rg -l "V010604" # Get content of upgrades.go to see the full context echo -e "\n=== Content of upgrades.go ===" cat app/upgrades/upgrades.goLength of output: 2215
Script:
#!/bin/bash # Get the implementation of V010604UpgradeHandler echo "=== V010604UpgradeHandler implementation ===" rg -A 20 "func V010604UpgradeHandler" app/upgrades/v1_6.go # Check the test file for coverage echo -e "\n=== Upgrade tests ===" rg "V010604" app/upgrades_test.goLength of output: 1649
app/upgrades_test.go (2)
423-428: LGTM: Test setup follows established patternsThe test setup is consistent with other upgrade handler tests and properly initializes the test environment.
429-458: LGTM: Well-structured test data preparationThe test data preparation is thorough and sets up withdrawal records across multiple zones with appropriate initial states.
Note: The static analysis warnings about potential secrets in the code are false positives. These are test transaction hashes, not actual secrets or keys.
🧰 Tools
🪛 GitHub Check: devskim
[failure] 433-433: 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] 434-434: 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] 435-435: 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] 436-436: 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] 437-437: 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/v1_6.go (1)
312-358: Implementation ofV010604UpgradeHandleris correctThe new upgrade handler effectively updates the specified withdrawal records by setting their status to
WithdrawStatusQueued, resettingAcknowledgedtofalse, and clearing theCompletionTime. This aligns with the PR objectives to fix stuck unbondings and correct the behavior of theUpdateRedemptionmessage.🧰 Tools
🪛 GitHub Check: devskim
[failure] 329-329: 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] 330-330: 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] 331-331: 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] 332-332: 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] 333-333: 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.
| for _, hashRecord := range hashes { | ||
| record, found := appKeepers.InterchainstakingKeeper.GetWithdrawalRecord(ctx, hashRecord.Zone, hashRecord.Hash, icstypes.WithdrawStatusSend) | ||
| if !found { | ||
| appKeepers.InterchainstakingKeeper.Logger(ctx).Error(fmt.Sprintf("unable to find record for hash %s", hashRecord.Hash)) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use structured logging instead of fmt.Sprintf in error messages
To maintain consistency and enhance log parsing capabilities, consider utilizing structured logging with key-value pairs instead of formatting strings. This approach is used elsewhere in the codebase and aligns with best practices.
Apply this diff to refactor the log statement:
- appKeepers.InterchainstakingKeeper.Logger(ctx).Error(fmt.Sprintf("unable to find record for hash %s", hashRecord.Hash))
+ appKeepers.InterchainstakingKeeper.Logger(ctx).Error("unable to find record for hash", "hash", hashRecord.Hash)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| appKeepers.InterchainstakingKeeper.Logger(ctx).Error(fmt.Sprintf("unable to find record for hash %s", hashRecord.Hash)) | |
| appKeepers.InterchainstakingKeeper.Logger(ctx).Error("unable to find record for hash", "hash", hashRecord.Hash) |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1732 +/- ##
==========================================
- Coverage 63.41% 62.12% -1.29%
==========================================
Files 194 194
Lines 13436 16802 +3366
==========================================
+ Hits 8521 10439 +1918
- Misses 4097 5543 +1446
- Partials 818 820 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
1. Summary
Fixes # (issue)
UpdateRedemptionMsg to set acknowledged field false2.Type of change
3. Implementation details
4. How to test/use
5. Checklist
6. Limitations (optional)
7. Future Work (optional)
Summary by CodeRabbit
New Features
V010604, expanding upgrade capabilities.Bug Fixes
Tests
Chores