🔃 Update vesting module to cosmos-sdk v0.50.6 #634
Conversation
bdeneux
changed the title
WalkthroughThe changes introduce new functionalities and enhancements to the Axone protocol, particularly focusing on the vesting module. Key updates include adding commands for managing genesis accounts, improving CLI commands for vesting accounts, refining Protobuf definitions, and enhancing test coverage. These modifications aim to streamline account management, improve documentation, and ensure robust testing for various vesting scenarios. Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant CLI
participant AxoneNode
participant VestingModule
User->>CLI: Execute add-genesis-account command
CLI->>AxoneNode: Parse and validate command
AxoneNode->>VestingModule: Add genesis account with vesting parameters
VestingModule-->>AxoneNode: Return success/failure
AxoneNode-->>CLI: Generate response
CLI-->>User: Display result
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
bdeneux
force-pushed
the
refactor/update-vesting
branch
from
5b9d1c6 to
85d03e6
Compare
ccamel
added
the
security audit
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
x/vesting/types/tx.pb.gois excluded by!**/*.pb.gox/vesting/types/vesting.pb.gois excluded by!**/*.pb.go
Files selected for processing (28)
- Makefile (1 hunks)
- cmd/axoned/cmd/genaccount.go (1 hunks)
- cmd/axoned/cmd/genaccount_test.go (1 hunks)
- cmd/axoned/cmd/root.go (1 hunks)
- docs/command/axoned_genesis_add-genesis-account.md (1 hunks)
- docs/command/axoned_tx_vesting_create-cliff-vesting-account.md (1 hunks)
- docs/command/axoned_tx_vesting_create-periodic-vesting-account.md (1 hunks)
- docs/proto/vesting.md (4 hunks)
- proto/buf.gen.doc.yml (1 hunks)
- proto/buf.yaml (1 hunks)
- proto/vesting/module/v1/module.proto (1 hunks)
- proto/vesting/v1beta1/tx.proto (4 hunks)
- proto/vesting/v1beta1/vesting.proto (6 hunks)
- scripts/protocgen-doc.sh (1 hunks)
- x/vesting/client/cli/test.json (1 hunks)
- x/vesting/client/cli/tx.go (14 hunks)
- x/vesting/client/cli/tx_test.go (1 hunks)
- x/vesting/module.go (5 hunks)
- x/vesting/msg_server.go (7 hunks)
- x/vesting/msg_server_test.go (14 hunks)
- x/vesting/types/codec.go (2 hunks)
- x/vesting/types/constants.go (1 hunks)
- x/vesting/types/genaccounts.go (1 hunks)
- x/vesting/types/genesis_test.go (1 hunks)
- x/vesting/types/msgs.go (4 hunks)
- x/vesting/types/period.go (2 hunks)
- x/vesting/types/vesting_account.go (16 hunks)
- x/vesting/types/vesting_account_test.go (30 hunks)
Files not reviewed due to errors (4)
- proto/vesting/v1beta1/tx.proto (no review received)
- x/vesting/types/genaccounts.go (no review received)
- x/vesting/client/cli/tx.go (no review received)
- x/vesting/msg_server.go (no review received)
Files skipped from review due to trivial changes (6)
- proto/buf.gen.doc.yml
- proto/buf.yaml
- x/vesting/client/cli/test.json
- x/vesting/types/constants.go
- x/vesting/types/msgs.go
- x/vesting/types/period.go
Additional comments not posted (36)
proto/vesting/module/v1/module.proto (1)
1-12: The protobuf definition for the vesting module configuration is correctly set up.scripts/protocgen-doc.sh (1)
14-17: The updated script optimizes the search for.protofiles and correctly handles the documentation generation and renaming.x/vesting/types/genesis_test.go (1)
24-25: The test properly handles the new error response fromNewBaseVestingAccount, ensuring robust error handling in genesis state validation.x/vesting/types/codec.go (1)
23-27: The new vesting account types and messages are correctly registered, ensuring proper serialization and deserialization within the application.Also applies to: 67-67
cmd/axoned/cmd/genaccount_test.go (1)
35-116: The tests forAddGenesisAccountCmdare comprehensive and correctly handle various scenarios, ensuring robust command validation.cmd/axoned/cmd/genaccount.go (1)
33-104: Ensure proper error handling and command flag descriptions inAddGenesisAccountCmd.Verification successful
Let's perform a detailed search for error handling and flag descriptions within the
AddGenesisAccountCmdfunction.
Error Handling
The function includes error handling patterns:
return errreturn fmt.Errorf("failed to get address from Keyring: %w", err)These patterns indicate that errors are being properly caught and returned.
Command Flag Descriptions
The command flag descriptions are present:
cmd.Flags().String(flags.FlagHome, defaultNodeHome, "The application home directory")cmd.Flags().String(flags.FlagKeyringBackend, flags.DefaultKeyringBackend, "Select keyring's backend (os|file|kwallet|pass|test)")cmd.Flags().String(flagVestingAmt, "", "amount of coins for vesting accounts")cmd.Flags().Int64(flagVestingStart, 0, "schedule start time (unix epoch) for vesting accounts")cmd.Flags().Int64(flagVestingCliff, 0, "schedule cliff time (unix epoch) for vesting accounts")cmd.Flags().Int64(flagVestingEnd, 0, "schedule end time (unix epoch) for vesting accounts")cmd.Flags().Bool(flagAppendMode, false, "append the coins to an account already in the genesis.json file")cmd.Flags().String(flagModuleName, "", "module account name")These descriptions appear to be clear and informative.
Conclusion
The function
AddGenesisAccountCmdhas proper error handling and well-documented command flag descriptions.
- : The function
AddGenesisAccountCmdincludes proper error handling and well-documented command flag descriptions.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify proper error handling and command flag descriptions in `AddGenesisAccountCmd`. # Test: Search for error handling patterns and flag descriptions in the function. rg --type go $'AddGenesisAccountCmd' cmd/axoned/cmd/genaccount.goLength of output: 226
Script:
#!/bin/bash # Description: Verify proper error handling and command flag descriptions in `AddGenesisAccountCmd`. # Test: Search for error handling patterns in the function. rg --type go 'return fmt\.Errorf|return err' cmd/axoned/cmd/genaccount.go # Test: Search for command flag descriptions in the function. rg --type go 'cmd\.Flags\.' cmd/axoned/cmd/genaccount.goLength of output: 237
x/vesting/module.go (2)
Line range hint
38-104: Review the implementation ofAppModuleBasicmethods for completeness and correctness.
Line range hint
88-146: Ensure correct use of keepers and implementation of module interfaces inAppModule.Verification successful
The
AppModulestructure correctly extendsAppModuleBasicand implements the necessary interfaces, includingappmodule.AppModuleandappmodule.HasServices. The keepers (accountKeeperandbankKeeper) are properly initialized in theNewAppModuleconstructor and used within theRegisterServicesmethod. Additionally, all required methods for the implemented interfaces are present and correctly defined.
- The keepers are initialized in the
NewAppModulefunction.- The
RegisterServicesmethod uses the keepers to register the message server.- The
AppModulestructure implements the necessary methods for theappmodule.AppModuleinterface.Therefore, the implementation appears to be correct.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct use of keepers and implementation of module interfaces in `AppModule`. # Test: Search for the use of keepers and module interfaces in the AppModule structure. rg --type go $'AppModule' x/vesting/module.goLength of output: 1696
proto/vesting/v1beta1/vesting.proto (3)
Line range hint
1-34: Review the Protobuf message definition forBaseVestingAccountfor correctness of field definitions and options.Verification successful
Verify the correctness of the
BaseVestingAccountmessage definitionThe extracted definition of the
BaseVestingAccountmessage appears to be correct in terms of field definitions and Protobuf options. The fields and options are consistent with the expected structure for a vesting account.
- The
base_accountfield is correctly embedded.- The
original_vesting,delegated_free, anddelegated_vestingfields are correctly defined with appropriate options.No issues were found in the provided snippet.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of field definitions and options in `BaseVestingAccount`. # Test: Search for the `BaseVestingAccount` message definition in the Protobuf file. rg --type proto $'BaseVestingAccount' proto/vesting/v1beta1/vesting.protoLength of output: 102
Script:
#!/bin/bash # Description: Verify the correctness of field definitions and options in `BaseVestingAccount`. # Test: Search for the `BaseVestingAccount` message definition in the Protobuf file. rg --type-add 'proto:*.proto' 'BaseVestingAccount' proto/vesting/v1beta1/vesting.protoLength of output: 621
Script:
#!/bin/bash # Description: Extract the full definition of the `BaseVestingAccount` message for detailed review. # Test: Extract the full message definition for `BaseVestingAccount`. rg --type-add 'proto:*.proto' -A 20 'message BaseVestingAccount' proto/vesting/v1beta1/vesting.protoLength of output: 972
64-69: Ensure correct field definitions and options in thePeriodmessage.Verification successful
Ensure correct field definitions and options in the
Periodmessage.The
Periodmessage in the Protobuf file is defined as follows:message Period { // Period duration in seconds. int64 length = 1; repeated cosmos.base.v1beta1.Coin amount = 2 [ (gogoproto.nullable) = false, (amino.dont_omitempty) = true, (amino.encoding) = "legacy_coins", (gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins" ]; }
- The
lengthfield is defined as anint64representing the period duration in seconds.- The
amountfield is a repeated field of typecosmos.base.v1beta1.Coinwith several options:
(gogoproto.nullable) = false(amino.dont_omitempty) = true(amino.encoding) = "legacy_coins"(gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins"These definitions and options appear to be correctly specified.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct field definitions and options in the `Period` message. # Test: Search for the `Period` message definition in the Protobuf file. rg --type proto $'Period' proto/vesting/v1beta1/vesting.protoLength of output: 90
Script:
#!/bin/bash # Description: Verify the correct field definitions and options in the `Period` message. # Test: Search for the `Period` message definition in the Protobuf file. rg --type proto 'message Period' proto/vesting/v1beta1/vesting.protoLength of output: 100
Script:
#!/bin/bash # Description: Verify the correct field definitions and options in the `Period` message. # Test: Search for the `Period` message definition in the Protobuf file. rg 'message Period' proto/vesting/v1beta1/vesting.protoLength of output: 107
Script:
#!/bin/bash # Description: Extract the `Period` message definition and verify the fields and options. # Test: Extract the `Period` message definition from the Protobuf file. rg -A 10 'message Period {' proto/vesting/v1beta1/vesting.protoLength of output: 161
Script:
#!/bin/bash # Description: Extract the `Period` message definition and verify the fields and options. # Test: Extract the `Period` message definition from the Protobuf file. rg -A 10 'message Period \{' proto/vesting/v1beta1/vesting.protoLength of output: 375
111-111: Review theCliffVestingAccountmessage for correct field definitions and options.Verification successful
Verification of
CliffVestingAccountMessage DefinitionThe
CliffVestingAccountmessage in the Protobuf file is correctly defined with appropriate field definitions and options.
- The message includes the necessary options:
(amino.name)and(gogoproto.goproto_getters).- The fields
base_vesting_account,start_time, andcliff_timeare correctly defined with appropriate types and options.No issues were found with the
CliffVestingAccountmessage definition.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct field definitions and options in the `CliffVestingAccount` message. # Test: Search for the `CliffVestingAccount` message definition in the Protobuf file. rg --type proto $'CliffVestingAccount' proto/vesting/v1beta1/vesting.protoLength of output: 103
Script:
#!/bin/bash # Description: Verify the correct field definitions and options in the `CliffVestingAccount` message. # Test: Search for the `CliffVestingAccount` message definition in the Protobuf file. rg 'message CliffVestingAccount' proto/vesting/v1beta1/vesting.protoLength of output: 100
Script:
#!/bin/bash # Description: Extract the full definition of the `CliffVestingAccount` message. # Test: Extract the `CliffVestingAccount` message definition from the Protobuf file. rg -A 20 'message CliffVestingAccount' proto/vesting/v1beta1/vesting.protoLength of output: 645
x/vesting/client/cli/tx_test.go (5)
54-140: The testing structure and execution inTestNewMsgCreateVestingAccountCmdare well-implemented and follow good practices.
142-232: The testing structure and execution inTestNewMsgCreateCliffVestingAccountCmdare correctly implemented for testing cliff vesting account creation.
234-311: The testing structure and execution inTestNewMsgCreatePermanentLockedAccountCmdare correctly implemented for testing permanent locked account creation.
313-371: The testing structure and execution inTestNewMsgCreatePeriodicVestingAccountCmdare correctly implemented for testing periodic vesting account creation.
30-52: The setup of theCLITestSuiteis comprehensive and well-structured, providing a solid foundation for the tests.cmd/axoned/cmd/root.go (1)
199-206: Customadd-genesis-accountcommand reintroduced correctly.docs/proto/vesting.md (3)
63-63: Documentation forcliff_timeinCliffVestingAccountadded correctly.
97-97: Documentation forlengthinPeriodadded correctly.
153-153: Documentation forcliff_timeinMsgCreateCliffVestingAccountadded correctly.Makefile (1)
322-322: Optimization of documentation generation process by limiting search depth implemented correctly.x/vesting/msg_server_test.go (4)
80-123: The test scenarios forCreateVestingAccountare comprehensive and well-structured.Also applies to: 141-155
213-230: The test scenarios forCreatePermanentLockedAccountare well-implemented and cover necessary edge cases.Also applies to: 255-269
312-391: The test scenarios forCreatePeriodicVestingAccountare thorough and effectively validate the functionality.Also applies to: 413-435
486-540: The test scenarios forCreateCliffVestingAccountare comprehensive and cover all critical edge cases.Also applies to: 558-572
x/vesting/types/vesting_account.go (5)
30-39: The implementation and validation logic inBaseVestingAccountare correctly handled.Also applies to: 152-164
186-198: TheContinuousVestingAccountis well-implemented with appropriate validation and utility methods.
283-301: ThePeriodicVestingAccountimplementation is robust, particularly the detailed validation logic for vesting periods.Also applies to: 374-394
415-427: TheDelayedVestingAccountis correctly implemented with appropriate validation and utility methods.
478-487: ThePermanentLockedAccountis well-implemented, ensuring that no end time is set for permanently locked accounts.x/vesting/types/vesting_account_test.go (6)
1-1: Ensure thenolint:llldirective is necessary.Verification successful
Ensure the
nolint:llldirective is necessary.
- The file
x/vesting/types/vesting_account_test.gocontains multiple lines exceeding typical length limits (100 characters), confirming the necessity of thenolint:llldirective.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the long line linting issue is prevalent in this file. # Test: Search for lines exceeding typical length limits. rg '.{100,}' x/vesting/types/vesting_account_test.goLength of output: 6426
13-13: Added imports are appropriate for the new test functionalities.
47-47: The integration ofruntime.NewKVStoreServiceand context setup withheader.Infois correctly implemented.Also applies to: 49-49, 62-62
76-77: The repeated pattern of creating vesting accounts and handling errors is consistent and correctly implemented across various test functions.Also applies to: 101-102, 122-123, 146-147, 153-154, 160-161, 171-172, 187-188, 195-196, 203-204, 212-213, 235-236, 252-253, 270-271, 299-300, 306-307, 314-315, 321-322, 337-338, 345-346, 353-354, 362-363, 389-390, 424-425, 485-486, 523-524, 553-554, 560-561, 567-568, 575-576, 583-584, 594-595, 615-616, 623-624, 631-632, 639-640, 648-649, 671-672, 688-689, 706-707, 730-731, 737-738, 744-745, 760-761, 768-769, 776-777, 785-786, 803-804, 832-833, 858-859, 888-889, 933-934
424-424: The test for overflow and negative values in vesting periods is a good addition for robustness.
988-988: The tests for various account validations are well-structured and cover a broad range of scenarios.Also applies to: 1012-1012, 1020-1020, 1028-1028, 1050-1050
bdeneux
changed the title
This commit updates the vesting module to match the changes introduced in cosmos-sdk v0.50.4.
…nclude cliff vesting account
bdeneux
force-pushed
the
refactor/update-vesting
branch
from
ba91548 to
f14c0e5
Compare
Codecov ReportAttention: Patch coverage is @@ Coverage Diff @@
## main #634 +/- ##
==========================================
- Coverage 53.25% 51.98% -1.28%
==========================================
Files 74 75 +1
Lines 3132 2824 -308
==========================================
- Hits 1668 1468 -200
+ Misses 1367 1260 -107
+ Partials 97 96 -1
|
There was a problem hiding this comment.
Actionable comments posted: 10
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (6)
docs/command/axoned_genesis_add-genesis-account.mdis excluded by!docs/command/**/*.mddocs/command/axoned_tx_vesting_create-cliff-vesting-account.mdis excluded by!docs/command/**/*.mddocs/command/axoned_tx_vesting_create-periodic-vesting-account.mdis excluded by!docs/command/**/*.mddocs/proto/vesting.mdis excluded by!docs/proto/**/*.mdx/vesting/types/tx.pb.gois excluded by!**/*.pb.gox/vesting/types/vesting.pb.gois excluded by!**/*.pb.go
Files selected for processing (25)
- .github/workflows/test.yml (1 hunks)
- Makefile (1 hunks)
- cmd/axoned/cmd/genaccount.go (1 hunks)
- cmd/axoned/cmd/genaccount_test.go (1 hunks)
- cmd/axoned/cmd/root.go (1 hunks)
- proto/buf.gen.doc.yml (1 hunks)
- proto/buf.yaml (1 hunks)
- proto/vesting/module/v1/module.proto (1 hunks)
- proto/vesting/v1beta1/tx.proto (4 hunks)
- proto/vesting/v1beta1/vesting.proto (6 hunks)
- scripts/protocgen-doc.sh (1 hunks)
- x/vesting/client/cli/test.json (1 hunks)
- x/vesting/client/cli/tx.go (14 hunks)
- x/vesting/client/cli/tx_test.go (1 hunks)
- x/vesting/module.go (5 hunks)
- x/vesting/msg_server.go (7 hunks)
- x/vesting/msg_server_test.go (14 hunks)
- x/vesting/types/codec.go (2 hunks)
- x/vesting/types/constants.go (1 hunks)
- x/vesting/types/genaccounts.go (1 hunks)
- x/vesting/types/genesis_test.go (1 hunks)
- x/vesting/types/msgs.go (4 hunks)
- x/vesting/types/period.go (2 hunks)
- x/vesting/types/vesting_account.go (16 hunks)
- x/vesting/types/vesting_account_test.go (30 hunks)
Files not reviewed due to errors (5)
- proto/vesting/v1beta1/vesting.proto (no review received)
- proto/vesting/v1beta1/tx.proto (no review received)
- x/vesting/types/genaccounts.go (no review received)
- x/vesting/client/cli/tx.go (no review received)
- x/vesting/msg_server.go (no review received)
Files skipped from review due to trivial changes (6)
- .github/workflows/test.yml
- proto/buf.gen.doc.yml
- proto/vesting/module/v1/module.proto
- x/vesting/client/cli/test.json
- x/vesting/types/msgs.go
- x/vesting/types/period.go
Files skipped from review as they are similar to previous changes (1)
- proto/buf.yaml
Additional context used
GitHub Check: codecov/patch
x/vesting/types/codec.go
[warning] 23-23: x/vesting/types/codec.go#L23
Added line #L23 was not covered by tests
[warning] 26-27: x/vesting/types/codec.go#L26-L27
Added lines #L26 - L27 were not covered by tests
[warning] 67-67: x/vesting/types/codec.go#L67
Added line #L67 was not covered by testsx/vesting/module.go
[warning] 70-70: x/vesting/module.go#L70
Added line #L70 was not covered by tests
[warning] 73-74: x/vesting/module.go#L73-L74
Added lines #L73 - L74 were not covered by tests
[warning] 88-88: x/vesting/module.go#L88
Added line #L88 was not covered by tests
[warning] 95-95: x/vesting/module.go#L95
Added line #L95 was not covered by tests
[warning] 98-98: x/vesting/module.go#L98
Added line #L98 was not covered by tests
[warning] 142-143: x/vesting/module.go#L142-L143
Added lines #L142 - L143 were not covered by tests
[warning] 145-145: x/vesting/module.go#L145
Added line #L145 was not covered by testsx/vesting/types/genaccounts.go
[warning] 32-35: x/vesting/types/genaccounts.go#L32-L35
Added lines #L32 - L35 were not covered by tests
[warning] 38-40: x/vesting/types/genaccounts.go#L38-L40
Added lines #L38 - L40 were not covered by tests
[warning] 44-44: x/vesting/types/genaccounts.go#L44
Added line #L44 was not covered by tests
[warning] 46-47: x/vesting/types/genaccounts.go#L46-L47
Added lines #L46 - L47 were not covered by tests
[warning] 49-53: x/vesting/types/genaccounts.go#L49-L53
Added lines #L49 - L53 were not covered by tests
[warning] 56-58: x/vesting/types/genaccounts.go#L56-L58
Added lines #L56 - L58 were not covered by tests
[warning] 61-63: x/vesting/types/genaccounts.go#L61-L63
Added lines #L61 - L63 were not covered by tests
[warning] 65-66: x/vesting/types/genaccounts.go#L65-L66
Added lines #L65 - L66 were not covered by tests
[warning] 68-69: x/vesting/types/genaccounts.go#L68-L69
Added lines #L68 - L69 were not covered by tests
[warning] 71-72: x/vesting/types/genaccounts.go#L71-L72
Added lines #L71 - L72 were not covered by tests
[warning] 74-77: x/vesting/types/genaccounts.go#L74-L77
Added lines #L74 - L77 were not covered by testsx/vesting/client/cli/tx.go
[warning] 28-28: x/vesting/client/cli/tx.go#L28
Added line #L28 was not covered by tests
[warning] 38-41: x/vesting/client/cli/tx.go#L38-L41
Added lines #L38 - L41 were not covered by tests
[warning] 202-203: x/vesting/client/cli/tx.go#L202-L203
Added lines #L202 - L203 were not covered by tests
[warning] 206-206: x/vesting/client/cli/tx.go#L206
Added line #L206 was not covered by testsx/vesting/msg_server.go
[warning] 71-71: x/vesting/msg_server.go#L71
Added line #L71 was not covered by tests
[warning] 138-138: x/vesting/msg_server.go#L138
Added line #L138 was not covered by tests
[warning] 213-213: x/vesting/msg_server.go#L213
Added line #L213 was not covered by tests
[warning] 281-281: x/vesting/msg_server.go#L281
Added line #L281 was not covered by tests
[warning] 315-315: x/vesting/msg_server.go#L315
Added line #L315 was not covered by tests
Additional comments not posted (27)
x/vesting/types/constants.go (1)
Line range hint
1-9: Ensure that the removed constantAttributeValueCategoryis not referenced elsewhere in the codebase.Verification successful
The constant
AttributeValueCategoryhas been successfully removed from the codebase and is not referenced elsewhere.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the removed constant `AttributeValueCategory` is not referenced elsewhere. # Test: Search for the constant usage. Expect: No occurrences. rg --type go 'AttributeValueCategory'Length of output: 37
Script:
#!/bin/bash # Description: Verify that the constant `AttributeValueCategory` has been removed from `constants.go`. # Test: Check the contents of `constants.go` for the presence of `AttributeValueCategory`. rg 'AttributeValueCategory' x/vesting/types/constants.goLength of output: 56
scripts/protocgen-doc.sh (1)
14-17: Optimization of documentation generation by limiting search depth is a good improvement.x/vesting/types/genesis_test.go (1)
24-25: Introduction ofNewBaseVestingAccountin tests aligns with the objective to enhance unit tests.cmd/axoned/cmd/genaccount_test.go (1)
1-116: Introduction of new imports and thetestMbmvariable enhances the setup for testingAddGenesisAccountCmd.cmd/axoned/cmd/genaccount.go (1)
33-104: Enhancement ofAddGenesisAccountCmdwith new vesting parameters improves flexibility in setting up genesis accounts.x/vesting/module.go (2)
142-143: Ensure theProvideModulefunction is covered by tests.Tools
GitHub Check: codecov/patch
[warning] 142-143: x/vesting/module.go#L142-L143
Added lines #L142 - L143 were not covered by tests
98-98: Ensure theIsAppModulemethod is covered by tests.Tools
GitHub Check: codecov/patch
[warning] 98-98: x/vesting/module.go#L98
Added line #L98 was not covered by testsx/vesting/client/cli/tx_test.go (4)
54-140: The implementation ofTestNewMsgCreateVestingAccountCmdcorrectly sets up and validates various scenarios for creating a vesting account. Good use of structured tests and error handling.
142-232: The implementation ofTestNewMsgCreateCliffVestingAccountCmdcorrectly sets up and validates various scenarios for creating a cliff vesting account. Proper handling of command context and error expectations.
234-311: The implementation ofTestNewMsgCreatePermanentLockedAccountCmdcorrectly sets up and validates various scenarios for creating a permanently locked vesting account. Proper handling of command context and error expectations.
313-371: The implementation ofTestNewMsgCreatePeriodicVestingAccountCmdcorrectly sets up and validates various scenarios for creating a periodic vesting account. Proper handling of command context and error expectations.cmd/axoned/cmd/root.go (1)
199-206: The modification to thegenesisCommandfunction correctly removes the defaultadd-genesis-accountcommand and adds a custom command. This aligns with the PR's objectives to reintroduce the custom command with additional vesting parameters.Makefile (1)
322-322: The modification to thedoc-prototarget in the Makefile correctly limits the search depth for.protofiles, optimizing the documentation generation process. This aligns with the PR's objectives to improve the Proto Docs Generation.x/vesting/msg_server_test.go (4)
80-123: Review error handling inTestCreateVestingAccountfor robustness and clarity.Verification successful
The error handling in
TestCreateVestingAccountappears robust and clear, covering various failure scenarios with descriptive error messages.
- Invalid 'from' address
- Invalid 'to' address
- Invalid coins
- Invalid end time
- Already exists
- Not allowed to receive funds
- Invalid start time
- Invalid period
- Invalid cliff time
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that error messages are clear and cover all failure scenarios. # Test: Search for error handling in `TestCreateVestingAccount`. Expect: Comprehensive and clear error messages. rg --type go 'expErr' x/vesting/msg_server_test.goLength of output: 2125
312-391: Ensure the handling of edge cases inTestCreatePeriodicVestingAccountis comprehensive.Verification successful
The edge cases in
TestCreatePeriodicVestingAccountappear to be comprehensive, covering various scenarios such as invalid addresses, invalid start time, invalid period, invalid coins, already existing accounts, and restrictions on receiving funds.
- Invalid 'from' address
- Invalid 'to' address
- Invalid start time
- Invalid period
- Invalid coins
- Already exists
- Not allowed to receive funds
- Invalid end time
- Invalid cliff time
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for comprehensive edge case handling in `TestCreatePeriodicVestingAccount`. # Test: Search for edge case tests. Expect: Comprehensive coverage. rg --type go 'expErr' x/vesting/msg_server_test.goLength of output: 2125
51-51: Ensure the block header time is set correctly for all test scenarios.Verification successful
The
VestingTestSuitestruct extendssuite.Suite, which is part of thetestifypackage. This package automatically calls theSetupTestmethod before each test case. TheSetupTestmethod inVestingTestSuitesets the block header time correctly.
- The block header time is set in the
SetupTestmethod.SetupTestis automatically called before each test case by thetestifyframework.This ensures that the block header time is set correctly for all test scenarios.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the block header time is set correctly in all test scenarios. # Test: Search for block header time setting. Expect: Consistent and correct usage across all tests. rg --type go 'WithBlockHeader' x/vesting/msg_server_test.goLength of output: 134
Script:
#!/bin/bash # Description: Identify all test functions in the file to verify if they set the block header time. # Test: Search for test functions. Expect: List of all test functions in the file. rg --type go 'func Test' x/vesting/msg_server_test.goLength of output: 97
Script:
#!/bin/bash # Description: Inspect the structure of the TestVestingTestSuite function to verify the block header time setting in each test case. # Test: Search for sub-tests or test cases within the TestVestingTestSuite function. rg --type go 'TestVestingTestSuite' -A 50 x/vesting/msg_server_test.goLength of output: 151
Script:
#!/bin/bash # Description: Search for the definition of VestingTestSuite and inspect its methods to verify the block header time setting. # Test: Search for the definition of VestingTestSuite. rg --type go 'type VestingTestSuite' -A 50 x/vesting/msg_server_test.goLength of output: 1575
Script:
#!/bin/bash # Description: Verify if SetupTest is called before each test case in the VestingTestSuite. # Test: Search for SetupTest calls in the VestingTestSuite. rg --type go 'SetupTest' x/vesting/msg_server_test.goLength of output: 94
Script:
#!/bin/bash # Description: Verify if suite.Suite is used and if it automatically calls SetupTest. # Test: Search for suite.Suite usage in the VestingTestSuite. rg --type go 'suite.Suite' x/vesting/msg_server_test.goLength of output: 68
213-239: Validate input parameters thoroughly inTestCreatePermanentLockedAccount.Verification successful
The test cases for
MsgCreatePermanentLockedAccountinx/vesting/msg_server_test.godo validate various input parameters, including empty addresses and invalid coins. This indicates that input validation is being thoroughly tested.
- Empty 'from' address: Validates that an empty 'from' address results in an error.
- Empty 'to' address: Validates that an empty 'to' address results in an error.
- Invalid coins: Validates that invalid coin amounts result in an error.
- Create for existing account: Validates that creating an account for an existing address results in an error.
- Create for blocked account: Validates that creating an account for a blocked address results in an error.
- Valid permanent locked account: Validates that a valid account creation succeeds without errors.
These test cases cover a broad range of input validation scenarios, ensuring that the function handles invalid inputs appropriately.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Ensure all input parameters are validated correctly. # Test: Search for input validation in `TestCreatePermanentLockedAccount`. Expect: Thorough validation for all inputs. rg --type go 'input:' x/vesting/msg_server_test.goLength of output: 1732
Script:
#!/bin/bash # Description: Search for the definition of `TestCreatePermanentLockedAccount` and extract relevant test cases to verify input validation. # Step 1: Locate the `TestCreatePermanentLockedAccount` function definition. rg --type go 'func TestCreatePermanentLockedAccount' x/vesting/msg_server_test.go # Step 2: Extract the test cases within the `TestCreatePermanentLockedAccount` function to verify input validation. rg --type go -A 20 'func TestCreatePermanentLockedAccount' x/vesting/msg_server_test.goLength of output: 173
Script:
#!/bin/bash # Description: Search for test cases related to `MsgCreatePermanentLockedAccount` and extract relevant sections to verify input validation. # Step 1: Locate test cases for `MsgCreatePermanentLockedAccount`. rg --type go 'MsgCreatePermanentLockedAccount' x/vesting/msg_server_test.go # Step 2: Extract the relevant sections around the located test cases to verify input validation. rg --type go -A 20 'MsgCreatePermanentLockedAccount' x/vesting/msg_server_test.goLength of output: 3073
x/vesting/types/vesting_account.go (6)
415-427: Review the creation ofDelayedVestingAccountfor correctness.
30-39: Ensure that theNewBaseVestingAccountfunction initializes all fields correctly.Verification successful
The
NewBaseVestingAccountfunction initializes all fields of theBaseVestingAccountstruct correctly. The fieldsBaseAccount,OriginalVesting,DelegatedFree,DelegatedVesting, andEndTimeare all set during the initialization.
BaseAccountis set tobaseAccount.OriginalVestingis set tooriginalVesting.DelegatedFreeis initialized withsdk.NewCoins().DelegatedVestingis initialized withsdk.NewCoins().EndTimeis set toendTime.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all fields in `NewBaseVestingAccount` are initialized correctly. # Test: Search for field initializations in `NewBaseVestingAccount`. Expect: Correct initialization for all fields. rg --type go 'BaseVestingAccount{' x/vesting/types/vesting_account.goLength of output: 173
Script:
#!/bin/bash # Description: Verify that all fields in `NewBaseVestingAccount` are initialized correctly. # Test: Search for field initializations in `NewBaseVestingAccount`. Expect: Correct initialization for all fields. rg --type go 'BaseVestingAccount\s*{' x/vesting/types/vesting_account.goLength of output: 182
Script:
#!/bin/bash # Description: Verify that all fields in `NewBaseVestingAccount` are initialized correctly. # Test: Search for field initializations in `NewBaseVestingAccount`. Expect: Correct initialization for all fields. rg --type go 'BaseVestingAccount\s*{' x/vesting/types/vesting_account.goLength of output: 182
Script:
#!/bin/bash # Description: Verify that all fields in `NewBaseVestingAccount` are initialized correctly. # Test: Search for the `NewBaseVestingAccount` function and extract its implementation. ast-grep --lang go --pattern 'func NewBaseVestingAccount($_, $_, $_) ($_ $_) { $$$ }' x/vesting/types/vesting_account.goLength of output: 124
Script:
#!/bin/bash # Description: Verify that all fields in `NewBaseVestingAccount` are initialized correctly. # Test: Search for the `NewBaseVestingAccount` function definition and display its implementation. rg 'func NewBaseVestingAccount' -A 10 x/vesting/types/vesting_account.goLength of output: 489
Line range hint
543-568: Validate the creation and initialization ofCliffVestingAccount.Verification successful
The search results indicate that the
CliffVestingAccountis being created and initialized in theNewCliffVestingAccountandNewCliffVestingAccountRawfunctions. Let's review these functions to ensure they correctly initialize theCliffVestingAccount.
NewCliffVestingAccountRaw:
func NewCliffVestingAccountRaw(bva *BaseVestingAccount, startTime, cliffTime int64) *CliffVestingAccount { return &CliffVestingAccount{ BaseVestingAccount: bva, StartTime: startTime, CliffTime: cliffTime, } }This function initializes a
CliffVestingAccountwith aBaseVestingAccount,startTime, andcliffTime. It appears to correctly set these fields.NewCliffVestingAccount:
func NewCliffVestingAccount(baseAcc *authtypes.BaseAccount, originalVesting sdk.Coins, startTime, cliffTime, endTime int64, ) (*CliffVestingAccount, error) { baseVestingAcc := &BaseVestingAccount{ BaseAccount: baseAcc, OriginalVesting: originalVesting, EndTime: endTime, } cliffVestingAccount := &CliffVestingAccount{ StartTime: startTime, CliffTime: cliffTime, BaseVestingAccount: baseVestingAcc, } return cliffVestingAccount, cliffVestingAccount.Validate() }This function initializes a
CliffVestingAccountwith aBaseAccount,originalVesting,startTime,cliffTime, andendTime. It also validates theCliffVestingAccountbefore returning it.Both functions appear to correctly initialize the
CliffVestingAccountwith the necessary fields. TheNewCliffVestingAccountfunction also includes validation, which adds an extra layer of robustness.Based on this review, the initialization logic for
CliffVestingAccountseems correct and robust.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the creation and initialization of `CliffVestingAccount`. # Test: Search for creation logic in `CliffVestingAccount`. Expect: Correct and robust initialization. rg --type go 'CliffVestingAccount' x/vesting/types/vesting_account.goLength of output: 1176
283-301: Check the calculation ofendTimeinNewPeriodicVestingAccountfor accuracy.Verification successful
The
endTimecalculation in theNewPeriodicVestingAccountfunction appears to be straightforward. It initializesendTimewithstartTimeand then iteratively adds the length of each period in theperiodsslice. This logic seems correct for accumulating the total vesting period.
- The
startTimeis correctly initialized.- Each period's length is correctly added to
endTime.No additional logic or edge cases affecting
endTimewere found in the provided context.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `endTime` is calculated accurately in `NewPeriodicVestingAccount`. # Test: Search for `endTime` calculation. Expect: Accurate calculation. rg --type go 'endTime' x/vesting/types/vesting_account.goLength of output: 655
Script:
#!/bin/bash # Extract the full implementation of the NewPeriodicVestingAccount function to verify the calculation of endTime. ast-grep --lang go --pattern $'func NewPeriodicVestingAccount($_, $_, $_, $_) (*PeriodicVestingAccount, error) { $$$ }' x/vesting/types/vesting_account.goLength of output: 1696
374-394: Ensure that the validation logic inPeriodicVestingAccount.Validateis comprehensive.Verification successful
Let's review the full implementation of the
PeriodicVestingAccount.ValidateandBaseVestingAccount.Validatemethods to ensure comprehensive validation.
The
PeriodicVestingAccount.Validatemethod includes comprehensive validation logic specific toPeriodicVestingAccount, such as checking the start and end times, validating each vesting period's length and amount, and ensuring the cumulative end time and original vesting amounts are correct. Additionally, it calls theBaseVestingAccount.Validatemethod, which further validates the end time, original vesting, and delegated vesting amounts.The validation logic appears to be thorough and covers various potential issues.
- : The validation logic in
PeriodicVestingAccount.Validateis comprehensive and includes checks for start and end times, vesting periods, and original vesting amounts. Additionally, it calls theBaseVestingAccount.Validatemethod for further validation.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for comprehensive validation logic in `PeriodicVestingAccount.Validate`. # Test: Search for validation logic. Expect: Comprehensive coverage. rg --type go 'Validate' x/vesting/types/vesting_account.goLength of output: 1319
Script:
#!/bin/bash # Retrieve the full implementation of the `PeriodicVestingAccount.Validate` method. ast-grep --lang go --pattern 'func (pva PeriodicVestingAccount) Validate() error { $$$ }' # Retrieve the full implementation of the `BaseVestingAccount.Validate` method. ast-grep --lang go --pattern 'func (bva BaseVestingAccount) Validate() error { $$$ }'Length of output: 3615
152-164: Validate error handling inBaseVestingAccount.Validate.x/vesting/types/vesting_account_test.go (4)
13-13: New import added forcosmossdk.io/core/header.This import is necessary for the updated context handling in the tests.
47-49: Updated context handling in test setup.The context now includes header information, which is crucial for tests that may depend on block headers.
76-77: Extensive use oftypes.New*VestingAccountacross various test functions.These changes are consistent with the PR's objective to enhance the vesting module's functionality and robustness. The error handling with
require.NoErrorensures that the tests will fail fast if the account creation does not proceed as expected.Also applies to: 101-102, 122-123, 146-147, 153-154, 160-161, 171-172, 187-188, 195-196, 203-204, 212-213, 235-236, 252-253, 270-271, 299-300, 306-307, 314-315, 321-322, 337-338, 345-346, 353-354, 362-363, 389-390, 424-473, 485-486, 523-524, 553-554, 560-561, 567-568, 575-576, 583-584, 594-595, 615-616, 623-624, 631-632, 639-640, 648-649, 671-672, 688-689, 706-707, 730-731, 737-738, 744-745, 760-761, 768-769, 776-777, 785-786, 802-803, 832-833, 858-859, 888-889, 933-934
988-989: Enhanced validation tests for various account types.The tests now cover a broader range of scenarios, including edge cases for vesting times and amounts. This is crucial for ensuring the robustness of the vesting logic.
Also applies to: 1012-1015, 1020-1023, 1028-1031, 1050-1053
📝 Purpose
This PR introduces several updates to the vesting module to align with the cosmos-sdk v0.50.4, while preserving the custom features implemented in our project, such as cliff vesting.
🕶 Changes
The main changes include also :
--vesting-cliff-timeargsSummary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests
Refactor