From 13a1a721bc0fa18128711e254f39820c35cf2f05 Mon Sep 17 00:00:00 2001 From: Eric Mokaya Date: Fri, 13 Sep 2024 08:43:41 +0300 Subject: [PATCH 1/4] feat(genutil/client): add better error messages for genesis validation --- x/genutil/client/cli/validate_genesis.go | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/x/genutil/client/cli/validate_genesis.go b/x/genutil/client/cli/validate_genesis.go index bc22ebda51ad..dfde0d450101 100644 --- a/x/genutil/client/cli/validate_genesis.go +++ b/x/genutil/client/cli/validate_genesis.go @@ -2,7 +2,10 @@ package cli import ( "encoding/json" + "errors" "fmt" + "io" + "strings" "github.com/spf13/cobra" @@ -32,7 +35,7 @@ func ValidateGenesisCmd(genMM genesisMM) *cobra.Command { appGenesis, err := types.AppGenesisFromFile(genesis) if err != nil { - return err + return enrichUnmarshalError(err) } if err := appGenesis.ValidateAndComplete(); err != nil { @@ -41,12 +44,19 @@ func ValidateGenesisCmd(genMM genesisMM) *cobra.Command { var genState map[string]json.RawMessage if err = json.Unmarshal(appGenesis.AppState, &genState); err != nil { + if strings.Contains(err.Error(), "unexpected end of JSON input") { + return fmt.Errorf("app_state is missing in the genesis file: %s", err.Error()) + } return fmt.Errorf("error unmarshalling genesis doc %s: %w", genesis, err) } if genMM != nil { if err = genMM.ValidateGenesis(genState); err != nil { - return fmt.Errorf("error validating genesis file %s: %w", genesis, err) + errStr := fmt.Sprintf("error validating genesis file %s: %s", genesis, err.Error()) + if errors.Is(err, io.EOF) { + errStr = fmt.Sprintf("%s: section is missing in the app_state", errStr) + } + return fmt.Errorf("%s", errStr) } } @@ -55,3 +65,11 @@ func ValidateGenesisCmd(genMM genesisMM) *cobra.Command { }, } } + +func enrichUnmarshalError(err error) error { + var syntaxErr *json.SyntaxError + if errors.As(err, &syntaxErr) { + return fmt.Errorf("error at offset %d: %s\n", syntaxErr.Offset, syntaxErr.Error()) + } + return err +} From 816f7d1a59dde0090bbfaaa05d29480507a9bdb4 Mon Sep 17 00:00:00 2001 From: Eric Mokaya Date: Fri, 13 Sep 2024 09:41:37 +0300 Subject: [PATCH 2/4] chore(genutil/client): changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 54e74e876f31..69e8fde3d293 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,8 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i ### Improvements +* (genutil) [#21701](https://github.com/cosmos/cosmos-sdk/pull/21701) Improved error messages for genesis validation. + ### Bug Fixes * (baseapp) [#21256](https://github.com/cosmos/cosmos-sdk/pull/21256) Halt height will not commit the block indicated, meaning that if halt-height is set to 10, only blocks until 9 (included) will be committed. This is to go back to the original behavior before a change was introduced in v0.50.0. From 25706deee532506fd2e0183cf192523091e9a063 Mon Sep 17 00:00:00 2001 From: Eric Mokaya Date: Fri, 13 Sep 2024 09:55:07 +0300 Subject: [PATCH 3/4] fix(genutil/client): lint --- x/genutil/client/cli/validate_genesis.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/genutil/client/cli/validate_genesis.go b/x/genutil/client/cli/validate_genesis.go index dfde0d450101..49c16054ccec 100644 --- a/x/genutil/client/cli/validate_genesis.go +++ b/x/genutil/client/cli/validate_genesis.go @@ -69,7 +69,7 @@ func ValidateGenesisCmd(genMM genesisMM) *cobra.Command { func enrichUnmarshalError(err error) error { var syntaxErr *json.SyntaxError if errors.As(err, &syntaxErr) { - return fmt.Errorf("error at offset %d: %s\n", syntaxErr.Offset, syntaxErr.Error()) + return fmt.Errorf("error at offset %d: %s", syntaxErr.Offset, syntaxErr.Error()) } return err } From 320ae50a020a7cc38fe9dffdc89ae6d2b41e1416 Mon Sep 17 00:00:00 2001 From: Eric Mokaya Date: Fri, 13 Sep 2024 10:24:58 +0300 Subject: [PATCH 4/4] test(genutil/client): update tests --- x/genutil/client/cli/validate_genesis_test.go | 46 +++++++++++++++---- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/x/genutil/client/cli/validate_genesis_test.go b/x/genutil/client/cli/validate_genesis_test.go index bd31a4b8eabb..2608f448d54b 100644 --- a/x/genutil/client/cli/validate_genesis_test.go +++ b/x/genutil/client/cli/validate_genesis_test.go @@ -6,9 +6,16 @@ import ( "github.com/stretchr/testify/require" + appmodulev2 "cosmossdk.io/core/appmodule/v2" + "cosmossdk.io/x/staking" + "github.com/cosmos/cosmos-sdk/client" + codectestutil "github.com/cosmos/cosmos-sdk/codec/testutil" "github.com/cosmos/cosmos-sdk/testutil" clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli" + "github.com/cosmos/cosmos-sdk/types/module" + testutilmod "github.com/cosmos/cosmos-sdk/types/module/testutil" + "github.com/cosmos/cosmos-sdk/x/genutil" "github.com/cosmos/cosmos-sdk/x/genutil/client/cli" ) @@ -32,15 +39,37 @@ var v037Exported = `{ }` func TestValidateGenesis(t *testing.T) { + cdc := testutilmod.MakeTestEncodingConfig(codectestutil.CodecOptions{}, genutil.AppModule{}).Codec testCases := []struct { - name string - genesis string - expErr bool + name string + genesis string + expErrStr string + genMM *module.Manager }{ + { + "invalid json", + `{"app_state": {x,}}`, + "error at offset 16: invalid character", + module.NewManagerFromMap(nil), + }, + { + "invalid: missing module config in app_state", + func() string { + bz, err := os.ReadFile("../../types/testdata/app_genesis.json") + require.NoError(t, err) + + return string(bz) + }(), + "section is missing in the app_state", + module.NewManagerFromMap(map[string]appmodulev2.AppModule{ + "custommod": staking.NewAppModule(cdc, nil, nil, nil), + }), + }, { "exported 0.37 genesis file", v037Exported, - true, + "make sure that you have correctly migrated all CometBFT consensus params", + module.NewManagerFromMap(nil), }, { "valid 0.50 genesis file", @@ -50,7 +79,8 @@ func TestValidateGenesis(t *testing.T) { return string(bz) }(), - false, + "", + module.NewManagerFromMap(nil), }, } @@ -59,9 +89,9 @@ func TestValidateGenesis(t *testing.T) { t.Run(tc.name, func(t *testing.T) { genesisFile := testutil.WriteToNewTempFile(t, tc.genesis) - _, err := clitestutil.ExecTestCLICmd(client.Context{}, cli.ValidateGenesisCmd(nil), []string{genesisFile.Name()}) - if tc.expErr { - require.Contains(t, err.Error(), "make sure that you have correctly migrated all CometBFT consensus params") + _, err := clitestutil.ExecTestCLICmd(client.Context{}, cli.ValidateGenesisCmd(tc.genMM), []string{genesisFile.Name()}) + if tc.expErrStr != "" { + require.Contains(t, err.Error(), tc.expErrStr) } else { require.NoError(t, err) }