From bcf3aaf102e8e7d658625be0189b34668f4b0677 Mon Sep 17 00:00:00 2001 From: krhubert Date: Mon, 30 Sep 2019 09:09:07 +0200 Subject: [PATCH 1/2] Add more validation to config --- config/config.go | 29 +++++++++++++++++++---------- config/config_test.go | 8 +++++++- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/config/config.go b/config/config.go index 11edab0f6..31f6c77ce 100644 --- a/config/config.go +++ b/config/config.go @@ -17,6 +17,7 @@ import ( "github.com/sirupsen/logrus" tmconfig "github.com/tendermint/tendermint/config" "github.com/tendermint/tendermint/crypto/ed25519" + "gopkg.in/go-playground/validator.v9" ) const ( @@ -30,24 +31,24 @@ const ( // Config contains all the configuration needed. type Config struct { - Name string - Path string + Name string `validate:"required"` + Path string `validate:"required"` Server struct { - Address string + Address string `validate:"required"` } Log struct { - Format string + Format string `validate:"required"` ForceColors bool - Level string + Level string `validate:"required"` } Database struct { - ServiceRelativePath string - InstanceRelativePath string - ExecutionRelativePath string - ProcessRelativePath string + ServiceRelativePath string `validate:"required"` + InstanceRelativePath string `validate:"required"` + ExecutionRelativePath string `validate:"required"` + ProcessRelativePath string `validate:"required"` } Tendermint struct { @@ -153,7 +154,7 @@ func (c *Config) Validate() error { if _, err := logrus.ParseLevel(c.Log.Level); err != nil { return err } - return nil + return validator.New().Struct(c) } // PubKeyEd25519 is type used to parse value provided by envconfig. @@ -183,6 +184,10 @@ type StdTx authtypes.StdTx // Decode parses string value as hex ed25519 key. func (tx *StdTx) Decode(value string) error { + if value == "" { + return nil + } + cdc := codec.New() codec.RegisterCrypto(cdc) sdktypes.RegisterCodec(cdc) @@ -191,5 +196,9 @@ func (tx *StdTx) Decode(value string) error { if err := cdc.UnmarshalJSON([]byte(value), tx); err != nil { return fmt.Errorf("unmarshal genesis validator error: %s", err) } + signers := authtypes.StdTx(*tx).GetSigners() + if l := len(signers); l == 0 || signers[l-1] == nil { + return fmt.Errorf("genesis validator error: no signer address") + } return nil } diff --git a/config/config_test.go b/config/config_test.go index 974d92f42..c06f8e65f 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -37,7 +37,8 @@ func TestLoad(t *testing.T) { "MESG_LOG_LEVEL": "", "MESG_LOG_FORCECOLORS": "", "MESG_TENDERMINT_P2P_PERSISTENTPEERS": "", - "MESG_COSMOS_VALIDATOR_PUB_KEY": "", + "MESG_COSMOS_VALIDATORPUBKEY": "", + "MESG_COSMOS_GENESISVALIDATORTX": "", } for key := range snapsnot { snapsnot[key] = os.Getenv(key) @@ -80,3 +81,8 @@ func TestValidate(t *testing.T) { c.Log.Level = "wrongValue" require.Error(t, c.Validate()) } + +func TestStdTXDecodeNoSignersError(t *testing.T) { + var tx StdTx + require.Error(t, tx.Decode(`{"msg":[{"type":"cosmos-sdk/MsgCreateValidator","value":{"description":{"identity":"","website":"","details":"create-first-validator"},"pubkey":"cosmosvalconspub1zcjduepqq0a87y3pur6vvzyp99t92me2zyxywz46kyq5vt7x2n4g987acmxszqey9p","value":{"denom":"stake","amount":"100000000"}}}],"fee":{"amount":[],"gas":"200000"}}`)) +} From 3254d4576ccbbe27a9ab62c548421e261e9a937f Mon Sep 17 00:00:00 2001 From: Nicolas Mahe Date: Mon, 30 Sep 2019 17:10:38 +0700 Subject: [PATCH 2/2] Add validation require on all Config. Check if Cosmos.GenesisValidatorTx is valid. Remove some test as there is require value without default value --- config/config.go | 32 +++++++++++++++----------------- config/config_test.go | 15 +-------------- 2 files changed, 16 insertions(+), 31 deletions(-) diff --git a/config/config.go b/config/config.go index 31f6c77ce..66b5b6e54 100644 --- a/config/config.go +++ b/config/config.go @@ -52,8 +52,8 @@ type Config struct { } Tendermint struct { - *tmconfig.Config - Path string + *tmconfig.Config `validate:"required"` + Path string `validate:"required"` } Cosmos CosmosConfig @@ -61,16 +61,14 @@ type Config struct { // CosmosConfig is the struct to hold cosmos related configs. type CosmosConfig struct { - Path string - ChainID string - GenesisTime time.Time - - GenesisValidatorTx StdTx - - ValidatorPubKey PubKeyEd25519 + Path string `validate:"required"` + ChainID string `validate:"required"` + GenesisTime time.Time `validate:"required"` + GenesisValidatorTx StdTx `validate:"required"` + ValidatorPubKey PubKeyEd25519 `validate:"required"` } -// New creates a new config with default values. +// Default creates a new config with default values. func Default() (*Config, error) { home, err := homedir.Dir() if err != nil { @@ -149,10 +147,13 @@ func (c *Config) Prepare() error { // Validate checks values and return an error if any validation failed. func (c *Config) Validate() error { if !xstrings.SliceContains([]string{"text", "json"}, c.Log.Format) { - return fmt.Errorf("value %q is not an allowed", c.Log.Format) + return fmt.Errorf("config.Log.Format value %q is not an allowed", c.Log.Format) } if _, err := logrus.ParseLevel(c.Log.Level); err != nil { - return err + return fmt.Errorf("config.Log.Level error: %w", err) + } + if err := authtypes.StdTx(c.Cosmos.GenesisValidatorTx).ValidateBasic(); err != nil { + return fmt.Errorf("config.Cosmos.GenesisValidatorTx error: %w", err.Stacktrace()) } return validator.New().Struct(c) } @@ -187,18 +188,15 @@ func (tx *StdTx) Decode(value string) error { if value == "" { return nil } - cdc := codec.New() codec.RegisterCrypto(cdc) sdktypes.RegisterCodec(cdc) stakingtypes.RegisterCodec(cdc) - if err := cdc.UnmarshalJSON([]byte(value), tx); err != nil { return fmt.Errorf("unmarshal genesis validator error: %s", err) } - signers := authtypes.StdTx(*tx).GetSigners() - if l := len(signers); l == 0 || signers[l-1] == nil { - return fmt.Errorf("genesis validator error: no signer address") + if err := authtypes.StdTx(*tx).ValidateBasic(); err != nil { + return err.Stacktrace() } return nil } diff --git a/config/config_test.go b/config/config_test.go index c06f8e65f..5dd5e945f 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -24,12 +24,6 @@ func TestDefaultValue(t *testing.T) { require.Equal(t, "engine", c.Name) } -func TestNew(t *testing.T) { - c, err := New() - require.NoError(t, err) - require.NotNil(t, c) -} - func TestLoad(t *testing.T) { snapsnot := map[string]string{ "MESG_SERVER_ADDRESS": "", @@ -71,14 +65,7 @@ func TestLoad(t *testing.T) { func TestValidate(t *testing.T) { c, _ := Default() - require.NoError(t, c.Validate()) - - c, _ = Default() - c.Log.Format = "wrongValue" - require.Error(t, c.Validate()) - - c, _ = Default() - c.Log.Level = "wrongValue" + c.Load() require.Error(t, c.Validate()) }