CDP revisions (#508)

* address review comments

* add kavadist to modaccount check

* cdp and deposit validation in genesis

* cleanup genesis validation

* add validation test for types

* don't error on augmented cdp loading

* simplify collateral auction logic

x/cdp/abci.go

@@ -1,13 +1,9 @@
 package cdp
 
 import (
-	"fmt"
-
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	abci "github.com/tendermint/tendermint/abci/types"
-
-	"github.com/kava-labs/kava/x/cdp/types"
 )
 
 // BeginBlocker compounds the debt in outstanding cdps and liquidates cdps that are below the required collateralization ratio
@@ -22,52 +18,33 @@ func BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock, k Keeper) {
 
 	for _, cp := range params.CollateralParams {
 
+		ok := k.UpdatePricefeedStatus(ctx, cp.MarketID)
+		if !ok {
+			continue
+		}
+
 		err := k.UpdateFeesForAllCdps(ctx, cp.Denom)
 
-		// handle if an error is returned then propagate up
 		if err != nil {
-			ctx.EventManager().EmitEvent(
+			panic(err)
-				sdk.NewEvent(
-					EventTypeBeginBlockerFatal,
-					sdk.NewAttribute(sdk.AttributeKeyModule, fmt.Sprintf("%s", ModuleName)),
-					sdk.NewAttribute(types.AttributeKeyError, fmt.Sprintf("%s", err)),
-				),
-			)
 		}
 
 		err = k.LiquidateCdps(ctx, cp.MarketID, cp.Denom, cp.LiquidationRatio)
 		if err != nil {
-			ctx.EventManager().EmitEvent(
+			panic(err)
-				sdk.NewEvent(
-					EventTypeBeginBlockerFatal,
-					sdk.NewAttribute(sdk.AttributeKeyModule, fmt.Sprintf("%s", ModuleName)),
-					sdk.NewAttribute(types.AttributeKeyError, fmt.Sprintf("%s", err)),
-				),
-			)
 		}
 	}
 	err := k.RunSurplusAndDebtAuctions(ctx)
 	if err != nil {
-		ctx.EventManager().EmitEvent(
+		panic(err)
-			sdk.NewEvent(
-				EventTypeBeginBlockerFatal,
-				sdk.NewAttribute(sdk.AttributeKeyModule, fmt.Sprintf("%s", ModuleName)),
-				sdk.NewAttribute(types.AttributeKeyError, fmt.Sprintf("%s", err)),
-			),
-		)
 	}
 	distTimeElapsed := sdk.NewInt(ctx.BlockTime().Unix() - previousDistTime.Unix())
-	if distTimeElapsed.GTE(sdk.NewInt(int64(params.SavingsDistributionFrequency.Seconds()))) {
+	if !distTimeElapsed.GTE(sdk.NewInt(int64(params.SavingsDistributionFrequency.Seconds()))) {
-		err := k.DistributeSavingsRate(ctx, params.DebtParam.Denom)
+		return
+	}
+	err = k.DistributeSavingsRate(ctx, params.DebtParam.Denom)
 	if err != nil {
-			ctx.EventManager().EmitEvent(
+		panic(err)
-				sdk.NewEvent(
-					EventTypeBeginBlockerFatal,
-					sdk.NewAttribute(sdk.AttributeKeyModule, fmt.Sprintf("%s", ModuleName)),
-					sdk.NewAttribute(types.AttributeKeyError, fmt.Sprintf("%s", err)),
-				),
-			)
 	}
 	k.SetPreviousSavingsDistribution(ctx, ctx.BlockTime())
 }
-}

x/cdp/doc.go

@@ -1,27 +0,0 @@
-/*
-Package CDP manages the storage of Collateralized Debt Positions. It handles their creation, modification, and stores the global state of all CDPs.
-
-Notes
- - sdk.Int is used for all the number types to maintain compatibility with internal type of sdk.Coin - saves type conversion when doing maths.
-   Also it allows for changes to a CDP to be expressed as a +ve or -ve number.
- - Only allowing one CDP per account-collateralDenom pair for now to keep things simple.
- - Genesis forces the global debt to start at zero, ie no stable coins in existence. This could be changed.
- - The cdp module fulfills the bank keeper interface and keeps track of the liquidator module's coins. This won't be needed with module accounts.
- - GetCDPs does not return an iterator, but instead reads out (potentially) all CDPs from the store. This isn't a huge performance concern as it is never used during a block, only for querying.
-   An iterator could be created, following the queue style construct in gov and auction, where CDP IDs are stored under ordered keys.
-   These keys could be a collateral-denom:collateral-ratio so that it is efficient to obtain the undercollateralized CDP for a given price and liquidation ratio.
-   However creating a byte sortable representation of a collateral ratio wasn't very easy so the simpler approach was chosen.
-
-TODO
- - A shorter name for an under-collateralized CDP would shorten a lot of function names
- - remove fake bank keeper and setup a proper liquidator module account
- - what happens if a collateral type is removed from the list of allowed ones?
- - Should the values used to generate a key for a stored struct be in the struct?
- - Add constants for the module and route names
- - Many more TODOs in the code
- - add more aggressive test cases
- - tags
- - custom error types, codespace
-
-*/
-package cdp

x/cdp/genesis.go

