fix(x/accounts/default/lockup) Lockup account track undelegation when unbonding entry is mature #22254
Conversation
![github-advanced-security[bot]](https://github.com/avatars/in/57789?s=60&v=4){kind=small}
| x.ValidatorAddress = value.Interface().(string) | ||
| default: | ||
| if fd.IsExtension() { | ||
| panic(fmt.Errorf("proto3 declared messages do not support extensions: cosmos.accounts.defaults.lockup.UnbondingEntry")) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
| if fd.IsExtension() { | ||
| panic(fmt.Errorf("proto3 declared messages do not support extensions: cosmos.accounts.defaults.lockup.UnbondingEntry")) | ||
| } | ||
| panic(fmt.Errorf("message cosmos.accounts.defaults.lockup.UnbondingEntry does not contain field %s", fd.FullName())) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
julienrbrt
changed the title
julienrbrt
added
the
backport/v0.52.x
|
maybe user should manually track this delegation through an execution since matured ubd doesnot mean that the fund has returned to the account yet |
| switch fd.FullName() { | ||
| default: | ||
| if fd.IsExtension() { | ||
| panic(fmt.Errorf("proto3 declared messages do not support extensions: cosmos.accounts.defaults.lockup.QueryUnbondingEntriesRequest")) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
| if fd.IsExtension() { | ||
| panic(fmt.Errorf("proto3 declared messages do not support extensions: cosmos.accounts.defaults.lockup.QueryUnbondingEntriesRequest")) | ||
| } | ||
| panic(fmt.Errorf("message cosmos.accounts.defaults.lockup.QueryUnbondingEntriesRequest does not contain field %s", fd.FullName())) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
| x.UnbondingEntries = *clv.list | ||
| default: | ||
| if fd.IsExtension() { | ||
| panic(fmt.Errorf("proto3 declared messages do not support extensions: cosmos.accounts.defaults.lockup.QueryUnbondingEntriesResponse")) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
| if fd.IsExtension() { | ||
| panic(fmt.Errorf("proto3 declared messages do not support extensions: cosmos.accounts.defaults.lockup.QueryUnbondingEntriesResponse")) | ||
| } | ||
| panic(fmt.Errorf("message cosmos.accounts.defaults.lockup.QueryUnbondingEntriesResponse does not contain field %s", fd.FullName())) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
| x.Id = value.Uint() | ||
| default: | ||
| if fd.IsExtension() { | ||
| panic(fmt.Errorf("proto3 declared messages do not support extensions: cosmos.accounts.defaults.lockup.MsgTrackUndelegation")) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
| if fd.IsExtension() { | ||
| panic(fmt.Errorf("proto3 declared messages do not support extensions: cosmos.accounts.defaults.lockup.MsgTrackUndelegation")) | ||
| } | ||
| panic(fmt.Errorf("message cosmos.accounts.defaults.lockup.MsgTrackUndelegation does not contain field %s", fd.FullName())) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
|
@sontrinh16 your pull request is missing a changelog! |
📝 WalkthroughWalkthroughThe changes in this pull request introduce new message types and functionalities to the lockup package in the Cosmos SDK. A new message type 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
Documentation and Community
|
![coderabbitai[bot]](https://github.com/avatars/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 25
🧹 Outside diff range and nitpick comments (17)
x/accounts/proto/cosmos/accounts/defaults/lockup/lockup.proto (2)
26-37: LGTM: UnbondingEntry message is well-defined, consider adding comments.The
UnbondingEntrymessage is correctly structured and uses appropriate types for each field. It aligns well with the PR objectives for tracking undelegation.Consider adding comments for each field to improve documentation. For example:
message UnbondingEntry { // Unique identifier for the unbonding entry uint64 id = 1; // Timestamp when the unbonding period ends google.protobuf.Timestamp end_time = 2 [(gogoproto.nullable) = false, (amino.dont_omitempty) = true, (gogoproto.stdtime) = true]; // Amount of coins being unbonded cosmos.base.v1beta1.Coin amount = 3 [(gogoproto.nullable) = false, (amino.dont_omitempty) = true]; // Address of the validator from which the delegation is being unbonded string validator_address = 4 [(cosmos_proto.scalar) = "cosmos.ValidatorAddressString"]; }This will enhance the clarity and maintainability of the code.
Missing Implementation for Managing
UnbondingEntryThe
UnbondingEntrymessage has been added tolockup.proto, but there are no corresponding functions or handlers implemented to manage these entries. To fully address the PR objectives, please ensure that the necessary logic for tracking, storing, and handlingUnbondingEntryinstances is included. Specifically, consider implementing:
- Functions to create and update
UnbondingEntryinstances.- Handlers to process matured entries and execute account operations based on them.
- Integration points where
UnbondingEntryis utilized within the account management workflow.🔗 Analysis chain
Line range hint
1-37: Implementation details needed for complete solution.The addition of the
UnbondingEntrymessage is a good start towards addressing the issue described in the PR objectives. However, to fully implement the solution, we need to see additional changes, such as:
- Logic for tracking and storing these
UnbondingEntryinstances.- Implementation of the process to check for matured entries before executing account operations.
- If going with the manual approach, a new message type for the owner to acknowledge unbonding and trigger
TrackUndelegation.Could you provide more information on how these
UnbondingEntryinstances will be used and managed? Here's a script to check for related implementations:🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for UnbondingEntry usage and related implementations echo "Searching for UnbondingEntry usage:" rg --type go "UnbondingEntry" -g '!*.pb.go' echo "\nSearching for TrackUndelegation implementation:" rg --type go "TrackUndelegation" echo "\nSearching for new message types related to manual unbonding acknowledgment:" rg --type proto "message\s+.*Unbond.*"Length of output: 35508
Script:
#!/bin/bash # Search for new message types related to manual unbonding acknowledgment in proto files echo "Searching for new message types related to manual unbonding acknowledgment:" rg "message\s+.*Unbond.*" -g '*.proto'Length of output: 1444
Script:
#!/bin/bash # Search for functions handling UnbondingEntry in Go files echo "Searching for functions that interact with UnbondingEntry:" rg "UnbondingEntry" -g '*.go' echo "\nSearching for handlers related to UnbondingEntry operations:" rg "HandleUnbondingEntry" -g '*.go'Length of output: 19860
tests/e2e/accounts/lockup/utils.go (2)
75-85: LGTM with a minor suggestion for consistency.The
queryUnbondingEntriesmethod is well-implemented and follows the existing patterns in the file. It enhances the test suite's capabilities for testing unbonding entries.For consistency with other query methods in this file, consider adding a comment describing the method's purpose, like so:
// queryUnbondingEntries queries the unbonding entries for a given account func (s *E2ETestSuite) queryUnbondingEntries(ctx sdk.Context, app *simapp.SimApp, accAddr []byte) *types.QueryUnbondingEntriesResponse { // ... (existing implementation) }
87-95: LGTM with a suggestion for improved flexibility.The
setupStakingParamsmethod is well-implemented and properly updates the staking parameters for testing purposes.To improve flexibility, consider parameterizing the unbonding time:
func (s *E2ETestSuite) setupStakingParams(ctx sdk.Context, app *simapp.SimApp, unbondingTime time.Duration) { params, err := app.StakingKeeper.Params.Get(ctx) require.NoError(s.T(), err) params.UnbondingTime = unbondingTime err = app.StakingKeeper.Params.Set(ctx, params) require.NoError(s.T(), err) }This change allows for different unbonding times in various test scenarios, enhancing the method's reusability.
x/accounts/proto/cosmos/accounts/defaults/lockup/query.proto (1)
46-53: Changes align with PR objectives, consider updating documentationThe addition of
QueryUnbondingEntriesRequestandQueryUnbondingEntriesResponsemessages aligns well with the PR objectives. These changes support the implementation of manual tracking of unbonding entries, addressing the concerns raised about accurate representation of spendable amounts in lockup accounts.To ensure clarity for users and developers:
- Consider updating the module's documentation to explain the purpose and usage of these new query types.
- It may be helpful to add examples of how these queries will be used in the context of manually acknowledging unbonding and triggering
TrackUndelegation.x/accounts/defaults/lockup/permanent_locking_account.go (1)
106-106: LGTM! Consider grouping related handlers.The addition of
TrackUndelegationEntryhandler is consistent with the PR objectives and follows the existing naming convention.Consider grouping related handlers together for better code organization. For example, you could place
TrackUndelegationEntrynext toUndelegatesince they are both related to undelegation operations.x/accounts/defaults/lockup/utils_test.go (1)
80-82: LGTM! Consider adding a comment for clarity.The change improves the mock context by providing a more realistic response for undelegation messages, which aligns with the PR objective. This will allow for more accurate testing of undelegation-related functionality.
Consider adding a brief comment explaining why this specific amount (1 "test" coin) was chosen for the mock response. This would enhance code readability and make it easier for other developers to understand the test setup.
case "/cosmos.staking.v1beta1.MsgUndelegate": + // Mock a successful undelegation of 1 "test" coin return &stakingtypes.MsgUndelegateResponse{ Amount: sdk.NewCoin("test", math.NewInt(1)), }, nilx/accounts/proto/cosmos/accounts/defaults/lockup/tx.proto (1)
110-119: LGTM! Consider adding a comment for theidfield.The new
MsgTrackUndelegationmessage is well-structured and consistent with other messages in the file. It appropriately uses thecosmos.AddressStringscalar type for thesenderfield and includes necessary options.For improved clarity and consistency with other messages in the file, consider adding a comment for the
idfield to explain its purpose. For example:string sender = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"]; - uint64 id = 2; + // id of the undelegation entry to track + uint64 id = 2;x/accounts/defaults/lockup/delayed_locking_account_test.go (1)
104-115: Improved undelegation testing logicThe changes enhance the test by focusing on the unbonding process, which aligns well with the PR objectives. The new checks for unbonding sequence and entry details provide a more comprehensive verification of the undelegation process.
Consider adding a comment explaining the significance of
ubdSeq-1to improve code readability. For example:// Use ubdSeq-1 as the UnbondingSequence is incremented after the entry is added ubdEntry, err := acc.UnbondEntries.Get(sdkCtx, ubdSeq-1)x/accounts/defaults/lockup/periodic_locking_account_test.go (2)
143-144: Userequire.Equalfor better assertion clarityCurrently, you're asserting string equality using
require.True:require.True(t, ubdEntry.ValidatorAddress == "val_address")For improved readability and consistency with other assertions, use
require.Equal:Apply this diff:
-require.True(t, ubdEntry.ValidatorAddress == "val_address") +require.Equal(t, "val_address", ubdEntry.ValidatorAddress)
154-154: AssertDelegatedFreeamount after undelegation trackingAfter tracking the undelegation, you assert that
DelegatedLockingis zero. It's important to also check theDelegatedFreeamount to confirm that the undelegated tokens are now free:require.True(t, delLocking.Equal(math.ZeroInt()))Add an assertion for
DelegatedFree:Apply this diff:
require.True(t, delLocking.Equal(math.ZeroInt())) +delFree, err := acc.DelegatedFree.Get(ctx, "test") +require.NoError(t, err) +require.True(t, delFree.Equal(math.NewInt(1)))tests/e2e/accounts/lockup/continous_lockup_test_suite.go (2)
Line range hint
1-1: Correct the filename spelling to 'continuous'.The filename
continous_lockup_test_suite.gois misspelled. It should becontinuous_lockup_test_suite.goto reflect correct spelling and improve codebase consistency.Apply this change to correct the filename:
-// File: continous_lockup_test_suite.go +// File: continuous_lockup_test_suite.go
179-191: Enhance test assertions to verifyDelegatedFreeamount after tracking undelegation.Currently, the test checks that
DelegatedLockingamount is zero after tracking undelegation. To fully validate the state, consider adding an assertion to verify that theDelegatedFreeamount has increased accordingly.Apply this diff to include the additional assertion:
require.True(t, delLocking.AmountOf("stake").Equal(math.ZeroInt())) + delFree := lockupAccountInfoResponse.DelegatedFree + require.True(t, delFree.AmountOf("stake").Equal(math.NewInt(100)))This ensures that both the locking and free delegated amounts reflect the expected values after the operation.
api/cosmos/accounts/defaults/lockup/lockup.pulsar.go (1)
1284-1291: Adjust field comments to follow Go conventionsAccording to the Uber Go Style Guide, comments on exported fields should start with the field name. Update the comments for
Id,EndTime,Amount, andValidatorAddressto begin with the field name for clarity and consistency.Apply this diff to correct the comments:
- // entry id + // Id is the entry ID. Id uint64 `protobuf:"varint,1,opt,name=id,proto3" json:"id,omitempty"` - // end time of entry + // EndTime represents the end time of the unbonding entry. EndTime *timestamppb.Timestamp `protobuf:"bytes,2,opt,name=end_time,json=endTime,proto3" json:"end_time,omitempty"` - // unbond amount + // Amount is the unbonded amount. Amount *v1beta1.Coin `protobuf:"bytes,3,opt,name=amount,proto3" json:"amount,omitempty"` - // validator address + // ValidatorAddress is the address of the validator. ValidatorAddress string `protobuf:"bytes,4,opt,name=validator_address,json=validatorAddress,proto3" json:"validator_address,omitempty"`api/cosmos/accounts/defaults/lockup/query.pulsar.go (2)
Line range hint
3570-3600: Correct the field numbering in protobuf descriptorsIn the
QueryLockupAccountInfoResponseand related message types, verify that the field numbers in the protobuf descriptors are correctly incremented and do not conflict with existing fields. Misnumbered fields can lead to serialization issues.Double-check the
.protofiles to ensure that the field numbers are unique and correctly assigned.
Line range hint
3843-3856: Align with Uber Go Style Guide for variable namingIn the generated code, some variables use underscores (e.g.,
file_cosmos_accounts_defaults_lockup_query_proto_depIdxs). According to the Uber Go Style Guide, variable names should be in mixedCaps or mixedCaps with initialisms. While some of these may be auto-generated, consider configuring the code generator to adhere more closely to the style guide if possible.Example:
var fileCosmosAccountsDefaultsLockupQueryProtoDepIdxs = []int32{ /* ... */ }api/cosmos/accounts/defaults/lockup/tx.pulsar.go (1)
6689-6690: Fix the grammatical error in the comment forMsgTrackUndelegation.The comment has a grammatical error and is missing a period at the end. According to the Uber Golang style guide, comments should be complete sentences ending with a period.
Please apply the following diff to correct the comment:
-// MsgTrackUndelegation defines a message that enable lockup account to update delegation tracking +// MsgTrackUndelegation defines a message that enables a lockup account to update delegation tracking.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
⛔ Files ignored due to path filters (3)
x/accounts/defaults/lockup/types/lockup.pb.gois excluded by!**/*.pb.gox/accounts/defaults/lockup/types/query.pb.gois excluded by!**/*.pb.gox/accounts/defaults/lockup/types/tx.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (21)
- api/cosmos/accounts/defaults/lockup/lockup.pulsar.go (6 hunks)
- api/cosmos/accounts/defaults/lockup/query.pulsar.go (13 hunks)
- api/cosmos/accounts/defaults/lockup/tx.pulsar.go (18 hunks)
- tests/e2e/accounts/lockup/continous_lockup_test_suite.go (2 hunks)
- tests/e2e/accounts/lockup/delayed_lockup_test_suite.go (2 hunks)
- tests/e2e/accounts/lockup/periodic_lockup_test_suite.go (3 hunks)
- tests/e2e/accounts/lockup/permanent_lockup_test_suite.go (3 hunks)
- tests/e2e/accounts/lockup/utils.go (2 hunks)
- x/accounts/defaults/lockup/continuous_locking_account.go (1 hunks)
- x/accounts/defaults/lockup/continuous_locking_account_test.go (1 hunks)
- x/accounts/defaults/lockup/delayed_locking_account.go (1 hunks)
- x/accounts/defaults/lockup/delayed_locking_account_test.go (1 hunks)
- x/accounts/defaults/lockup/lockup.go (5 hunks)
- x/accounts/defaults/lockup/periodic_locking_account.go (1 hunks)
- x/accounts/defaults/lockup/periodic_locking_account_test.go (1 hunks)
- x/accounts/defaults/lockup/permanent_locking_account.go (1 hunks)
- x/accounts/defaults/lockup/permanent_locking_account_test.go (1 hunks)
- x/accounts/defaults/lockup/utils_test.go (1 hunks)
- x/accounts/proto/cosmos/accounts/defaults/lockup/lockup.proto (2 hunks)
- x/accounts/proto/cosmos/accounts/defaults/lockup/query.proto (1 hunks)
- x/accounts/proto/cosmos/accounts/defaults/lockup/tx.proto (1 hunks)
🧰 Additional context used
📓 Path-based instructions (18)
api/cosmos/accounts/defaults/lockup/lockup.pulsar.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.api/cosmos/accounts/defaults/lockup/query.pulsar.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.api/cosmos/accounts/defaults/lockup/tx.pulsar.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.tests/e2e/accounts/lockup/continous_lockup_test_suite.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"tests/e2e/accounts/lockup/delayed_lockup_test_suite.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"tests/e2e/accounts/lockup/periodic_lockup_test_suite.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"tests/e2e/accounts/lockup/permanent_lockup_test_suite.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"tests/e2e/accounts/lockup/utils.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"x/accounts/defaults/lockup/continuous_locking_account.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/accounts/defaults/lockup/continuous_locking_account_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/accounts/defaults/lockup/delayed_locking_account.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/accounts/defaults/lockup/delayed_locking_account_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/accounts/defaults/lockup/lockup.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/accounts/defaults/lockup/periodic_locking_account.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/accounts/defaults/lockup/periodic_locking_account_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/accounts/defaults/lockup/permanent_locking_account.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/accounts/defaults/lockup/permanent_locking_account_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/accounts/defaults/lockup/utils_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
🪛 GitHub Check: CodeQL
api/cosmos/accounts/defaults/lockup/lockup.pulsar.go
[warning] 805-805: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
[warning] 807-807: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain haltapi/cosmos/accounts/defaults/lockup/query.pulsar.go
[warning] 1785-1785: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
[warning] 1787-1787: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
[warning] 2214-2214: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
[warning] 2216-2216: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain haltapi/cosmos/accounts/defaults/lockup/tx.pulsar.go
[warning] 4358-4358: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
[warning] 4360-4360: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
🔇 Additional comments (18)
x/accounts/proto/cosmos/accounts/defaults/lockup/lockup.proto (1)
7-9: LGTM: New imports are appropriate for the added message.The new imports for
cosmos_proto/cosmos.protoandgoogle/protobuf/timestamp.protoare correctly added to support the newUnbondingEntrymessage definition.tests/e2e/accounts/lockup/utils.go (1)
Line range hint
1-95: Overall, the changes enhance the e2e test suite capabilities.The additions to
tests/e2e/accounts/lockup/utils.goimprove the testing framework for the lockup module. The new methodsqueryUnbondingEntriesandsetupStakingParamsallow for more comprehensive e2e tests, particularly for testing unbonding entries and custom staking parameters.The changes adhere to the Uber Golang style guide and are consistent with the existing code structure. These enhancements will contribute to better test coverage for the changes associated with the pull request.
x/accounts/proto/cosmos/accounts/defaults/lockup/query.proto (2)
46-47: LGTM: QueryUnbondingEntriesRequest message addedThe addition of the
QueryUnbondingEntriesRequestmessage is appropriate. It follows the common pattern of using an empty message for simple requests that don't require additional parameters.
49-53: LGTM: QueryUnbondingEntriesResponse message added, but verify UnbondingEntry typeThe
QueryUnbondingEntriesResponsemessage is well-structured for returning a list of unbonding entries. However, please ensure that theUnbondingEntrytype is properly defined and imported.Run the following script to verify the UnbondingEntry type definition:
✅ Verification successful
Let's attempt to locate the
UnbondingEntrytype definition by searching within all.protofiles.
Verified: UnbondingEntry type is properly defined
The
UnbondingEntrytype is defined inx/accounts/proto/cosmos/accounts/defaults/lockup/lockup.proto, ensuring it is properly defined and accessible for theQueryUnbondingEntriesResponsemessage.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the UnbondingEntry type definition # Test: Search for UnbondingEntry definition in proto files rg --type proto 'message\s+UnbondingEntry' # Test: Check for imports of files that might contain UnbondingEntry rg --type proto 'import.*UnbondingEntry'Length of output: 147
Script:
#!/bin/bash # Description: Locate the UnbondingEntry type definition within .proto files # Find all .proto files and search for the UnbondingEntry message fd --extension proto --type f | xargs rg 'message\s+UnbondingEntry'Length of output: 156
x/accounts/defaults/lockup/permanent_locking_account.go (1)
111-111: LGTM! Verify the implementation of QueryUnbondingEntries.The addition of
QueryUnbondingEntrieshandler is consistent with the PR objectives and follows the existing naming convention.Please ensure that the
QueryUnbondingEntriesmethod is properly implemented in thePermanentLockingAccountstruct. Run the following script to verify:x/accounts/defaults/lockup/utils_test.go (1)
Line range hint
1-124: Overall assessment: The changes improve test utilities.The modification to the
newMockContextfunction enhances the simulation of undelegation messages in the test environment. This change aligns well with the PR objectives and should lead to more accurate testing of undelegation-related functionality in the lockup package.x/accounts/defaults/lockup/delayed_locking_account_test.go (1)
Line range hint
1-190: Overall assessment of changesThe modifications to the
TestDelayedAccountUndelegatefunction effectively address the PR objectives by improving the testing of the undelegation process. The focus on unbonding sequences and entries provides a more accurate representation of the account's state during undelegation.The test coverage for the changes associated with the pull request appears sufficient, as it directly tests the new functionality related to tracking undelegation entries.
x/accounts/defaults/lockup/continuous_locking_account.go (1)
203-203: LGTM! Verify the implementation of QueryUnbondingEntries.The addition of the
QueryUnbondingEntriesquery handler registration is appropriate and aligns with the PR objectives. This enhancement will allow theContinuousLockingAccountto handle queries related to unbonding entries.To ensure completeness, let's verify the implementation of the
QueryUnbondingEntriesmethod:✅ Verification successful
Implementation of
QueryUnbondingEntriesVerified SuccessfullyThe
QueryUnbondingEntriesmethod is correctly implemented inx/accounts/defaults/lockup/lockup.go. The registration incontinuous_locking_account.goproperly references this method, ensuring that unbonding entries are handled as intended.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of QueryUnbondingEntries method # Test: Search for the QueryUnbondingEntries method implementation rg --type go -A 5 'func \(.*\) QueryUnbondingEntries\('Length of output: 644
x/accounts/defaults/lockup/periodic_locking_account.go (1)
346-346: LGTM. Please provide implementation details forQueryUnbondingEntries.The addition of the
QueryUnbondingEntriesquery handler aligns with the PR objectives to address issues with the lockup account's handling of undelegation. This change enhances the account's capabilities to manage and respond to queries about unbonding processes.Could you please provide the implementation details of the
QueryUnbondingEntriesmethod? This will help ensure that it correctly handles the querying of unbonding entries and aligns with the overall functionality of thePeriodicLockingAccount.tests/e2e/accounts/lockup/permanent_lockup_test_suite.go (1)
26-26: Initialize staking parameters for accurate test executionSetting up staking parameters ensures that the staking-related tests operate under the intended conditions, preventing unexpected behavior.
x/accounts/defaults/lockup/continuous_locking_account_test.go (1)
121-121: Confirm delegated locking amount is updated correctlyThe assertion that
delLockingequals zero after tracking the undelegation entry ensures that the delegated locking amount is updated appropriately. This verification is crucial to confirm that the undelegation logic functions as expected.tests/e2e/accounts/lockup/delayed_lockup_test_suite.go (1)
26-26: LGTMThe addition of
s.setupStakingParams(ctx, app)correctly initializes the staking parameters necessary for the test, ensuring that the subsequent staking operations function as expected.tests/e2e/accounts/lockup/continous_lockup_test_suite.go (1)
26-26: Verify the implementation ofsetupStakingParams.Ensure that the
s.setupStakingParams(ctx, app)function is properly defined and sets up the staking parameters as intended. This setup is crucial for the tests that depend on staking configurations.If
setupStakingParamsis not defined, please implement it or remove the call if it's unnecessary.tests/e2e/accounts/lockup/periodic_lockup_test_suite.go (1)
26-26: Configuration of staking parameters is appropriateThe addition of
s.setupStakingParams(ctx, app)initializes the staking parameters necessary for the test environment. This ensures that staking-related operations perform as expected.api/cosmos/accounts/defaults/lockup/query.pulsar.go (3)
1671-1996: Ensure consistency in method implementationsThe methods for
fastReflection_QueryUnbondingEntriesRequestare generated but lack specific implementations due to the empty struct. Verify if this is intentional. If the request type does not contain any fields, consider whether it's necessary, or if fields should be added.Please confirm that the empty implementation is appropriate for your use case.
🧰 Tools
🪛 GitHub Check: CodeQL
[warning] 1785-1785: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
[warning] 1787-1787: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
2214-2216: Avoid potential panics in consensus methodsStatic analysis tools have identified possible panics in the consensus-related methods, which could cause a chain halt.
This issue has been previously flagged. Ensure that proper error handling is implemented to prevent consensus failures.
🧰 Tools
🪛 GitHub Check: CodeQL
[warning] 2214-2214: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
[warning] 2216-2216: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
2050-2290:⚠️ Potential issueHandle potential nil pointer dereferences
In the
fastReflection_QueryUnbondingEntriesResponsemethods, ensure that nil checks are in place when accessing pointers. While the generated code handles some nil checks, it's crucial to prevent possible panics due to nil pointer dereferences, especially in methods likeGet,Set, andMutable.Review the methods to ensure that all pointer accesses are safe. For example:
func (x *fastReflection_QueryUnbondingEntriesResponse) Get(descriptor protoreflect.FieldDescriptor) protoreflect.Value { if x == nil { // Handle nil receiver } // Existing implementation... }🧰 Tools
🪛 GitHub Check: CodeQL
[warning] 2214-2214: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
[warning] 2216-2216: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain haltapi/cosmos/accounts/defaults/lockup/tx.pulsar.go (1)
4181-4647: LGTM!The code correctly defines the new message type
MsgTrackUndelegationand its associated methods, adhering to the protobuf definitions and Go coding conventions.🧰 Tools
🪛 GitHub Check: CodeQL
[warning] 4358-4358: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
[warning] 4360-4360: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
| ubdSeq, err := acc.UnbondingSequence.Peek(sdkCtx) | ||
| require.NoError(t, err) | ||
| // sequence should be the previous one | ||
| ubdEntry, err := acc.UnbondEntries.Get(sdkCtx, ubdSeq-1) | ||
| require.NoError(t, err) | ||
| require.True(t, ubdEntry.Amount.Amount.Equal(math.NewInt(1))) | ||
| require.True(t, ubdEntry.ValidatorAddress == "val_address") | ||
|
|
||
| _, err = acc.TrackUndelegationEntry(sdkCtx, &lockuptypes.MsgTrackUndelegation{ | ||
| Sender: "owner", | ||
| Id: ubdSeq - 1, | ||
| }) | ||
| require.NoError(t, err) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance test coverage and improve error handling
The additions to TestPermanentAccountUndelegate improve the test coverage by verifying the state after undelegation. However, consider the following suggestions to further enhance the test:
- Add assertions to verify the state before undelegation, providing a more comprehensive check.
- Use
require.NoErrorconsistently for all error checks to ensure the test fails immediately on any error. - Improve the comment on line 83 to be more descriptive, e.g., "Retrieve the unbonding entry for the previous sequence".
- Add assertions to verify the state after tracking the undelegation entry.
Here's a suggested improvement:
// Verify state before undelegation
delLockingBefore, err := acc.DelegatedLocking.Get(ctx, "test")
require.NoError(t, err)
require.True(t, delLockingBefore.Equal(math.NewInt(1)))
// Undelegate
_, err = acc.Undelegate(sdkCtx, &lockuptypes.MsgUndelegate{
Sender: "owner",
ValidatorAddress: "val_address",
Amount: sdk.NewCoin("test", math.NewInt(1)),
})
require.NoError(t, err)
ubdSeq, err := acc.UnbondingSequence.Peek(sdkCtx)
require.NoError(t, err)
// Retrieve the unbonding entry for the previous sequence
ubdEntry, err := acc.UnbondEntries.Get(sdkCtx, ubdSeq-1)
require.NoError(t, err)
require.True(t, ubdEntry.Amount.Amount.Equal(math.NewInt(1)))
require.Equal(t, "val_address", ubdEntry.ValidatorAddress)
_, err = acc.TrackUndelegationEntry(sdkCtx, &lockuptypes.MsgTrackUndelegation{
Sender: "owner",
Id: ubdSeq - 1,
})
require.NoError(t, err)
// Verify state after tracking undelegation
delLockingAfter, err := acc.DelegatedLocking.Get(ctx, "test")
require.NoError(t, err)
require.True(t, delLockingAfter.Equal(math.ZeroInt()))These changes will provide a more comprehensive test of the undelegation process.
Consider adding a brief comment explaining the purpose of TrackUndelegationEntry to improve code readability.
| @@ -149,4 +149,5 @@ func (dva DelayedLockingAccount) RegisterExecuteHandlers(builder *accountstd.Exe | |||
|
|
|||
| func (dva DelayedLockingAccount) RegisterQueryHandlers(builder *accountstd.QueryBuilder) { | |||
| accountstd.RegisterQueryHandler(builder, dva.QueryVestingAccountInfo) | |||
| accountstd.RegisterQueryHandler(builder, dva.QueryUnbondingEntries) | |||
There was a problem hiding this comment.
💡 Codebase verification
🔍 Missing Implementation of QueryUnbondingEntries Method
The QueryUnbondingEntries query handler has been registered, but the implementation of the QueryUnbondingEntries method was not found in the codebase.
- Ensure that the
QueryUnbondingEntriesmethod is implemented within theDelayedLockingAccountstruct. - Verify that the method adheres to the expected signature and correctly handles the request and response types.
🔗 Analysis chain
LGTM. Verify the implementation of QueryUnbondingEntries.
The addition of the QueryUnbondingEntries query handler is consistent with the PR objectives and follows the established pattern for registering query handlers. However, the implementation of this method is not visible in the current file.
Please run the following script to verify the existence and implementation of the QueryUnbondingEntries method:
If the method is not found or the types are not defined, please implement them to ensure the newly added query handler functions correctly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the implementation of QueryUnbondingEntries method
# Test: Search for the QueryUnbondingEntries method implementation
ast-grep --lang go --pattern $'func (dva DelayedLockingAccount) QueryUnbondingEntries(ctx context.Context, req *lockuptypes.QueryUnbondingEntriesRequest) (*lockuptypes.QueryUnbondingEntriesResponse, error) {
$$$
}'
# Test: Check if the QueryUnbondingEntriesRequest and QueryUnbondingEntriesResponse types are defined
rg --type go -e 'type QueryUnbondingEntriesRequest struct' -e 'type QueryUnbondingEntriesResponse struct'
Length of output: 693
| t.Run("ok - execute tracking unbonding entry", func(t *testing.T) { | ||
| msg := &types.MsgTrackUndelegation{ | ||
| Sender: ownerAddrStr, | ||
| Id: 0, | ||
| } | ||
| err = s.executeTx(ctx, msg, app, accountAddr, accOwner) | ||
| require.NoError(t, err) | ||
|
|
||
| // check if tracking is updated accordingly | ||
| lockupAccountInfoResponse := s.queryLockupAccInfo(ctx, app, accountAddr) | ||
| delLocking := lockupAccountInfoResponse.DelegatedLocking | ||
| require.True(t, delLocking.AmountOf("stake").Equal(math.ZeroInt())) | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add test cases for multiple unbonding entries
Currently, the test tracks undelegation for a single entry with Id: 0. To ensure comprehensive coverage, consider adding test cases that handle multiple unbonding entries and verify that tracking works correctly for each.
| require.True(t, entries[0].Amount.Amount.Equal(math.NewInt(100))) | ||
| require.True(t, entries[0].ValidatorAddress == val.OperatorAddress) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use require.Equal for clearer assertions
Using require.Equal instead of require.True with an equality condition provides more informative error messages if the test fails.
Apply this diff:
- require.True(t, entries[0].ValidatorAddress == val.OperatorAddress)
+ require.Equal(t, val.OperatorAddress, entries[0].ValidatorAddress)📝 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.
| require.True(t, entries[0].Amount.Amount.Equal(math.NewInt(100))) | |
| require.True(t, entries[0].ValidatorAddress == val.OperatorAddress) | |
| require.True(t, entries[0].Amount.Amount.Equal(math.NewInt(100))) | |
| require.Equal(t, val.OperatorAddress, entries[0].ValidatorAddress) |
| // check if an entry is added | ||
| unbondingEntriesResponse := s.queryUnbondingEntries(ctx, app, accountAddr) | ||
| entries := unbondingEntriesResponse.UnbondingEntries | ||
| require.True(t, entries[0].Amount.Amount.Equal(math.NewInt(100))) | ||
| require.True(t, entries[0].ValidatorAddress == val.OperatorAddress) |
There was a problem hiding this comment.
Check for empty unbonding entries before accessing
Accessing entries[0] without verifying that entries is not empty could lead to an index out of range error if no unbonding entries are present. Please add a check to ensure entries contains at least one element before accessing it.
Apply this diff to add the length check:
+ require.Greater(t, len(entries), 0)
require.True(t, entries[0].Amount.Amount.Equal(math.NewInt(100)))
require.Equal(t, val.OperatorAddress, entries[0].ValidatorAddress)📝 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.
| // check if an entry is added | |
| unbondingEntriesResponse := s.queryUnbondingEntries(ctx, app, accountAddr) | |
| entries := unbondingEntriesResponse.UnbondingEntries | |
| require.True(t, entries[0].Amount.Amount.Equal(math.NewInt(100))) | |
| require.True(t, entries[0].ValidatorAddress == val.OperatorAddress) | |
| // check if an entry is added | |
| unbondingEntriesResponse := s.queryUnbondingEntries(ctx, app, accountAddr) | |
| entries := unbondingEntriesResponse.UnbondingEntries | |
| require.Greater(t, len(entries), 0) | |
| require.True(t, entries[0].Amount.Amount.Equal(math.NewInt(100))) | |
| require.Equal(t, val.OperatorAddress, entries[0].ValidatorAddress) |
| msgUndelegateResp, err := accountstd.UnpackAny[stakingtypes.MsgUndelegateResponse](resp[0]) | ||
| if err != nil { |
There was a problem hiding this comment.
Check response length before accessing resp[0]
Before accessing the first element of resp, ensure that the response slice is not empty to prevent a potential index out-of-range panic.
Apply this diff to add a length check:
resp, err := sendMessage(ctx, msgUndelegate)
if err != nil {
return nil, err
}
+if len(resp) == 0 {
+ return nil, sdkerrors.ErrInvalidResponse.Wrap("no response received from MsgUndelegate")
+}
msgUndelegateResp, err := accountstd.UnpackAny[stakingtypes.MsgUndelegateResponse](resp[0])
if err != nil {
return nil, err
}This ensures that you safely handle cases where the response might be empty.
📝 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.
| msgUndelegateResp, err := accountstd.UnpackAny[stakingtypes.MsgUndelegateResponse](resp[0]) | |
| if err != nil { | |
| if len(resp) == 0 { | |
| return nil, sdkerrors.ErrInvalidResponse.Wrap("no response received from MsgUndelegate") | |
| } | |
| msgUndelegateResp, err := accountstd.UnpackAny[stakingtypes.MsgUndelegateResponse](resp[0]) | |
| if err != nil { |
| entry, err := bva.UnbondEntries.Get(ctx, msg.Id) | ||
| if err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
Handle missing unbonding entries explicitly
When retrieving the unbonding entry with bva.UnbondEntries.Get(ctx, msg.Id), if the entry does not exist, it may return an error that's not descriptive. To improve user experience, check for a collections.ErrNotFound error and return a more specific message.
Apply this diff to enhance error handling:
entry, err := bva.UnbondEntries.Get(ctx, msg.Id)
if err != nil {
- return nil, err
+ if errors.Is(err, collections.ErrNotFound) {
+ return nil, sdkerrors.ErrNotFound.Wrapf("unbonding entry with ID %d not found", msg.Id)
+ }
+ return nil, err
}This provides clearer feedback when an unbonding entry is not found.
📝 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.
| entry, err := bva.UnbondEntries.Get(ctx, msg.Id) | |
| if err != nil { | |
| return nil, err | |
| } | |
| entry, err := bva.UnbondEntries.Get(ctx, msg.Id) | |
| if err != nil { | |
| if errors.Is(err, collections.ErrNotFound) { | |
| return nil, sdkerrors.ErrNotFound.Wrapf("unbonding entry with ID %d not found", msg.Id) | |
| } | |
| return nil, err | |
| } |
| UnbondEntries: collections.NewMap(d.SchemaBuilder, UnbondEntriesPrefix, "unbond_entries", collections.Uint64Key, codec.CollValue[lockuptypes.UnbondingEntry](d.LegacyStateCodec)), | ||
| UnbondingSequence: collections.NewSequence(d.SchemaBuilder, UnbondingSequencePrefix, "unbonding_sequence"), |
There was a problem hiding this comment.
Initialize renamed field correctly in newBaseLockup
Following the renaming of WithdrawedCoins to WithdrawnCoins, make sure to update its initialization in the newBaseLockup function.
Apply this diff:
BaseLockup := &BaseLockup{
Owner: collections.NewItem(d.SchemaBuilder, OwnerPrefix, "owner", collections.BytesValue),
OriginalLocking: collections.NewMap(d.SchemaBuilder, OriginalLockingPrefix, "original_locking", collections.StringKey, sdk.IntValue),
DelegatedFree: collections.NewMap(d.SchemaBuilder, DelegatedFreePrefix, "delegated_free", collections.StringKey, sdk.IntValue),
DelegatedLocking: collections.NewMap(d.SchemaBuilder, DelegatedLockingPrefix, "delegated_locking", collections.StringKey, sdk.IntValue),
- WithdrawedCoins: collections.NewMap(d.SchemaBuilder, WithdrawedCoinsPrefix, "withdrawed_coins", collections.StringKey, sdk.IntValue),
+ WithdrawnCoins: collections.NewMap(d.SchemaBuilder, WithdrawnCoinsPrefix, "withdrawn_coins", collections.StringKey, sdk.IntValue),
UnbondEntries: collections.NewMap(d.SchemaBuilder, UnbondEntriesPrefix, "unbond_entries", collections.Uint64Key, codec.CollValue[lockuptypes.UnbondingEntry](d.LegacyStateCodec)),
UnbondingSequence: collections.NewSequence(d.SchemaBuilder, UnbondingSequencePrefix, "unbonding_sequence"),
addressCodec: d.AddressCodec,Also, update the prefix variable if necessary.
Committable suggestion was skipped due to low confidence.
| var ( | ||
| md_QueryUnbondingEntriesRequest protoreflect.MessageDescriptor | ||
| ) | ||
|
|
||
| func init() { | ||
| file_cosmos_accounts_defaults_lockup_query_proto_init() | ||
| md_QueryUnbondingEntriesRequest = File_cosmos_accounts_defaults_lockup_query_proto.Messages().ByName("QueryUnbondingEntriesRequest") | ||
| } | ||
|
|
||
| var _ protoreflect.Message = (*fastReflection_QueryUnbondingEntriesRequest)(nil) | ||
|
|
||
| type fastReflection_QueryUnbondingEntriesRequest QueryUnbondingEntriesRequest | ||
|
|
||
| func (x *QueryUnbondingEntriesRequest) ProtoReflect() protoreflect.Message { | ||
| return (*fastReflection_QueryUnbondingEntriesRequest)(x) | ||
| } | ||
|
|
||
| func (x *QueryUnbondingEntriesRequest) slowProtoReflect() protoreflect.Message { | ||
| mi := &file_cosmos_accounts_defaults_lockup_query_proto_msgTypes[2] | ||
| if protoimpl.UnsafeEnabled && x != nil { | ||
| ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) | ||
| if ms.LoadMessageInfo() == nil { | ||
| ms.StoreMessageInfo(mi) | ||
| } | ||
| return ms | ||
| } | ||
| return mi.MessageOf(x) | ||
| } |
There was a problem hiding this comment.
Add documentation comments for new message types
The newly introduced QueryUnbondingEntriesRequest lacks a documentation comment explaining its purpose and usage. According to the Go coding guidelines and best practices, exported types and functions should include comments.
Consider adding a comment like:
// QueryUnbondingEntriesRequest is the request type for querying unbonding entries.
type QueryUnbondingEntriesRequest struct {
// Fields...
}| // QueryUnbondingEntriesRequest is used to query the lockup account unbonding entries. | ||
| type QueryUnbondingEntriesRequest struct { | ||
| state protoimpl.MessageState | ||
| sizeCache protoimpl.SizeCache | ||
| unknownFields protoimpl.UnknownFields | ||
| } | ||
|
|
||
| func (x *QueryUnbondingEntriesRequest) Reset() { | ||
| *x = QueryUnbondingEntriesRequest{} | ||
| if protoimpl.UnsafeEnabled { | ||
| mi := &file_cosmos_accounts_defaults_lockup_query_proto_msgTypes[2] | ||
| ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) | ||
| ms.StoreMessageInfo(mi) | ||
| } | ||
| } | ||
|
|
||
| func (x *QueryUnbondingEntriesRequest) String() string { | ||
| return protoimpl.X.MessageStringOf(x) | ||
| } | ||
|
|
||
| func (*QueryUnbondingEntriesRequest) ProtoMessage() {} | ||
|
|
||
| // Deprecated: Use QueryUnbondingEntriesRequest.ProtoReflect.Descriptor instead. | ||
| func (*QueryUnbondingEntriesRequest) Descriptor() ([]byte, []int) { | ||
| return file_cosmos_accounts_defaults_lockup_query_proto_rawDescGZIP(), []int{2} | ||
| } | ||
|
|
||
| // QueryUnbondingEntriesResponse returns the lockup account unbonding entries. | ||
| type QueryUnbondingEntriesResponse struct { | ||
| state protoimpl.MessageState | ||
| sizeCache protoimpl.SizeCache | ||
| unknownFields protoimpl.UnknownFields | ||
|
|
||
| // UnbondingEntry defines the list of unbonding entries. | ||
| UnbondingEntries []*UnbondingEntry `protobuf:"bytes,1,rep,name=unbonding_entries,json=unbondingEntries,proto3" json:"unbonding_entries,omitempty"` | ||
| } | ||
|
|
||
| func (x *QueryUnbondingEntriesResponse) Reset() { | ||
| *x = QueryUnbondingEntriesResponse{} | ||
| if protoimpl.UnsafeEnabled { | ||
| mi := &file_cosmos_accounts_defaults_lockup_query_proto_msgTypes[3] | ||
| ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) | ||
| ms.StoreMessageInfo(mi) | ||
| } | ||
| } | ||
|
|
||
| func (x *QueryUnbondingEntriesResponse) String() string { | ||
| return protoimpl.X.MessageStringOf(x) | ||
| } | ||
|
|
||
| func (*QueryUnbondingEntriesResponse) ProtoMessage() {} | ||
|
|
||
| // Deprecated: Use QueryUnbondingEntriesResponse.ProtoReflect.Descriptor instead. | ||
| func (*QueryUnbondingEntriesResponse) Descriptor() ([]byte, []int) { | ||
| return file_cosmos_accounts_defaults_lockup_query_proto_rawDescGZIP(), []int{3} | ||
| } | ||
|
|
||
| func (x *QueryUnbondingEntriesResponse) GetUnbondingEntries() []*UnbondingEntry { | ||
| if x != nil { | ||
| return x.UnbondingEntries | ||
| } | ||
| return nil | ||
| } | ||
|
|
There was a problem hiding this comment.
Add documentation comments for new response types
The QueryUnbondingEntriesResponse struct is missing a documentation comment. Following the Uber Go Style Guide, exported types should have comments to improve code readability and maintainability.
Add a comment to describe the response type:
// QueryUnbondingEntriesResponse is the response type containing unbonding entries.
type QueryUnbondingEntriesResponse struct {
// Fields...
}
Description
TrackUndelegationto update thedelegateLockinganddelegateFreeamount, this is incorrect behavior since legacy vesting account callTrackUndelegationwhen we call x/bankUndelegateCoinswhich is when the unbonding entry already matured. This makedelegateLockingupdated before the ubd amount actually sent back to the account, which make the spendable amount incorrect when we try to sent token from lockup accountTrackUndelegationwith that entry. This check should be run before every execution the account made.TrackUndelegationAuthor 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 changeCHANGELOG.mdReviewers 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.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
New Features
UnbondingEntryfor tracking unbonding operations.QueryUnbondingEntriesRequestandQueryUnbondingEntriesResponse.MsgTrackUndelegationfor tracking undelegation operations.Bug Fixes
Tests