From 86c0225174948ffd5b16f80eabe0b16b9d5cc746 Mon Sep 17 00:00:00 2001 From: Nick DeLuca Date: Fri, 10 Jul 2020 08:16:05 -0500 Subject: [PATCH] encapsulate total principle calculation within cdp type and use (#610) throughout codebase --- x/cdp/genesis.go | 4 ++-- x/cdp/keeper/cdp.go | 2 +- x/cdp/keeper/deposit.go | 8 ++++---- x/cdp/keeper/draw.go | 10 +++++----- x/cdp/keeper/fees.go | 4 ++-- x/cdp/keeper/seize.go | 6 +++--- x/cdp/types/cdp.go | 5 +++++ x/cdp/types/cdp_test.go | 8 ++++++++ x/incentive/keeper/rewards.go | 2 +- 9 files changed, 31 insertions(+), 18 deletions(-) diff --git a/x/cdp/genesis.go b/x/cdp/genesis.go index 55607468..347cc316 100644 --- a/x/cdp/genesis.go +++ b/x/cdp/genesis.go @@ -72,9 +72,9 @@ func InitGenesis(ctx sdk.Context, k Keeper, pk types.PricefeedKeeper, sk types.S panic(fmt.Sprintf("error setting cdp: %v", err)) } k.IndexCdpByOwner(ctx, cdp) - ratio := k.CalculateCollateralToDebtRatio(ctx, cdp.Collateral, cdp.Principal.Add(cdp.AccumulatedFees)) + ratio := k.CalculateCollateralToDebtRatio(ctx, cdp.Collateral, cdp.GetTotalPrincipal()) k.IndexCdpByCollateralRatio(ctx, cdp.Collateral.Denom, cdp.ID, ratio) - k.IncrementTotalPrincipal(ctx, cdp.Collateral.Denom, cdp.Principal.Add(cdp.AccumulatedFees)) + k.IncrementTotalPrincipal(ctx, cdp.Collateral.Denom, cdp.GetTotalPrincipal()) } k.SetNextCdpID(ctx, gs.StartingCdpID) diff --git a/x/cdp/keeper/cdp.go b/x/cdp/keeper/cdp.go index a68b0209..be3de016 100644 --- a/x/cdp/keeper/cdp.go +++ b/x/cdp/keeper/cdp.go @@ -437,7 +437,7 @@ func (k Keeper) LoadAugmentedCDP(ctx sdk.Context, cdp types.CDP) types.Augmented return types.AugmentedCDP{CDP: cdp} } // convert collateral value to debt coin - totalDebt := cdp.Principal.Amount.Add(cdp.AccumulatedFees.Amount) + totalDebt := cdp.GetTotalPrincipal().Amount collateralValueInDebtDenom := sdk.NewDecFromInt(totalDebt).Mul(collateralizationRatio) collateralValueInDebt := sdk.NewCoin(cdp.Principal.Denom, collateralValueInDebtDenom.RoundInt()) // create new augmuented cdp diff --git a/x/cdp/keeper/deposit.go b/x/cdp/keeper/deposit.go index 8c2caa93..1dadcd13 100644 --- a/x/cdp/keeper/deposit.go +++ b/x/cdp/keeper/deposit.go @@ -42,11 +42,11 @@ func (k Keeper) DepositCollateral(ctx sdk.Context, owner, depositor sdk.AccAddre k.SetDeposit(ctx, deposit) - oldCollateralToDebtRatio := k.CalculateCollateralToDebtRatio(ctx, cdp.Collateral, cdp.Principal.Add(cdp.AccumulatedFees)) + oldCollateralToDebtRatio := k.CalculateCollateralToDebtRatio(ctx, cdp.Collateral, cdp.GetTotalPrincipal()) k.RemoveCdpCollateralRatioIndex(ctx, cdp.Collateral.Denom, cdp.ID, oldCollateralToDebtRatio) cdp.Collateral = cdp.Collateral.Add(collateral) - collateralToDebtRatio := k.CalculateCollateralToDebtRatio(ctx, cdp.Collateral, cdp.Principal.Add(cdp.AccumulatedFees)) + collateralToDebtRatio := k.CalculateCollateralToDebtRatio(ctx, cdp.Collateral, cdp.GetTotalPrincipal()) return k.SetCdpAndCollateralRatioIndex(ctx, cdp, collateralToDebtRatio) } @@ -88,11 +88,11 @@ func (k Keeper) WithdrawCollateral(ctx sdk.Context, owner, depositor sdk.AccAddr if err != nil { panic(err) } - oldCollateralToDebtRatio := k.CalculateCollateralToDebtRatio(ctx, cdp.Collateral, cdp.Principal.Add(cdp.AccumulatedFees)) + oldCollateralToDebtRatio := k.CalculateCollateralToDebtRatio(ctx, cdp.Collateral, cdp.GetTotalPrincipal()) k.RemoveCdpCollateralRatioIndex(ctx, cdp.Collateral.Denom, cdp.ID, oldCollateralToDebtRatio) cdp.Collateral = cdp.Collateral.Sub(collateral) - collateralToDebtRatio := k.CalculateCollateralToDebtRatio(ctx, cdp.Collateral, cdp.Principal.Add(cdp.AccumulatedFees)) + collateralToDebtRatio := k.CalculateCollateralToDebtRatio(ctx, cdp.Collateral, cdp.GetTotalPrincipal()) err = k.SetCdpAndCollateralRatioIndex(ctx, cdp, collateralToDebtRatio) if err != nil { return err diff --git a/x/cdp/keeper/draw.go b/x/cdp/keeper/draw.go index 193646fd..78ccff08 100644 --- a/x/cdp/keeper/draw.go +++ b/x/cdp/keeper/draw.go @@ -57,7 +57,7 @@ func (k Keeper) AddPrincipal(ctx sdk.Context, owner sdk.AccAddress, denom string ) // remove old collateral:debt index - oldCollateralToDebtRatio := k.CalculateCollateralToDebtRatio(ctx, cdp.Collateral, cdp.Principal.Add(cdp.AccumulatedFees)) + oldCollateralToDebtRatio := k.CalculateCollateralToDebtRatio(ctx, cdp.Collateral, cdp.GetTotalPrincipal()) k.RemoveCdpCollateralRatioIndex(ctx, denom, cdp.ID, oldCollateralToDebtRatio) // update cdp state @@ -67,7 +67,7 @@ func (k Keeper) AddPrincipal(ctx sdk.Context, owner sdk.AccAddress, denom string k.IncrementTotalPrincipal(ctx, cdp.Collateral.Denom, principal) // set cdp state and indexes in the store - collateralToDebtRatio := k.CalculateCollateralToDebtRatio(ctx, cdp.Collateral, cdp.Principal.Add(cdp.AccumulatedFees)) + collateralToDebtRatio := k.CalculateCollateralToDebtRatio(ctx, cdp.Collateral, cdp.GetTotalPrincipal()) return k.SetCdpAndCollateralRatioIndex(ctx, cdp, collateralToDebtRatio) } @@ -86,7 +86,7 @@ func (k Keeper) RepayPrincipal(ctx sdk.Context, owner sdk.AccAddress, denom stri } // Note: assumes cdp.Principal and cdp.AccumulatedFees don't change during calculations - totalPrincipal := cdp.Principal.Add(cdp.AccumulatedFees) + totalPrincipal := cdp.GetTotalPrincipal() // calculate fee and principal payment feePayment, principalPayment := k.calculatePayment(ctx, totalPrincipal, cdp.AccumulatedFees, payment) @@ -167,13 +167,13 @@ func (k Keeper) RepayPrincipal(ctx sdk.Context, owner sdk.AccAddress, denom stri } // set cdp state and update indexes - collateralToDebtRatio := k.CalculateCollateralToDebtRatio(ctx, cdp.Collateral, cdp.Principal.Add(cdp.AccumulatedFees)) + collateralToDebtRatio := k.CalculateCollateralToDebtRatio(ctx, cdp.Collateral, cdp.GetTotalPrincipal()) return k.SetCdpAndCollateralRatioIndex(ctx, cdp, collateralToDebtRatio) } // ValidatePaymentCoins validates that the input coins are valid for repaying debt func (k Keeper) ValidatePaymentCoins(ctx sdk.Context, cdp types.CDP, payment sdk.Coin) error { - debt := cdp.Principal.Add(cdp.AccumulatedFees) + debt := cdp.GetTotalPrincipal() if payment.Denom != debt.Denom { return sdkerrors.Wrapf(types.ErrInvalidPayment, "cdp %d: expected %s, got %s", cdp.ID, debt.Denom, payment.Denom) } diff --git a/x/cdp/keeper/fees.go b/x/cdp/keeper/fees.go index e3bec6ff..3e0f91dc 100644 --- a/x/cdp/keeper/fees.go +++ b/x/cdp/keeper/fees.go @@ -26,7 +26,7 @@ func (k Keeper) CalculateFees(ctx sdk.Context, principal sdk.Coin, periods sdk.I func (k Keeper) UpdateFeesForAllCdps(ctx sdk.Context, collateralDenom string) error { var iterationErr error k.IterateCdpsByDenom(ctx, collateralDenom, func(cdp types.CDP) bool { - oldCollateralToDebtRatio := k.CalculateCollateralToDebtRatio(ctx, cdp.Collateral, cdp.Principal.Add(cdp.AccumulatedFees)) + oldCollateralToDebtRatio := k.CalculateCollateralToDebtRatio(ctx, cdp.Collateral, cdp.GetTotalPrincipal()) // periods = bblock timestamp - fees updated periods := sdk.NewInt(ctx.BlockTime().Unix()).Sub(sdk.NewInt(cdp.FeesUpdated.Unix())) @@ -84,7 +84,7 @@ func (k Keeper) UpdateFeesForAllCdps(ctx sdk.Context, collateralDenom string) er // and set the fees updated time to the current block time since we just updated it cdp.FeesUpdated = ctx.BlockTime() - collateralToDebtRatio := k.CalculateCollateralToDebtRatio(ctx, cdp.Collateral, cdp.Principal.Add(cdp.AccumulatedFees)) + collateralToDebtRatio := k.CalculateCollateralToDebtRatio(ctx, cdp.Collateral, cdp.GetTotalPrincipal()) k.RemoveCdpCollateralRatioIndex(ctx, cdp.Collateral.Denom, cdp.ID, oldCollateralToDebtRatio) err = k.SetCdpAndCollateralRatioIndex(ctx, cdp, collateralToDebtRatio) if err != nil { diff --git a/x/cdp/keeper/seize.go b/x/cdp/keeper/seize.go index ca8a073f..e536c9e0 100644 --- a/x/cdp/keeper/seize.go +++ b/x/cdp/keeper/seize.go @@ -17,11 +17,11 @@ import ( // (this is the equivalent of saying that fees are no longer accumulated by a cdp once it gets liquidated) func (k Keeper) SeizeCollateral(ctx sdk.Context, cdp types.CDP) error { // Calculate the previous collateral ratio - oldCollateralToDebtRatio := k.CalculateCollateralToDebtRatio(ctx, cdp.Collateral, cdp.Principal.Add(cdp.AccumulatedFees)) + oldCollateralToDebtRatio := k.CalculateCollateralToDebtRatio(ctx, cdp.Collateral, cdp.GetTotalPrincipal()) // Move debt coins from cdp to liquidator account deposits := k.GetDeposits(ctx, cdp.ID) - debt := cdp.Principal.Amount.Add(cdp.AccumulatedFees.Amount) + debt := cdp.GetTotalPrincipal().Amount modAccountDebt := k.getModAccountDebt(ctx, types.ModuleName) debt = sdk.MinInt(debt, modAccountDebt) debtCoin := sdk.NewCoin(k.GetDebtDenom(ctx), debt) @@ -54,7 +54,7 @@ func (k Keeper) SeizeCollateral(ctx sdk.Context, cdp types.CDP) error { } // Decrement total principal for this collateral type - coinsToDecrement := cdp.Principal.Add(cdp.AccumulatedFees) + coinsToDecrement := cdp.GetTotalPrincipal() k.DecrementTotalPrincipal(ctx, cdp.Collateral.Denom, coinsToDecrement) // Delete CDP from state diff --git a/x/cdp/types/cdp.go b/x/cdp/types/cdp.go index 0b1fa7e0..7b3a080b 100644 --- a/x/cdp/types/cdp.go +++ b/x/cdp/types/cdp.go @@ -76,6 +76,11 @@ func (cdp CDP) Validate() error { return nil } +// GetTotalPrinciple returns the total principle for the cdp +func (cdp CDP) GetTotalPrincipal() sdk.Coin { + return cdp.Principal.Add(cdp.AccumulatedFees) +} + // CDPs a collection of CDP objects type CDPs []CDP diff --git a/x/cdp/types/cdp_test.go b/x/cdp/types/cdp_test.go index 0e3a0dc7..54439e74 100644 --- a/x/cdp/types/cdp_test.go +++ b/x/cdp/types/cdp_test.go @@ -157,7 +157,15 @@ func (suite *CdpValidationSuite) TestDepositValidation() { } }) } +} +func (suite *CdpValidationSuite) TestCdpGetTotalPrinciple() { + principal := sdk.Coin{"usdx", sdk.NewInt(100500)} + acummulatedFees := sdk.Coin{"usdx", sdk.NewInt(25000)} + + cdp := types.CDP{Principal: principal, AccumulatedFees: acummulatedFees} + + suite.Require().Equal(cdp.GetTotalPrincipal(), principal.Add(acummulatedFees)) } func TestCdpValidationSuite(t *testing.T) { diff --git a/x/incentive/keeper/rewards.go b/x/incentive/keeper/rewards.go index c07a9f1a..c3cf711b 100644 --- a/x/incentive/keeper/rewards.go +++ b/x/incentive/keeper/rewards.go @@ -73,7 +73,7 @@ func (k Keeper) ApplyRewardsToCdps(ctx sdk.Context) { rewardsThisPeriod := rp.Reward.Amount.Mul(timeElapsed) id := k.GetNextClaimPeriodID(ctx, rp.Denom) k.cdpKeeper.IterateCdpsByDenom(ctx, rp.Denom, func(cdp cdptypes.CDP) bool { - rewardsShare := sdk.NewDecFromInt(cdp.Principal.Amount.Add(cdp.AccumulatedFees.Amount)).Quo(sdk.NewDecFromInt(totalPrincipal)) + rewardsShare := sdk.NewDecFromInt(cdp.GetTotalPrincipal().Amount).Quo(sdk.NewDecFromInt(totalPrincipal)) // sanity check - don't create zero claims if rewardsShare.IsZero() { return false