@@ -42,6 +42,9 @@ func InitGenesis(ctx sdk.Context, k Keeper, pk types.PricefeedKeeper, sk types.S
 		if !found {
 			panic(fmt.Sprintf("%s collateral not found in pricefeed", col.Denom))
 		}
+		// sets the status of the pricefeed in the store
+		// if pricefeed not active, debt operations are paused
+		_ = k.UpdatePricefeedStatus(ctx, col.MarketID)
 	}
 
 	k.SetParams(ctx, gs.Params)

x/cdp/keeper/auctions.go

@@ -70,26 +70,18 @@ func (k Keeper) NetSurplusAndDebt(ctx sdk.Context) error {
 	if netAmount.IsZero() {
 		return nil
 	}
+
 	// burn debt coins equal to netAmount
 	err := k.supplyKeeper.BurnCoins(ctx, types.LiquidatorMacc, sdk.NewCoins(sdk.NewCoin(k.GetDebtDenom(ctx), netAmount)))
 	if err != nil {
 		return err
 	}
-	// burn stable coins equal to netAmount
+
+	// burn stable coins equal to min(balance, netAmount)
 	dp := k.GetParams(ctx).DebtParam
 	balance := k.supplyKeeper.GetModuleAccount(ctx, types.LiquidatorMacc).GetCoins().AmountOf(dp.Denom)
-	if balance.LT(netAmount) {
+	burnAmount := sdk.MinInt(balance, netAmount)
-		err = k.supplyKeeper.BurnCoins(ctx, types.LiquidatorMacc, sdk.NewCoins(sdk.NewCoin(dp.Denom, balance)))
+	return k.supplyKeeper.BurnCoins(ctx, types.LiquidatorMacc, sdk.NewCoins(sdk.NewCoin(dp.Denom, burnAmount)))
-		if err != nil {
-			return err
-		}
-		return nil
-	}
-	err = k.supplyKeeper.BurnCoins(ctx, types.LiquidatorMacc, sdk.NewCoins(sdk.NewCoin(dp.Denom, netAmount)))
-	if err != nil {
-		return err
-	}
-	return nil
 }
 
 // GetTotalSurplus returns the total amount of surplus tokens held by the liquidator module account
@@ -124,12 +116,10 @@ func (k Keeper) RunSurplusAndDebtAuctions(ctx sdk.Context) error {
 	}
 
 	surplus := k.supplyKeeper.GetModuleAccount(ctx, types.LiquidatorMacc).GetCoins().AmountOf(params.DebtParam.Denom)
-	if surplus.GTE(params.SurplusAuctionThreshold) {
+	if !surplus.GTE(params.SurplusAuctionThreshold) {
-		surplusLot := sdk.NewCoin(params.DebtParam.Denom, surplus)
-		_, err := k.auctionKeeper.StartSurplusAuction(ctx, types.LiquidatorMacc, surplusLot, k.GetGovDenom(ctx))
-		if err != nil {
-			return err
-		}
-	}
 		return nil
 	}
+	surplusLot := sdk.NewCoin(params.DebtParam.Denom, surplus)
+	_, err := k.auctionKeeper.StartSurplusAuction(ctx, types.LiquidatorMacc, surplusLot, k.GetGovDenom(ctx))
+	return err
+}

x/cdp/keeper/cdp.go

@@ -64,6 +64,19 @@ func (k Keeper) AddCdp(ctx sdk.Context, owner sdk.AccAddress, collateral sdk.Coi
 		panic(err)
 	}
 
+	// update total principal for input collateral type
+	k.IncrementTotalPrincipal(ctx, collateral.Denom, principal)
+
+	// set the cdp, deposit, and indexes in the store
+	collateralToDebtRatio := k.CalculateCollateralToDebtRatio(ctx, collateral, principal)
+	err = k.SetCdpAndCollateralRatioIndex(ctx, cdp, collateralToDebtRatio)
+	if err != nil {
+		return err
+	}
+	k.IndexCdpByOwner(ctx, cdp)
+	k.SetDeposit(ctx, deposit)
+	k.SetNextCdpID(ctx, id+1)
+
 	// emit events for cdp creation, deposit, and draw
 	ctx.EventManager().EmitEvent(
 		sdk.NewEvent(
@@ -86,18 +99,6 @@ func (k Keeper) AddCdp(ctx sdk.Context, owner sdk.AccAddress, collateral sdk.Coi
 		),
 	)
 
-	// update total principal for input collateral type
-	k.IncrementTotalPrincipal(ctx, collateral.Denom, principal)
-
-	// set the cdp, deposit, and indexes in the store
-	collateralToDebtRatio := k.CalculateCollateralToDebtRatio(ctx, collateral, principal)
-	err = k.SetCdpAndCollateralRatioIndex(ctx, cdp, collateralToDebtRatio)
-	if err != nil {
-		return err
-	}
-	k.IndexCdpByOwner(ctx, cdp)
-	k.SetDeposit(ctx, deposit)
-	k.SetNextCdpID(ctx, id+1)
 	return nil
 }
 
