feat(x/staking)!: Add metadata field to validator info #21315
Conversation
new IsEmpty() method for description
WalkthroughWalkthroughThe changes introduced in the codebase enhance the staking module by adding a Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Recent review detailsConfiguration used: .coderabbit.yml Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
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
|
github-actions
bot
added
C:x/staking
C:CLI
C:Simulations
C:x/genutil
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -79,6 +78,9 @@ message Description { | |||
| string security_contact = 4; | |||
| // details define other optional details. | |||
| string details = 5; | |||
| // metadata is a map of string keys to any values that can be used to store additional information about the | |||
| // validator. | |||
| map<string, google.protobuf.Any> metadata = 6; | |||
There was a problem hiding this comment.
Wallet and explorer complained that proposal metadata were too loose and not typed (even though we had recommended json structure).
Shall we instead be opinionated about which metadata can be set? Using an any will lead to the same complaints I think.
ziscky
force-pushed
the
ziscky/9988-staking-validator-metadata
branch
from
937a95e
to
14eaff9
Compare
This comment has been minimized.
This comment has been minimized.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (2)
x/staking/client/cli/tx.go (1)
76-76: Include metadata in the example JSON.The example JSON now includes a
"metadata"field for profile pictures. Ensure the documentation reflects this change.testutil/network/network.go (1)
492-492: Ensure all usages ofstakingtypes.NewDescriptioninclude the metadata parameter.The following files contain usages of
stakingtypes.NewDescriptionthat do not include the newstakingtypes.Metadata{}parameter:
tests/integration/slashing/slashing_test.gotests/integration/staking/keeper/deterministic_test.gotests/integration/gov/common_test.gosimapp/v2/simdv2/cmd/testnet.goPlease update these instances to include the metadata parameter to align with the PR objectives.
Analysis chain
LGTM! Verify the usage of
stakingtypes.NewDescription.The addition of
stakingtypes.Metadata{}to thestakingtypes.NewDescriptionfunction call is consistent with the PR objectives of adding metadata for validator info.Ensure that all usages of
stakingtypes.NewDescriptionacross the codebase are updated to include the new metadata parameter.Run the following script to verify the usage:
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all usages of `stakingtypes.NewDescription` include the new metadata parameter. # Test: Search for the function usage. Expect: Only occurrences with the new metadata parameter. rg --type go -A 5 $'stakingtypes.NewDescription'Length of output: 5767
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (2)
x/staking/proto/buf.lockis excluded by!**/*.lockx/staking/types/staking.pb.gois excluded by!**/*.pb.go
Files selected for processing (11)
- simapp/simd/cmd/testnet.go (1 hunks)
- testutil/network/network.go (1 hunks)
- x/genutil/gentx_test.go (1 hunks)
- x/genutil/types/genesis_state_test.go (2 hunks)
- x/staking/client/cli/flags.go (1 hunks)
- x/staking/client/cli/tx.go (8 hunks)
- x/staking/client/cli/utils.go (3 hunks)
- x/staking/keeper/msg_server.go (1 hunks)
- x/staking/proto/cosmos/staking/v1beta1/staking.proto (4 hunks)
- x/staking/simulation/operations.go (2 hunks)
- x/staking/types/validator.go (4 hunks)
Additional context used
Path-based instructions (10)
x/staking/client/cli/flags.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/staking/client/cli/utils.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/genutil/types/genesis_state_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/genutil/gentx_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/staking/client/cli/tx.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/staking/types/validator.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.simapp/simd/cmd/testnet.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/staking/keeper/msg_server.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.testutil/network/network.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/staking/simulation/operations.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (23)
x/staking/client/cli/flags.go (1)
24-24: Addition ofFlagMetadatais appropriate.The new constant
FlagMetadatais added correctly and adheres to the Uber Golang style guide. It enhances the CLI functionality by allowing metadata specification. Ensure that this flag is utilized in relevant CLI commands.x/staking/client/cli/utils.go (3)
27-27: Addition ofMetadatafield invalidatorstruct is correct.The inclusion of the
Metadatafield in thevalidatorstruct enhances its capability to store additional information. This aligns with the PR objective of managing validator profile images.
41-41: Ensure correct handling ofMetadatain JSON parsing.The addition of the
Metadatafield in theinternalValstruct and its JSON tag is correctly implemented. This ensures that metadata is properly parsed and validated.
100-100: Return ofMetadatainparseAndValidateValidatorJSONis appropriate.The function correctly returns the
Metadatafield as part of thevalidatorstruct. This ensures that metadata is included in the validator information.x/genutil/types/genesis_state_test.go (2)
42-42: Inclusion ofMetadatainTestValidateGenesisMultipleMessagesis correct.The test function now includes the
Metadataparameter, ensuring comprehensive validation of the new field. This aligns with the PR objectives.
69-69: Inclusion ofMetadatainTestValidateGenesisBadMessageis correct.The test function now includes the
Metadataparameter, ensuring comprehensive validation of the new field. This aligns with the PR objectives.x/genutil/gentx_test.go (1)
39-39: Ensure test coverage for the new metadata field.The
Descriptioninstantiation now includes astakingtypes.Metadata{}parameter. Verify that test cases validate the handling of metadata.x/staking/client/cli/tx.go (6)
4-4: Import statement forjsonis necessary.The
jsonpackage is used for unmarshalling metadata strings.
134-141: Validate metadata parsing logic.The metadata string is parsed from JSON and assigned to the
metadatavariable. Ensure proper error handling and validation.
197-197: Ensure metadata is correctly included inNewDescription.The
NewDescriptionfunction now includes themetadataparameter. Verify that this change is correctly handled in the function logic.
278-278: AddMetadatafield toTxCreateValidatorConfig.The
Metadatafield is added to theTxCreateValidatorConfigstruct. Ensure this field is correctly populated and used in transactions.
318-327: Validate metadata parsing inPrepareConfigForTxCreateValidator.The metadata string is parsed and assigned to the
Metadatafield. Ensure proper error handling and validation.
407-407: Include metadata inBuildCreateValidatorMsg.The
BuildCreateValidatorMsgfunction now includes theMetadatafield in the description. Verify that this change is correctly handled in the function logic.x/staking/types/validator.go (4)
6-6: Import statement forurlis necessary.The
urlpackage is used for validating theProfilePicUriin metadata.
192-199: Include metadata inNewDescription.The
NewDescriptionfunction now includes ametadataparameter. Ensure this change is correctly integrated into the function logic.
261-268: Validate metadata inDescription.Validate.The
Validatemethod forDescriptionnow callsmetadata.Validate(). Ensure this validation logic is robust and correctly implemented.
270-279: ValidateProfilePicUriinMetadata.Validate.The
Validatemethod forMetadatachecks theProfilePicUriformat. Ensure this validation logic is robust and correctly implemented.x/staking/proto/cosmos/staking/v1beta1/staking.proto (2)
81-83: Addition ofmetadatafield inDescriptionmessage is appropriate.The inclusion of the
metadatafield enhances theDescriptionmessage by allowing additional validator information, such as profile pictures. The non-nullable constraint ensures that this information is always present.
85-90: Introduction ofMetadatamessage is well-structured.The
Metadatamessage provides a clear and structured way to include additional information, such as a profile picture URI, enhancing the validator's description.simapp/simd/cmd/testnet.go (1)
351-351: Inclusion ofMetadataininitTestnetFilesenhances validator initialization.The addition of the
Metadataparameter to thestakingtypes.NewDescriptionfunction call allows for richer validator descriptions, aligning with the PR's objectives.x/staking/keeper/msg_server.go (1)
109-109: Switch toValidate()inCreateValidatorenhances validation.Replacing
EnsureLength()withValidate()likely provides a more comprehensive validation mechanism, improving the robustness of input validation for validator creation.x/staking/simulation/operations.go (2)
181-183: LGTM! Metadata parameter added toSimulateMsgCreateValidator.The addition of the
types.Metadataparameter with aProfilePicUrifield enhances the simulation function by allowing it to handle metadata related to a validator's profile picture.
283-285: LGTM! Metadata parameter added toSimulateMsgEditValidator.The addition of the
types.Metadataparameter with aProfilePicUrifield enhances the simulation function by allowing it to handle metadata related to a validator's profile picture.
x/staking/types/validator.go
Outdated
| d.Metadata, // TODO: how should we check for the DoNotModifyDesc | ||
| ).Validate() |
There was a problem hiding this comment.
Consider handling DoNotModifyDesc for metadata.
The UpdateDescription function currently does not handle DoNotModifyDesc for metadata. Consider implementing this logic.
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (8)
- CHANGELOG.md (1 hunks)
- simapp/simd/cmd/testnet.go (1 hunks)
- simapp/v2/simdv2/cmd/testnet.go (1 hunks)
- tests/integration/gov/common_test.go (1 hunks)
- tests/integration/slashing/slashing_test.go (1 hunks)
- tests/integration/staking/keeper/deterministic_test.go (13 hunks)
- tests/integration/tx/aminojson/aminojson_test.go (1 hunks)
- testutil/network/network.go (1 hunks)
Files skipped from review as they are similar to previous changes (5)
- CHANGELOG.md
- tests/integration/gov/common_test.go
- tests/integration/slashing/slashing_test.go
- tests/integration/tx/aminojson/aminojson_test.go
- testutil/network/network.go
Additional context used
Path-based instructions (3)
simapp/v2/simdv2/cmd/testnet.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.simapp/simd/cmd/testnet.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.tests/integration/staking/keeper/deterministic_test.go (3)
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"
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
Additional comments not posted (4)
simapp/v2/simdv2/cmd/testnet.go (1)
298-298: Review of Metadata Addition in Validator DescriptionThe addition of the
metadatafield to thestakingtypes.NewDescriptionfunction call is a crucial enhancement. This change aligns with the PR's objectives to allow validators to directly link their profile pictures, improving the management of validator profile images and reducing reliance on external services like Keybase.However, it's important to ensure that the
metadatafield is being utilized correctly across the application, especially in areas where validator descriptions are displayed or processed.Verification successful
Metadata Field Integration Verified
The
metadatafield in thestakingtypes.NewDescriptionfunction is consistently used across the codebase, including in test files. This indicates that the change is well-integrated and potentially covered by tests. Ensure that these tests verify the correct behavior of themetadatafield to maintain robustness.
- Files with Usage:
simapp/v2/simdv2/cmd/testnet.gosimapp/simd/cmd/testnet.gotests/integration/staking/keeper/deterministic_test.gotests/integration/slashing/slashing_test.gotests/integration/gov/common_test.gox/genutil/gentx_test.gox/genutil/types/genesis_state_test.gotestutil/network/network.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new metadata field across the application. # Test: Search for the usage of `stakingtypes.NewDescription` with the new metadata field. ast-grep --lang go --pattern $'stakingtypes.NewDescription($_, $_, $_, $_, $_, stakingtypes.Metadata{})'Length of output: 2385
tests/integration/staking/keeper/deterministic_test.go (3)
212-230: Review of themetadataGeneratorandgenerateUrifunctionsThe
metadataGeneratorfunction is well-implemented, using therapidlibrary to generate random metadata for testing, which includes a profile picture URI and social handle URIs. ThegenerateUrifunction, used withinmetadataGenerator, correctly constructs URIs using random strings for the host and path, ensuring they are formatted as HTTPS URLs.
- Correctness: The implementation correctly adheres to the PR's objective of adding a metadata field.
- Performance: The use of
rapidfor generating test data is appropriate and efficient for test environments.- Best Practices: The code is clean, well-structured, and follows Go best practices.
- Readability: The functions are concise and easy to understand.
- Test Coverage: These functions are crucial for generating test data that includes the new metadata field, ensuring that the staking module's handling of this data is robustly tested.
Overall, the implementation of these functions meets the requirements and contributes effectively to the test suite.
251-251: Ensure Metadata Integration in Validator CreationThe integration of the
metadataGeneratorwithin thecreateValidatorfunction is crucial for testing the new metadata functionality. By drawing metadata directly within the validator's description setup, this ensures that every test validator created has a metadata field populated, which aligns with the PR's objectives.
- Correctness: Correctly integrates the new metadata field in test validators.
- Best Practices: Follows good practice by ensuring that new functionality is included in integration tests.
- Test Coverage: Adequately tests the new functionality by incorporating it into the standard validator creation process used across various tests.
This change is essential for ensuring that the new metadata field behaves as expected under different test scenarios.
327-327: Static Validators Metadata InitializationThe updates to
getStaticValidatorandgetStaticValidator2functions to include an emptystakingtypes.Metadata{}instance are minimal but necessary. This ensures that even static validators used in tests have the metadata field initialized, preventing any nil pointer exceptions and maintaining consistency with dynamic validators.
- Correctness: Properly initializes the metadata field to an empty struct, which is safe and prevents potential runtime errors.
- Consistency: Ensures that all validators, whether dynamically or statically generated, have their metadata fields initialized.
- Best Practices: The approach of initializing complex structs to their zero values is a standard practice in Go, particularly in testing scenarios.
These changes are small but improve the robustness and maintainability of the test suite by ensuring consistency across different types of validators.
Also applies to: 363-363
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (8)
- CHANGELOG.md (1 hunks)
- simapp/simd/cmd/testnet.go (1 hunks)
- simapp/v2/simdv2/cmd/testnet.go (1 hunks)
- tests/integration/gov/common_test.go (1 hunks)
- tests/integration/slashing/slashing_test.go (1 hunks)
- tests/integration/staking/keeper/deterministic_test.go (13 hunks)
- tests/integration/tx/aminojson/aminojson_test.go (1 hunks)
- testutil/network/network.go (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- CHANGELOG.md
- tests/integration/tx/aminojson/aminojson_test.go
- testutil/network/network.go
Additional context used
Path-based instructions (5)
tests/integration/gov/common_test.go (3)
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"
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"tests/integration/slashing/slashing_test.go (3)
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"
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"simapp/v2/simdv2/cmd/testnet.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.simapp/simd/cmd/testnet.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.tests/integration/staking/keeper/deterministic_test.go (3)
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"
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
Additional comments not posted (7)
tests/integration/slashing/slashing_test.go (2)
85-85: Test structure adheres to the Uber Golang style guide.The test is well-structured and follows best practices for Go testing, including the use of
requirefor assertions and clear separation of setup, execution, and verification phases. This structure helps in maintaining readability and ensuring that each part of the test is focused on a specific aspect of the functionality.
85-85: Verify the correct instantiation of themetadataparameter.The
metadataparameter is instantiated as an empty struct, which is correct for the purpose of this test. However, to fully test the functionality, consider adding a test case wheremetadatacontains actual data (e.g., a valid URL to a profile picture). This would ensure that the system correctly handles and stores this new type of data.Verification successful
Metadata parameter is tested with actual data elsewhere.
The
stakingtypes.Metadataparameter is instantiated with actual data intests/integration/staking/keeper/deterministic_test.go, which ensures that the handling of non-empty metadata is tested. While the current test intests/integration/slashing/slashing_test.gocorrectly uses an empty metadata struct, consider adding a test case with non-empty metadata for completeness and enhanced coverage.
tests/integration/staking/keeper/deterministic_test.go: Tests withProfilePicUriandSocialHandleUris.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the metadata parameter is correctly instantiated and handled. # Test: Search for tests that instantiate the metadata parameter with actual data. rg --type go -A 5 $'stakingtypes.Metadata{'Length of output: 152
Script:
#!/bin/bash # Description: Verify that the metadata parameter is correctly instantiated and handled. # Test: Search for tests that instantiate the metadata parameter with actual data. rg --type go -A 5 $'stakingtypes.Metadata\\{'Length of output: 5566
simapp/v2/simdv2/cmd/testnet.go (1)
298-298: Review the addition of the metadata field in validator descriptions.The change at line 298 introduces a new
stakingtypes.Metadata{}instance in thestakingtypes.NewDescriptionfunction call. This aligns with the PR's objectives to enhance validator descriptions with additional metadata, such as profile pictures. However, it's important to verify whether this metadata is intended to be populated dynamically elsewhere in the code or if further modifications are needed to handle the metadata effectively.Verification successful
Ensure dynamic population of metadata in validator descriptions.
The
stakingtypes.Metadata{}instance insimapp/v2/simdv2/cmd/testnet.gois currently a placeholder. While the metadata structure is capable of holding data such as profile pictures and social handles, as seen in the test filedeterministic_test.go, it is important to ensure that this metadata is populated dynamically or in the appropriate contexts within the application. Consider reviewing where and how metadata should be populated to fully utilize its capabilities.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if the metadata is populated dynamically elsewhere in the code. # Test: Search for assignments to `stakingtypes.Metadata` fields. Expect: Assignments that populate the metadata. ast-grep --lang go --pattern $'stakingtypes.Metadata { $$$ }'Length of output: 1632
simapp/simd/cmd/testnet.go (1)
356-356: Approve addition of metadata field in validator descriptions.The addition of the
stakingtypes.Metadata{}parameter in theNewDescriptionfunction call aligns with the PR's objectives to enhance validator descriptions with profile pictures. This change is crucial for reducing reliance on external services like Keybase and improving the management of validator profile images.Please ensure that the integration of the new metadata field is thoroughly tested, especially in environments where the validator descriptions are consumed, such as block explorers and wallets.
tests/integration/staking/keeper/deterministic_test.go (3)
212-230: Review of themetadataGeneratorandgenerateUrifunctionsThe
metadataGeneratorfunction is well-implemented, using therapidlibrary to generate random metadata for testing. The function correctly constructs aMetadataobject with a profile picture URI and social handle URIs, which aligns with the PR's objectives to handle validator profile pictures more effectively.The
generateUrifunction also appears correctly implemented. It uses therapidlibrary to generate random strings for the host and path, and constructs a URL using thenet/urlpackage. This ensures that the URIs are valid and well-formed.Overall, these changes are consistent with the PR's goal of enhancing the metadata handling in validator information. The use of property-based testing with
rapidis appropriate for this context, ensuring a robust test suite.
251-251: Integration ofmetadataGeneratorincreateValidatorfunctionThe integration of
metadataGeneratorwithin thecreateValidatorfunction is crucial for testing the new metadata functionality. By drawing aMetadatainstance during validator creation, the test effectively simulates the real-world usage of this new feature.This change is essential for ensuring that the metadata field is populated correctly and behaves as expected under various conditions. It's a good practice to include such integrations in unit tests to catch potential issues early in the development cycle.
327-327: Static Validator Functions with Default MetadataThe updates to
getStaticValidatorandgetStaticValidator2functions to include a defaultstakingtypes.Metadata{}instance are a necessary adjustment to maintain consistency with the new metadata field in the validator struct.These changes ensure that even static validators used in tests have the metadata field populated, albeit with an empty default value. This is important for ensuring that all parts of the test suite acknowledge and support the new metadata structure, preventing any regressions or overlooked bugs related to uninitialized fields.
Also applies to: 363-363
CHANGELOG.md
Outdated
There was a problem hiding this comment.
All those should be moved under staking/CHANGELOG.md
|
You should run |
github-actions
bot
added
C:server/v2
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (2)
x/staking/simulation/rand_util.go (2)
12-24: Add unit tests for the function.The function does not have any unit tests. Unit tests are essential to ensure the correctness and reliability of the function.
Consider adding unit tests to cover various scenarios, including edge cases.
Do you want me to generate the unit testing code or open a GitHub issue to track this task?
27-36: Add unit tests for the function.The function does not have any unit tests. Unit tests are essential to ensure the correctness and reliability of the function.
Consider adding unit tests to cover various scenarios, including edge cases.
Do you want me to generate the unit testing code or open a GitHub issue to track this task?
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (3)
- x/staking/simulation/msg_factory.go (2 hunks)
- x/staking/simulation/rand_util.go (1 hunks)
- x/staking/simulation/rand_util_test.go (1 hunks)
Additional context used
Path-based instructions (3)
x/staking/simulation/msg_factory.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/staking/simulation/rand_util.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/staking/simulation/rand_util_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"
Additional comments not posted (4)
x/staking/simulation/rand_util_test.go (2)
14-39: LGTM!The test function
TestRandURIOfHostLengthis well-structured and follows the Golang testing conventions. It uses a table-driven approach to test theRandURIOfHostLengthfunction with different input sizes, including edge cases. The test correctly parses the generated URI and checks the host length using therequirepackage for assertions.
41-60: LGTM!The test function
TestRandSocialHandleURIsis well-structured and follows the Golang testing conventions. It uses a table-driven approach to test theRandSocialHandleURIsfunction with different numbers of social handles. The test correctly checks the length of the generated social handle URIs using therequirepackage for assertions.x/staking/simulation/msg_factory.go (2)
42-52: LGTM!The changes to the
MsgCreateValidatorFactoryfunction look good. Thetypes.NewDescriptionmethod now includes atypes.Metadataparameter, which is populated with aProfilePicUriandSocialHandleUris. This enhances the validator's description with profile and social media information.
146-149: LGTM!The changes to the
MsgEditValidatorFactoryfunction look good. Thetypes.NewDescriptionmethod now includes atypes.Metadataparameter, which is populated with aProfilePicUriandSocialHandleUris. This enhances the edited validator's description with profile and social media information.
| if n == 0 { | ||
| return "" | ||
| } | ||
| tld := ".com" |
There was a problem hiding this comment.
Consider making the TLD configurable.
Using a fixed TLD of ".com" might not be suitable for all use cases. Consider making the TLD configurable by accepting it as a parameter or using a constant that can be easily modified.
* main: docs: amend docs for 52 changes (#21992) test: migrate e2e/authz to system tests (#21819) refactor(runtime/v2): use StoreBuilder (#21989) feat(schema): add API descriptors, struct, oneof & list types, and wire encoding spec (#21482) docs: add instructions to change DefaultGenesis (#21680) feat(x/staking)!: Add metadata field to validator info (#21315) chore(x/authz)!: Remove account keeper dependency (#21632)
* main: docs: amend docs for 52 changes (#21992) test: migrate e2e/authz to system tests (#21819) refactor(runtime/v2): use StoreBuilder (#21989) feat(schema): add API descriptors, struct, oneof & list types, and wire encoding spec (#21482) docs: add instructions to change DefaultGenesis (#21680) feat(x/staking)!: Add metadata field to validator info (#21315) chore(x/authz)!: Remove account keeper dependency (#21632) chore(contributing): delete link (#21990) test(gov): Migrate e2e to system test (#21927) test: e2e/client to system tests (#21981)
Description
Closes: #9988
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking 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
Enhancements
Bug Fixes