x/pricefeed: GenesisState validation (#514)

* x/pricefeed: GenesisState validation

x/pricefeed/genesis.go

@@ -6,10 +6,6 @@ import (
 
 // InitGenesis sets distribution information for genesis.
 func InitGenesis(ctx sdk.Context, keeper Keeper, gs GenesisState) {
-	err := gs.Validate()
-	if err != nil {
-		panic(err)
-	}
 	// Set the markets and oracles from params
 	keeper.SetParams(ctx, gs.Params)
 
@@ -24,19 +20,21 @@ func InitGenesis(ctx sdk.Context, keeper Keeper, gs GenesisState) {
 
 	// Set the current price (if any) based on what's now in the store
 	for _, market := range params.Markets {
-		if market.Active {
+		if !market.Active {
+			continue
+		}
 		rps, err := keeper.GetRawPrices(ctx, market.MarketID)
 		if err != nil {
 			panic(err)
 		}
-			if len(rps) > 0 {
+		if len(rps) == 0 {
-				err := keeper.SetCurrentPrices(ctx, market.MarketID)
+			continue
+		}
+		err = keeper.SetCurrentPrices(ctx, market.MarketID)
 		if err != nil {
 			panic(err)
 		}
 	}
-		}
-	}
 }
 
 // ExportGenesis returns a GenesisState for a given context and keeper.

x/pricefeed/types/genesis.go

@@ -7,7 +7,7 @@ import (
 // GenesisState - pricefeed state that must be provided at genesis
 type GenesisState struct {
 	Params       Params       `json:"params" yaml:"params"`
-	PostedPrices []PostedPrice `json:"posted_prices" yaml:"posted_prices"`
+	PostedPrices PostedPrices `json:"posted_prices" yaml:"posted_prices"`
 }
 
 // NewGenesisState creates a new genesis state for the pricefeed module
@@ -38,12 +38,11 @@ func (gs GenesisState) IsEmpty() bool {
 	return gs.Equal(GenesisState{})
 }
 
-// ValidateGenesis performs basic validation of genesis data returning an
+// Validate performs basic validation of genesis data returning an
 // error for any failed validation criteria.
 func (gs GenesisState) Validate() error {
-
 	if err := gs.Params.Validate(); err != nil {
 		return err
 	}
-	return nil
+	return gs.PostedPrices.Validate()
 }

x/pricefeed/types/genesis_test.go

@@ -0,0 +1,88 @@
+package types
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+
+	tmtypes "github.com/tendermint/tendermint/types"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+)
+
+func TestGenesisStateValidate(t *testing.T) {
+	now := time.Now()
+	addr := sdk.AccAddress(tmtypes.NewMockPV().GetPubKey().Address())
+
+	testCases := []struct {
+		msg          string
+		genesisState GenesisState
+		expPass      bool
+	}{
+		{
+			msg:          "default",
+			genesisState: DefaultGenesisState(),
+			expPass:      true,
+		},
+		{
+			msg: "valid genesis",
+			genesisState: NewGenesisState(
+				NewParams(Markets{
+					{"market", "xrp", "bnb", []sdk.AccAddress{addr}, true},
+				}),
+				[]PostedPrice{NewPostedPrice("xrp", addr, sdk.OneDec(), now)},
+			),
+			expPass: true,
+		},
+		{
+			msg: "invalid param",
+			genesisState: NewGenesisState(
+				NewParams(Markets{
+					{"", "xrp", "bnb", []sdk.AccAddress{addr}, true},
+				}),
+				[]PostedPrice{NewPostedPrice("xrp", addr, sdk.OneDec(), now)},
+			),
+			expPass: false,
+		},
+		{
+			msg: "dup market param",
+			genesisState: NewGenesisState(
+				NewParams(Markets{
+					{"market", "xrp", "bnb", []sdk.AccAddress{addr}, true},
+					{"market", "xrp", "bnb", []sdk.AccAddress{addr}, true},
+				}),
+				[]PostedPrice{NewPostedPrice("xrp", addr, sdk.OneDec(), now)},
+			),
+			expPass: false,
+		},
+		{
+			msg: "invalid posted price",
+			genesisState: NewGenesisState(
+				NewParams(Markets{}),
+				[]PostedPrice{NewPostedPrice("xrp", nil, sdk.OneDec(), now)},
+			),
+			expPass: false,
+		},
+		{
+			msg: "duplicated posted price",
+			genesisState: NewGenesisState(
+				NewParams(Markets{}),
+				[]PostedPrice{
+					NewPostedPrice("xrp", addr, sdk.OneDec(), now),
+					NewPostedPrice("xrp", addr, sdk.OneDec(), now),
+				},
+			),
+			expPass: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		err := tc.genesisState.Validate()
+		if tc.expPass {
+			require.NoError(t, err, tc.msg)
+		} else {
+			require.Error(t, err, tc.msg)
+		}
+	}
+}

x/pricefeed/types/market.go

@@ -1,6 +1,7 @@
 package types
 
 import (
+	"errors"
 	"fmt"
 	"strings"
 	"time"
@@ -10,6 +11,7 @@ import (
 
 // Market an asset in the pricefeed
 type Market struct {
+	// TODO: rename to ID
 	MarketID   string           `json:"market_id" yaml:"market_id"`
 	BaseAsset  string           `json:"base_asset" yaml:"base_asset"`
 	QuoteAsset string           `json:"quote_asset" yaml:"quote_asset"`
@@ -18,19 +20,59 @@ type Market struct {
 }
 
 // String implement fmt.Stringer
-func (a Market) String() string {
+func (m Market) String() string {
 	return fmt.Sprintf(`Asset:
 	Market ID: %s
 	Base Asset: %s
 	Quote Asset: %s
 	Oracles: %s
 	Active: %t`,
-		a.MarketID, a.BaseAsset, a.QuoteAsset, a.Oracles, a.Active)
+		m.MarketID, m.BaseAsset, m.QuoteAsset, m.Oracles, m.Active)
+}
+
+// Validate performs a basic validation of the market params
+func (m Market) Validate() error {
+	if strings.TrimSpace(m.MarketID) == "" {
+		return errors.New("market id cannot be blank")
+	}
+	if err := sdk.ValidateDenom(m.BaseAsset); err != nil {
+		return fmt.Errorf("invalid base asset: %w", err)
+	}
+	if err := sdk.ValidateDenom(m.QuoteAsset); err != nil {
+		return fmt.Errorf("invalid quote asset: %w", err)
+	}
+	seenOracles := make(map[string]bool)
+	for i, oracle := range m.Oracles {
+		if oracle.Empty() {
+			return fmt.Errorf("oracle %d is empty", i)
+		}
+		if seenOracles[oracle.String()] {
+			return fmt.Errorf("duplicated oracle %s", oracle)
+		}
+		seenOracles[oracle.String()] = true
+	}
+	return nil
 }
 
 // Markets array type for oracle
 type Markets []Market
 
+// Validate checks if all the markets are valid and there are no duplicated
+// entries.
+func (ms Markets) Validate() error {
+	seenMarkets := make(map[string]bool)
+	for _, m := range ms {
+		if seenMarkets[m.MarketID] {
+			return fmt.Errorf("duplicated market %s", m.MarketID)
+		}
+		if err := m.Validate(); err != nil {
+			return err
+		}
+		seenMarkets[m.MarketID] = true
+	}
+	return nil
+}
+
 // String implements fmt.Stringer
 func (ms Markets) String() string {
 	out := "Markets:\n"
@@ -72,9 +114,44 @@ func NewPostedPrice(marketID string, oracle sdk.AccAddress, price sdk.Dec, expir
 	}
 }
 
+// Validate performs a basic check of a PostedPrice params.
+func (pp PostedPrice) Validate() error {
+	if strings.TrimSpace(pp.MarketID) == "" {
+		return errors.New("market id cannot be blank")
+	}
+	if pp.OracleAddress.Empty() {
+		return errors.New("oracle address cannot be empty")
+	}
+	if pp.Price.IsNegative() {
+		return fmt.Errorf("posted price cannot be negative %s", pp.Price)
+	}
+	if pp.Expiry.IsZero() {
+		return errors.New("expiry time cannot be zero")
+	}
+	return nil
+}
+
 // PostedPrices type for an array of PostedPrice
 type PostedPrices []PostedPrice
 
+// Validate checks if all the posted prices are valid and there are no duplicated
+// entries.
+func (pps PostedPrices) Validate() error {
+	seenPrices := make(map[string]bool)
+	for _, pp := range pps {
+		if pp.OracleAddress != nil && seenPrices[pp.MarketID+pp.OracleAddress.String()] {
+			return fmt.Errorf("duplicated posted price for marked id %s and oracle address %s", pp.MarketID, pp.OracleAddress)
+		}
+
+		if err := pp.Validate(); err != nil {
+			return err
+		}
+		seenPrices[pp.MarketID+pp.OracleAddress.String()] = true
+	}
+
+	return nil
+}
+
 // implement fmt.Stringer
 func (cp CurrentPrice) String() string {
 	return strings.TrimSpace(fmt.Sprintf(`Market ID: %s

x/pricefeed/types/market_test.go

@@ -0,0 +1,152 @@
+package types
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+
+	tmtypes "github.com/tendermint/tendermint/types"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+)
+
+func TestMarketValidate(t *testing.T) {
+	addr := sdk.AccAddress(tmtypes.NewMockPV().GetPubKey().Address())
+
+	testCases := []struct {
+		msg     string
+		market  Market
+		expPass bool
+	}{
+		{
+			"valid market",
+			Market{
+				MarketID:   "market",
+				BaseAsset:  "xrp",
+				QuoteAsset: "bnb",
+				Oracles:    []sdk.AccAddress{addr},
+				Active:     true,
+			},
+			true,
+		},
+		{
+			"invalid id",
+			Market{
+				MarketID: " ",
+			},
+			false,
+		},
+		{
+			"invalid base asset",
+			Market{
+				MarketID:  "market",
+				BaseAsset: "XRP",
+			},
+			false,
+		},
+		{
+			"invalid market",
+			Market{
+				MarketID:   "market",
+				BaseAsset:  "xrp",
+				QuoteAsset: "BNB",
+			},
+			false,
+		},
+		{
+			"empty oracle address ",
+			Market{
+				MarketID:   "market",
+				BaseAsset:  "xrp",
+				QuoteAsset: "bnb",
+				Oracles:    []sdk.AccAddress{nil},
+			},
+			false,
+		},
+		{
+			"empty oracle address ",
+			Market{
+				MarketID:   "market",
+				BaseAsset:  "xrp",
+				QuoteAsset: "bnb",
+				Oracles:    []sdk.AccAddress{addr, addr},
+			},
+			false,
+		},
+	}
+
+	for _, tc := range testCases {
+		err := tc.market.Validate()
+		if tc.expPass {
+			require.NoError(t, err)
+		} else {
+			require.Error(t, err)
+		}
+	}
+}
+
+func TestPostedPriceValidate(t *testing.T) {
+	now := time.Now()
+	addr := sdk.AccAddress(tmtypes.NewMockPV().GetPubKey().Address())
+
+	testCases := []struct {
+		msg         string
+		postedPrice PostedPrice
+		expPass     bool
+	}{
+		{
+			"valid posted price",
+			PostedPrice{
+				MarketID:      "market",
+				OracleAddress: addr,
+				Price:         sdk.OneDec(),
+				Expiry:        now,
+			},
+			true,
+		},
+		{
+			"invalid id",
+			PostedPrice{
+				MarketID: " ",
+			},
+			false,
+		},
+		{
+			"invalid oracle",
+			PostedPrice{
+				MarketID:      "market",
+				OracleAddress: nil,
+			},
+			false,
+		},
+		{
+			"invalid price",
+			PostedPrice{
+				MarketID:      "market",
+				OracleAddress: addr,
+				Price:         sdk.NewDec(-1),
+			},
+			false,
+		},
+		{
+			"zero expiry time ",
+			PostedPrice{
+				MarketID:      "market",
+				OracleAddress: addr,
+				Price:         sdk.OneDec(),
+				Expiry:        time.Time{},
+			},
+			false,
+		},
+	}
+
+	for _, tc := range testCases {
+		err := tc.postedPrice.Validate()
+		if tc.expPass {
+			require.NoError(t, err)
+		} else {
+			require.Error(t, err)
+		}
+	}
+}

x/pricefeed/types/params.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"strings"
 
-	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/cosmos/cosmos-sdk/x/params"
 )
 
@@ -64,12 +63,5 @@ func validateMarketParams(i interface{}) error {
 		return fmt.Errorf("invalid parameter type: %T", i)
 	}
 
-	// iterate over assets and verify them
+	return markets.Validate()
-	for _, asset := range markets {
-		if strings.TrimSpace(asset.MarketID) == "" {
-			return sdkerrors.Wrapf(ErrInvalidMarket, "market id for asset %s cannot be blank", asset)
-		}
-	}
-
-	return nil
 }