Improved validation of node types

sentinel-official · Jul 11, 2021 · c59db84 · c59db84
1 parent ac1aa23
commit c59db84
Show file tree

Hide file tree

Showing 4 changed files with 141 additions and 62 deletions.
diff --git a/x/node/types/errors.go b/x/node/types/errors.go
@@ -5,14 +5,17 @@ import (
 )
 
 var (
-	ErrorInvalidFieldFrom            = errors.Register(ModuleName, 101, "invalid value for field from; expected a valid address")
-	ErrorProviderDoesNotExist        = errors.Register(ModuleName, 102, "provider does not exist")
-	ErrorDuplicateNode               = errors.Register(ModuleName, 103, "duplicate node")
-	ErrorNodeDoesNotExist            = errors.Register(ModuleName, 104, "node does not exist")
-	ErrorInvalidFieldProviderOrPrice = errors.Register(ModuleName, 105, "invalid value for provider or price; expected either price or provider to be empty")
-	ErrorInvalidFieldProvider        = errors.Register(ModuleName, 106, "invalid value for field provider; expected a valid provider address")
-	ErrorInvalidFieldPrice           = errors.Register(ModuleName, 107, "invalid value for field price; expected a valid price")
-	ErrorInvalidFieldRemoteURL       = errors.Register(ModuleName, 108, "invalid value for field remote_url; expected length between 1 and 64")
-	ErrorInvalidFieldStatus          = errors.Register(ModuleName, 109, "invalid value for field status; expected either Active or Inactive")
-	ErrorInvalidPlanCount            = errors.Register(ModuleName, 110, "invalid plan count")
+	ErrorInvalidField     = errors.Register(ModuleName, 101, "invalid field")
+	ErrorInvalidFrom      = errors.Register(ModuleName, 102, "invalid from")
+	ErrorInvalidProvider  = errors.Register(ModuleName, 103, "invalid provider")
+	ErrorInvalidPrice     = errors.Register(ModuleName, 104, "invalid price")
+	ErrorInvalidRemoteURL = errors.Register(ModuleName, 105, "invalid remote_url")
+	ErrorInvalidStatus    = errors.Register(ModuleName, 106, "invalid status")
+)
+
+var (
+	ErrorProviderDoesNotExist = errors.Register(ModuleName, 201, "provider does not exist")
+	ErrorDuplicateNode        = errors.Register(ModuleName, 202, "duplicate node")
+	ErrorNodeDoesNotExist     = errors.Register(ModuleName, 203, "node does not exist")
+	ErrorInvalidPlanCount     = errors.Register(ModuleName, 204, "invalid plan count")
 )
diff --git a/x/node/types/msg.go b/x/node/types/msg.go
@@ -2,8 +2,10 @@ package types
 
 import (
 	"fmt"
+	"net/url"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/types/errors"
 
 	hubtypes "github.com/sentinel-official/hub/types"
 )
