From ad7c08cfc31dd0037b8ca884ae53106cf8735efe Mon Sep 17 00:00:00 2001 From: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Date: Thu, 21 May 2020 00:49:27 -0400 Subject: [PATCH] x/pricefeed: GenesisState validation (#514) * x/pricefeed: GenesisState validation --- x/pricefeed/genesis.go | 28 +++--- x/pricefeed/types/genesis.go | 9 +- x/pricefeed/types/genesis_test.go | 88 +++++++++++++++++ x/pricefeed/types/market.go | 81 +++++++++++++++- x/pricefeed/types/market_test.go | 152 ++++++++++++++++++++++++++++++ x/pricefeed/types/params.go | 10 +- 6 files changed, 337 insertions(+), 31 deletions(-) create mode 100644 x/pricefeed/types/genesis_test.go create mode 100644 x/pricefeed/types/market_test.go diff --git a/x/pricefeed/genesis.go b/x/pricefeed/genesis.go index 476d4c56..0bfe7f48 100644 --- a/x/pricefeed/genesis.go +++ b/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,17 +20,19 @@ 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 { - rps, err := keeper.GetRawPrices(ctx, market.MarketID) - if err != nil { - panic(err) - } - if len(rps) > 0 { - err := keeper.SetCurrentPrices(ctx, market.MarketID) - if err != nil { - panic(err) - } - } + if !market.Active { + continue + } + rps, err := keeper.GetRawPrices(ctx, market.MarketID) + if err != nil { + panic(err) + } + if len(rps) == 0 { + continue + } + err = keeper.SetCurrentPrices(ctx, market.MarketID) + if err != nil { + panic(err) } } } diff --git a/x/pricefeed/types/genesis.go b/x/pricefeed/types/genesis.go index eb8ba380..d43b6e09 100644 --- a/x/pricefeed/types/genesis.go +++ b/x/pricefeed/types/genesis.go @@ -6,8 +6,8 @@ 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"` + Params Params `json:"params" yaml:"params"` + 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() } diff --git a/x/pricefeed/types/genesis_test.go b/x/pricefeed/types/genesis_test.go new file mode 100644 index 00000000..f64e730d --- /dev/null +++ b/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) + } + } +} diff --git a/x/pricefeed/types/market.go b/x/pricefeed/types/market.go index 16231be8..66c0d9c0 100644 --- a/x/pricefeed/types/market.go +++ b/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 diff --git a/x/pricefeed/types/market_test.go b/x/pricefeed/types/market_test.go new file mode 100644 index 00000000..dd3d1bb6 --- /dev/null +++ b/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) + } + } +} diff --git a/x/pricefeed/types/params.go b/x/pricefeed/types/params.go index acb748d4..61770d4f 100644 --- a/x/pricefeed/types/params.go +++ b/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 - for _, asset := range markets { - if strings.TrimSpace(asset.MarketID) == "" { - return sdkerrors.Wrapf(ErrInvalidMarket, "market id for asset %s cannot be blank", asset) - } - } - - return nil + return markets.Validate() }