Minor audit fixes (#540)

* use mod and quo to calculate how many auctions to start

* check both return values in getDenomPrefix

* split auctions using weighted buckets

x/cdp/keeper/auctions.go

@@ -32,32 +32,33 @@ func (k Keeper) CreateAuctionsFromDeposit(
 	ctx sdk.Context, collateral sdk.Coin, returnAddr sdk.AccAddress, debt, auctionSize sdk.Int,
 	principalDenom string) (err error) {
 
-	amountToAuction := collateral.Amount
-	totalCollateralAmount := collateral.Amount
-	remainingDebt := debt
-	if !amountToAuction.IsPositive() {
-		return nil
-	}
-	for amountToAuction.GT(auctionSize) {
-		debtCoveredByAuction := (sdk.NewDecFromInt(auctionSize).Quo(sdk.NewDecFromInt(totalCollateralAmount))).Mul(sdk.NewDecFromInt(debt)).RoundInt()
-		penalty := k.ApplyLiquidationPenalty(ctx, collateral.Denom, debtCoveredByAuction)
-		_, err := k.auctionKeeper.StartCollateralAuction(
-			ctx, types.LiquidatorMacc, sdk.NewCoin(collateral.Denom, auctionSize), sdk.NewCoin(principalDenom, debtCoveredByAuction.Add(penalty)), []sdk.AccAddress{returnAddr},
-			[]sdk.Int{auctionSize}, sdk.NewCoin(k.GetDebtDenom(ctx), debtCoveredByAuction))
-		if err != nil {
-			return err
-		}
-		amountToAuction = amountToAuction.Sub(auctionSize)
-		remainingDebt = remainingDebt.Sub(debtCoveredByAuction)
-	}
-	penalty := k.ApplyLiquidationPenalty(ctx, collateral.Denom, remainingDebt)
-	_, err = k.auctionKeeper.StartCollateralAuction(
-		ctx, types.LiquidatorMacc, sdk.NewCoin(collateral.Denom, amountToAuction), sdk.NewCoin(principalDenom, remainingDebt.Add(penalty)), []sdk.AccAddress{returnAddr},
-		[]sdk.Int{amountToAuction}, sdk.NewCoin(k.GetDebtDenom(ctx), remainingDebt))
-	if err != nil {
-		return err
-	}
+	// the number of auctions to start with lot = auctionSize
+	wholeAuctions := collateral.Amount.Quo(auctionSize)
+	// remaining collateral (< lot) to auction
+	partialAuctionAmount := collateral.Amount.Mod(auctionSize)
+	auctionLots := []sdk.Int{}
 
+	for i := int64(0); i < wholeAuctions.Int64(); i++ {
+		auctionLots = append(auctionLots, auctionSize)
+	}
+	if partialAuctionAmount.IsPositive() {
+		auctionLots = append(auctionLots, partialAuctionAmount)
+	}
+	// use the auction lots as weights to split the debt into buckets,
+	// where each bucket represents how much debt that auction will attempt to cover
+	debtAmounts := splitIntIntoWeightedBuckets(debt, auctionLots)
+	debtDenom := k.GetDebtDenom(ctx)
+	for i, debtAmount := range debtAmounts {
+		penalty := k.ApplyLiquidationPenalty(ctx, collateral.Denom, debtAmount)
+		_, err := k.auctionKeeper.StartCollateralAuction(
+			ctx, types.LiquidatorMacc, sdk.NewCoin(collateral.Denom, auctionLots[i]),
+			sdk.NewCoin(principalDenom, debtAmount.Add(penalty)), []sdk.AccAddress{returnAddr},
+			[]sdk.Int{auctionLots[i]}, sdk.NewCoin(debtDenom, debtAmount),
+		)
+		if err != nil {
+			return err
+		}
+	}
 	return nil
 }
 

x/cdp/keeper/auctions_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/stretchr/testify/suite"
 
 	abci "github.com/tendermint/tendermint/abci/types"
+	"github.com/tendermint/tendermint/crypto"
 	tmtime "github.com/tendermint/tendermint/types/time"
 )
 
