Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(x/staking): make metadata nullable #22556

Merged
merged 9 commits into from
Nov 26, 2024
Merged

fix(x/staking): make metadata nullable #22556

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5cc08df
fix(x/staking): make metadata nullable
julienrbrt Nov 19, 2024
a4a5a6b
cl
julienrbrt Nov 19, 2024
f4d6a37
fix
julienrbrt Nov 20, 2024
cc6c67b
Merge branch 'main' into julien/nullable
julienrbrt Nov 20, 2024
accb7ec
build
julienrbrt Nov 26, 2024
09e8221
fix isEmpty
julienrbrt Nov 26, 2024
826779d
updates
julienrbrt Nov 26, 2024
3313e2b
Merge branch 'main' into julien/nullable
julienrbrt Nov 26, 2024
6edb300
updates
julienrbrt Nov 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/cosmos/staking/v1beta1/staking.pulsar.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion testutil/network/network.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ func New(l Logger, baseDir string, cfg Config) (NetworkI, error) {
sdk.ValAddress(addr).String(),
valPubKeys[i],
sdk.NewCoin(cfg.BondDenom, cfg.BondedTokens),
stakingtypes.NewDescription(nodeDirName, "", "", "", "", stakingtypes.Metadata{}),
stakingtypes.NewDescription(nodeDirName, "", "", "", "", &stakingtypes.Metadata{}),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Several test files need to be updated to use pointer metadata

The NewDescription function in x/staking/types/validator.go expects a pointer *Metadata, but some test files are still passing value types. The following locations need to be updated to use &stakingtypes.Metadata{}:

  • tests/integration/slashing/slashing_test.go: stakingtypes.NewDescription("foo_moniker", "", "", "", "", stakingtypes.Metadata{})
  • tests/integration/staking/keeper/msg_server_test.go: types.NewDescription("Validator", "", "", "", "", types.Metadata{})
  • tests/integration/staking/keeper/genesis_test.go: Multiple occurrences of types.NewDescription(..., types.Metadata{})
  • tests/integration/gov/common_test.go: stakingtypes.NewDescription("T", "E", "S", "T", "Z", stakingtypes.Metadata{})
  • simapp/simd/cmd/testnet.go: stakingtypes.NewDescription(nodeDirName, "", "", "", "", stakingtypes.Metadata{})
🔗 Analysis chain

LGTM! Change aligns with making metadata nullable.

The modification to use a pointer for metadata aligns with the PR objective of making the field nullable, which helps avoid the need for validator migrations.

Let's verify that this change doesn't break existing tests:

The search results show that there are several test files and simulation code using NewDescription with metadata. I notice that some files are already using a pointer (&stakingtypes.Metadata{}), while others are using value types (stakingtypes.Metadata{}). Let's verify if the NewDescription function signature has been updated to expect a pointer consistently.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for existing test cases that might be affected by the metadata change
rg -l "NewDescription.*Metadata" --type go | while read -r file; do
    echo "=== $file ==="
    rg -A 5 "NewDescription.*Metadata" "$file"
done

Length of output: 6401

Script:

#!/bin/bash
# Check the NewDescription function signature in the staking types
ast-grep --pattern 'func NewDescription($$$) Description'

Length of output: 708

stakingtypes.NewCommissionRates(commission, sdkmath.LegacyOneDec(), sdkmath.LegacyOneDec()),
sdkmath.OneInt(),
)
Expand Down
2 changes: 1 addition & 1 deletion x/genutil/gentx_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ var (
pk2 = priv2.PubKey()
addr1 = sdk.AccAddress(pk1.Address())
addr2 = sdk.AccAddress(pk2.Address())
desc = stakingtypes.NewDescription("testname", "", "", "", "", stakingtypes.Metadata{})
desc = stakingtypes.NewDescription("testname", "", "", "", "", &stakingtypes.Metadata{})
comm = stakingtypes.CommissionRates{}
)

Expand Down
4 changes: 2 additions & 2 deletions x/genutil/types/genesis_state_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestNetGenesisState(t *testing.T) {
}

