feat(x/genutil): add better error messages for genesis validation (ba…

…ckport #21701) (#21708) Co-authored-by: Eric Mokaya <[email protected]> Co-authored-by: marbar3778 <[email protected]>
cosmos · Sep 13, 2024 · f78ed49 · f78ed49
1 parent fe37dfb
commit f78ed49
Show file tree

Hide file tree

Showing 3 changed files with 74 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -46,6 +46,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 ### Improvements
 
 * (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.
 
 ### Bug Fixes
 

diff --git 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,24 +40,39 @@ 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 {
 				return fmt.Errorf("make sure that you have correctly migrated all CometBFT consensus params. Refer the UPGRADING.md (%s): %w", chainUpgradeGuide, err)
 			}
 
 			var genState map[string]json.RawMessage
-			if err = json.Unmarshal(appGenesis.AppState, &genState); err != nil {
-				return fmt.Errorf("error unmarshalling genesis doc %s: %s", genesis, err.Error())
+			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 err = mbm.ValidateGenesis(cdc, clientCtx.TxConfig, genState); err != nil {
-				return fmt.Errorf("error validating genesis file %s: %s", genesis, err.Error())
+				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)
 			}
 
 			fmt.Fprintf(cmd.OutOrStdout(), "File at %s is a valid genesis file\n", genesis)
 			return nil
 		},
 	}
 }
+
+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
@@ -1,14 +1,19 @@
 package cli_test
 
 import (
+	"encoding/json"
+	"fmt"
+	"io"
 	"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"
 	"github.com/cosmos/cosmos-sdk/x/genutil/client/cli"
 )
 
@@ -33,14 +38,33 @@ var v037Exported = `{
 
 func TestValidateGenesis(t *testing.T) {
 	testCases := []struct {
-		name    string
-		genesis string
-		expErr  bool
+		name         string
+		genesis      string
+		expErrStr    string
+		basicManager module.BasicManager
 	}{
+		{
+			"invalid json",
+			`{"app_state": {x,}}`,
+			"error at offset 16: invalid character",
+			module.NewBasicManager(),
+		},
+		{
+			"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.NewBasicManager(mockModule{}),
+		},
 		{
 			"exported 0.37 genesis file",
 			v037Exported,
-			true,
+			"make sure that you have correctly migrated all CometBFT consensus params",
+			module.NewBasicManager(),
 		},
 		{
 			"valid 0.50 genesis file",
@@ -50,7 +74,8 @@ func TestValidateGenesis(t *testing.T) {
 
 				return string(bz)
 			}(),
-			false,
+			"",
+			module.NewBasicManager(),
 		},
 	}
 
@@ -59,12 +84,30 @@ 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.basicManager), []string{genesisFile.Name()})
+			if tc.expErrStr != "" {
+				require.Contains(t, err.Error(), tc.expErrStr)
 			} else {
 				require.NoError(t, err)
 			}
 		})
 	}
 }
+
+var _ module.HasGenesisBasics = mockModule{}
+
+type mockModule struct {
+	module.AppModuleBasic
+}
+
+func (m mockModule) Name() string {
+	return "mock"
+}
+
+func (m mockModule) DefaultGenesis(codec.JSONCodec) json.RawMessage {
+	return json.RawMessage(`{"foo": "bar"}`)
+}
+
+func (m mockModule) ValidateGenesis(codec.JSONCodec, client.TxEncodingConfig, json.RawMessage) error {
+	return fmt.Errorf("mock section is missing: %w", io.EOF)
+}