fix(x/staking): fix validator metadata update #23461
Conversation
📝 WalkthroughWalkthroughThe pull request addresses a bug in the Changes
Assessment against linked issues
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)
⏰ Context from checks skipped due to timeout of 90000ms (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
|
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: 0
🧹 Nitpick comments (1)
x/staking/types/validator_test.go (1)
35-38: LGTM! Consider adding more test cases.The test cases effectively cover the basic metadata update scenarios. However, consider adding tests for:
- Invalid profile picture URI
- Invalid social handle URIs
- Nil metadata updates
Also applies to: 46-49, 57-57
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
x/staking/types/validator.go(1 hunks)x/staking/types/validator_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
x/staking/types/validator.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
x/staking/types/validator_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"
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: tests (00)
- GitHub Check: test-simapp-v2
- GitHub Check: test-system-v2
- GitHub Check: build (amd64)
- GitHub Check: Analyze
- GitHub Check: Summary
🔇 Additional comments (1)
x/staking/types/validator.go (1)
242-242: LGTM! The metadata update fix is correct.The change properly handles metadata updates by passing d2.Metadata instead of d.Metadata, consistent with how other Description fields are updated.
Let's verify that this is the only instance where metadata needs to be updated:
✅ Verification successful
✓ Metadata update change is correct and complete
The verification confirms this is the only instance of NewDescription usage, and the metadata update follows the established pattern with proper validation flow. The change from
d.Metadatatod2.Metadatais consistent with the codebase's metadata handling logic, including the DoNotModify check implemented earlier in the function.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other potential metadata update locations ast-grep --pattern 'NewDescription($$$, metadata)'Length of output: 52
Script:
#!/bin/bash # Search for all NewDescription calls ast-grep --pattern 'NewDescription($$$)' # Also search for metadata references in the context rg "Metadata" -A 2 -B 2 x/staking/types/validator.goLength of output: 1701
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
x/staking/CHANGELOG.md (1)
59-59: Enhance the changelog message with more details.While the format is correct, consider making the changelog message more descriptive by explaining the previous behavior and the impact. For example:
-* [#23461](https://github.com/cosmos/cosmos-sdk/pull/23461) Fix `UpdateDescription` to correctly update the `Metadata` field. +* [#23461](https://github.com/cosmos/cosmos-sdk/pull/23461) Fix `UpdateDescription` to correctly update the `Metadata` field by passing the new description's metadata instead of the existing metadata, ensuring validator metadata updates are properly applied.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
x/staking/CHANGELOG.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
x/staking/CHANGELOG.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: test-system-v2
- GitHub Check: Analyze
- GitHub Check: Summary
| @@ -56,6 +56,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ | |||
|
|
|||
| * [#20688](https://github.com/cosmos/cosmos-sdk/pull/20688) Avoid overslashing unbonding delegations after a redelegation. | |||
| * [#19226](https://github.com/cosmos/cosmos-sdk/pull/19226) Ensure `GetLastValidators` in `x/staking` does not return an error when `MaxValidators` exceeds total number of bonded validators. | |||
| * [#23461](https://github.com/cosmos/cosmos-sdk/pull/23461) Fix `UpdateDescription` to correctly update the `Metadata` field. | |||
There was a problem hiding this comment.
no changelog needed, this feature is on main only.
Description
Closes: #23413
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
metadata-profile-pic-uriin thecreate-validatorJSON and a new CLI flag foredit-validator.Bug Fixes
UpdateDescriptionfunction to ensure it correctly updates theMetadatafield.Tests
These changes improve the flexibility of validator descriptions by allowing more comprehensive profile information while maintaining existing core functionality.