func TestValidateGenesisMultipleMessages(t *testing.T) {
desc := stakingtypes.NewDescription("testname", "", "", "", "", stakingtypes.Metadata{})
desc := stakingtypes.NewDescription("testname", "", "", "", "", &stakingtypes.Metadata{})
comm := stakingtypes.CommissionRates{}
valAc := codectestutil.CodecOptions{}.GetValidatorCodec()

Expand All @@ -66,7 +66,7 @@ func TestValidateGenesisMultipleMessages(t *testing.T) {
}

func TestValidateGenesisBadMessage(t *testing.T) {
desc := stakingtypes.NewDescription("testname", "", "", "", "", stakingtypes.Metadata{})
desc := stakingtypes.NewDescription("testname", "", "", "", "", &stakingtypes.Metadata{})
pk1Addr, err := codectestutil.CodecOptions{}.GetValidatorCodec().BytesToString(pk1.Address())
require.NoError(t, err)
msg1 := stakingtypes.NewMsgEditValidator(pk1Addr, desc, nil, nil)
Expand Down
2 changes: 1 addition & 1 deletion x/staking/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

* [#19537](https://github.com/cosmos/cosmos-sdk/pull/19537) Changing `MinCommissionRate` in `MsgUpdateParams` now updates the minimum commission rate for all validators.
* [#20434](https://github.com/cosmos/cosmos-sdk/pull/20434) Add consensus address to validator query response
* [#21315](https://github.com/cosmos/cosmos-sdk/pull/21315) Create metadata type and add metadata field in validator details proto
* [#21315](https://github.com/cosmos/cosmos-sdk/pull/21315), [#22556](https://github.com/cosmos/cosmos-sdk/pull/22556) Create metadata type and add metadata field in validator details proto
* Add parsing of `metadata-profile-pic-uri` in `create-validator` JSON.
* Add cli flag: `metadata-profile-pic-uri` to `edit-validator` cmd.

Expand Down
4 changes: 2 additions & 2 deletions x/staking/client/cli/tx.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func NewEditValidatorCmd() *cobra.Command {
profilePicUri, _ := cmd.Flags().GetString(FlagMetadataProfilePicUri)
socialHandlesUris, _ := cmd.Flags().GetStringArray(FlagMetadataSocialHandleUris)

metadata := types.Metadata{
metadata := &types.Metadata{
ProfilePicUri: profilePicUri,
SocialHandleUris: socialHandlesUris,
}
Expand Down Expand Up @@ -401,7 +401,7 @@ func BuildCreateValidatorMsg(clientCtx client.Context, config TxCreateValidatorC
config.Website,
config.SecurityContact,
config.Details,
config.Metadata,
&config.Metadata,
)

// get the initial validator commission parameters
Expand Down
16 changes: 10 additions & 6 deletions x/staking/client/cli/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ type validator struct {
Website string
Security string
Details string
Metadata types.Metadata
Metadata *types.Metadata
CommissionRates types.CommissionRates
MinSelfDelegation math.Int
}
Expand Down Expand Up @@ -134,11 +134,15 @@ func buildCommissionRates(rateStr, maxRateStr, maxChangeRateStr string) (commiss
return commission, nil
}

func buildMetadata(profilePicUri string, socialHandlesUris []string) (metadata types.Metadata, err error) {
metadata.ProfilePicUri = profilePicUri
metadata.SocialHandleUris = socialHandlesUris
func buildMetadata(profilePicUri string, socialHandlesUris []string) (*types.Metadata, error) {
metadata := types.Metadata{
ProfilePicUri: profilePicUri,
SocialHandleUris: socialHandlesUris,
}

if err := metadata.Validate(); err != nil {
return metadata, err
return nil, err
}
return metadata, nil

return &metadata, nil
}
2 changes: 1 addition & 1 deletion x/staking/proto/cosmos/staking/v1beta1/staking.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ message Description {
// details define other optional details.
string details = 5;
// metadata defines extra information about the validator.
Metadata metadata = 6 [(gogoproto.nullable) = false, (amino.dont_omitempty) = true];
Metadata metadata = 6 [(gogoproto.nullable) = true, (amino.dont_omitempty) = false];
}

// Metadata defines extra information about the validator.
Expand Down
4 changes: 2 additions & 2 deletions x/staking/simulation/msg_factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func MsgCreateValidatorFactory(k *keeper.Keeper) simsx.SimMsgFactoryFn[*types.Ms
r.StringN(10),
r.StringN(10),
r.StringN(10),
types.Metadata{
&types.Metadata{
ProfilePicUri: RandURIOfHostLength(r.Rand, 10),
SocialHandleUris: RandSocialHandleURIs(r.Rand, 2, 10),
},
Expand Down Expand Up @@ -143,7 +143,7 @@ func MsgEditValidatorFactory(k *keeper.Keeper) simsx.SimMsgFactoryFn[*types.MsgE
}
valOpAddrBz := must(k.ValidatorAddressCodec().StringToBytes(val.GetOperator()))
valOper := testData.GetAccountbyAccAddr(reporter, valOpAddrBz)
d := types.NewDescription(r.StringN(10), r.StringN(10), r.StringN(10), r.StringN(10), r.StringN(10), types.Metadata{
d := types.NewDescription(r.StringN(10), r.StringN(10), r.StringN(10), r.StringN(10), r.StringN(10), &types.Metadata{
ProfilePicUri: RandURIOfHostLength(r.Rand, 10),
SocialHandleUris: RandSocialHandleURIs(r.Rand, 2, 10),
})
Expand Down
Loading
Loading