From a6031172a1edf80d999beb18656b998a662d020a Mon Sep 17 00:00:00 2001 From: Kevin Davis Date: Mon, 6 Apr 2020 09:56:59 -0400 Subject: [PATCH 1/2] feat: clean up pricefeed code --- x/pricefeed/alias.go | 58 +++++++++++++----------- x/pricefeed/keeper/keeper.go | 38 ++++++---------- x/pricefeed/simulation/decoder.go | 23 +++++++++- x/pricefeed/simulation/decoder_test.go | 63 ++++++++++++++++++++++++++ x/pricefeed/types/key.go | 26 +++++++---- x/pricefeed/types/market.go | 30 ++++++++++++ 6 files changed, 176 insertions(+), 62 deletions(-) create mode 100644 x/pricefeed/simulation/decoder_test.go diff --git a/x/pricefeed/alias.go b/x/pricefeed/alias.go index be764b2c..1e5aa2be 100644 --- a/x/pricefeed/alias.go +++ b/x/pricefeed/alias.go @@ -1,8 +1,8 @@ // nolint // autogenerated code using github.com/rigelrozanski/multitool // aliases generated for the following subdirectories: -// ALIASGEN: github.com/kava-labs/kava/x/pricefeed/types/ -// ALIASGEN: github.com/kava-labs/kava/x/pricefeed/keeper/ +// ALIASGEN: github.com/kava-labs/kava/x/pricefeed/keeper +// ALIASGEN: github.com/kava-labs/kava/x/pricefeed/types package pricefeed import ( @@ -31,48 +31,54 @@ const ( RouterKey = types.RouterKey QuerierRoute = types.QuerierRoute DefaultParamspace = types.DefaultParamspace - RawPriceFeedPrefix = types.RawPriceFeedPrefix - CurrentPricePrefix = types.CurrentPricePrefix - MarketPrefix = types.MarketPrefix - OraclePrefix = types.OraclePrefix TypeMsgPostPrice = types.TypeMsgPostPrice - QueryPrice = types.QueryPrice - QueryRawPrices = types.QueryRawPrices + QueryGetParams = types.QueryGetParams QueryMarkets = types.QueryMarkets + QueryOracles = types.QueryOracles + QueryRawPrices = types.QueryRawPrices + QueryPrice = types.QueryPrice ) var ( // functions aliases - RegisterCodec = types.RegisterCodec - ErrEmptyInput = types.ErrEmptyInput - ErrExpired = types.ErrExpired - ErrNoValidPrice = types.ErrNoValidPrice - ErrInvalidMarket = types.ErrInvalidMarket - ErrInvalidOracle = types.ErrInvalidOracle - NewGenesisState = types.NewGenesisState - DefaultGenesisState = types.DefaultGenesisState - NewMsgPostPrice = types.NewMsgPostPrice - NewParams = types.NewParams - DefaultParams = types.DefaultParams - ParamKeyTable = types.ParamKeyTable - NewKeeper = keeper.NewKeeper - NewQuerier = keeper.NewQuerier + NewKeeper = keeper.NewKeeper + NewQuerier = keeper.NewQuerier + RegisterCodec = types.RegisterCodec + ErrEmptyInput = types.ErrEmptyInput + ErrExpired = types.ErrExpired + ErrNoValidPrice = types.ErrNoValidPrice + ErrInvalidMarket = types.ErrInvalidMarket + ErrInvalidOracle = types.ErrInvalidOracle + NewGenesisState = types.NewGenesisState + DefaultGenesisState = types.DefaultGenesisState + CurrentPriceKey = types.CurrentPriceKey + RawPriceKey = types.RawPriceKey + NewCurrentPrice = types.NewCurrentPrice + NewPostedPrice = types.NewPostedPrice + NewMsgPostPrice = types.NewMsgPostPrice + NewParams = types.NewParams + DefaultParams = types.DefaultParams + ParamKeyTable = types.ParamKeyTable + NewQueryWithMarketIDParams = types.NewQueryWithMarketIDParams // variable aliases - ModuleCdc = types.ModuleCdc - KeyMarkets = types.KeyMarkets - DefaultMarkets = types.DefaultMarkets + ModuleCdc = types.ModuleCdc + CurrentPricePrefix = types.CurrentPricePrefix + RawPriceFeedPrefix = types.RawPriceFeedPrefix + KeyMarkets = types.KeyMarkets + DefaultMarkets = types.DefaultMarkets ) type ( + Keeper = keeper.Keeper GenesisState = types.GenesisState Market = types.Market Markets = types.Markets CurrentPrice = types.CurrentPrice PostedPrice = types.PostedPrice + PostedPrices = types.PostedPrices SortDecs = types.SortDecs MsgPostPrice = types.MsgPostPrice Params = types.Params QueryWithMarketIDParams = types.QueryWithMarketIDParams - Keeper = keeper.Keeper ) diff --git a/x/pricefeed/keeper/keeper.go b/x/pricefeed/keeper/keeper.go index 0e4fc8e9..bd769662 100644 --- a/x/pricefeed/keeper/keeper.go +++ b/x/pricefeed/keeper/keeper.go @@ -58,13 +58,9 @@ func (k Keeper) SetPrice( } // set the price for that particular oracle if found { - prices[index] = types.PostedPrice{ - MarketID: marketID, OracleAddress: oracle, - Price: price, Expiry: expiry} + prices[index] = types.NewPostedPrice(marketID, oracle, price, expiry) } else { - prices = append(prices, types.PostedPrice{ - MarketID: marketID, OracleAddress: oracle, - Price: price, Expiry: expiry}) + prices = append(prices, types.NewPostedPrice(marketID, oracle, price, expiry)) index = len(prices) - 1 } @@ -79,7 +75,7 @@ func (k Keeper) SetPrice( ), ) store.Set( - []byte(types.RawPriceFeedPrefix+marketID), k.cdc.MustMarshalBinaryBare(prices), + types.RawPriceKey(marketID), k.cdc.MustMarshalBinaryBare(prices), ) return prices[index], nil } @@ -101,20 +97,17 @@ func (k Keeper) SetCurrentPrices(ctx sdk.Context, marketID string) sdk.Error { } prices := k.GetRawPrices(ctx, marketID) - var notExpiredPrices []types.CurrentPrice + var notExpiredPrices types.CurrentPrices // filter out expired prices for _, v := range prices { if v.Expiry.After(ctx.BlockTime()) { - notExpiredPrices = append(notExpiredPrices, types.CurrentPrice{ - MarketID: v.MarketID, - Price: v.Price, - }) + notExpiredPrices = append(notExpiredPrices, types.NewCurrentPrice(v.MarketID, v.Price)) } } if len(notExpiredPrices) == 0 { store := ctx.KVStore(k.key) store.Set( - []byte(types.CurrentPricePrefix+marketID), k.cdc.MustMarshalBinaryBare(types.CurrentPrice{}), + types.CurrentPriceKey(marketID), k.cdc.MustMarshalBinaryBare(types.CurrentPrice{}), ) return types.ErrNoValidPrice(k.codespace) } @@ -135,20 +128,17 @@ func (k Keeper) SetCurrentPrices(ctx sdk.Context, marketID string) sdk.Error { } store := ctx.KVStore(k.key) - currentPrice := types.CurrentPrice{ - MarketID: marketID, - Price: medianPrice, - } + currentPrice := types.NewCurrentPrice(marketID, medianPrice) store.Set( - []byte(types.CurrentPricePrefix+marketID), k.cdc.MustMarshalBinaryBare(currentPrice), + types.CurrentPriceKey(marketID), k.cdc.MustMarshalBinaryBare(currentPrice), ) return nil } // CalculateMedianPrice calculates the median prices for the input prices. -func (k Keeper) CalculateMedianPrice(ctx sdk.Context, prices []types.CurrentPrice) sdk.Dec { +func (k Keeper) CalculateMedianPrice(ctx sdk.Context, prices types.CurrentPrices) sdk.Dec { l := len(prices) if l == 1 { @@ -169,7 +159,7 @@ func (k Keeper) CalculateMedianPrice(ctx sdk.Context, prices []types.CurrentPric } -func (k Keeper) calculateMeanPrice(ctx sdk.Context, prices []types.CurrentPrice) sdk.Dec { +func (k Keeper) calculateMeanPrice(ctx sdk.Context, prices types.CurrentPrices) sdk.Dec { sum := prices[0].Price.Add(prices[1].Price) mean := sum.Quo(sdk.NewDec(2)) return mean @@ -178,7 +168,7 @@ func (k Keeper) calculateMeanPrice(ctx sdk.Context, prices []types.CurrentPrice) // GetCurrentPrice fetches the current median price of all oracles for a specific market func (k Keeper) GetCurrentPrice(ctx sdk.Context, marketID string) (types.CurrentPrice, sdk.Error) { store := ctx.KVStore(k.key) - bz := store.Get([]byte(types.CurrentPricePrefix + marketID)) + bz := store.Get(types.CurrentPriceKey(marketID)) if bz == nil { return types.CurrentPrice{}, types.ErrNoValidPrice(k.codespace) @@ -192,10 +182,10 @@ func (k Keeper) GetCurrentPrice(ctx sdk.Context, marketID string) (types.Current } // GetRawPrices fetches the set of all prices posted by oracles for an asset -func (k Keeper) GetRawPrices(ctx sdk.Context, marketID string) []types.PostedPrice { +func (k Keeper) GetRawPrices(ctx sdk.Context, marketID string) types.PostedPrices { store := ctx.KVStore(k.key) - bz := store.Get([]byte(types.RawPriceFeedPrefix + marketID)) - var prices []types.PostedPrice + bz := store.Get(types.RawPriceKey(marketID)) + var prices types.PostedPrices k.cdc.MustUnmarshalBinaryBare(bz, &prices) return prices } diff --git a/x/pricefeed/simulation/decoder.go b/x/pricefeed/simulation/decoder.go index 115565b3..3d4a07c4 100644 --- a/x/pricefeed/simulation/decoder.go +++ b/x/pricefeed/simulation/decoder.go @@ -1,12 +1,31 @@ package simulation import ( + "bytes" + "fmt" + "github.com/cosmos/cosmos-sdk/codec" cmn "github.com/tendermint/tendermint/libs/common" + + "github.com/kava-labs/kava/x/pricefeed/types" ) // DecodeStore unmarshals the KVPair's Value to the corresponding pricefeed type func DecodeStore(cdc *codec.Codec, kvA, kvB cmn.KVPair) string { - // TODO implement this - return "" + switch { + case bytes.Contains(kvA.Key[:1], []byte(types.CurrentPricePrefix)): + var priceA, priceB types.CurrentPrice + cdc.MustUnmarshalBinaryBare(kvA.Value, &priceA) + cdc.MustUnmarshalBinaryBare(kvB.Value, &priceB) + return fmt.Sprintf("%s\n%s", priceA, priceB) + + case bytes.Contains(kvA.Key[:1], []byte(types.RawPriceFeedPrefix)): + var postedPriceA, postedPriceB types.PostedPrices + cdc.MustUnmarshalBinaryBare(kvA.Value, &postedPriceA) + cdc.MustUnmarshalBinaryBare(kvB.Value, &postedPriceB) + return fmt.Sprintf("%s\n%s", postedPriceA, postedPriceB) + + default: + panic(fmt.Sprintf("invalid %s key prefix %X", types.ModuleName, kvA.Key[:1])) + } } diff --git a/x/pricefeed/simulation/decoder_test.go b/x/pricefeed/simulation/decoder_test.go new file mode 100644 index 00000000..635ffa27 --- /dev/null +++ b/x/pricefeed/simulation/decoder_test.go @@ -0,0 +1,63 @@ +package simulation_test + +import ( + "fmt" + "testing" + "time" + + "github.com/cosmos/cosmos-sdk/codec" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/kava-labs/kava/app" + "github.com/kava-labs/kava/x/pricefeed/simulation" + "github.com/kava-labs/kava/x/pricefeed/types" + "github.com/stretchr/testify/suite" + cmn "github.com/tendermint/tendermint/libs/common" + tmtime "github.com/tendermint/tendermint/types/time" +) + +type decoderTest struct { + name string + expectedLog string +} + +type DecoderTestSuite struct { + suite.Suite + + tests []decoderTest +} + +func makeTestCodec() (cdc *codec.Codec) { + cdc = codec.New() + sdk.RegisterCodec(cdc) + codec.RegisterCrypto(cdc) + types.RegisterCodec(cdc) + return +} + +func (suite *DecoderTestSuite) TestDecodeStore() { + cdc := makeTestCodec() + price := types.NewCurrentPrice("bnb:usd", sdk.MustNewDecFromStr("12.0")) + _, addrs := app.GeneratePrivKeyAddressPairs(1) + rawPrices := types.PostedPrices{ + types.NewPostedPrice("bnb:usd", addrs[0], sdk.MustNewDecFromStr("12.0"), tmtime.Now().Add(time.Hour*2)), + } + kvPairs := cmn.KVPairs{ + cmn.KVPair{Key: types.CurrentPriceKey("bnb:usd"), Value: cdc.MustMarshalBinaryBare(price)}, + cmn.KVPair{Key: types.RawPriceKey("bnb:usd"), Value: cdc.MustMarshalBinaryBare(rawPrices)}, + } + + decoderTests := []decoderTest{ + decoderTest{"current price", fmt.Sprintf("%s\n%s", price, price)}, + decoderTest{"raw prices", fmt.Sprintf("%s\n%s", rawPrices, rawPrices)}, + } + + for i, t := range decoderTests { + suite.Run(t.name, func() { + suite.Equal(t.expectedLog, simulation.DecodeStore(cdc, kvPairs[i], kvPairs[i])) + }) + } +} + +func TestDecoderTestSuite(t *testing.T) { + suite.Run(t, new(DecoderTestSuite)) +} diff --git a/x/pricefeed/types/key.go b/x/pricefeed/types/key.go index a1107954..74a8a81b 100644 --- a/x/pricefeed/types/key.go +++ b/x/pricefeed/types/key.go @@ -15,16 +15,22 @@ const ( // DefaultParamspace default namestore DefaultParamspace = ModuleName +) + +var ( + // CurrentPricePrefix prefix for the current price of an asset + CurrentPricePrefix = []byte{0x00} // RawPriceFeedPrefix prefix for the raw pricefeed of an asset - RawPriceFeedPrefix = StoreKey + ":raw:" - - // CurrentPricePrefix prefix for the current price of an asset - CurrentPricePrefix = StoreKey + ":currentprice:" - - // MarketPrefix Prefix for the assets in the pricefeed system - MarketPrefix = StoreKey + ":markets" - - // OraclePrefix store prefix for the oracle accounts - OraclePrefix = StoreKey + ":oracles" + RawPriceFeedPrefix = []byte{0x01} ) + +// CurrentPriceKey returns the prefix for the current price +func CurrentPriceKey(marketID string) []byte { + return append(CurrentPricePrefix, []byte(marketID)...) +} + +// RawPriceKey returns the prefix for the raw price +func RawPriceKey(marketID string) []byte { + return append(RawPriceFeedPrefix, []byte(marketID)...) +} diff --git a/x/pricefeed/types/market.go b/x/pricefeed/types/market.go index a357db73..16231be8 100644 --- a/x/pricefeed/types/market.go +++ b/x/pricefeed/types/market.go @@ -46,6 +46,14 @@ type CurrentPrice struct { Price sdk.Dec `json:"price" yaml:"price"` } +// NewCurrentPrice returns an instance of CurrentPrice +func NewCurrentPrice(marketID string, price sdk.Dec) CurrentPrice { + return CurrentPrice{MarketID: marketID, Price: price} +} + +// CurrentPrices type for an array of CurrentPrice +type CurrentPrices []CurrentPrice + // PostedPrice price for market posted by a specific oracle type PostedPrice struct { MarketID string `json:"market_id" yaml:"market_id"` @@ -54,6 +62,19 @@ type PostedPrice struct { Expiry time.Time `json:"expiry" yaml:"expiry"` } +// NewPostedPrice returns a new PostedPrice +func NewPostedPrice(marketID string, oracle sdk.AccAddress, price sdk.Dec, expiry time.Time) PostedPrice { + return PostedPrice{ + MarketID: marketID, + OracleAddress: oracle, + Price: price, + Expiry: expiry, + } +} + +// PostedPrices type for an array of PostedPrice +type PostedPrices []PostedPrice + // implement fmt.Stringer func (cp CurrentPrice) String() string { return strings.TrimSpace(fmt.Sprintf(`Market ID: %s @@ -68,6 +89,15 @@ Price: %s Expiry: %s`, pp.MarketID, pp.OracleAddress, pp.Price, pp.Expiry)) } +// String implements fmt.Stringer +func (ps PostedPrices) String() string { + out := "Posted Prices:\n" + for _, p := range ps { + out += fmt.Sprintf("%s\n", p.String()) + } + return strings.TrimSpace(out) +} + // SortDecs provides the interface needed to sort sdk.Dec slices type SortDecs []sdk.Dec From 8f9aece875a9eed3771de92f156b0cd6ddf97409 Mon Sep 17 00:00:00 2001 From: Kevin Davis Date: Mon, 13 Apr 2020 14:08:14 -0400 Subject: [PATCH 2/2] address review comments --- x/pricefeed/genesis.go | 10 ++++++++-- x/pricefeed/keeper/keeper.go | 28 +++++++++++++++++++++------- x/pricefeed/keeper/keeper_test.go | 9 ++++++--- x/pricefeed/keeper/querier.go | 14 ++++++++------ 4 files changed, 43 insertions(+), 18 deletions(-) diff --git a/x/pricefeed/genesis.go b/x/pricefeed/genesis.go index 9919f6a7..476d4c56 100644 --- a/x/pricefeed/genesis.go +++ b/x/pricefeed/genesis.go @@ -25,7 +25,10 @@ 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 := keeper.GetRawPrices(ctx, market.MarketID) + 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 { @@ -44,7 +47,10 @@ func ExportGenesis(ctx sdk.Context, keeper Keeper) GenesisState { var postedPrices []PostedPrice for _, market := range keeper.GetMarkets(ctx) { - pp := keeper.GetRawPrices(ctx, market.MarketID) + pp, err := keeper.GetRawPrices(ctx, market.MarketID) + if err != nil { + panic(err) + } postedPrices = append(postedPrices, pp...) } diff --git a/x/pricefeed/keeper/keeper.go b/x/pricefeed/keeper/keeper.go index 393643a7..211e797a 100644 --- a/x/pricefeed/keeper/keeper.go +++ b/x/pricefeed/keeper/keeper.go @@ -46,7 +46,10 @@ func (k Keeper) SetPrice( // If the expiry is less than or equal to the current blockheight, we consider the price valid if expiry.After(ctx.BlockTime()) { store := ctx.KVStore(k.key) - prices := k.GetRawPrices(ctx, marketID) + prices, err := k.GetRawPrices(ctx, marketID) + if err != nil { + return types.PostedPrice{}, err + } var index int found := false for i := range prices { @@ -96,7 +99,10 @@ func (k Keeper) SetCurrentPrices(ctx sdk.Context, marketID string) sdk.Error { validPrevPrice = false } - prices := k.GetRawPrices(ctx, marketID) + prices, err := k.GetRawPrices(ctx, marketID) + if err != nil { + return err + } var notExpiredPrices types.CurrentPrices // filter out expired prices for _, v := range prices { @@ -174,8 +180,10 @@ func (k Keeper) GetCurrentPrice(ctx sdk.Context, marketID string) (types.Current return types.CurrentPrice{}, types.ErrNoValidPrice(k.codespace) } var price types.CurrentPrice - k.cdc.MustUnmarshalBinaryBare(bz, &price) - + err := k.cdc.UnmarshalBinaryBare(bz, &price) + if err != nil { + return types.CurrentPrice{}, sdk.ErrInternal(sdk.AppendMsgToErr("failed to unmarshal result", err.Error())) + } if price.Price.Equal(sdk.ZeroDec()) { return types.CurrentPrice{}, types.ErrNoValidPrice(k.codespace) } @@ -183,12 +191,18 @@ func (k Keeper) GetCurrentPrice(ctx sdk.Context, marketID string) (types.Current } // GetRawPrices fetches the set of all prices posted by oracles for an asset -func (k Keeper) GetRawPrices(ctx sdk.Context, marketID string) types.PostedPrices { +func (k Keeper) GetRawPrices(ctx sdk.Context, marketID string) (types.PostedPrices, sdk.Error) { store := ctx.KVStore(k.key) bz := store.Get(types.RawPriceKey(marketID)) + if bz == nil { + return types.PostedPrices{}, nil + } var prices types.PostedPrices - k.cdc.MustUnmarshalBinaryBare(bz, &prices) - return prices + err := k.cdc.UnmarshalBinaryBare(bz, &prices) + if err != nil { + return types.PostedPrices{}, sdk.ErrInternal(sdk.AppendMsgToErr("failed to unmarshal result", err.Error())) + } + return prices, nil } // Codespace return the codespace for the keeper diff --git a/x/pricefeed/keeper/keeper_test.go b/x/pricefeed/keeper/keeper_test.go index 0f79b026..2288fa87 100644 --- a/x/pricefeed/keeper/keeper_test.go +++ b/x/pricefeed/keeper/keeper_test.go @@ -67,7 +67,8 @@ func TestKeeper_GetSetPrice(t *testing.T) { time.Now().Add(1*time.Hour)) require.NoError(t, err) // Get raw prices - rawPrices := keeper.GetRawPrices(ctx, "tstusd") + rawPrices, err := keeper.GetRawPrices(ctx, "tstusd") + require.NoError(t, err) require.Equal(t, len(rawPrices), 1) require.Equal(t, rawPrices[0].Price.Equal(sdk.MustNewDecFromStr("0.33")), true) // Set price by oracle 2 @@ -77,7 +78,8 @@ func TestKeeper_GetSetPrice(t *testing.T) { time.Now().Add(time.Hour*1)) require.NoError(t, err) - rawPrices = keeper.GetRawPrices(ctx, "tstusd") + rawPrices, err = keeper.GetRawPrices(ctx, "tstusd") + require.NoError(t, err) require.Equal(t, len(rawPrices), 2) require.Equal(t, rawPrices[1].Price.Equal(sdk.MustNewDecFromStr("0.35")), true) @@ -87,7 +89,8 @@ func TestKeeper_GetSetPrice(t *testing.T) { sdk.MustNewDecFromStr("0.37"), time.Now().Add(time.Hour*1)) require.NoError(t, err) - rawPrices = keeper.GetRawPrices(ctx, "tstusd") + rawPrices, err = keeper.GetRawPrices(ctx, "tstusd") + require.NoError(t, err) require.Equal(t, rawPrices[0].Price.Equal(sdk.MustNewDecFromStr("0.37")), true) } diff --git a/x/pricefeed/keeper/querier.go b/x/pricefeed/keeper/querier.go index 94015535..1a111fdf 100644 --- a/x/pricefeed/keeper/querier.go +++ b/x/pricefeed/keeper/querier.go @@ -56,18 +56,20 @@ func queryPrice(ctx sdk.Context, req abci.RequestQuery, keeper Keeper) (res []by func queryRawPrices(ctx sdk.Context, req abci.RequestQuery, keeper Keeper) (res []byte, sdkErr sdk.Error) { var requestParams types.QueryWithMarketIDParams - err := keeper.cdc.UnmarshalJSON(req.Data, &requestParams) - if err != nil { - return nil, sdk.ErrInternal(fmt.Sprintf("failed to parse params: %s", err)) + err2 := keeper.cdc.UnmarshalJSON(req.Data, &requestParams) + if err2 != nil { + return nil, sdk.ErrInternal(fmt.Sprintf("failed to parse params: %s", err2)) } _, found := keeper.GetMarket(ctx, requestParams.MarketID) if !found { return []byte{}, sdk.ErrUnknownRequest("asset not found") } - rawPrices := keeper.GetRawPrices(ctx, requestParams.MarketID) - - bz, err := codec.MarshalJSONIndent(keeper.cdc, rawPrices) + rawPrices, err := keeper.GetRawPrices(ctx, requestParams.MarketID) if err != nil { + return nil, err + } + bz, err2 := codec.MarshalJSONIndent(keeper.cdc, rawPrices) + if err2 != nil { panic("could not marshal result to JSON") }