@@ -347,10 +348,14 @@ func (k Keeper) SetGovDenom(ctx sdk.Context, denom string) {
 
 // ValidateCollateral validates that a collateral is valid for use in cdps
 func (k Keeper) ValidateCollateral(ctx sdk.Context, collateral sdk.Coin) error {
-	_, found := k.GetCollateral(ctx, collateral.Denom)
+	cp, found := k.GetCollateral(ctx, collateral.Denom)
 	if !found {
 		return sdkerrors.Wrap(types.ErrCollateralNotSupported, collateral.Denom)
 	}
+	ok := k.GetMarketStatus(ctx, cp.MarketID)
+	if !ok {
+		return sdkerrors.Wrap(types.ErrPricefeedDown, collateral.Denom)
+	}
 	return nil
 }
 
@@ -423,11 +428,11 @@ func (k Keeper) CalculateCollateralToDebtRatio(ctx sdk.Context, collateral sdk.C
 }
 
 // LoadAugmentedCDP creates a new augmented CDP from an existing CDP
-func (k Keeper) LoadAugmentedCDP(ctx sdk.Context, cdp types.CDP) (types.AugmentedCDP, error) {
+func (k Keeper) LoadAugmentedCDP(ctx sdk.Context, cdp types.CDP) types.AugmentedCDP {
 	// calculate collateralization ratio
 	collateralizationRatio, err := k.CalculateCollateralizationRatio(ctx, cdp.Collateral, cdp.Principal, cdp.AccumulatedFees)
 	if err != nil {
-		return types.AugmentedCDP{}, err
+		return types.AugmentedCDP{CDP: cdp}
 	}
 
 	// total debt is the sum of all outstanding principal and fees
@@ -442,7 +447,7 @@ func (k Keeper) LoadAugmentedCDP(ctx sdk.Context, cdp types.CDP) (types.Augmente
 
 	// create new augmuented cdp
 	augmentedCDP := types.NewAugmentedCDP(cdp, collateralValueInDebt, collateralizationRatio)
-	return augmentedCDP, nil
+	return augmentedCDP
 }
 
 // CalculateCollateralizationRatio returns the collateralization ratio of the input collateral to the input debt plus fees
@@ -480,6 +485,32 @@ func (k Keeper) CalculateCollateralizationRatioFromAbsoluteRatio(ctx sdk.Context
 	return respectiveCollateralRatio, nil
 }
 
+// SetMarketStatus sets the status of the input market, true means the market is up and running, false means it is down
+func (k Keeper) SetMarketStatus(ctx sdk.Context, marketID string, up bool) {
+	store := prefix.NewStore(ctx.KVStore(k.key), types.PricefeedStatusKeyPrefix)
+	store.Set([]byte(marketID), k.cdc.MustMarshalBinaryBare(up))
+	return
+}
+
+// GetMarketStatus returns true if the market has a price, otherwise false
+func (k Keeper) GetMarketStatus(ctx sdk.Context, marketID string) (up bool) {
+	store := prefix.NewStore(ctx.KVStore(k.key), types.PricefeedStatusKeyPrefix)
+	bz := store.Get([]byte(marketID))
+	k.cdc.MustUnmarshalBinaryBare(bz, &up)
+	return up
+}
+
+// UpdatePricefeedStatus determines if the price of an asset is available and updates the global status of the market
+func (k Keeper) UpdatePricefeedStatus(ctx sdk.Context, marketID string) (ok bool) {
+	_, err := k.pricefeedKeeper.GetCurrentPrice(ctx, marketID)
+	if err != nil {
+		k.SetMarketStatus(ctx, marketID, false)
+		return false
+	}
+	k.SetMarketStatus(ctx, marketID, true)
+	return true
+}
+
 // converts the input collateral to base units (ie multiplies the input by 10^(-ConversionFactor))
 func (k Keeper) convertCollateralToBaseUnits(ctx sdk.Context, collateral sdk.Coin) (baseUnits sdk.Dec) {
 	cp, _ := k.GetCollateral(ctx, collateral.Denom)

x/cdp/keeper/cdp_test.go

@@ -64,10 +64,14 @@ func (suite *CdpTestSuite) TestAddCdp() {
 	pk := suite.app.GetPriceFeedKeeper()
 	err = pk.SetCurrentPrices(ctx, "xrp:usd")
 	suite.Error(err)
+	ok := suite.keeper.UpdatePricefeedStatus(ctx, "xrp:usd")
+	suite.False(ok)
 	err = suite.keeper.AddCdp(ctx, addrs[0], c("xrp", 100000000), c("usdx", 10000000))
-	suite.Error(err) // no prices in pricefeed
+	suite.Require().True(errors.Is(err, types.ErrPricefeedDown))
 
 	err = pk.SetCurrentPrices(suite.ctx, "xrp:usd")
+	ok = suite.keeper.UpdatePricefeedStatus(suite.ctx, "xrp:usd")
+	suite.True(ok)
 	suite.NoError(err)
 	err = suite.keeper.AddCdp(suite.ctx, addrs[0], c("xrp", 100000000), c("usdx", 10000000))
 	suite.NoError(err)

x/cdp/keeper/deposit.go

@@ -11,7 +11,8 @@ import (
 )
 
 // DepositCollateral adds collateral to a cdp
-func (k Keeper) DepositCollateral(ctx sdk.Context, owner sdk.AccAddress, depositor sdk.AccAddress, collateral sdk.Coin) error {
+func (k Keeper) DepositCollateral(ctx sdk.Context, owner, depositor sdk.AccAddress, collateral sdk.Coin) error {
+	// check that collateral exists and has a functioning pricefeed
 	err := k.ValidateCollateral(ctx, collateral)
 	if err != nil {
 		return err
@@ -54,7 +55,7 @@ func (k Keeper) DepositCollateral(ctx sdk.Context, owner sdk.AccAddress, deposit
 }
 
 // WithdrawCollateral removes collateral from a cdp if it does not put the cdp below the liquidation ratio
-func (k Keeper) WithdrawCollateral(ctx sdk.Context, owner sdk.AccAddress, depositor sdk.AccAddress, collateral sdk.Coin) error {
+func (k Keeper) WithdrawCollateral(ctx sdk.Context, owner, depositor sdk.AccAddress, collateral sdk.Coin) error {
 	err := k.ValidateCollateral(ctx, collateral)
 	if err != nil {
 		return err
@@ -102,6 +103,7 @@ func (k Keeper) WithdrawCollateral(ctx sdk.Context, owner sdk.AccAddress, deposi
 	}
 
 	deposit.Amount = deposit.Amount.Sub(collateral)
+	// delete deposits if amount is 0
 	if deposit.Amount.IsZero() {
 		k.DeleteDeposit(ctx, deposit.CdpID, deposit.Depositor)
 	} else {

x/cdp/keeper/draw.go

@@ -84,7 +84,7 @@ func (k Keeper) RepayPrincipal(ctx sdk.Context, owner sdk.AccAddress, denom stri
 		return sdkerrors.Wrapf(types.ErrCdpNotFound, "owner %s, denom %s", owner, denom)
 	}
 
-	err := k.ValidatePaymentCoins(ctx, cdp, payment, cdp.Principal.Add(cdp.AccumulatedFees))
+	err := k.ValidatePaymentCoins(ctx, cdp, payment)
 	if err != nil {
 		return err
 	}
@@ -92,6 +92,10 @@ func (k Keeper) RepayPrincipal(ctx sdk.Context, owner sdk.AccAddress, denom stri
 	// calculate fee and principal payment
 	feePayment, principalPayment := k.calculatePayment(ctx, cdp.Principal.Add(cdp.AccumulatedFees), cdp.AccumulatedFees, payment)
 
+	err = k.validatePrincipalPayment(ctx, cdp, principalPayment)
+	if err != nil {
+		return err
+	}
 	// send the payment from the sender to the cpd module
 	err = k.supplyKeeper.SendCoinsFromAccountToModule(ctx, owner, types.ModuleName, sdk.NewCoins(feePayment.Add(principalPayment)))
 	if err != nil {
@@ -168,18 +172,15 @@ func (k Keeper) RepayPrincipal(ctx sdk.Context, owner sdk.AccAddress, denom stri
 }
 
 // ValidatePaymentCoins validates that the input coins are valid for repaying debt
-func (k Keeper) ValidatePaymentCoins(ctx sdk.Context, cdp types.CDP, payment sdk.Coin, debt sdk.Coin) error {
+func (k Keeper) ValidatePaymentCoins(ctx sdk.Context, cdp types.CDP, payment sdk.Coin) error {
+	debt := cdp.Principal.Add(cdp.AccumulatedFees)
 	if payment.Denom != debt.Denom {
 		return sdkerrors.Wrapf(types.ErrInvalidPayment, "cdp %d: expected %s, got %s", cdp.ID, debt.Denom, payment.Denom)
 	}
-	dp, found := k.GetDebtParam(ctx, payment.Denom)
+	_, found := k.GetDebtParam(ctx, payment.Denom)
 	if !found {
 		return sdkerrors.Wrapf(types.ErrInvalidPayment, "payment denom %s not found", payment.Denom)
 	}
-	proposedBalance := cdp.Principal.Amount.Sub(payment.Amount)
-	if proposedBalance.GT(sdk.ZeroInt()) && proposedBalance.LT(dp.DebtFloor) {
-		return sdkerrors.Wrapf(types.ErrBelowDebtFloor, "proposed %s < minimum %s", sdk.NewCoin(payment.Denom, proposedBalance), dp.DebtFloor)
-	}
 	return nil
 }
 
@@ -195,12 +196,17 @@ func (k Keeper) ReturnCollateral(ctx sdk.Context, cdp types.CDP) {
 	}
 }
 
-func (k Keeper) calculatePayment(ctx sdk.Context, owed sdk.Coin, fees sdk.Coin, payment sdk.Coin) (sdk.Coin, sdk.Coin) {
+// calculatePayment divides the input payment into the portions that will be used to repay fees and principal
+// owed - Principal + AccumulatedFees
+// fees - AccumulatedFees
+// CONTRACT: owned and payment denoms must be checked before calling this function.
+func (k Keeper) calculatePayment(ctx sdk.Context, owed, fees, payment sdk.Coin) (sdk.Coin, sdk.Coin) {
 	// divides repayment into principal and fee components, with fee payment applied first.
 
 	feePayment := sdk.NewCoin(payment.Denom, sdk.ZeroInt())
 	principalPayment := sdk.NewCoin(payment.Denom, sdk.ZeroInt())
 	var overpayment sdk.Coin
+	// return zero value coins if payment amount is invalid
 	if !payment.Amount.IsPositive() {
 		return feePayment, principalPayment
 	}
@@ -222,3 +228,14 @@ func (k Keeper) calculatePayment(ctx sdk.Context, owed sdk.Coin, fees sdk.Coin,
 	}
 	return feePayment, principalPayment
 }
+
+// validatePrincipalPayment checks that the payment is either full or does not put the cdp below the debt floor
+// CONTRACT: payment denom must be checked before calling this function.
+func (k Keeper) validatePrincipalPayment(ctx sdk.Context, cdp types.CDP, payment sdk.Coin) error {
+	proposedBalance := cdp.Principal.Amount.Sub(payment.Amount)
+	dp, _ := k.GetDebtParam(ctx, payment.Denom)
+	if proposedBalance.GT(sdk.ZeroInt()) && proposedBalance.LT(dp.DebtFloor) {
+		return sdkerrors.Wrapf(types.ErrBelowDebtFloor, "proposed %s < minimum %s", sdk.NewCoin(payment.Denom, proposedBalance), dp.DebtFloor)
+	}
+	return nil
+}

x/cdp/keeper/fees.go

@@ -78,7 +78,7 @@ func (k Keeper) UpdateFeesForAllCdps(ctx sdk.Context, collateralDenom string) er
 			return true
 		}
 
-		// now add the new fees fees to the accumulated fees for the cdp
+		// now add the new fees to the accumulated fees for the cdp
 		cdp.AccumulatedFees = cdp.AccumulatedFees.Add(newFees)
 
 		// and set the fees updated time to the current block time since we just updated it
@@ -92,11 +92,8 @@ func (k Keeper) UpdateFeesForAllCdps(ctx sdk.Context, collateralDenom string) er
 		}
 		return false // this returns true when you want to stop iterating. Since we want to iterate through all we return false
 	})
-	if iterationErr != nil {
 	return iterationErr
 }
-	return nil
-}
 
 // IncrementTotalPrincipal increments the total amount of debt that has been drawn with that collateral type
 func (k Keeper) IncrementTotalPrincipal(ctx sdk.Context, collateralDenom string, principal sdk.Coin) {

x/cdp/keeper/querier.go

@@ -1,8 +1,6 @@
 package keeper
 
 import (
-	"fmt"
-
 	abci "github.com/tendermint/tendermint/abci/types"
 
 	"github.com/cosmos/cosmos-sdk/codec"
@@ -50,10 +48,7 @@ func queryGetCdp(ctx sdk.Context, req abci.RequestQuery, keeper Keeper) ([]byte,
 		return nil, sdkerrors.Wrapf(types.ErrCdpNotFound, "owner %s, denom %s", requestParams.Owner, requestParams.CollateralDenom)
 	}
 
-	augmentedCDP, err := keeper.LoadAugmentedCDP(ctx, cdp)
+	augmentedCDP := keeper.LoadAugmentedCDP(ctx, cdp)
-	if err != nil {
-		return nil, sdkerrors.Wrap(types.ErrLoadingAugmentedCDP, fmt.Sprintf("%v: %d", err.Error(), cdp.ID))
-	}
 
 	bz, err := codec.MarshalJSONIndent(types.ModuleCdc, augmentedCDP)
 	if err != nil {
@@ -112,11 +107,9 @@ func queryGetCdpsByRatio(ctx sdk.Context, req abci.RequestQuery, keeper Keeper)
 	// augment CDPs by adding collateral value and collateralization ratio
 	var augmentedCDPs types.AugmentedCDPs
 	for _, cdp := range cdps {
-		augmentedCDP, err := keeper.LoadAugmentedCDP(ctx, cdp)
+		augmentedCDP := keeper.LoadAugmentedCDP(ctx, cdp)
-		if err == nil {
 		augmentedCDPs = append(augmentedCDPs, augmentedCDP)
 	}
-	}
 	bz, err := codec.MarshalJSONIndent(types.ModuleCdc, augmentedCDPs)
 	if err != nil {
 		return nil, sdkerrors.Wrap(sdkerrors.ErrJSONMarshal, err.Error())
@@ -140,11 +133,9 @@ func queryGetCdpsByDenom(ctx sdk.Context, req abci.RequestQuery, keeper Keeper)
 	// augment CDPs by adding collateral value and collateralization ratio
 	var augmentedCDPs types.AugmentedCDPs
 	for _, cdp := range cdps {
-		augmentedCDP, err := keeper.LoadAugmentedCDP(ctx, cdp)
+		augmentedCDP := keeper.LoadAugmentedCDP(ctx, cdp)
-		if err == nil {
 		augmentedCDPs = append(augmentedCDPs, augmentedCDP)
 	}
-	}
 	bz, err := codec.MarshalJSONIndent(types.ModuleCdc, augmentedCDPs)
 	if err != nil {
 		return nil, sdkerrors.Wrap(sdkerrors.ErrJSONMarshal, err.Error())

x/cdp/keeper/querier_test.go

@@ -101,8 +101,7 @@ func (suite *QuerierTestSuite) SetupTest() {
 		c, f := suite.keeper.GetCDP(suite.ctx, collateral, uint64(j+1))
 		suite.True(f)
 		cdps[j] = c
-		aCDP, err := suite.keeper.LoadAugmentedCDP(suite.ctx, c)
+		aCDP := suite.keeper.LoadAugmentedCDP(suite.ctx, c)
-		suite.NoError(err)
 		augmentedCDPs[j] = aCDP
 	}
 

x/cdp/keeper/savings.go

@@ -12,6 +12,7 @@ import (
 
 	auctiontypes "github.com/kava-labs/kava/x/auction/types"
 	"github.com/kava-labs/kava/x/cdp/types"
+	kdtypes "github.com/kava-labs/kava/x/kavadist/types"
 )
 
 // DistributeSavingsRate distributes surplus that has accumulated in the liquidator account to address holding stable coins according the the savings rate
@@ -60,11 +61,8 @@ func (k Keeper) DistributeSavingsRate(ctx sdk.Context, debtDenom string) error {
 		surplusDistributed = surplusDistributed.Add(interest)
 		return false
 	})
-	if iterationErr != nil {
 	return iterationErr
 }
-	return nil
-}
 
 // GetPreviousSavingsDistribution get the time of the previous savings rate distribution
 func (k Keeper) GetPreviousSavingsDistribution(ctx sdk.Context) (distTime time.Time, found bool) {
@@ -92,6 +90,7 @@ func (k Keeper) getModuleAccountCoins(ctx sdk.Context, denom string) sdk.Coins {
 	auctionMaccCoinAmount := k.supplyKeeper.GetModuleAccount(ctx, auctiontypes.ModuleName).GetCoins().AmountOf(denom)
 	liquidatorMaccCoinAmount := k.supplyKeeper.GetModuleAccount(ctx, types.LiquidatorMacc).GetCoins().AmountOf(denom)
 	feeMaccCoinAmount := k.supplyKeeper.GetModuleAccount(ctx, authtypes.FeeCollectorName).GetCoins().AmountOf(denom)
-	totalModAccountAmount := savingsRateMaccCoinAmount.Add(cdpMaccCoinAmount).Add(auctionMaccCoinAmount).Add(liquidatorMaccCoinAmount).Add(feeMaccCoinAmount)
+	kavadistMaccCoinAmount := k.supplyKeeper.GetModuleAccount(ctx, kdtypes.ModuleName).GetCoins().AmountOf(denom)
+	totalModAccountAmount := savingsRateMaccCoinAmount.Add(cdpMaccCoinAmount).Add(auctionMaccCoinAmount).Add(liquidatorMaccCoinAmount).Add(feeMaccCoinAmount).Add(kavadistMaccCoinAmount)
 	return sdk.NewCoins(sdk.NewCoin(denom, totalModAccountAmount))
 }

x/cdp/spec/01_concepts.md

@@ -11,7 +11,7 @@ Once created, stable assets are free to be transferred between users, but a CDP
 User interactions with this module:
 
 - create a new CDP by depositing a supported coin as collateral and minting debt
-- deposit to a CDP controlled a different owner address
+- deposit to a CDP controlled by a different owner address
 - withdraw deposited collateral, if it doesn't put the CDP below the liquidation ratio
 - issue stable coins from this CDP (up to a fraction of the value of the collateral)
 - repay debt by paying back stable coins (including paying any fees accrued)
@@ -73,4 +73,10 @@ The CDP module relies on a supply keeper to move assets between its module accou
 
 ## Dependency: pricefeed
 
-The CDP module needs to know the current price of collateral assets in order to determine if CDPs are under collateralized. This is provided by a "pricefeed" module that returns a price for a given collateral in units (usually US Dollars) which are the target for the stable asset.
+The CDP module needs to know the current price of collateral assets in order to determine if CDPs are under collateralized. This is provided by a "pricefeed" module that returns a price for a given collateral in units (usually US Dollars) which are the target for the stable asset. The status of the pricefeed for each collateral is checked at the beginning of each block. In the event that the pricefeed does not return a price for a collateral asset:
+
+1. Liquidation of CDPs is suspended until a price is reported
+2. Accumulation of fees is suspended until a price is reported
+3. Deposits and withdrawals of collateral are suspended until a price is reported
+4. Creation of new CDPs is suspended until a price is reported
+5. Drawing of additional debt off of existing CDPs is suspended until a price is reported

x/cdp/spec/04_begin_block.md

@@ -2,6 +2,8 @@
 
 At the start of every block the BeginBlocker of the cdp module:
 
+- updates the status of the pricefeed for each collateral asset
+- If the pricefeed is active (reporting a price):
   - updates fees for CDPs
   - liquidates CDPs under the collateral ratio
 - nets out system debt and, if necessary, starts auctions to re-balance it

x/cdp/types/cdp.go

@@ -1,11 +1,13 @@
 package types
 
 import (
+	"errors"
 	"fmt"
 	"strings"
 	"time"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
 
 // CDP is the state of a single collateralized debt position.
@@ -51,6 +53,29 @@ func (cdp CDP) String() string {
 	))
 }
 
+// Validate performs a basic validation of the CDP fields.
+func (cdp CDP) Validate() error {
+	if cdp.ID == 0 {
+		return errors.New("cdp id cannot be 0")
+	}
+	if cdp.Owner.Empty() {
+		return errors.New("cdp owner cannot be empty")
+	}
+	if !cdp.Collateral.IsValid() {
+		return sdkerrors.Wrapf(sdkerrors.ErrInvalidCoins, "collateral %s", cdp.Collateral)
+	}
+	if !cdp.Principal.IsValid() {
+		return sdkerrors.Wrapf(sdkerrors.ErrInvalidCoins, "principal %s", cdp.Principal)
+	}
+	if !cdp.AccumulatedFees.IsValid() {
+		return sdkerrors.Wrapf(sdkerrors.ErrInvalidCoins, "accumulated fees %s", cdp.AccumulatedFees)
+	}
+	if cdp.FeesUpdated.IsZero() {
+		return errors.New("cdp updated fee time cannot be zero")
+	}
+	return nil
+}
+
 // CDPs a collection of CDP objects
 type CDPs []CDP
 
@@ -63,6 +88,16 @@ func (cdps CDPs) String() string {
 	return out
 }
 
+// Validate validates each CDP
+func (cdps CDPs) Validate() error {
+	for _, cdp := range cdps {
+		if err := cdp.Validate(); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // AugmentedCDP provides additional information about an active CDP
 type AugmentedCDP struct {
 	CDP                    `json:"cdp" yaml:"cdp"`

x/cdp/types/cdp_test.go

@@ -0,0 +1,165 @@
+package types_test
+
+import (
+	"math/rand"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/suite"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	"github.com/tendermint/tendermint/crypto/secp256k1"
+	tmtime "github.com/tendermint/tendermint/types/time"
+
+	"github.com/kava-labs/kava/x/cdp/types"
+)
+
+type CdpValidationSuite struct {
+	suite.Suite
+
+	addrs []sdk.AccAddress
+}
+
+func (suite *CdpValidationSuite) SetupTest() {
+	r := rand.New(rand.NewSource(12345))
+	privkeySeed := make([]byte, 15)
+	r.Read(privkeySeed)
+	addr := sdk.AccAddress(secp256k1.GenPrivKeySecp256k1(privkeySeed).PubKey().Address())
+	suite.addrs = []sdk.AccAddress{addr}
+}
+
+func (suite *CdpValidationSuite) TestCdpValidation() {
+	type errArgs struct {
+		expectPass bool
+		contains   string
+	}
+	testCases := []struct {
+		name    string
+		cdp     types.CDP
+		errArgs errArgs
+	}{
+		{
+			name: "valid cdp",
+			cdp:  types.NewCDP(1, suite.addrs[0], sdk.NewInt64Coin("bnb", 100000), sdk.NewInt64Coin("usdx", 100000), tmtime.Now()),
+			errArgs: errArgs{
+				expectPass: true,
+				contains:   "",
+			},
+		},
+		{
+			name: "invalid cdp id",
+			cdp:  types.NewCDP(0, suite.addrs[0], sdk.NewInt64Coin("bnb", 100000), sdk.NewInt64Coin("usdx", 100000), tmtime.Now()),
+			errArgs: errArgs{
+				expectPass: false,
+				contains:   "cdp id cannot be 0",
+			},
+		},
+		{
+			name: "invalid collateral",
+			cdp:  types.CDP{1, suite.addrs[0], sdk.Coin{"", sdk.NewInt(100)}, sdk.Coin{"usdx", sdk.NewInt(100)}, sdk.Coin{"usdx", sdk.NewInt(0)}, tmtime.Now()},
+			errArgs: errArgs{
+				expectPass: false,
+				contains:   "invalid coins: collateral",
+			},
+		},
+		{
+			name: "invalid prinicpal",
+			cdp:  types.CDP{1, suite.addrs[0], sdk.Coin{"xrp", sdk.NewInt(100)}, sdk.Coin{"", sdk.NewInt(100)}, sdk.Coin{"usdx", sdk.NewInt(0)}, tmtime.Now()},
+			errArgs: errArgs{
+				expectPass: false,
+				contains:   "invalid coins: principal",
+			},
+		},
+		{
+			name: "invalid fees",
+			cdp:  types.CDP{1, suite.addrs[0], sdk.Coin{"xrp", sdk.NewInt(100)}, sdk.Coin{"usdx", sdk.NewInt(100)}, sdk.Coin{"", sdk.NewInt(0)}, tmtime.Now()},
+			errArgs: errArgs{
+				expectPass: false,
+				contains:   "invalid coins: accumulated fees",
+			},
+		},
+		{
+			name: "invalid fees updated",
+			cdp:  types.CDP{1, suite.addrs[0], sdk.Coin{"xrp", sdk.NewInt(100)}, sdk.Coin{"usdx", sdk.NewInt(100)}, sdk.Coin{"usdx", sdk.NewInt(0)}, time.Time{}},
+			errArgs: errArgs{
+				expectPass: false,
+				contains:   "cdp updated fee time cannot be zero",
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		suite.Run(tc.name, func() {
+			err := tc.cdp.Validate()
+			if tc.errArgs.expectPass {
+				suite.Require().NoError(err, tc.name)
+			} else {
+				suite.Require().Error(err, tc.name)
+				suite.Require().True(strings.Contains(err.Error(), tc.errArgs.contains))
+			}
+		})
+	}
+}
+
+func (suite *CdpValidationSuite) TestDepositValidation() {
+	type errArgs struct {
+		expectPass bool
+		contains   string
+	}
+	testCases := []struct {
+		name    string
+		deposit types.Deposit
+		errArgs errArgs
+	}{
+		{
+			name:    "valid deposit",
+			deposit: types.NewDeposit(1, suite.addrs[0], sdk.NewInt64Coin("bnb", 1000000)),
+			errArgs: errArgs{
+				expectPass: true,
+				contains:   "",
+			},
+		},
+		{
+			name:    "invalid cdp id",
+			deposit: types.NewDeposit(0, suite.addrs[0], sdk.NewInt64Coin("bnb", 1000000)),
+			errArgs: errArgs{
+				expectPass: false,
+				contains:   "deposit's cdp id cannot be 0",
+			},
+		},
+		{
+			name:    "empty depositor",
+			deposit: types.NewDeposit(1, sdk.AccAddress{}, sdk.NewInt64Coin("bnb", 1000000)),
+			errArgs: errArgs{
+				expectPass: false,
+				contains:   "depositor cannot be empty",
+			},
+		},
+		{
+			name:    "invalid deposit coins",
+			deposit: types.NewDeposit(1, suite.addrs[0], sdk.Coin{"Invalid Denom", sdk.NewInt(1000000)}),
+			errArgs: errArgs{
+				expectPass: false,
+				contains:   "invalid coins: deposit",
+			},
+		},
+	}
+	for _, tc := range testCases {
+		suite.Run(tc.name, func() {
+			err := tc.deposit.Validate()
+			if tc.errArgs.expectPass {
+				suite.Require().NoError(err, tc.name)
+			} else {
+				suite.Require().Error(err, tc.name)
+				suite.Require().True(strings.Contains(err.Error(), tc.errArgs.contains))
+			}
+		})
+	}
+
+}
+
+func TestCdpValidationSuite(t *testing.T) {
+	suite.Run(t, new(CdpValidationSuite))
+}

x/cdp/types/deposit.go

@@ -1,9 +1,11 @@
 package types
 
 import (
+	"errors"
 	"fmt"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
 
 // Deposit defines an amount of coins deposited by an account to a cdp
@@ -26,6 +28,20 @@ func (d Deposit) String() string {
 		d.CdpID, d.Depositor, d.Amount)
 }
 
+// Validate performs a basic validation of the deposit fields.
+func (d Deposit) Validate() error {
+	if d.CdpID == 0 {
+		return errors.New("deposit's cdp id cannot be 0")
+	}
+	if d.Depositor.Empty() {
+		return errors.New("depositor cannot be empty")
+	}
+	if !d.Amount.IsValid() {
+		return sdkerrors.Wrapf(sdkerrors.ErrInvalidCoins, "deposit %s", d.Amount)
+	}
+	return nil
+}
+
 // Deposits a collection of Deposit objects
 type Deposits []Deposit
 
@@ -41,6 +57,16 @@ func (ds Deposits) String() string {
 	return out
 }
 
+// Validate validates each deposit
+func (ds Deposits) Validate() error {
+	for _, d := range ds {
+		if err := d.Validate(); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // Equals returns whether two deposits are equal.
 func (d Deposit) Equals(comp Deposit) bool {
 	return d.Depositor.Equals(comp.Depositor) && d.CdpID == comp.CdpID && d.Amount.IsEqual(comp.Amount)

x/cdp/types/errors.go

@@ -41,4 +41,6 @@ var (
 	ErrInvalidDebtRequest = sdkerrors.Register(ModuleName, 17, "only one principal type per cdp")
 	// ErrDenomPrefixNotFound error for denom prefix not found
 	ErrDenomPrefixNotFound = sdkerrors.Register(ModuleName, 18, "denom prefix not found")
+	// ErrPricefeedDown error for when a price for the input denom is not found
+	ErrPricefeedDown = sdkerrors.Register(ModuleName, 19, "no price found for collateral")
 )

x/cdp/types/genesis.go

@@ -53,7 +53,15 @@ func (gs GenesisState) Validate() error {
 		return err
 	}
 
-	if gs.PreviousDistributionTime.Equal(time.Time{}) {
+	if err := gs.CDPs.Validate(); err != nil {
+		return err
+	}
+
+	if err := gs.Deposits.Validate(); err != nil {
+		return err
+	}
+
+	if gs.PreviousDistributionTime.IsZero() {
 		return fmt.Errorf("previous distribution time not set")
 	}
 

x/cdp/types/keys.go

@@ -46,6 +46,7 @@ var sep = []byte(":")
 // - 0x06<denom>:totalPrincipal
 // - 0x07<denom>:feeRate
 // - 0x08:previousDistributionTime
+// - 0x09<marketID>:downTime
 
 // KVStore key prefixes
 var (
@@ -58,6 +59,7 @@ var (
 	DepositKeyPrefix            = []byte{0x06}
 	PrincipalKeyPrefix          = []byte{0x07}
 	PreviousDistributionTimeKey = []byte{0x08}
+	PricefeedStatusKeyPrefix    = []byte{0x09}
 )
 
 // GetCdpIDBytes returns the byte representation of the cdpID