@@ -22,14 +23,18 @@ type AuctionTestSuite struct {
 	keeper keeper.Keeper
 	app    app.TestApp
 	ctx    sdk.Context
+	addrs  []sdk.AccAddress
 }
 
 func (suite *AuctionTestSuite) SetupTest() {
 	config := sdk.GetConfig()
 	app.SetBech32AddressPrefixes(config)
 	tApp := app.NewTestApp()
+	taddr := sdk.AccAddress(crypto.AddressHash([]byte("KavaTestUser1")))
+	authGS := app.NewAuthGenState([]sdk.AccAddress{taddr}, []sdk.Coins{cs(c("usdx", 21000000000))})
 	ctx := tApp.NewContext(true, abci.Header{Height: 1, Time: tmtime.Now()})
 	tApp.InitializeFromGenesisStates(
+		authGS,
 		NewPricefeedGenStateMulti(),
 		NewCDPGenStateMulti(),
 	)
@@ -37,6 +42,7 @@ func (suite *AuctionTestSuite) SetupTest() {
 	suite.app = tApp
 	suite.ctx = ctx
 	suite.keeper = keeper
+	suite.addrs = []sdk.AccAddress{taddr}
 	return
 }
 
@@ -51,6 +57,15 @@ func (suite *AuctionTestSuite) TestNetDebtSurplus() {
 	suite.Equal(cs(c("debt", 90)), acc.GetCoins())
 }
 
+func (suite *AuctionTestSuite) TestCollateralAuction() {
+	sk := suite.app.GetSupplyKeeper()
+	err := sk.MintCoins(suite.ctx, types.LiquidatorMacc, cs(c("debt", 21000000000), c("bnb", 190000000000)))
+	suite.Require().NoError(err)
+	testDeposit := types.NewDeposit(1, suite.addrs[0], c("bnb", 190000000000))
+	err = suite.keeper.AuctionCollateral(suite.ctx, types.Deposits{testDeposit}, i(21000000000), "usdx")
+	suite.Require().NoError(err)
+}
+
 func (suite *AuctionTestSuite) TestSurplusAuction() {
 	sk := suite.app.GetSupplyKeeper()
 	err := sk.MintCoins(suite.ctx, types.LiquidatorMacc, cs(c("usdx", 600000000000)))

x/cdp/keeper/cdp.go

@@ -302,14 +302,20 @@ func (k Keeper) RemoveCdpOwnerIndex(ctx sdk.Context, cdp types.CDP) {
 // IndexCdpByCollateralRatio sets the cdp id in the store, indexed by the collateral type and collateral to debt ratio
 func (k Keeper) IndexCdpByCollateralRatio(ctx sdk.Context, denom string, id uint64, collateralRatio sdk.Dec) {
 	store := prefix.NewStore(ctx.KVStore(k.key), types.CollateralRatioIndexPrefix)
-	db, _ := k.GetDenomPrefix(ctx, denom)
+	db, found := k.GetDenomPrefix(ctx, denom)
+	if !found {
+		panic(fmt.Sprintf("denom %s prefix not found", denom))
+	}
 	store.Set(types.CollateralRatioKey(db, id, collateralRatio), types.GetCdpIDBytes(id))
 }
 
 // RemoveCdpCollateralRatioIndex deletes the cdp id from the store's index of cdps by collateral type and collateral to debt ratio
 func (k Keeper) RemoveCdpCollateralRatioIndex(ctx sdk.Context, denom string, id uint64, collateralRatio sdk.Dec) {
 	store := prefix.NewStore(ctx.KVStore(k.key), types.CollateralRatioIndexPrefix)
-	db, _ := k.GetDenomPrefix(ctx, denom)
+	db, found := k.GetDenomPrefix(ctx, denom)
+	if !found {
+		panic(fmt.Sprintf("denom %s prefix not found", denom))
+	}
 	store.Delete(types.CollateralRatioKey(db, id, collateralRatio))
 }
 

x/cdp/keeper/integration_test.go

@@ -83,6 +83,7 @@ func NewPricefeedGenStateMulti() app.GenesisState {
 			Markets: []pricefeed.Market{
 				{MarketID: "btc:usd", BaseAsset: "btc", QuoteAsset: "usd", Oracles: []sdk.AccAddress{}, Active: true},
 				{MarketID: "xrp:usd", BaseAsset: "xrp", QuoteAsset: "usd", Oracles: []sdk.AccAddress{}, Active: true},
+				{MarketID: "bnb:usd", BaseAsset: "bnb", QuoteAsset: "usd", Oracles: []sdk.AccAddress{}, Active: true},
 			},
 		},
 		PostedPrices: []pricefeed.PostedPrice{
@@ -98,6 +99,12 @@ func NewPricefeedGenStateMulti() app.GenesisState {
 				Price:         sdk.MustNewDecFromStr("0.25"),
 				Expiry:        time.Now().Add(1 * time.Hour),
 			},
+			{
+				MarketID:      "bnb:usd",
+				OracleAddress: sdk.AccAddress{},
+				Price:         sdk.MustNewDecFromStr("17.25"),
+				Expiry:        time.Now().Add(1 * time.Hour),
+			},
 		},
 	}
 	return app.GenesisState{pricefeed.ModuleName: pricefeed.ModuleCdc.MustMarshalJSON(pfGenesis)}
@@ -105,7 +112,7 @@ func NewPricefeedGenStateMulti() app.GenesisState {
 func NewCDPGenStateMulti() app.GenesisState {
 	cdpGenesis := cdp.GenesisState{
 		Params: cdp.Params{
-			GlobalDebtLimit:              sdk.NewInt64Coin("usdx", 1000000000000),
+			GlobalDebtLimit:              sdk.NewInt64Coin("usdx", 1500000000000),
 			SurplusAuctionThreshold:      cdp.DefaultSurplusThreshold,
 			SurplusAuctionLot:            cdp.DefaultSurplusLot,
 			DebtAuctionThreshold:         cdp.DefaultDebtThreshold,
@@ -136,6 +143,18 @@ func NewCDPGenStateMulti() app.GenesisState {
 					LiquidationMarketID: "btc:usd",
 					ConversionFactor:    i(8),
 				},
+				{
+					Denom:               "bnb",
+					LiquidationRatio:    sdk.MustNewDecFromStr("1.5"),
+					DebtLimit:           sdk.NewInt64Coin("usdx", 500000000000),
+					StabilityFee:        sdk.MustNewDecFromStr("1.000000001547125958"), // %5 apr
+					LiquidationPenalty:  d("0.05"),
+					AuctionSize:         i(50000000000),
+					Prefix:              0x22,
+					SpotMarketID:        "bnb:usd",
+					LiquidationMarketID: "bnb:usd",
+					ConversionFactor:    i(8),
+				},
 			},
 			DebtParam: cdp.DebtParam{
 				Denom:            "usdx",

x/cdp/keeper/keeper.go

@@ -45,7 +45,10 @@ func NewKeeper(cdc *codec.Codec, key sdk.StoreKey, paramstore subspace.Subspace,
 // CdpDenomIndexIterator returns an sdk.Iterator for all cdps with matching collateral denom
 func (k Keeper) CdpDenomIndexIterator(ctx sdk.Context, denom string) sdk.Iterator {
 	store := prefix.NewStore(ctx.KVStore(k.key), types.CdpKeyPrefix)
-	db, _ := k.GetDenomPrefix(ctx, denom)
+	db, found := k.GetDenomPrefix(ctx, denom)
+	if !found {
+		panic(fmt.Sprintf("denom %s prefix not found", denom))
+	}
 	return sdk.KVStorePrefixIterator(store, types.DenomIterKey(db))
 }
 
@@ -53,7 +56,10 @@ func (k Keeper) CdpDenomIndexIterator(ctx sdk.Context, denom string) sdk.Iterato
 // matching denom and collateral:debt ratio LESS THAN targetRatio
 func (k Keeper) CdpCollateralRatioIndexIterator(ctx sdk.Context, denom string, targetRatio sdk.Dec) sdk.Iterator {
 	store := prefix.NewStore(ctx.KVStore(k.key), types.CollateralRatioIndexPrefix)
-	db, _ := k.GetDenomPrefix(ctx, denom)
+	db, found := k.GetDenomPrefix(ctx, denom)
+	if !found {
+		panic(fmt.Sprintf("denom %s prefix not found", denom))
+	}
 	return store.Iterator(types.CollateralRatioIterKey(db, sdk.ZeroDec()), types.CollateralRatioIterKey(db, targetRatio))
 }
 

x/cdp/keeper/math.go

@@ -0,0 +1,81 @@
+package keeper
+
+import (
+	"sort"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+)
+
+// splitIntIntoWeightedBuckets divides an initial +ve integer among several buckets in proportion to the buckets' weights
+// It uses the largest remainder method: https://en.wikipedia.org/wiki/Largest_remainder_method
+// See also: https://stackoverflow.com/questions/13483430/how-to-make-rounded-percentages-add-up-to-100
+// note - copied from auction, tests are located there.
+func splitIntIntoWeightedBuckets(amount sdk.Int, buckets []sdk.Int) []sdk.Int {
+	// Limit input to +ve numbers as algorithm hasn't been scoped to work with -ve numbers.
+	if amount.IsNegative() {
+		panic("negative amount")
+	}
+	if len(buckets) < 1 {
+		panic("no buckets")
+	}
+	for _, bucket := range buckets {
+		if bucket.IsNegative() {
+			panic("negative bucket")
+		}
+	}
+
+	// 1) Split the amount by weights, recording whole number part and remainder
+
+	totalWeights := totalInts(buckets...)
+	if !totalWeights.IsPositive() {
+		panic("total weights must sum to > 0")
+	}
+
+	quotients := make([]quoRem, len(buckets))
+	for i := range buckets {
+		// amount * ( weight/total_weight )
+		q := amount.Mul(buckets[i]).Quo(totalWeights)
+		r := amount.Mul(buckets[i]).Mod(totalWeights)
+		quotients[i] = quoRem{index: i, quo: q, rem: r}
+	}
+
+	// 2) Calculate total left over from remainders, and apportion it to buckets with the highest remainder (to minimize error)
+
+	// sort by decreasing remainder order
+	sort.Slice(quotients, func(i, j int) bool {
+		return quotients[i].rem.GT(quotients[j].rem)
+	})
+
+	// calculate total left over from remainders
+	allocated := sdk.ZeroInt()
+	for _, qr := range quotients {
+		allocated = allocated.Add(qr.quo)
+	}
+	leftToAllocate := amount.Sub(allocated)
+
+	// apportion according to largest remainder
+	results := make([]sdk.Int, len(quotients))
+	for _, qr := range quotients {
+		results[qr.index] = qr.quo
+		if !leftToAllocate.IsZero() {
+			results[qr.index] = results[qr.index].Add(sdk.OneInt())
+			leftToAllocate = leftToAllocate.Sub(sdk.OneInt())
+		}
+	}
+	return results
+}
+
+type quoRem struct {
+	index int
+	quo   sdk.Int
+	rem   sdk.Int
+}
+
+// totalInts adds together sdk.Ints
+func totalInts(is ...sdk.Int) sdk.Int {
+	total := sdk.ZeroInt()
+	for _, i := range is {
+		total = total.Add(i)
+	}
+	return total
+}