From 1099dfbd7d7b141d979931cb4ddba487b1ace918 Mon Sep 17 00:00:00 2001 From: Kevin Davis Date: Wed, 13 May 2020 10:19:19 -0400 Subject: [PATCH] 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 | 53 ++++------- x/cdp/doc.go | 27 ------ x/cdp/genesis.go | 3 + x/cdp/keeper/auctions.go | 30 +++---- x/cdp/keeper/cdp.go | 63 +++++++++---- x/cdp/keeper/cdp_test.go | 6 +- x/cdp/keeper/deposit.go | 6 +- x/cdp/keeper/draw.go | 33 +++++-- x/cdp/keeper/fees.go | 7 +- x/cdp/keeper/querier.go | 19 ++-- x/cdp/keeper/querier_test.go | 3 +- x/cdp/keeper/savings.go | 9 +- x/cdp/spec/01_concepts.md | 10 ++- x/cdp/spec/04_begin_block.md | 6 +- x/cdp/types/cdp.go | 35 ++++++++ x/cdp/types/cdp_test.go | 165 +++++++++++++++++++++++++++++++++++ x/cdp/types/deposit.go | 26 ++++++ x/cdp/types/errors.go | 2 + x/cdp/types/genesis.go | 10 ++- x/cdp/types/keys.go | 2 + 20 files changed, 372 insertions(+), 143 deletions(-) delete mode 100644 x/cdp/doc.go create mode 100644 x/cdp/types/cdp_test.go diff --git a/x/cdp/abci.go b/x/cdp/abci.go index 92781fa0..603704b7 100644 --- a/x/cdp/abci.go +++ b/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( - sdk.NewEvent( - EventTypeBeginBlockerFatal, - sdk.NewAttribute(sdk.AttributeKeyModule, fmt.Sprintf("%s", ModuleName)), - sdk.NewAttribute(types.AttributeKeyError, fmt.Sprintf("%s", err)), - ), - ) + panic(err) } err = k.LiquidateCdps(ctx, cp.MarketID, cp.Denom, cp.LiquidationRatio) if err != nil { - ctx.EventManager().EmitEvent( - sdk.NewEvent( - EventTypeBeginBlockerFatal, - sdk.NewAttribute(sdk.AttributeKeyModule, fmt.Sprintf("%s", ModuleName)), - sdk.NewAttribute(types.AttributeKeyError, fmt.Sprintf("%s", err)), - ), - ) + panic(err) } } err := k.RunSurplusAndDebtAuctions(ctx) if err != nil { - ctx.EventManager().EmitEvent( - sdk.NewEvent( - EventTypeBeginBlockerFatal, - sdk.NewAttribute(sdk.AttributeKeyModule, fmt.Sprintf("%s", ModuleName)), - sdk.NewAttribute(types.AttributeKeyError, fmt.Sprintf("%s", err)), - ), - ) + panic(err) } distTimeElapsed := sdk.NewInt(ctx.BlockTime().Unix() - previousDistTime.Unix()) - if distTimeElapsed.GTE(sdk.NewInt(int64(params.SavingsDistributionFrequency.Seconds()))) { - err := k.DistributeSavingsRate(ctx, params.DebtParam.Denom) - if err != nil { - ctx.EventManager().EmitEvent( - sdk.NewEvent( - EventTypeBeginBlockerFatal, - sdk.NewAttribute(sdk.AttributeKeyModule, fmt.Sprintf("%s", ModuleName)), - sdk.NewAttribute(types.AttributeKeyError, fmt.Sprintf("%s", err)), - ), - ) - } - k.SetPreviousSavingsDistribution(ctx, ctx.BlockTime()) + if !distTimeElapsed.GTE(sdk.NewInt(int64(params.SavingsDistributionFrequency.Seconds()))) { + return } + err = k.DistributeSavingsRate(ctx, params.DebtParam.Denom) + if err != nil { + panic(err) + } + k.SetPreviousSavingsDistribution(ctx, ctx.BlockTime()) } diff --git a/x/cdp/doc.go b/x/cdp/doc.go deleted file mode 100644 index 1207e925..00000000 --- a/x/cdp/doc.go +++ /dev/null @@ -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 diff --git a/x/cdp/genesis.go b/x/cdp/genesis.go index 176554c3..9eaae0d3 100644 --- a/x/cdp/genesis.go +++ b/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) diff --git a/x/cdp/keeper/auctions.go b/x/cdp/keeper/auctions.go index e7c0b7ec..6741d0f4 100644 --- a/x/cdp/keeper/auctions.go +++ b/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) { - err = k.supplyKeeper.BurnCoins(ctx, types.LiquidatorMacc, sdk.NewCoins(sdk.NewCoin(dp.Denom, balance))) - 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 + burnAmount := sdk.MinInt(balance, netAmount) + return k.supplyKeeper.BurnCoins(ctx, types.LiquidatorMacc, sdk.NewCoins(sdk.NewCoin(dp.Denom, burnAmount))) } // 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) { - surplusLot := sdk.NewCoin(params.DebtParam.Denom, surplus) - _, err := k.auctionKeeper.StartSurplusAuction(ctx, types.LiquidatorMacc, surplusLot, k.GetGovDenom(ctx)) - if err != nil { - return err - } + if !surplus.GTE(params.SurplusAuctionThreshold) { + return nil } - return nil + surplusLot := sdk.NewCoin(params.DebtParam.Denom, surplus) + _, err := k.auctionKeeper.StartSurplusAuction(ctx, types.LiquidatorMacc, surplusLot, k.GetGovDenom(ctx)) + return err } diff --git a/x/cdp/keeper/cdp.go b/x/cdp/keeper/cdp.go index e667006b..ebd71adf 100644 --- a/x/cdp/keeper/cdp.go +++ b/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) diff --git a/x/cdp/keeper/cdp_test.go b/x/cdp/keeper/cdp_test.go index 87d0d8ef..e2fddd58 100644 --- a/x/cdp/keeper/cdp_test.go +++ b/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) diff --git a/x/cdp/keeper/deposit.go b/x/cdp/keeper/deposit.go index 7d1d46bb..20ead918 100644 --- a/x/cdp/keeper/deposit.go +++ b/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 { diff --git a/x/cdp/keeper/draw.go b/x/cdp/keeper/draw.go index 43ef9ff6..2616e4c9 100644 --- a/x/cdp/keeper/draw.go +++ b/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 +} diff --git a/x/cdp/keeper/fees.go b/x/cdp/keeper/fees.go index 1d2c4555..1cd98ffa 100644 --- a/x/cdp/keeper/fees.go +++ b/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,10 +92,7 @@ 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 + return iterationErr } // IncrementTotalPrincipal increments the total amount of debt that has been drawn with that collateral type diff --git a/x/cdp/keeper/querier.go b/x/cdp/keeper/querier.go index 54b79668..edfdf54b 100644 --- a/x/cdp/keeper/querier.go +++ b/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) - if err != nil { - return nil, sdkerrors.Wrap(types.ErrLoadingAugmentedCDP, fmt.Sprintf("%v: %d", err.Error(), cdp.ID)) - } + augmentedCDP := keeper.LoadAugmentedCDP(ctx, cdp) bz, err := codec.MarshalJSONIndent(types.ModuleCdc, augmentedCDP) if err != nil { @@ -112,10 +107,8 @@ 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) - if err == nil { - augmentedCDPs = append(augmentedCDPs, augmentedCDP) - } + augmentedCDP := keeper.LoadAugmentedCDP(ctx, cdp) + augmentedCDPs = append(augmentedCDPs, augmentedCDP) } bz, err := codec.MarshalJSONIndent(types.ModuleCdc, augmentedCDPs) if err != nil { @@ -140,10 +133,8 @@ 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) - if err == nil { - augmentedCDPs = append(augmentedCDPs, augmentedCDP) - } + augmentedCDP := keeper.LoadAugmentedCDP(ctx, cdp) + augmentedCDPs = append(augmentedCDPs, augmentedCDP) } bz, err := codec.MarshalJSONIndent(types.ModuleCdc, augmentedCDPs) if err != nil { diff --git a/x/cdp/keeper/querier_test.go b/x/cdp/keeper/querier_test.go index 879ab5ac..343b1d0e 100644 --- a/x/cdp/keeper/querier_test.go +++ b/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) - suite.NoError(err) + aCDP := suite.keeper.LoadAugmentedCDP(suite.ctx, c) augmentedCDPs[j] = aCDP } diff --git a/x/cdp/keeper/savings.go b/x/cdp/keeper/savings.go index fe5a8840..f20898b0 100644 --- a/x/cdp/keeper/savings.go +++ b/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,10 +61,7 @@ func (k Keeper) DistributeSavingsRate(ctx sdk.Context, debtDenom string) error { surplusDistributed = surplusDistributed.Add(interest) return false }) - if iterationErr != nil { - return iterationErr - } - return nil + return iterationErr } // GetPreviousSavingsDistribution get the time of the previous savings rate distribution @@ -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)) } diff --git a/x/cdp/spec/01_concepts.md b/x/cdp/spec/01_concepts.md index cb55e6d2..26805251 100644 --- a/x/cdp/spec/01_concepts.md +++ b/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 diff --git a/x/cdp/spec/04_begin_block.md b/x/cdp/spec/04_begin_block.md index f85f3470..73dac9cf 100644 --- a/x/cdp/spec/04_begin_block.md +++ b/x/cdp/spec/04_begin_block.md @@ -2,8 +2,10 @@ At the start of every block the BeginBlocker of the cdp module: -- updates fees for CDPs -- liquidates CDPs under the collateral ratio +- 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 - pays out the savings rate if sufficient time has past - records the last savings rate distribution, if one occurred diff --git a/x/cdp/types/cdp.go b/x/cdp/types/cdp.go index c5dae9cf..0b1fa7e0 100644 --- a/x/cdp/types/cdp.go +++ b/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"` diff --git a/x/cdp/types/cdp_test.go b/x/cdp/types/cdp_test.go new file mode 100644 index 00000000..0e3a0dc7 --- /dev/null +++ b/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)) +} diff --git a/x/cdp/types/deposit.go b/x/cdp/types/deposit.go index a6baae72..995d7cf6 100644 --- a/x/cdp/types/deposit.go +++ b/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) diff --git a/x/cdp/types/errors.go b/x/cdp/types/errors.go index bee71992..d28416bf 100644 --- a/x/cdp/types/errors.go +++ b/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") ) diff --git a/x/cdp/types/genesis.go b/x/cdp/types/genesis.go index 2c837366..64c56f06 100644 --- a/x/cdp/types/genesis.go +++ b/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") } diff --git a/x/cdp/types/keys.go b/x/cdp/types/keys.go index 4dc57b38..53570c66 100644 --- a/x/cdp/types/keys.go +++ b/x/cdp/types/keys.go @@ -46,6 +46,7 @@ var sep = []byte(":") // - 0x06:totalPrincipal // - 0x07:feeRate // - 0x08:previousDistributionTime +// - 0x09: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