@@ -32,31 +34,47 @@ func (m *MsgRegisterRequest) Type() string {
 }
 
 func (m *MsgRegisterRequest) ValidateBasic() error {
+	if m.From == "" {
+		return errors.Wrap(ErrorInvalidFrom, "from cannot be empty")
+	}
 	if _, err := sdk.AccAddressFromBech32(m.From); err != nil {
-		return ErrorInvalidFieldFrom
+		return errors.Wrapf(ErrorInvalidFrom, "%s", err)
 	}
-
-	// Either provider or price should be empty
-	if (m.Provider != "" && m.Price != nil) ||
-		(m.Provider == "" && m.Price == nil) {
-		return ErrorInvalidFieldProviderOrPrice
+	if m.Provider == "" && m.Price == nil {
+		return errors.Wrap(ErrorInvalidField, "both provider and price cannot be empty")
+	}
+	if m.Provider != "" && m.Price != nil {
+		return errors.Wrap(ErrorInvalidField, "either provider or price must be empty")
 	}
-
-	// Provider can be empty. If not, it should be valid
 	if m.Provider != "" {
 		if _, err := hubtypes.ProvAddressFromBech32(m.Provider); err != nil {
-			return ErrorInvalidFieldProvider
+			return errors.Wrapf(ErrorInvalidProvider, "%s", err)
 		}
 	}
-
-	// Price can be nil. If not, it should be valid
-	if m.Price != nil && !m.Price.IsValid() {
-		return ErrorInvalidFieldPrice
+	if m.Price != nil {
+		if m.Price.Len() == 0 {
+			return errors.Wrap(ErrorInvalidPrice, "price cannot be empty")
+		}
+		if !m.Price.IsValid() {
+			return errors.Wrap(ErrorInvalidPrice, "price must be valid")
+		}
+	}
+	if m.RemoteURL == "" {
+		return errors.Wrap(ErrorInvalidRemoteURL, "remote_url cannot be empty")
+	}
+	if len(m.RemoteURL) > 64 {
+		return errors.Wrapf(ErrorInvalidRemoteURL, "remote_url length cannot be greater than %d", 64)
 	}
 
-	// RemoteURL length should be between 1 and 64
-	if len(m.RemoteURL) == 0 || len(m.RemoteURL) > 64 {
-		return ErrorInvalidFieldRemoteURL
+	remoteURL, err := url.ParseRequestURI(m.RemoteURL)
+	if err != nil {
+		return errors.Wrapf(ErrorInvalidRemoteURL, "%s", err)
+	}
+	if remoteURL.Scheme != "https" {
+		return errors.Wrap(ErrorInvalidRemoteURL, "remote_url scheme must be https")
+	}
+	if remoteURL.Port() == "" {
+		return errors.Wrap(ErrorInvalidRemoteURL, "remote_url port cannot be empty")
 	}
 
 	return nil
@@ -93,29 +111,43 @@ func (m *MsgUpdateRequest) Type() string {
 }
 
 func (m *MsgUpdateRequest) ValidateBasic() error {
+	if m.From == "" {
+		return errors.Wrap(ErrorInvalidFrom, "from cannot be empty")
+	}
 	if _, err := hubtypes.NodeAddressFromBech32(m.From); err != nil {
-		return ErrorInvalidFieldFrom
+		return errors.Wrapf(ErrorInvalidFrom, "%s", err)
 	}
-
 	if m.Provider != "" && m.Price != nil {
-		return ErrorInvalidFieldProviderOrPrice
+		return errors.Wrap(ErrorInvalidField, "either provider or price must be empty")
 	}
-
-	// Provider can be empty. If not, it should be valid
 	if m.Provider != "" {
 		if _, err := hubtypes.ProvAddressFromBech32(m.Provider); err != nil {
-			return ErrorInvalidFieldProvider
+			return errors.Wrapf(ErrorInvalidProvider, "%s", err)
 		}
 	}
-
-	// Price can be nil. If not, it should be valid
-	if m.Price != nil && !m.Price.IsValid() {
-		return ErrorInvalidFieldPrice
+	if m.Price != nil {
+		if m.Price.Len() == 0 {
+			return errors.Wrap(ErrorInvalidPrice, "price cannot be empty")
+		}
+		if !m.Price.IsValid() {
+			return errors.Wrap(ErrorInvalidPrice, "price must be valid")
+		}
 	}
+	if m.RemoteURL != "" {
+		if len(m.RemoteURL) > 64 {
+			return errors.Wrapf(ErrorInvalidRemoteURL, "remote_url length cannot be greater than %d", 64)
+		}
 
-	// RemoteURL length should be between 0 and 64
-	if len(m.RemoteURL) > 64 {
-		return ErrorInvalidFieldRemoteURL
+		remoteURL, err := url.ParseRequestURI(m.RemoteURL)
+		if err != nil {
+			return errors.Wrapf(ErrorInvalidRemoteURL, "%s", err)
+		}
+		if remoteURL.Scheme != "https" {
+			return errors.Wrap(ErrorInvalidRemoteURL, "remote_url scheme must be https")
+		}
+		if remoteURL.Port() == "" {
+			return errors.Wrap(ErrorInvalidRemoteURL, "remote_url port cannot be empty")
+		}
 	}
 
 	return nil
@@ -150,13 +182,14 @@ func (m *MsgSetStatusRequest) Type() string {
 }
 
 func (m *MsgSetStatusRequest) ValidateBasic() error {
+	if m.From == "" {
+		return errors.Wrap(ErrorInvalidFrom, "from cannot be empty")
+	}
 	if _, err := hubtypes.NodeAddressFromBech32(m.From); err != nil {
-		return ErrorInvalidFieldFrom
+		return errors.Wrapf(ErrorInvalidFrom, "%s", err)
 	}
-
-	// Status should be either Active or Inactive
 	if !m.Status.Equal(hubtypes.StatusActive) && !m.Status.Equal(hubtypes.StatusInactive) {
-		return ErrorInvalidFieldStatus
+		return errors.Wrap(ErrorInvalidStatus, "status must be either active or inactive")
 	}
 
 	return nil

diff --git a/x/node/types/node.go b/x/node/types/node.go
@@ -2,8 +2,10 @@ package types
 
 import (
 	"fmt"
+	"net/url"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/types/errors"
 
 	hubtypes "github.com/sentinel-official/hub/types"
 )
@@ -35,27 +37,54 @@ func (n *Node) GetProvider() hubtypes.ProvAddress {
 }
 
 func (n *Node) Validate() error {
+	if n.Address == "" {
+		return fmt.Errorf("address cannot be empty")
+	}
 	if _, err := hubtypes.NodeAddressFromBech32(n.Address); err != nil {
-		return err
+		return errors.Wrapf(err, "invalid address %s", n.Address)
+	}
+	if n.Provider == "" && n.Price == nil {
+		return fmt.Errorf("both provider and price cannot be empty")
+	}
+	if n.Provider != "" && n.Price != nil {
+		return fmt.Errorf("either provider or price must be empty")
+	}
+	if n.Provider != "" {
+		if _, err := hubtypes.ProvAddressFromBech32(n.Provider); err != nil {
+			return errors.Wrapf(err, "invalid provider %s", n.Provider)
+		}
+	}
+	if n.Price != nil {
+		if n.Price.Len() == 0 {
+			return fmt.Errorf("price cannot be empty")
+		}
+		if !n.Price.IsValid() {
+			return fmt.Errorf("price must be valid")
+		}
 	}
-	if (n.Provider != "" && n.Price != nil) ||
-		(n.Provider == "" && n.Price == nil) {
-		return fmt.Errorf("invalid provider and price combination; expected one of them empty")
+	if n.RemoteURL == "" {
+		return fmt.Errorf("remote_url cannot be empty")
 	}
-	if _, err := hubtypes.ProvAddressFromBech32(n.Provider); err != nil {
-		return err
+	if len(n.RemoteURL) > 64 {
+		return fmt.Errorf("remote_url length cannot be greater than %d", 64)
 	}
-	if n.Price != nil && !n.Price.IsValid() {
-		return fmt.Errorf("invalid price; expected non-nil and valid value")
+
+	remoteURL, err := url.ParseRequestURI(n.RemoteURL)
+	if err != nil {
+		return errors.Wrapf(err, "invalid remote_url %s", n.RemoteURL)
 	}
-	if len(n.RemoteURL) == 0 || len(n.RemoteURL) > 64 {
-		return fmt.Errorf("invalid remote url length; expected length is between 1 and 64")
+	if remoteURL.Scheme != "https" {
+		return fmt.Errorf("remote_url scheme must be https")
 	}
+	if remoteURL.Port() == "" {
+		return fmt.Errorf("remote_url port cannot be empty")
+	}
+
 	if !n.Status.Equal(hubtypes.StatusActive) && !n.Status.Equal(hubtypes.StatusInactive) {
-		return fmt.Errorf("invalid status; exptected active or inactive")
+		return fmt.Errorf("status must be either active or inactive")
 	}
 	if n.StatusAt.IsZero() {
-		return fmt.Errorf("invalid status at; expected non-zero value")
+		return fmt.Errorf("status_at cannot be zero")
 	}
 
 	return nil
@@ -89,4 +118,6 @@ func (n *Node) BytesForCoin(coin sdk.Coin) (sdk.Int, error) {
 	return coin.Amount.Quo(y), nil
 }
 
-type Nodes []Node
+type (
+	Nodes []Node
+)
diff --git a/x/node/types/params.go b/x/node/types/params.go
@@ -26,11 +26,17 @@ var (
 )
 
 func (p *Params) Validate() error {
+	if p.Deposit.IsNegative() {
+		return fmt.Errorf("deposit cannot be negative")
+	}
 	if !p.Deposit.IsValid() {
-		return fmt.Errorf("deposit should be valid")
+		return fmt.Errorf("invalid deposit %s", p.Deposit)
+	}
+	if p.InactiveDuration < 0 {
+		return fmt.Errorf("inactive_duration cannot be negative")
 	}
-	if p.InactiveDuration <= 0 {
-		return fmt.Errorf("inactive_duration should be positive")
+	if p.InactiveDuration == 0 {
+		return fmt.Errorf("inactive_duration cannot be zero")
 	}
 
 	return nil
@@ -47,8 +53,11 @@ func (p *Params) ParamSetPairs() params.ParamSetPairs {
 					return fmt.Errorf("invalid parameter type %T", v)
 				}
 
+				if value.IsNegative() {
+					return fmt.Errorf("deposit cannot be negative")
+				}
 				if !value.IsValid() {
-					return fmt.Errorf("deposit value should be valid")
+					return fmt.Errorf("invalid deposit %s", value)
 				}
 
 				return nil
@@ -63,8 +72,11 @@ func (p *Params) ParamSetPairs() params.ParamSetPairs {
 					return fmt.Errorf("invalid parameter type %T", v)
 				}
 
-				if value <= 0 {
-					return fmt.Errorf("inactive duration value should be positive")
+				if value < 0 {
+					return fmt.Errorf("inactive_duration cannot be negative")
+				}
+				if value == 0 {
+					return fmt.Errorf("inactive_duration cannot be zero")
 				}
 
 				return nil