diff --git a/app/test_common.go b/app/test_common.go index f6a99036..f25c1c23 100644 --- a/app/test_common.go +++ b/app/test_common.go @@ -97,8 +97,9 @@ func (tApp TestApp) InitializeFromGenesisStates(genesisStates ...GenesisState) T } func (tApp TestApp) CheckBalance(t *testing.T, ctx sdk.Context, owner sdk.AccAddress, expectedCoins sdk.Coins) { - actualCoins := tApp.GetAccountKeeper().GetAccount(ctx, owner).GetCoins() - require.Equal(t, expectedCoins, actualCoins) + acc := tApp.GetAccountKeeper().GetAccount(ctx, owner) + require.NotNilf(t, acc, "account with address '%s' doesn't exist", owner) + require.Equal(t, expectedCoins, acc.GetCoins()) } // Create a new auth genesis state from some addresses and coins. The state is returned marshalled into a map. diff --git a/x/auction/abci_test.go b/x/auction/abci_test.go index bb6a6585..79f526b5 100644 --- a/x/auction/abci_test.go +++ b/x/auction/abci_test.go @@ -19,7 +19,8 @@ func TestKeeper_EndBlocker(t *testing.T) { // Setup _, addrs := app.GeneratePrivKeyAddressPairs(2) buyer := addrs[0] - recipient := addrs[1] + returnAddrs := addrs[1:] + returnWeights := []sdk.Int{sdk.NewInt(1)} sellerModName := liquidator.ModuleName //sellerAddr := supply.NewModuleAddress(sellerModName) @@ -36,7 +37,7 @@ func TestKeeper_EndBlocker(t *testing.T) { ctx := tApp.NewContext(true, abci.Header{}) keeper := tApp.GetAuctionKeeper() - auctionID, err := keeper.StartForwardReverseAuction(ctx, sellerModName, c("token1", 20), c("token2", 50), recipient) + auctionID, err := keeper.StartForwardReverseAuction(ctx, sellerModName, c("token1", 20), c("token2", 50), returnAddrs, returnWeights) require.NoError(t, err) require.NoError(t, keeper.PlaceBid(ctx, auctionID, buyer, c("token2", 30), c("token1", 20))) diff --git a/x/auction/keeper/auctions.go b/x/auction/keeper/auctions.go index a1215624..96d305bd 100644 --- a/x/auction/keeper/auctions.go +++ b/x/auction/keeper/auctions.go @@ -46,12 +46,16 @@ func (k Keeper) StartReverseAuction(ctx sdk.Context, buyer string, bid sdk.Coin, } // StartForwardReverseAuction starts an auction where bidders bid up to a maxBid, then switch to bidding down on price. -func (k Keeper) StartForwardReverseAuction(ctx sdk.Context, seller string, lot sdk.Coin, maxBid sdk.Coin, otherPerson sdk.AccAddress) (types.ID, sdk.Error) { +func (k Keeper) StartForwardReverseAuction(ctx sdk.Context, seller string, lot sdk.Coin, maxBid sdk.Coin, lotReturnAddrs []sdk.AccAddress, lotReturnWeights []sdk.Int) (types.ID, sdk.Error) { // create auction - auction := types.NewForwardReverseAuction(seller, lot, ctx.BlockTime().Add(types.DefaultMaxAuctionDuration), maxBid, otherPerson) + weightedAddresses, err := types.NewWeightedAddresses(lotReturnAddrs, lotReturnWeights) + if err != nil { + return 0, err + } + auction := types.NewForwardReverseAuction(seller, lot, ctx.BlockTime().Add(types.DefaultMaxAuctionDuration), maxBid, weightedAddresses) // take coins from module account - err := k.supplyKeeper.SendCoinsFromModuleToModule(ctx, seller, types.ModuleName, sdk.NewCoins(lot)) + err = k.supplyKeeper.SendCoinsFromModuleToModule(ctx, seller, types.ModuleName, sdk.NewCoins(lot)) if err != nil { return 0, err } @@ -206,10 +210,17 @@ func (k Keeper) PlaceBidForwardReverse(ctx sdk.Context, a types.ForwardReverseAu if err != nil { return a, err } - err = k.supplyKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, a.OtherPerson, sdk.NewCoins(lotDecrement)) + // FIXME paying out rateably to cdp depositors is vulnerable to errors compounding over multiple bids + lotPayouts, err := splitCoinIntoWeightedBuckets(lotDecrement, a.LotReturns.Weights) if err != nil { return a, err } + for i, payout := range lotPayouts { + err = k.supplyKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, a.LotReturns.Addresses[i], sdk.NewCoins(payout)) + if err != nil { + return a, err + } + } // Update Auction a.Bidder = bidder @@ -318,3 +329,17 @@ func earliestTime(t1, t2 time.Time) time.Time { return t2 // also returned if times are equal } } + +func splitCoinIntoWeightedBuckets(coin sdk.Coin, buckets []sdk.Int) ([]sdk.Coin, sdk.Error) { + for _, bucket := range buckets { + if bucket.IsNegative() { + return nil, sdk.ErrInternal("cannot split coin into bucket with negative weight") + } + } + amounts := splitIntIntoWeightedBuckets(coin.Amount, buckets) + result := make([]sdk.Coin, len(amounts)) + for i, a := range amounts { + result[i] = sdk.NewCoin(coin.Denom, a) + } + return result, nil +} diff --git a/x/auction/keeper/auctions_test.go b/x/auction/keeper/auctions_test.go index 77f174e7..1bc48650 100644 --- a/x/auction/keeper/auctions_test.go +++ b/x/auction/keeper/auctions_test.go @@ -3,6 +3,7 @@ package keeper_test import ( "testing" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth" authexported "github.com/cosmos/cosmos-sdk/x/auth/exported" "github.com/cosmos/cosmos-sdk/x/supply" @@ -94,9 +95,10 @@ func TestReverseAuctionBasic(t *testing.T) { func TestForwardReverseAuctionBasic(t *testing.T) { // Setup - _, addrs := app.GeneratePrivKeyAddressPairs(2) + _, addrs := app.GeneratePrivKeyAddressPairs(4) buyer := addrs[0] - recipient := addrs[1] + returnAddrs := addrs[1:] + returnWeights := []sdk.Int{i(30), i(20), i(10)} sellerModName := liquidator.ModuleName sellerAddr := supply.NewModuleAddress(sellerModName) @@ -106,7 +108,9 @@ func TestForwardReverseAuctionBasic(t *testing.T) { tApp.InitializeFromGenesisStates( NewAuthGenStateFromAccs(authexported.GenesisAccounts{ auth.NewBaseAccount(buyer, cs(c("token1", 100), c("token2", 100)), nil, 0, 0), - auth.NewBaseAccount(recipient, cs(c("token1", 100), c("token2", 100)), nil, 0, 0), + auth.NewBaseAccount(returnAddrs[0], cs(c("token1", 100), c("token2", 100)), nil, 0, 0), + auth.NewBaseAccount(returnAddrs[1], cs(c("token1", 100), c("token2", 100)), nil, 0, 0), + auth.NewBaseAccount(returnAddrs[2], cs(c("token1", 100), c("token2", 100)), nil, 0, 0), sellerAcc, }), ) @@ -114,7 +118,7 @@ func TestForwardReverseAuctionBasic(t *testing.T) { keeper := tApp.GetAuctionKeeper() // Start auction - auctionID, err := keeper.StartForwardReverseAuction(ctx, sellerModName, c("token1", 20), c("token2", 50), recipient) // seller, lot, maxBid, otherPerson + auctionID, err := keeper.StartForwardReverseAuction(ctx, sellerModName, c("token1", 20), c("token2", 50), returnAddrs, returnWeights) // seller, lot, maxBid, otherPerson require.NoError(t, err) // Check seller's coins have decreased tApp.CheckBalance(t, ctx, sellerAddr, cs(c("token1", 80), c("token2", 100))) @@ -125,8 +129,10 @@ func TestForwardReverseAuctionBasic(t *testing.T) { tApp.CheckBalance(t, ctx, buyer, cs(c("token1", 100), c("token2", 90))) // Check seller's coins have increased tApp.CheckBalance(t, ctx, sellerAddr, cs(c("token1", 80), c("token2", 110))) - // Check recipient has not received coins - tApp.CheckBalance(t, ctx, recipient, cs(c("token1", 100), c("token2", 100))) + // Check return addresses have not received coins + for _, ra := range returnAddrs { + tApp.CheckBalance(t, ctx, ra, cs(c("token1", 100), c("token2", 100))) + } // Place a reverse bid require.NoError(t, keeper.PlaceBid(ctx, 0, buyer, c("token2", 50), c("token1", 15))) // bid, lot @@ -134,8 +140,10 @@ func TestForwardReverseAuctionBasic(t *testing.T) { tApp.CheckBalance(t, ctx, buyer, cs(c("token1", 100), c("token2", 50))) // Check seller's coins have increased tApp.CheckBalance(t, ctx, sellerAddr, cs(c("token1", 80), c("token2", 150))) - // Check "recipient" has received coins - tApp.CheckBalance(t, ctx, recipient, cs(c("token1", 105), c("token2", 100))) + // Check return addresses have received coins + tApp.CheckBalance(t, ctx, returnAddrs[0], cs(c("token1", 102), c("token2", 100))) + tApp.CheckBalance(t, ctx, returnAddrs[1], cs(c("token1", 102), c("token2", 100))) + tApp.CheckBalance(t, ctx, returnAddrs[2], cs(c("token1", 101), c("token2", 100))) // Close auction at just after auction expiry ctx = ctx.WithBlockTime(ctx.BlockTime().Add(types.DefaultBidDuration)) diff --git a/x/auction/keeper/integration_test.go b/x/auction/keeper/integration_test.go index 3ad2500e..38d9e4ce 100644 --- a/x/auction/keeper/integration_test.go +++ b/x/auction/keeper/integration_test.go @@ -10,6 +10,13 @@ import ( func c(denom string, amount int64) sdk.Coin { return sdk.NewInt64Coin(denom, amount) } func cs(coins ...sdk.Coin) sdk.Coins { return sdk.NewCoins(coins...) } +func i(n int64) sdk.Int { return sdk.NewInt(n) } +func is(ns ...int64) (is []sdk.Int) { + for _, n := range ns { + is = append(is, sdk.NewInt(n)) + } + return +} func NewAuthGenStateFromAccs(accounts authexported.GenesisAccounts) app.GenesisState { authGenesis := auth.NewGenesisState(auth.DefaultParams(), accounts) diff --git a/x/auction/keeper/math.go b/x/auction/keeper/math.go new file mode 100644 index 00000000..437b4f22 --- /dev/null +++ b/x/auction/keeper/math.go @@ -0,0 +1,69 @@ +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 +func splitIntIntoWeightedBuckets(amount sdk.Int, buckets []sdk.Int) []sdk.Int { + // TODO ideally change algorithm to work with -ve numbers. Limiting to +ve numbers until them + if amount.IsNegative() { + panic("negative amount") + } + for _, bucket := range buckets { + if bucket.IsNegative() { + panic("negative bucket") + } + } + + totalWeights := totalInts(buckets...) + + // split amount by weights, recording whole number part and remainder + quotients := make([]quoRem, len(buckets)) + for i := range buckets { + q := amount.Mul(buckets[i]).Quo(totalWeights) + r := amount.Mul(buckets[i]).Mod(totalWeights) + quotients[i] = quoRem{index: i, quo: q, rem: r} + } + + // apportion left over to buckets with the highest remainder (to minimize error) + sort.Slice(quotients, func(i, j int) bool { + return quotients[i].rem.GT(quotients[j].rem) // decreasing remainder order + }) + + allocated := sdk.ZeroInt() + for _, qr := range quotients { + allocated = allocated.Add(qr.quo) + } + leftToAllocate := amount.Sub(allocated) + + 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 +} diff --git a/x/auction/keeper/math_test.go b/x/auction/keeper/math_test.go new file mode 100644 index 00000000..58e33e7e --- /dev/null +++ b/x/auction/keeper/math_test.go @@ -0,0 +1,37 @@ +package keeper + +import ( + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/require" +) + +func TestSplitIntIntoWeightedBuckets(t *testing.T) { + testCases := []struct { + name string + amount sdk.Int + buckets []sdk.Int + want []sdk.Int + }{ + {"2split1,1", i(2), is(1, 1), is(1, 1)}, + {"100split1,9", i(100), is(1, 9), is(10, 90)}, + {"7split1,2", i(7), is(1, 2), is(2, 5)}, + {"17split1,1,1", i(17), is(1, 1, 1), is(6, 6, 5)}, + // TODO more tests + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got := splitIntIntoWeightedBuckets(tc.amount, tc.buckets) + require.Equal(t, tc.want, got) + }) + } +} + +func i(n int64) sdk.Int { return sdk.NewInt(n) } +func is(ns ...int64) (is []sdk.Int) { + for _, n := range ns { + is = append(is, sdk.NewInt(n)) + } + return +} diff --git a/x/auction/types/auctions.go b/x/auction/types/auctions.go index 9e45ee5c..b293c5ee 100644 --- a/x/auction/types/auctions.go +++ b/x/auction/types/auctions.go @@ -133,8 +133,8 @@ func NewReverseAuction(buyerModAccName string, bid sdk.Coin, initialLot sdk.Coin // ForwardReverseAuction type for forward reverse auction type ForwardReverseAuction struct { BaseAuction - MaxBid sdk.Coin - OtherPerson sdk.AccAddress // TODO rename, this is normally the original CDP owner, will have to be updated to account for deposits + MaxBid sdk.Coin + LotReturns WeightedAddresses // return addresses to pay out reductions in the lot amount to. Lot is bid down during reverse phase. } // WithID returns an auction with the ID set @@ -149,15 +149,15 @@ func (a ForwardReverseAuction) String() string { End Time: %s Max End Time: %s Max Bid %s - Other Person %s`, + LotReturns %s`, a.GetID(), a.Initiator, a.Lot, a.Bidder, a.Bid, a.GetEndTime().String(), - a.MaxEndTime.String(), a.MaxBid, a.OtherPerson, + a.MaxEndTime.String(), a.MaxBid, a.LotReturns, ) } // NewForwardReverseAuction creates a new forward reverse auction -func NewForwardReverseAuction(seller string, lot sdk.Coin, EndTime time.Time, maxBid sdk.Coin, otherPerson sdk.AccAddress) ForwardReverseAuction { +func NewForwardReverseAuction(seller string, lot sdk.Coin, EndTime time.Time, maxBid sdk.Coin, lotReturns WeightedAddresses) ForwardReverseAuction { auction := ForwardReverseAuction{ BaseAuction: BaseAuction{ // no ID @@ -167,8 +167,28 @@ func NewForwardReverseAuction(seller string, lot sdk.Coin, EndTime time.Time, ma Bid: sdk.NewInt64Coin(maxBid.Denom, 0), EndTime: EndTime, MaxEndTime: EndTime}, - MaxBid: maxBid, - OtherPerson: otherPerson, + MaxBid: maxBid, + LotReturns: lotReturns, } return auction } + +type WeightedAddresses struct { + Addresses []sdk.AccAddress + Weights []sdk.Int +} + +func NewWeightedAddresses(addrs []sdk.AccAddress, weights []sdk.Int) (WeightedAddresses, sdk.Error) { + if len(addrs) != len(weights) { + return WeightedAddresses{}, sdk.ErrInternal("number of addresses doesn't match number of weights") + } + for _, w := range weights { + if w.IsNegative() { + return WeightedAddresses{}, sdk.ErrInternal("weights contain a negative amount") + } + } + return WeightedAddresses{ + Addresses: addrs, + Weights: weights, + }, nil +}