From 4a55e5a9a6fa736d1c2d2664149716503673a58c Mon Sep 17 00:00:00 2001 From: Eric Mokaya <4112301+ziscky@users.noreply.github.com> Date: Fri, 13 Sep 2024 12:40:13 +0300 Subject: [PATCH 1/4] feat(x/genutil): add better error messages for genesis validation (#21701) (cherry picked from commit 9c5cea08c8fb5858cff44b4acec7a63a79ac8354) # Conflicts: # CHANGELOG.md # x/genutil/client/cli/validate_genesis.go --- CHANGELOG.md | 4 ++ x/genutil/client/cli/validate_genesis.go | 30 +++++++++++- x/genutil/client/cli/validate_genesis_test.go | 46 +++++++++++++++---- 3 files changed, 71 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e513e0c1cb7..da94cc6043b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,7 +45,11 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements +<<<<<<< HEAD * (x/bank) [#21460](https://github.com/cosmos/cosmos-sdk/pull/21460) Added `Sender` attribute in `MsgMultiSend` event. +======= +* (genutil) [#21701](https://github.com/cosmos/cosmos-sdk/pull/21701) Improved error messages for genesis validation. +>>>>>>> 9c5cea08c (feat(x/genutil): add better error messages for genesis validation (#21701)) ### Bug Fixes diff --git a/x/genutil/client/cli/validate_genesis.go b/x/genutil/client/cli/validate_genesis.go index f3a57320ac16..cc161591c596 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" @@ -37,7 +40,7 @@ func ValidateGenesisCmd(mbm module.BasicManager) *cobra.Command { appGenesis, err := types.AppGenesisFromFile(genesis) if err != nil { - return err + return enrichUnmarshalError(err) } if err := appGenesis.ValidateAndComplete(); err != nil { @@ -46,11 +49,28 @@ func ValidateGenesisCmd(mbm module.BasicManager) *cobra.Command { var genState map[string]json.RawMessage if err = json.Unmarshal(appGenesis.AppState, &genState); err != nil { +<<<<<<< HEAD return fmt.Errorf("error unmarshalling genesis doc %s: %s", genesis, err.Error()) } if err = mbm.ValidateGenesis(cdc, clientCtx.TxConfig, genState); err != nil { return fmt.Errorf("error validating genesis file %s: %s", genesis, err.Error()) +======= + 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 { + 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) + } +>>>>>>> 9c5cea08c (feat(x/genutil): add better error messages for genesis validation (#21701)) } fmt.Fprintf(cmd.OutOrStdout(), "File at %s is a valid genesis file\n", genesis) @@ -58,3 +78,11 @@ func ValidateGenesisCmd(mbm module.BasicManager) *cobra.Command { }, } } + +func enrichUnmarshalError(err error) error { + var syntaxErr *json.SyntaxError + if errors.As(err, &syntaxErr) { + return fmt.Errorf("error at offset %d: %s", syntaxErr.Offset, syntaxErr.Error()) + } + return err +} 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) } From f07eaa1dffb01b40cfeec1c777d92a131db33aaf Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Fri, 13 Sep 2024 12:11:07 +0200 Subject: [PATCH 2/4] fixes --- CHANGELOG.md | 3 --- x/genutil/client/cli/validate_genesis.go | 23 ++++++------------- x/genutil/client/cli/validate_genesis_test.go | 19 ++++++--------- 3 files changed, 14 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da94cc6043b3..088c7bfa0c3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,11 +45,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements -<<<<<<< HEAD * (x/bank) [#21460](https://github.com/cosmos/cosmos-sdk/pull/21460) Added `Sender` attribute in `MsgMultiSend` event. -======= * (genutil) [#21701](https://github.com/cosmos/cosmos-sdk/pull/21701) Improved error messages for genesis validation. ->>>>>>> 9c5cea08c (feat(x/genutil): add better error messages for genesis validation (#21701)) ### Bug Fixes diff --git a/x/genutil/client/cli/validate_genesis.go b/x/genutil/client/cli/validate_genesis.go index cc161591c596..93fe9ba086e7 100644 --- a/x/genutil/client/cli/validate_genesis.go +++ b/x/genutil/client/cli/validate_genesis.go @@ -49,28 +49,19 @@ func ValidateGenesisCmd(mbm module.BasicManager) *cobra.Command { var genState map[string]json.RawMessage if err = json.Unmarshal(appGenesis.AppState, &genState); err != nil { -<<<<<<< HEAD - return fmt.Errorf("error unmarshalling genesis doc %s: %s", genesis, err.Error()) - } - - if err = mbm.ValidateGenesis(cdc, clientCtx.TxConfig, genState); err != nil { - return fmt.Errorf("error validating genesis file %s: %s", genesis, err.Error()) -======= 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) + + return fmt.Errorf("error unmarshalling genesis doc %s: %s", genesis, err.Error()) } - if genMM != nil { - if err = genMM.ValidateGenesis(genState); err != nil { - 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) + if err = mbm.ValidateGenesis(cdc, clientCtx.TxConfig, genState); err != nil { + 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) } ->>>>>>> 9c5cea08c (feat(x/genutil): add better error messages for genesis validation (#21701)) + return fmt.Errorf("%s", errStr) } fmt.Fprintf(cmd.OutOrStdout(), "File at %s is a valid genesis file\n", genesis) diff --git a/x/genutil/client/cli/validate_genesis_test.go b/x/genutil/client/cli/validate_genesis_test.go index 2608f448d54b..025900975d5e 100644 --- a/x/genutil/client/cli/validate_genesis_test.go +++ b/x/genutil/client/cli/validate_genesis_test.go @@ -6,17 +6,14 @@ 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" + "github.com/cosmos/cosmos-sdk/x/staking" ) // An example exported genesis file from a 0.37 chain. Note that evidence @@ -39,18 +36,18 @@ var v037Exported = `{ }` func TestValidateGenesis(t *testing.T) { - cdc := testutilmod.MakeTestEncodingConfig(codectestutil.CodecOptions{}, genutil.AppModule{}).Codec + cdc := testutilmod.MakeTestEncodingConfig(genutil.AppModule{}).Codec testCases := []struct { name string genesis string expErrStr string - genMM *module.Manager + genMM module.BasicManager }{ { "invalid json", `{"app_state": {x,}}`, "error at offset 16: invalid character", - module.NewManagerFromMap(nil), + module.NewBasicManager(), }, { "invalid: missing module config in app_state", @@ -61,15 +58,13 @@ func TestValidateGenesis(t *testing.T) { return string(bz) }(), "section is missing in the app_state", - module.NewManagerFromMap(map[string]appmodulev2.AppModule{ - "custommod": staking.NewAppModule(cdc, nil, nil, nil), - }), + module.NewBasicManager(staking.NewAppModule(cdc, nil, nil, nil, nil)), }, { "exported 0.37 genesis file", v037Exported, "make sure that you have correctly migrated all CometBFT consensus params", - module.NewManagerFromMap(nil), + module.NewBasicManager(), }, { "valid 0.50 genesis file", @@ -80,7 +75,7 @@ func TestValidateGenesis(t *testing.T) { return string(bz) }(), "", - module.NewManagerFromMap(nil), + module.NewBasicManager(), }, } From e0b60ebe6b191b659e2cfe217a00a066208d8f03 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Fri, 13 Sep 2024 12:19:41 +0200 Subject: [PATCH 3/4] fix test --- x/genutil/client/cli/validate_genesis_test.go | 34 +++++++++++++------ 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/x/genutil/client/cli/validate_genesis_test.go b/x/genutil/client/cli/validate_genesis_test.go index 025900975d5e..daaab7b325ac 100644 --- a/x/genutil/client/cli/validate_genesis_test.go +++ b/x/genutil/client/cli/validate_genesis_test.go @@ -1,19 +1,18 @@ package cli_test import ( + "encoding/json" "os" "testing" "github.com/stretchr/testify/require" "github.com/cosmos/cosmos-sdk/client" + "github.com/cosmos/cosmos-sdk/codec" "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" - "github.com/cosmos/cosmos-sdk/x/staking" ) // An example exported genesis file from a 0.37 chain. Note that evidence @@ -36,12 +35,11 @@ var v037Exported = `{ }` func TestValidateGenesis(t *testing.T) { - cdc := testutilmod.MakeTestEncodingConfig(genutil.AppModule{}).Codec testCases := []struct { - name string - genesis string - expErrStr string - genMM module.BasicManager + name string + genesis string + expErrStr string + basicManager module.BasicManager }{ { "invalid json", @@ -58,7 +56,7 @@ func TestValidateGenesis(t *testing.T) { return string(bz) }(), "section is missing in the app_state", - module.NewBasicManager(staking.NewAppModule(cdc, nil, nil, nil, nil)), + module.NewBasicManager(mockModule{}), }, { "exported 0.37 genesis file", @@ -84,7 +82,7 @@ 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(tc.genMM), []string{genesisFile.Name()}) + _, err := clitestutil.ExecTestCLICmd(client.Context{}, cli.ValidateGenesisCmd(tc.basicManager), []string{genesisFile.Name()}) if tc.expErrStr != "" { require.Contains(t, err.Error(), tc.expErrStr) } else { @@ -93,3 +91,19 @@ func TestValidateGenesis(t *testing.T) { }) } } + +type mockModule struct { + module.AppModuleBasic +} + +func (mockModule) Name() string { + return "mock" +} + +func (mockModule) DefaultGenesis(codec.JSONCodec) json.RawMessage { + return json.RawMessage(`{"foo": "bar"}`) +} + +func (mockModule) ValidateGenesis(codec.JSONCodec, client.TxEncodingConfig, json.RawMessage) error { + return nil +} From 9490a943e966090a111e32cf72b8d3bc42890030 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Fri, 13 Sep 2024 12:32:56 +0200 Subject: [PATCH 4/4] fix test --- x/genutil/client/cli/validate_genesis.go | 5 ++--- x/genutil/client/cli/validate_genesis_test.go | 12 ++++++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/x/genutil/client/cli/validate_genesis.go b/x/genutil/client/cli/validate_genesis.go index 93fe9ba086e7..94e9a3a04c31 100644 --- a/x/genutil/client/cli/validate_genesis.go +++ b/x/genutil/client/cli/validate_genesis.go @@ -48,12 +48,11 @@ func ValidateGenesisCmd(mbm module.BasicManager) *cobra.Command { } var genState map[string]json.RawMessage - if err = json.Unmarshal(appGenesis.AppState, &genState); err != nil { + 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: %s", genesis, err.Error()) + return fmt.Errorf("error unmarshalling genesis doc %s: %w", genesis, err) } if err = mbm.ValidateGenesis(cdc, clientCtx.TxConfig, genState); err != nil { diff --git a/x/genutil/client/cli/validate_genesis_test.go b/x/genutil/client/cli/validate_genesis_test.go index daaab7b325ac..fba0338660cc 100644 --- a/x/genutil/client/cli/validate_genesis_test.go +++ b/x/genutil/client/cli/validate_genesis_test.go @@ -2,6 +2,8 @@ package cli_test import ( "encoding/json" + "fmt" + "io" "os" "testing" @@ -92,18 +94,20 @@ func TestValidateGenesis(t *testing.T) { } } +var _ module.HasGenesisBasics = mockModule{} + type mockModule struct { module.AppModuleBasic } -func (mockModule) Name() string { +func (m mockModule) Name() string { return "mock" } -func (mockModule) DefaultGenesis(codec.JSONCodec) json.RawMessage { +func (m mockModule) DefaultGenesis(codec.JSONCodec) json.RawMessage { return json.RawMessage(`{"foo": "bar"}`) } -func (mockModule) ValidateGenesis(codec.JSONCodec, client.TxEncodingConfig, json.RawMessage) error { - return nil +func (m mockModule) ValidateGenesis(codec.JSONCodec, client.TxEncodingConfig, json.RawMessage) error { + return fmt.Errorf("mock section is missing: %w", io.EOF) }