CDP begin blocker optimization 1 (#754)

* refactor begin blocker tests

* add begin blocker benchmark

* optimize by consolidating param fetches

* return error rather than panic

x/cdp/abci_test.go

@@ -1,6 +1,7 @@
 package cdp_test
 
 import (
+	"fmt"
 	"math/rand"
 	"testing"
 	"time"
@@ -21,12 +22,10 @@ import (
 type ModuleTestSuite struct {
 	suite.Suite
 
-	keeper       cdp.Keeper
-	addrs        []sdk.AccAddress
-	app          app.TestApp
-	cdps         cdp.CDPs
-	ctx          sdk.Context
-	liquidations liquidationTracker
+	keeper cdp.Keeper
+	addrs  []sdk.AccAddress
+	app    app.TestApp
+	ctx    sdk.Context
 }
 
 type liquidationTracker struct {
@@ -39,56 +38,31 @@ func (suite *ModuleTestSuite) SetupTest() {
 	tApp := app.NewTestApp()
 	ctx := tApp.NewContext(true, abci.Header{Height: 1, Time: tmtime.Now()})
 	coins := []sdk.Coins{}
-	tracker := liquidationTracker{}
 
-	for j := 0; j < 100; j++ {
+	const numAddrs = 100
+	for j := 0; j < numAddrs; j++ {
 		coins = append(coins, cs(c("btc", 100000000), c("xrp", 10000000000)))
 	}
-	_, addrs := app.GeneratePrivKeyAddressPairs(100)
+	_, addrs := app.GeneratePrivKeyAddressPairs(numAddrs)
 
-	authGS := app.NewAuthGenState(
-		addrs, coins)
 	tApp.InitializeFromGenesisStates(
-		authGS,
+		app.NewAuthGenState(addrs, coins),
 		NewPricefeedGenStateMulti(),
 		NewCDPGenStateMulti(),
 	)
 	suite.ctx = ctx
 	suite.app = tApp
 	suite.keeper = tApp.GetCDPKeeper()
-	suite.cdps = cdp.CDPs{}
 	suite.addrs = addrs
-	suite.liquidations = tracker
 }
 
-func (suite *ModuleTestSuite) createCdps() {
-	tApp := app.NewTestApp()
-	ctx := tApp.NewContext(true, abci.Header{Height: 1, Time: tmtime.Now()})
-	cdps := make(cdp.CDPs, 100)
-	_, addrs := app.GeneratePrivKeyAddressPairs(100)
-	coins := []sdk.Coins{}
+func createCDPs(ctx sdk.Context, keeper cdp.Keeper, addrs []sdk.AccAddress, numCDPs int) (liquidationTracker, error) {
 	tracker := liquidationTracker{}
 
-	for j := 0; j < 100; j++ {
-		coins = append(coins, cs(c("btc", 100000000), c("xrp", 10000000000)))
-	}
+	for j := 0; j < numCDPs; j++ {
+		var collateral string
+		var amount, debt int
 
-	authGS := app.NewAuthGenState(
-		addrs, coins)
-	tApp.InitializeFromGenesisStates(
-		authGS,
-		NewPricefeedGenStateMulti(),
-		NewCDPGenStateMulti(),
-	)
-
-	suite.ctx = ctx
-	suite.app = tApp
-	suite.keeper = tApp.GetCDPKeeper()
-
-	for j := 0; j < 100; j++ {
-		collateral := "xrp"
-		amount := 10000000000
-		debt := simulation.RandIntBetween(rand.New(rand.NewSource(int64(j))), 750000000, 1249000000)
 		if j%2 == 0 {
 			collateral = "btc"
 			amount = 100000000
@@ -98,67 +72,76 @@ func (suite *ModuleTestSuite) createCdps() {
 				tracker.debt += int64(debt)
 			}
 		} else {
+			collateral = "xrp"
+			amount = 10000000000
+			debt = simulation.RandIntBetween(rand.New(rand.NewSource(int64(j))), 750000000, 1249000000)
 			if debt >= 1000000000 {
 				tracker.xrp = append(tracker.xrp, uint64(j+1))
 				tracker.debt += int64(debt)
 			}
 		}
-		suite.Nil(suite.keeper.AddCdp(suite.ctx, addrs[j], c(collateral, int64(amount)), c("usdx", int64(debt)), collateral+"-a"))
-		c, f := suite.keeper.GetCDP(suite.ctx, collateral+"-a", uint64(j+1))
-		suite.True(f)
-		cdps[j] = c
+		err := keeper.AddCdp(ctx, addrs[j], c(collateral, int64(amount)), c("usdx", int64(debt)), collateral+"-a")
+		if err != nil {
+			return liquidationTracker{}, err
+		}
+		_, f := keeper.GetCDP(ctx, collateral+"-a", uint64(j+1))
+		if !f {
+			return liquidationTracker{}, fmt.Errorf("created cdp, but could not find in store")
+		}
 	}
-
-	suite.cdps = cdps
-	suite.addrs = addrs
-	suite.liquidations = tracker
+	return tracker, nil
 }
 
 func (suite *ModuleTestSuite) setPrice(price sdk.Dec, market string) {
 	pfKeeper := suite.app.GetPriceFeedKeeper()
 
-	pfKeeper.SetPrice(suite.ctx, sdk.AccAddress{}, market, price, suite.ctx.BlockTime().Add(time.Hour*3))
-	err := pfKeeper.SetCurrentPrices(suite.ctx, market)
+	_, err := pfKeeper.SetPrice(suite.ctx, sdk.AccAddress{}, market, price, suite.ctx.BlockTime().Add(time.Hour*3))
+	suite.NoError(err)
+	err = pfKeeper.SetCurrentPrices(suite.ctx, market)
 	suite.NoError(err)
 	pp, err := pfKeeper.GetCurrentPrice(suite.ctx, market)
 	suite.NoError(err)
 	suite.Equal(price, pp.Price)
 }
 func (suite *ModuleTestSuite) TestBeginBlock() {
-	suite.createCdps()
+	liquidations, err := createCDPs(suite.ctx, suite.keeper, suite.addrs, 100)
+	suite.Require().NoError(err)
+
 	sk := suite.app.GetSupplyKeeper()
 	acc := sk.GetModuleAccount(suite.ctx, cdp.ModuleName)
 	originalXrpCollateral := acc.GetCoins().AmountOf("xrp")
+	originalBtcCollateral := acc.GetCoins().AmountOf("btc")
+
 	suite.setPrice(d("0.2"), "xrp:usd")
+	suite.setPrice(d("6000"), "btc:usd")
 	cdp.BeginBlocker(suite.ctx, abci.RequestBeginBlock{Header: suite.ctx.BlockHeader()}, suite.keeper)
+
 	acc = sk.GetModuleAccount(suite.ctx, cdp.ModuleName)
 	finalXrpCollateral := acc.GetCoins().AmountOf("xrp")
 	seizedXrpCollateral := originalXrpCollateral.Sub(finalXrpCollateral)
 	xrpLiquidations := int(seizedXrpCollateral.Quo(i(10000000000)).Int64())
-	suite.Equal(len(suite.liquidations.xrp), xrpLiquidations)
+	suite.Equal(len(liquidations.xrp), xrpLiquidations)
 
-	acc = sk.GetModuleAccount(suite.ctx, cdp.ModuleName)
-	originalBtcCollateral := acc.GetCoins().AmountOf("btc")
-	suite.setPrice(d("6000"), "btc:usd")
-	cdp.BeginBlocker(suite.ctx, abci.RequestBeginBlock{Header: suite.ctx.BlockHeader()}, suite.keeper)
 	acc = sk.GetModuleAccount(suite.ctx, cdp.ModuleName)
 	finalBtcCollateral := acc.GetCoins().AmountOf("btc")
 	seizedBtcCollateral := originalBtcCollateral.Sub(finalBtcCollateral)
 	btcLiquidations := int(seizedBtcCollateral.Quo(i(100000000)).Int64())
-	suite.Equal(len(suite.liquidations.btc), btcLiquidations)
+	suite.Equal(len(liquidations.btc), btcLiquidations)
 
 	acc = sk.GetModuleAccount(suite.ctx, auction.ModuleName)
-	suite.Equal(suite.liquidations.debt, acc.GetCoins().AmountOf("debt").Int64())
-
+	suite.Equal(liquidations.debt, acc.GetCoins().AmountOf("debt").Int64())
 }
 
 func (suite *ModuleTestSuite) TestSeizeSingleCdpWithFees() {
 	err := suite.keeper.AddCdp(suite.ctx, suite.addrs[0], c("xrp", 10000000000), c("usdx", 1000000000), "xrp-a")
 	suite.NoError(err)
+
 	suite.Equal(i(1000000000), suite.keeper.GetTotalPrincipal(suite.ctx, "xrp-a", "usdx"))
+
 	sk := suite.app.GetSupplyKeeper()
 	cdpMacc := sk.GetModuleAccount(suite.ctx, cdp.ModuleName)
 	suite.Equal(i(1000000000), cdpMacc.GetCoins().AmountOf("debt"))
+
 	for i := 0; i < 100; i++ {
 		suite.ctx = suite.ctx.WithBlockTime(suite.ctx.BlockTime().Add(time.Second * 6))
 		cdp.BeginBlocker(suite.ctx, abci.RequestBeginBlock{Header: suite.ctx.BlockHeader()}, suite.keeper)
@@ -177,3 +160,37 @@ func (suite *ModuleTestSuite) TestSeizeSingleCdpWithFees() {
 func TestModuleTestSuite(t *testing.T) {
 	suite.Run(t, new(ModuleTestSuite))
 }
+
+func BenchmarkBeginBlocker(b *testing.B) {
+	tApp := app.NewTestApp()
+	ctx := tApp.NewContext(true, abci.Header{Height: 1, Time: tmtime.Now()})
+
+	const numAddrs = 10_000
+	coins := []sdk.Coins{}
+	for j := 0; j < numAddrs; j++ {
+		coins = append(coins, cs(c("btc", 100_000_000), c("xrp", 10_000_000_000)))
+	}
+	_, addrs := app.GeneratePrivKeyAddressPairs(numAddrs)
+
+	tApp.InitializeFromGenesisStates(
+		app.NewAuthGenState(addrs, coins),
+		NewPricefeedGenStateMulti(),
+		NewCDPGenStateMulti(),
+	)
+	_, err := createCDPs(ctx, tApp.GetCDPKeeper(), addrs, 2000)
+	if err != nil {
+		b.Fatal(err)
+	}
+	// note: price has not been lowered, so there will be no liquidations in the begin blocker
+
+	b.ResetTimer() // don't count the expensive cdp creation in the benchmark
+	for n := 0; n < b.N; n++ {
+		// Use a copy of the store in the begin blocker to discard any writes and avoid loop iterations interfering.
+		// Exclude this operation from the benchmark time
+		b.StopTimer()
+		cacheCtx, _ := ctx.CacheContext()
+		b.StartTimer()
+
+		cdp.BeginBlocker(cacheCtx, abci.RequestBeginBlock{Header: cacheCtx.BlockHeader()}, tApp.GetCDPKeeper())
+	}
+}

x/cdp/integration_test.go

@@ -106,7 +106,7 @@ func NewPricefeedGenStateMulti() app.GenesisState {
 func NewCDPGenStateMulti() app.GenesisState {
 	cdpGenesis := cdp.GenesisState{
 		Params: cdp.Params{
-			GlobalDebtLimit:              sdk.NewInt64Coin("usdx", 1000000000000),
+			GlobalDebtLimit:              sdk.NewInt64Coin("usdx", 10_000_000_000_000),
 			SurplusAuctionThreshold:      cdp.DefaultSurplusThreshold,
 			SurplusAuctionLot:            cdp.DefaultSurplusLot,
 			DebtAuctionThreshold:         cdp.DefaultDebtThreshold,
@@ -117,7 +117,7 @@ func NewCDPGenStateMulti() app.GenesisState {
 					Denom:               "xrp",
 					Type:                "xrp-a",
 					LiquidationRatio:    sdk.MustNewDecFromStr("2.0"),
-					DebtLimit:           sdk.NewInt64Coin("usdx", 500000000000),
+					DebtLimit:           sdk.NewInt64Coin("usdx", 5_000_000_000_000),
 					StabilityFee:        sdk.MustNewDecFromStr("1.000000001547125958"), // %5 apr
 					LiquidationPenalty:  d("0.05"),
 					AuctionSize:         i(7000000000),
@@ -130,7 +130,7 @@ func NewCDPGenStateMulti() app.GenesisState {
 					Denom:               "btc",
 					Type:                "btc-a",
 					LiquidationRatio:    sdk.MustNewDecFromStr("1.5"),
-					DebtLimit:           sdk.NewInt64Coin("usdx", 500000000000),
+					DebtLimit:           sdk.NewInt64Coin("usdx", 5_000_000_000_000),
 					StabilityFee:        sdk.MustNewDecFromStr("1.000000000782997609"), // %2.5 apr
 					LiquidationPenalty:  d("0.025"),
 					AuctionSize:         i(10000000),

x/cdp/keeper/cdp.go

@@ -421,13 +421,19 @@ func (k Keeper) ValidateCollateralizationRatio(ctx sdk.Context, collateral sdk.C
 
 // CalculateCollateralToDebtRatio returns the collateral to debt ratio of the input collateral and debt amounts
 func (k Keeper) CalculateCollateralToDebtRatio(ctx sdk.Context, collateral sdk.Coin, collateralType string, debt sdk.Coin) sdk.Dec {
-	debtTotal := k.convertDebtToBaseUnits(ctx, debt)
+	cp, _ := k.GetCollateral(ctx, collateralType)
+	dp, _ := k.GetDebtParam(ctx, debt.Denom)
+	return calculateCollateralToDebtRatio(collateral, cp, debt, dp)
+}
+
+func calculateCollateralToDebtRatio(collateral sdk.Coin, collateralParam types.CollateralParam, debt sdk.Coin, debtParam types.DebtParam) sdk.Dec {
+	debtTotal := convertDebtToBaseUnits(debt, debtParam)
 
 	if debtTotal.IsZero() || debtTotal.GTE(types.MaxSortableDec) {
 		return types.MaxSortableDec.Sub(sdk.SmallestDec())
 	}
 
-	collateralBaseUnits := k.convertCollateralToBaseUnits(ctx, collateral, collateralType)
+	collateralBaseUnits := convertCollateralToBaseUnits(collateral, collateralParam)
 	return collateralBaseUnits.Quo(debtTotal)
 }
 
@@ -529,13 +535,27 @@ func (k Keeper) UpdatePricefeedStatus(ctx sdk.Context, marketID string) (ok bool
 // converts the input collateral to base units (ie multiplies the input by 10^(-ConversionFactor))
 func (k Keeper) convertCollateralToBaseUnits(ctx sdk.Context, collateral sdk.Coin, collateralType string) (baseUnits sdk.Dec) {
 	cp, _ := k.GetCollateral(ctx, collateralType)
-	return sdk.NewDecFromInt(collateral.Amount).Mul(sdk.NewDecFromIntWithPrec(sdk.OneInt(), cp.ConversionFactor.Int64()))
+	return convertCollateralToBaseUnits(collateral, cp)
+}
+
+func convertCollateralToBaseUnits(collateral sdk.Coin, collateralParam types.CollateralParam) (baseUnits sdk.Dec) {
+	if collateral.Denom != collateralParam.Denom {
+		panic(fmt.Sprintf("mismatched collateral denom (%s) and param denom (%s)", collateral.Denom, collateralParam.Denom))
+	}
+	return sdk.NewDecFromInt(collateral.Amount).Mul(sdk.NewDecFromIntWithPrec(sdk.OneInt(), collateralParam.ConversionFactor.Int64()))
 }
 
 // converts the input debt to base units (ie multiplies the input by 10^(-ConversionFactor))
 func (k Keeper) convertDebtToBaseUnits(ctx sdk.Context, debt sdk.Coin) (baseUnits sdk.Dec) {
 	dp, _ := k.GetDebtParam(ctx, debt.Denom)
-	return sdk.NewDecFromInt(debt.Amount).Mul(sdk.NewDecFromIntWithPrec(sdk.OneInt(), dp.ConversionFactor.Int64()))
+	return convertDebtToBaseUnits(debt, dp)
+}
+
+func convertDebtToBaseUnits(debt sdk.Coin, debtParam types.DebtParam) (baseUnits sdk.Dec) {
+	if debt.Denom != debtParam.Denom {
+		panic(fmt.Sprintf("mismatched debt denom (%s) and param denom (%s)", debt.Denom, debtParam.Denom))
+	}
+	return sdk.NewDecFromInt(debt.Amount).Mul(sdk.NewDecFromIntWithPrec(sdk.OneInt(), debtParam.ConversionFactor.Int64()))
 }
 
 type pricefeedType string

x/cdp/keeper/fees.go

@@ -5,17 +5,22 @@ import (
 
 	"github.com/cosmos/cosmos-sdk/store/prefix"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 
 	"github.com/kava-labs/kava/x/cdp/types"
 )
 
 // CalculateFees returns the fees accumulated since fees were last calculated based on
 // the input amount of outstanding debt (principal) and the number of periods (seconds) that have passed
 func (k Keeper) CalculateFees(ctx sdk.Context, principal sdk.Coin, periods sdk.Int, collateralType string) sdk.Coin {
+	feePerSecond := k.getFeeRate(ctx, collateralType)
+	return calculateFees(principal, periods, feePerSecond)
+}
+
+func calculateFees(principal sdk.Coin, periods sdk.Int, feePerSecond sdk.Dec) sdk.Coin {
 	// how fees are calculated:
 	// feesAccumulated = (outstandingDebt * (feeRate^periods)) - outstandingDebt
 	// Note that since we can't do x^y using sdk.Decimal, we are converting to int and using RelativePow
-	feePerSecond := k.getFeeRate(ctx, collateralType)
 	scalar := sdk.NewInt(1000000000000000000)
 	feeRateInt := feePerSecond.Mul(sdk.NewDecFromInt(scalar)).TruncateInt()
 	accumulator := sdk.NewDecFromInt(types.RelativePow(feeRateInt, periods, scalar)).Mul(sdk.SmallestDec())
@@ -27,12 +32,22 @@ func (k Keeper) CalculateFees(ctx sdk.Context, principal sdk.Coin, periods sdk.I
 // UpdateFeesForAllCdps updates the fees for each of the CDPs
 func (k Keeper) UpdateFeesForAllCdps(ctx sdk.Context, collateralType string) error {
 	var iterationErr error
+
+	// Load params outside the loop to speed up iterations.
+	// This is safe as long as params are not updated during the loop.
+	collateralParam, found := k.GetCollateral(ctx, collateralType)
+	if !found {
+		return sdkerrors.Wrap(types.ErrCollateralNotSupported, collateralType)
+	}
+	debtParam := k.GetParams(ctx).DebtParam
+	feePerSecond := k.getFeeRate(ctx, collateralType)
+
 	k.IterateCdpsByCollateralType(ctx, collateralType, func(cdp types.CDP) bool {
-		oldCollateralToDebtRatio := k.CalculateCollateralToDebtRatio(ctx, cdp.Collateral, cdp.Type, cdp.GetTotalPrincipal())
-		// periods = bblock timestamp - fees updated
+		oldCollateralToDebtRatio := calculateCollateralToDebtRatio(cdp.Collateral, collateralParam, cdp.GetTotalPrincipal(), debtParam)
+		// periods = block timestamp - fees updated
 		periods := sdk.NewInt(ctx.BlockTime().Unix()).Sub(sdk.NewInt(cdp.FeesUpdated.Unix()))
 
-		newFees := k.CalculateFees(ctx, cdp.Principal, periods, collateralType)
+		newFees := calculateFees(cdp.Principal, periods, feePerSecond)
 
 		// exit without updating fees if amount has rounded down to zero
 		// cdp will get updated next block when newFees, newFeesSavings, newFeesSurplus >0
@@ -86,7 +101,7 @@ func (k Keeper) UpdateFeesForAllCdps(ctx sdk.Context, collateralType string) err
 
 		// 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.Type, cdp.GetTotalPrincipal())
+		collateralToDebtRatio := calculateCollateralToDebtRatio(cdp.Collateral, collateralParam, cdp.GetTotalPrincipal(), debtParam)
 		k.RemoveCdpCollateralRatioIndex(ctx, cdp.Type, cdp.ID, oldCollateralToDebtRatio)
 		err = k.SetCdpAndCollateralRatioIndex(ctx, cdp, collateralToDebtRatio)
 		if err != nil {