From dc6f5c6c830ba14d791329a83d8e594801fe0f23 Mon Sep 17 00:00:00 2001 From: Ruaridh Date: Mon, 26 Jul 2021 20:07:24 +0100 Subject: [PATCH] Fix incentive usdx/borrow/supply reward calculation bug (#974) * extract borrow sync logic into separate func * fix borrow reward calculations Use the normalized borrow as the source shares in reward calculations. * extract supply sync logic into separate func * prepare to fix supply reward calculations * fix deposit reward calculations Use the normalized deposit as the source shares in reward calculations. * extract usdx sync logic into separate func * prepare to fix usdx reward calculations * fix cdp reward calculations Use the normalized cdp debt as the source shares in reward calculations. * fix compile error from messed up partial stage * Fix incentive usdx reward bug (#976) * minor test refactors * fix overpayment bug Init methods should not read params. Add test to cover bug * fix typos --- x/cdp/types/cdp.go | 14 ++ x/cdp/types/cdp_test.go | 62 +++++ x/hard/types/borrow.go | 30 +++ x/hard/types/borrow_test.go | 117 ++++++++++ x/hard/types/deposit.go | 30 +++ x/hard/types/deposit_test.go | 117 ++++++++++ x/incentive/keeper/cdp_test.go | 71 ------ x/incentive/keeper/hooks.go | 2 +- x/incentive/keeper/integration_test.go | 38 ++-- x/incentive/keeper/rewards_borrow.go | 82 ++++--- .../keeper/rewards_borrow_init_test.go | 30 +-- .../keeper/rewards_borrow_sync_test.go | 106 ++++++--- x/incentive/keeper/rewards_borrow_test.go | 69 +++++- .../keeper/rewards_borrow_update_test.go | 37 +-- x/incentive/keeper/rewards_supply.go | 72 +++--- .../keeper/rewards_supply_init_test.go | 30 +-- .../keeper/rewards_supply_sync_test.go | 106 ++++++--- x/incentive/keeper/rewards_supply_test.go | 70 +++++- .../keeper/rewards_supply_update_test.go | 37 +-- x/incentive/keeper/rewards_usdx.go | 48 ++-- x/incentive/keeper/rewards_usdx_test.go | 213 ++++++++++++++++++ x/incentive/keeper/rewards_usdx_unit_test.go | 83 +++---- x/incentive/testutil/integration.go | 88 +++++++- 23 files changed, 1165 insertions(+), 387 deletions(-) create mode 100644 x/hard/types/borrow_test.go create mode 100644 x/hard/types/deposit_test.go delete mode 100644 x/incentive/keeper/cdp_test.go diff --git a/x/cdp/types/cdp.go b/x/cdp/types/cdp.go index 20b9e5b8..897c92e5 100644 --- a/x/cdp/types/cdp.go +++ b/x/cdp/types/cdp.go @@ -104,6 +104,20 @@ func (cdp CDP) GetTotalPrincipal() sdk.Coin { return cdp.Principal.Add(cdp.AccumulatedFees) } +// GetNormalizedPrincipal returns the total cdp principal divided by the interest factor. +// +// Multiplying the normalized principal by the current global factor gives the current debt (ie including all interest, ie a synced cdp). +// The normalized principal is effectively how big the principal would have been if it had been borrowed at time 0 and not touched since. +// +// An error is returned if the cdp interest factor is in an invalid state. +func (cdp CDP) GetNormalizedPrincipal() (sdk.Dec, error) { + unsyncedDebt := cdp.GetTotalPrincipal().Amount + if cdp.InterestFactor.LT(sdk.OneDec()) { + return sdk.Dec{}, fmt.Errorf("interest factor '%s' must be ≥ 1", cdp.InterestFactor) + } + return unsyncedDebt.ToDec().Quo(cdp.InterestFactor), nil +} + // 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 86452d80..a96a6cd0 100644 --- a/x/cdp/types/cdp_test.go +++ b/x/cdp/types/cdp_test.go @@ -176,6 +176,68 @@ func (suite *CdpValidationSuite) TestCdpGetTotalPrinciple() { suite.Require().Equal(cdp.GetTotalPrincipal(), principal.Add(accumulatedFees)) } +func (suite *CdpValidationSuite) TestCDPGetNormalizedPrincipal() { + type expectedErr struct { + expectPass bool + contains string + } + testCases := []struct { + name string + cdp types.CDP + expected sdk.Dec + expectedErr expectedErr + }{ + { + name: "principal + fees is divided by factor correctly", + cdp: types.CDP{ + Principal: sdk.NewInt64Coin("usdx", 1e9), + AccumulatedFees: sdk.NewInt64Coin("usdx", 1e6), + InterestFactor: sdk.MustNewDecFromStr("2"), + }, + expected: sdk.MustNewDecFromStr("500500000"), + expectedErr: expectedErr{ + expectPass: true, + }, + }, + { + name: "factor < 1 returns error", + cdp: types.CDP{ + Principal: sdk.NewInt64Coin("usdx", 1e9), + AccumulatedFees: sdk.NewInt64Coin("usdx", 1e6), + InterestFactor: sdk.MustNewDecFromStr("0.999999999999999999"), + }, + expectedErr: expectedErr{ + contains: "must be ≥ 1", + }, + }, + { + name: "0 factor returns error rather than div by 0 panic", + cdp: types.CDP{ + Principal: sdk.NewInt64Coin("usdx", 1e9), + AccumulatedFees: sdk.NewInt64Coin("usdx", 1e6), + InterestFactor: sdk.MustNewDecFromStr("0"), + }, + expectedErr: expectedErr{ + contains: "must be ≥ 1", + }, + }, + } + + for _, tc := range testCases { + suite.Run(tc.name, func() { + np, err := tc.cdp.GetNormalizedPrincipal() + + if tc.expectedErr.expectPass { + suite.Require().NoError(err, tc.name) + suite.Equal(tc.expected, np) + } else { + suite.Require().Error(err, tc.name) + suite.Contains(err.Error(), tc.expectedErr.contains) + } + }) + } +} + func TestCdpValidationSuite(t *testing.T) { suite.Run(t, new(CdpValidationSuite)) } diff --git a/x/hard/types/borrow.go b/x/hard/types/borrow.go index 80f8bc83..2e749da4 100644 --- a/x/hard/types/borrow.go +++ b/x/hard/types/borrow.go @@ -23,6 +23,36 @@ func NewBorrow(borrower sdk.AccAddress, amount sdk.Coins, index BorrowInterestFa } } +// NormalizedBorrow is the borrow amounts divided by the interest factors. +// +// Multiplying the normalized borrow by the current global factors gives the current borrow (ie including all interest, ie a synced borrow). +// The normalized borrow is effectively how big the borrow would have been if it had been borrowed at time 0 and not touched since. +// +// An error is returned if the borrow is in an invalid state. +func (b Borrow) NormalizedBorrow() (sdk.DecCoins, error) { + + normalized := sdk.NewDecCoins() + + for _, coin := range b.Amount { + + factor, found := b.Index.GetInterestFactor(coin.Denom) + if !found { + return nil, fmt.Errorf("borrowed amount '%s' missing interest factor", coin.Denom) + } + if factor.LT(sdk.OneDec()) { + return nil, fmt.Errorf("interest factor '%s' < 1", coin.Denom) + } + + normalized = normalized.Add( + sdk.NewDecCoinFromDec( + coin.Denom, + coin.Amount.ToDec().Quo(factor), + ), + ) + } + return normalized, nil +} + // Validate deposit validation func (b Borrow) Validate() error { if b.Borrower.Empty() { diff --git a/x/hard/types/borrow_test.go b/x/hard/types/borrow_test.go new file mode 100644 index 00000000..5f7994bb --- /dev/null +++ b/x/hard/types/borrow_test.go @@ -0,0 +1,117 @@ +package types_test + +import ( + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/require" + + "github.com/kava-labs/kava/x/hard/types" +) + +func TestBorrow_NormalizedBorrow(t *testing.T) { + testCases := []struct { + name string + borrow types.Borrow + expect sdk.DecCoins + expectErr string + }{ + { + name: "multiple denoms are calculated correctly", + borrow: types.Borrow{ + Amount: sdk.NewCoins( + sdk.NewInt64Coin("bnb", 100e8), + sdk.NewInt64Coin("xrpb", 1e8), + ), + Index: types.BorrowInterestFactors{ + { + Denom: "xrpb", + Value: sdk.MustNewDecFromStr("1.25"), + }, + { + Denom: "bnb", + Value: sdk.MustNewDecFromStr("2.0"), + }, + }, + }, + expect: sdk.NewDecCoins( + sdk.NewInt64DecCoin("bnb", 50e8), + sdk.NewInt64DecCoin("xrpb", 8e7), + ), + }, + { + name: "empty borrow amount returns empty dec coins", + borrow: types.Borrow{ + Amount: sdk.Coins{}, + Index: types.BorrowInterestFactors{}, + }, + expect: sdk.DecCoins{}, + }, + { + name: "nil borrow amount returns empty dec coins", + borrow: types.Borrow{ + Amount: nil, + Index: types.BorrowInterestFactors{}, + }, + expect: sdk.DecCoins{}, + }, + { + name: "missing indexes return error", + borrow: types.Borrow{ + Amount: sdk.NewCoins( + sdk.NewInt64Coin("bnb", 100e8), + ), + Index: types.BorrowInterestFactors{ + { + Denom: "xrpb", + Value: sdk.MustNewDecFromStr("1.25"), + }, + }, + }, + expectErr: "missing interest factor", + }, + { + name: "invalid indexes return error", + borrow: types.Borrow{ + Amount: sdk.NewCoins( + sdk.NewInt64Coin("bnb", 100e8), + ), + Index: types.BorrowInterestFactors{ + { + Denom: "bnb", + Value: sdk.MustNewDecFromStr("0.999999999999999999"), + }, + }, + }, + expectErr: "< 1", + }, + { + name: "zero indexes return error rather than panicking", + borrow: types.Borrow{ + Amount: sdk.NewCoins( + sdk.NewInt64Coin("bnb", 100e8), + ), + Index: types.BorrowInterestFactors{ + { + Denom: "bnb", + Value: sdk.MustNewDecFromStr("0"), + }, + }, + }, + expectErr: "< 1", + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + nb, err := tc.borrow.NormalizedBorrow() + + require.Equal(t, tc.expect, nb) + + if len(tc.expectErr) > 0 { + require.Error(t, err) + require.Contains(t, err.Error(), tc.expectErr) + } + }) + } + +} diff --git a/x/hard/types/deposit.go b/x/hard/types/deposit.go index ad517e14..3cde8b62 100644 --- a/x/hard/types/deposit.go +++ b/x/hard/types/deposit.go @@ -23,6 +23,36 @@ func NewDeposit(depositor sdk.AccAddress, amount sdk.Coins, indexes SupplyIntere } } +// NormalizedDeposit is the deposit amounts divided by the interest factors. +// +// Multiplying the normalized deposit by the current global factors gives the current deposit (ie including all interest, ie a synced deposit). +// The normalized deposit is effectively how big the deposit would have been if it had been supplied at time 0 and not touched since. +// +// An error is returned if the deposit is in an invalid state. +func (b Deposit) NormalizedDeposit() (sdk.DecCoins, error) { + + normalized := sdk.NewDecCoins() + + for _, coin := range b.Amount { + + factor, found := b.Index.GetInterestFactor(coin.Denom) + if !found { + return nil, fmt.Errorf("deposited amount '%s' missing interest factor", coin.Denom) + } + if factor.LT(sdk.OneDec()) { + return nil, fmt.Errorf("interest factor '%s' < 1", coin.Denom) + } + + normalized = normalized.Add( + sdk.NewDecCoinFromDec( + coin.Denom, + coin.Amount.ToDec().Quo(factor), + ), + ) + } + return normalized, nil +} + // Validate deposit validation func (d Deposit) Validate() error { if d.Depositor.Empty() { diff --git a/x/hard/types/deposit_test.go b/x/hard/types/deposit_test.go new file mode 100644 index 00000000..7247c13b --- /dev/null +++ b/x/hard/types/deposit_test.go @@ -0,0 +1,117 @@ +package types_test + +import ( + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/require" + + "github.com/kava-labs/kava/x/hard/types" +) + +func TestDeposit_NormalizedDeposit(t *testing.T) { + testCases := []struct { + name string + deposit types.Deposit + expect sdk.DecCoins + expectErr string + }{ + { + name: "multiple denoms are calculated correctly", + deposit: types.Deposit{ + Amount: sdk.NewCoins( + sdk.NewInt64Coin("bnb", 100e8), + sdk.NewInt64Coin("xrpb", 1e8), + ), + Index: types.SupplyInterestFactors{ + { + Denom: "xrpb", + Value: sdk.MustNewDecFromStr("1.25"), + }, + { + Denom: "bnb", + Value: sdk.MustNewDecFromStr("2.0"), + }, + }, + }, + expect: sdk.NewDecCoins( + sdk.NewInt64DecCoin("bnb", 50e8), + sdk.NewInt64DecCoin("xrpb", 8e7), + ), + }, + { + name: "empty deposit amount returns empty dec coins", + deposit: types.Deposit{ + Amount: sdk.Coins{}, + Index: types.SupplyInterestFactors{}, + }, + expect: sdk.DecCoins{}, + }, + { + name: "nil deposit amount returns empty dec coins", + deposit: types.Deposit{ + Amount: nil, + Index: types.SupplyInterestFactors{}, + }, + expect: sdk.DecCoins{}, + }, + { + name: "missing indexes return error", + deposit: types.Deposit{ + Amount: sdk.NewCoins( + sdk.NewInt64Coin("bnb", 100e8), + ), + Index: types.SupplyInterestFactors{ + { + Denom: "xrpb", + Value: sdk.MustNewDecFromStr("1.25"), + }, + }, + }, + expectErr: "missing interest factor", + }, + { + name: "invalid indexes return error", + deposit: types.Deposit{ + Amount: sdk.NewCoins( + sdk.NewInt64Coin("bnb", 100e8), + ), + Index: types.SupplyInterestFactors{ + { + Denom: "bnb", + Value: sdk.MustNewDecFromStr("0.999999999999999999"), + }, + }, + }, + expectErr: "< 1", + }, + { + name: "zero indexes return error rather than panicking", + deposit: types.Deposit{ + Amount: sdk.NewCoins( + sdk.NewInt64Coin("bnb", 100e8), + ), + Index: types.SupplyInterestFactors{ + { + Denom: "bnb", + Value: sdk.MustNewDecFromStr("0"), + }, + }, + }, + expectErr: "< 1", + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + nb, err := tc.deposit.NormalizedDeposit() + + require.Equal(t, tc.expect, nb) + + if len(tc.expectErr) > 0 { + require.Error(t, err) + require.Contains(t, err.Error(), tc.expectErr) + } + }) + } + +} diff --git a/x/incentive/keeper/cdp_test.go b/x/incentive/keeper/cdp_test.go deleted file mode 100644 index 83f256b7..00000000 --- a/x/incentive/keeper/cdp_test.go +++ /dev/null @@ -1,71 +0,0 @@ -package keeper_test - -import ( - "testing" - "time" - - "github.com/stretchr/testify/require" - abci "github.com/tendermint/tendermint/abci/types" - - "github.com/kava-labs/kava/app" - "github.com/kava-labs/kava/x/incentive/testutil" - "github.com/kava-labs/kava/x/incentive/types" -) - -func TestRiskyCDPsAccumulateRewards(t *testing.T) { - genesisTime := time.Date(2020, 12, 15, 14, 0, 0, 0, time.UTC) - _, addrs := app.GeneratePrivKeyAddressPairs(5) - - initialCollateral := c("bnb", 1_000_000_000) - user := addrs[0] - authBuilder := app.NewAuthGenesisBuilder(). - WithSimpleAccount(user, cs(initialCollateral)) - - collateralType := "bnb-a" - rewardsPerSecond := c(types.USDXMintingRewardDenom, 1_000_000) - - incentBuilder := testutil.NewIncentiveGenesisBuilder(). - WithGenesisTime(genesisTime). - WithSimpleUSDXRewardPeriod(collateralType, rewardsPerSecond) - - tApp := app.NewTestApp() - tApp.InitializeFromGenesisStatesWithTime( - genesisTime, - authBuilder.BuildMarshalled(), - NewPricefeedGenStateMultiFromTime(genesisTime), - NewCDPGenStateMulti(), - incentBuilder.BuildMarshalled(), - ) - ctx := tApp.NewContext(true, abci.Header{Height: 1, Time: genesisTime}) - - // Setup cdp state containing one CDP - cdpKeeper := tApp.GetCDPKeeper() - err := cdpKeeper.AddCdp(ctx, user, initialCollateral, c("usdx", 100_000_000), collateralType) - require.NoError(t, err) - - // Skip ahead two blocks to accumulate both interest and usdx reward for the cdp - // Two blocks are required because the cdp begin blocker runs before incentive begin blocker. - // In the first begin block the cdp is synced, which triggers its claim to sync. But no global rewards have accumulated yet so the sync does nothing. - // Global rewards accumulate immediately after during the incentive begin blocker. - // Rewards are added to the cdp's claim in the next block when the cdp is synced. - _ = tApp.EndBlocker(ctx, abci.RequestEndBlock{}) - ctx = ctx.WithBlockTime(ctx.BlockTime().Add(10 * time.Minute)) - _ = tApp.BeginBlocker(ctx, abci.RequestBeginBlock{}) // height and time in header are ignored by module begin blockers - - _ = tApp.EndBlocker(ctx, abci.RequestEndBlock{}) - ctx = ctx.WithBlockTime(ctx.BlockTime().Add(10 * time.Minute)) - _ = tApp.BeginBlocker(ctx, abci.RequestBeginBlock{}) - - // check cdp rewards - cdp, found := cdpKeeper.GetCdpByOwnerAndCollateralType(ctx, user, collateralType) - require.True(t, found) - // This additional sync adds the rewards accumulated at the end of the last begin block. - // They weren't added during the begin blocker as the incentive BB runs after the CDP BB. - incentiveKeeper := tApp.GetIncentiveKeeper() - incentiveKeeper.SynchronizeUSDXMintingReward(ctx, cdp) - claim, found := incentiveKeeper.GetUSDXMintingClaim(ctx, user) - require.True(t, found) - - // rewards are roughly rewardsPerSecond * secondsElapsed (10mins) * num blocks (2) - require.Equal(t, c(types.USDXMintingRewardDenom, 1_200_000_557), claim.Reward) -} diff --git a/x/incentive/keeper/hooks.go b/x/incentive/keeper/hooks.go index 7ea59886..3b04c4ac 100644 --- a/x/incentive/keeper/hooks.go +++ b/x/incentive/keeper/hooks.go @@ -138,7 +138,7 @@ func (h Hooks) AfterValidatorBonded(ctx sdk.Context, consAddr sdk.ConsAddress, v func (h Hooks) AfterDelegationModified(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) { } -// BeforeDelegationRemoved runs directly before a delegation is deleted +// BeforeDelegationRemoved runs directly before a delegation is deleted. BeforeDelegationSharesModified is run prior to this. func (h Hooks) BeforeDelegationRemoved(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) { } diff --git a/x/incentive/keeper/integration_test.go b/x/incentive/keeper/integration_test.go index 18912b42..41671844 100644 --- a/x/incentive/keeper/integration_test.go +++ b/x/incentive/keeper/integration_test.go @@ -109,6 +109,8 @@ func NewCDPGenStateMulti() app.GenesisState { } func NewPricefeedGenStateMultiFromTime(t time.Time) app.GenesisState { + expiry := 100 * 365 * 24 * time.Hour // 100 years + pfGenesis := pricefeed.GenesisState{ Params: pricefeed.Params{ Markets: []pricefeed.Market{ @@ -125,37 +127,37 @@ func NewPricefeedGenStateMultiFromTime(t time.Time) app.GenesisState { MarketID: "kava:usd", OracleAddress: sdk.AccAddress{}, Price: sdk.MustNewDecFromStr("2.00"), - Expiry: t.Add(1 * time.Hour), + Expiry: t.Add(expiry), }, { MarketID: "btc:usd", OracleAddress: sdk.AccAddress{}, Price: sdk.MustNewDecFromStr("8000.00"), - Expiry: t.Add(1 * time.Hour), + Expiry: t.Add(expiry), }, { MarketID: "xrp:usd", OracleAddress: sdk.AccAddress{}, Price: sdk.MustNewDecFromStr("0.25"), - Expiry: t.Add(1 * time.Hour), + Expiry: t.Add(expiry), }, { MarketID: "bnb:usd", OracleAddress: sdk.AccAddress{}, Price: sdk.MustNewDecFromStr("17.25"), - Expiry: t.Add(1 * time.Hour), + Expiry: t.Add(expiry), }, { MarketID: "busd:usd", OracleAddress: sdk.AccAddress{}, Price: sdk.OneDec(), - Expiry: t.Add(1 * time.Hour), + Expiry: t.Add(expiry), }, { MarketID: "zzz:usd", OracleAddress: sdk.AccAddress{}, Price: sdk.MustNewDecFromStr("2.00"), - Expiry: t.Add(1 * time.Hour), + Expiry: t.Add(expiry), }, }, } @@ -186,22 +188,20 @@ func NewStakingGenesisState() app.GenesisState { } } -func NewCommitteeGenesisState(members []sdk.AccAddress) app.GenesisState { +func NewCommitteeGenesisState(committeeID uint64, members ...sdk.AccAddress) app.GenesisState { genState := committeetypes.DefaultGenesisState() + genState.Committees = committeetypes.Committees{ - committeetypes.MemberCommittee{ - BaseCommittee: committeetypes.BaseCommittee{ - ID: genState.NextProposalID, - Description: "This committee is for testing.", - Members: members, - Permissions: []committeetypes.Permission{committeetypes.GodPermission{}}, - VoteThreshold: d("0.667"), - ProposalDuration: time.Hour * 24 * 7, - TallyOption: committeetypes.FirstPastThePost, - }, - }, + committeetypes.NewMemberCommittee( + committeeID, + "This committee is for testing.", + members, + []committeetypes.Permission{committeetypes.GodPermission{}}, + sdk.MustNewDecFromStr("0.666666667"), + time.Hour*24*7, + committeetypes.FirstPastThePost, + ), } - genState.NextProposalID += 1 return app.GenesisState{ committeetypes.ModuleName: committeetypes.ModuleCdc.MustMarshalJSON(genState), } diff --git a/x/incentive/keeper/rewards_borrow.go b/x/incentive/keeper/rewards_borrow.go index 61b8d768..eb534b94 100644 --- a/x/incentive/keeper/rewards_borrow.go +++ b/x/incentive/keeper/rewards_borrow.go @@ -38,8 +38,14 @@ func (k Keeper) AccumulateHardBorrowRewards(ctx sdk.Context, rewardPeriod types. } // getHardBorrowTotalSourceShares fetches the sum of all source shares for a borrow reward. -// In the case of hard borrow, this is the total borrowed divided by the borrow interest factor. -// This give the "pre interest" value of the total borrowed. +// +// In the case of hard borrow, this is the total borrowed divided by the borrow interest factor (for a particular denom). +// This gives the "pre interest" or "normalized" value of the total borrowed. This is an amount, that if it was borrowed when +// the interest factor was zero (ie at time 0), the current value of it with interest would be equal to the current total borrowed. +// +// The normalized borrow is also used for each individual borrow's source shares amount. Normalized amounts do not change except through +// user input. This is essential as claims must be synced before any change to a source shares amount. The actual borrowed amounts cannot +// be used as they increase every block due to interest. func (k Keeper) getHardBorrowTotalSourceShares(ctx sdk.Context, denom string) sdk.Dec { totalBorrowedCoins, found := k.hardKeeper.GetBorrowedCoins(ctx) if !found { @@ -87,40 +93,56 @@ func (k Keeper) SynchronizeHardBorrowReward(ctx sdk.Context, borrow hardtypes.Bo return } - for _, coin := range borrow.Amount { - globalRewardIndexes, found := k.GetHardBorrowRewardIndexes(ctx, coin.Denom) - if !found { - // The global factor is only not found if - // - the borrowed denom has not started accumulating rewards yet (either there is no reward specified in params, or the reward start time hasn't been hit) - // - OR it was wrongly deleted from state (factors should never be removed while unsynced claims exist) - // If not found we could either skip this sync, or assume the global factor is zero. - // Skipping will avoid storing unnecessary factors in the claim for non rewarded denoms. - // And in the event a global factor is wrongly deleted, it will avoid this function panicking when calculating rewards. - continue - } + // Source shares for hard borrows is their normalized borrow amount + normalizedBorrows, err := borrow.NormalizedBorrow() + if err != nil { + panic(fmt.Sprintf("during borrow reward sync, could not get normalized borrow for %s: %s", borrow.Borrower, err.Error())) + } - userRewardIndexes, found := claim.BorrowRewardIndexes.Get(coin.Denom) - if !found { - // Normally the reward indexes should always be found. - // But if a denom was not rewarded then becomes rewarded (ie a reward period is added to params), then the indexes will be missing from claims for that borrowed denom. - // So given the reward period was just added, assume the starting value for any global reward indexes, which is an empty slice. - userRewardIndexes = types.RewardIndexes{} - } + for _, normedBorrow := range normalizedBorrows { - newRewards, err := k.CalculateRewards(userRewardIndexes, globalRewardIndexes, coin.Amount.ToDec()) - if err != nil { - // Global reward factors should never decrease, as it would lead to a negative update to claim.Rewards. - // This panics if a global reward factor decreases or disappears between the old and new indexes. - panic(fmt.Sprintf("corrupted global reward indexes found: %v", err)) - } - - claim.Reward = claim.Reward.Add(newRewards...) - claim.BorrowRewardIndexes = claim.BorrowRewardIndexes.With(coin.Denom, globalRewardIndexes) + claim = k.synchronizeSingleHardBorrowReward(ctx, claim, normedBorrow.Denom, normedBorrow.Amount) } k.SetHardLiquidityProviderClaim(ctx, claim) } -// UpdateHardBorrowIndexDenoms adds any new borrow denoms to the claim's borrow reward index +// synchronizeSingleHardBorrowReward synchronizes a single rewarded borrow denom in a hard claim. +// It returns the claim without setting in the store. +// The public methods for accessing and modifying claims are preferred over this one. Direct modification of claims is easy to get wrong. +func (k Keeper) synchronizeSingleHardBorrowReward(ctx sdk.Context, claim types.HardLiquidityProviderClaim, denom string, sourceShares sdk.Dec) types.HardLiquidityProviderClaim { + globalRewardIndexes, found := k.GetHardBorrowRewardIndexes(ctx, denom) + if !found { + // The global factor is only not found if + // - the borrowed denom has not started accumulating rewards yet (either there is no reward specified in params, or the reward start time hasn't been hit) + // - OR it was wrongly deleted from state (factors should never be removed while unsynced claims exist) + // If not found we could either skip this sync, or assume the global factor is zero. + // Skipping will avoid storing unnecessary factors in the claim for non rewarded denoms. + // And in the event a global factor is wrongly deleted, it will avoid this function panicking when calculating rewards. + return claim + } + + userRewardIndexes, found := claim.BorrowRewardIndexes.Get(denom) + if !found { + // Normally the reward indexes should always be found. + // But if a denom was not rewarded then becomes rewarded (ie a reward period is added to params), then the indexes will be missing from claims for that borrowed denom. + // So given the reward period was just added, assume the starting value for any global reward indexes, which is an empty slice. + userRewardIndexes = types.RewardIndexes{} + } + + newRewards, err := k.CalculateRewards(userRewardIndexes, globalRewardIndexes, sourceShares) + if err != nil { + // Global reward factors should never decrease, as it would lead to a negative update to claim.Rewards. + // This panics if a global reward factor decreases or disappears between the old and new indexes. + panic(fmt.Sprintf("corrupted global reward indexes found: %v", err)) + } + + claim.Reward = claim.Reward.Add(newRewards...) + claim.BorrowRewardIndexes = claim.BorrowRewardIndexes.With(denom, globalRewardIndexes) + + return claim +} + +// UpdateHardBorrowIndexDenoms adds or removes reward indexes from a claim to match the denoms in the borrow. func (k Keeper) UpdateHardBorrowIndexDenoms(ctx sdk.Context, borrow hardtypes.Borrow) { claim, found := k.GetHardLiquidityProviderClaim(ctx, borrow.Borrower) if !found { diff --git a/x/incentive/keeper/rewards_borrow_init_test.go b/x/incentive/keeper/rewards_borrow_init_test.go index 296041e2..39d3815e 100644 --- a/x/incentive/keeper/rewards_borrow_init_test.go +++ b/x/incentive/keeper/rewards_borrow_init_test.go @@ -5,19 +5,10 @@ import ( "github.com/stretchr/testify/suite" - hardtypes "github.com/kava-labs/kava/x/hard/types" "github.com/kava-labs/kava/x/incentive/types" ) // InitializeHardBorrowRewardTests runs unit tests for the keeper.InitializeHardBorrowReward method -// -// inputs -// - claim in store if it exists (only claim.BorrowRewardIndexes) -// - global indexes in store -// - borrow function arg (only borrow.Amount) -// -// outputs -// - sets or creates a claim type InitializeHardBorrowRewardTests struct { unitTester } @@ -41,10 +32,9 @@ func (suite *InitializeHardBorrowRewardTests) TestClaimIndexesAreSetWhenClaimExi globalIndexes := nonEmptyMultiRewardIndexes suite.storeGlobalBorrowIndexes(globalIndexes) - borrow := hardtypes.Borrow{ - Borrower: claim.Owner, - Amount: arbitraryCoinsWithDenoms(extractCollateralTypes(globalIndexes)...), - } + borrow := NewBorrowBuilder(claim.Owner). + WithArbitrarySourceShares(extractCollateralTypes(globalIndexes)...). + Build() suite.keeper.InitializeHardBorrowReward(suite.ctx, borrow) @@ -56,10 +46,9 @@ func (suite *InitializeHardBorrowRewardTests) TestClaimIndexesAreSetWhenClaimDoe suite.storeGlobalBorrowIndexes(globalIndexes) owner := arbitraryAddress() - borrow := hardtypes.Borrow{ - Borrower: owner, - Amount: arbitraryCoinsWithDenoms(extractCollateralTypes(globalIndexes)...), - } + borrow := NewBorrowBuilder(owner). + WithArbitrarySourceShares(extractCollateralTypes(globalIndexes)...). + Build() suite.keeper.InitializeHardBorrowReward(suite.ctx, borrow) @@ -77,10 +66,9 @@ func (suite *InitializeHardBorrowRewardTests) TestClaimIndexesAreSetEmptyForMiss // This happens when a borrow denom has no rewards associated with it. expectedIndexes := appendUniqueEmptyMultiRewardIndex(globalIndexes) borrowedDenoms := extractCollateralTypes(expectedIndexes) - borrow := hardtypes.Borrow{ - Borrower: owner, - Amount: arbitraryCoinsWithDenoms(borrowedDenoms...), - } + borrow := NewBorrowBuilder(owner). + WithArbitrarySourceShares(borrowedDenoms...). + Build() suite.keeper.InitializeHardBorrowReward(suite.ctx, borrow) diff --git a/x/incentive/keeper/rewards_borrow_sync_test.go b/x/incentive/keeper/rewards_borrow_sync_test.go index 3be2ae46..728f3974 100644 --- a/x/incentive/keeper/rewards_borrow_sync_test.go +++ b/x/incentive/keeper/rewards_borrow_sync_test.go @@ -14,14 +14,6 @@ import ( ) // SynchronizeHardBorrowRewardTests runs unit tests for the keeper.SynchronizeHardBorrowReward method -// -// inputs -// - claim in store (only claim.BorrowRewardIndexes, claim.Reward) -// - global indexes in store -// - borrow function arg (only borrow.Amount) -// -// outputs -// - sets a claim type SynchronizeHardBorrowRewardTests struct { unitTester } @@ -43,10 +35,10 @@ func (suite *SynchronizeHardBorrowRewardTests) TestClaimIndexesAreUpdatedWhenGlo globalIndexes := increaseAllRewardFactors(nonEmptyMultiRewardIndexes) suite.storeGlobalBorrowIndexes(globalIndexes) - borrow := hardtypes.Borrow{ - Borrower: claim.Owner, - Amount: arbitraryCoinsWithDenoms(extractCollateralTypes(claim.BorrowRewardIndexes)...), - } + + borrow := NewBorrowBuilder(claim.Owner). + WithArbitrarySourceShares(extractCollateralTypes(claim.BorrowRewardIndexes)...). + Build() suite.keeper.SynchronizeHardBorrowReward(suite.ctx, borrow) @@ -68,10 +60,9 @@ func (suite *SynchronizeHardBorrowRewardTests) TestClaimIndexesAreUnchangedWhenG suite.storeGlobalBorrowIndexes(unchangingIndexes) - borrow := hardtypes.Borrow{ - Borrower: claim.Owner, - Amount: arbitraryCoinsWithDenoms(extractCollateralTypes(unchangingIndexes)...), - } + borrow := NewBorrowBuilder(claim.Owner). + WithArbitrarySourceShares(extractCollateralTypes(unchangingIndexes)...). + Build() suite.keeper.SynchronizeHardBorrowReward(suite.ctx, borrow) @@ -93,10 +84,9 @@ func (suite *SynchronizeHardBorrowRewardTests) TestClaimIndexesAreUpdatedWhenNew globalIndexes := appendUniqueMultiRewardIndex(nonEmptyMultiRewardIndexes) suite.storeGlobalBorrowIndexes(globalIndexes) - borrow := hardtypes.Borrow{ - Borrower: claim.Owner, - Amount: arbitraryCoinsWithDenoms(extractCollateralTypes(globalIndexes)...), - } + borrow := NewBorrowBuilder(claim.Owner). + WithArbitrarySourceShares(extractCollateralTypes(globalIndexes)...). + Build() suite.keeper.SynchronizeHardBorrowReward(suite.ctx, borrow) @@ -119,10 +109,9 @@ func (suite *SynchronizeHardBorrowRewardTests) TestClaimIndexesAreUpdatedWhenNew globalIndexes := appendUniqueRewardIndexToFirstItem(nonEmptyMultiRewardIndexes) suite.storeGlobalBorrowIndexes(globalIndexes) - borrow := hardtypes.Borrow{ - Borrower: claim.Owner, - Amount: arbitraryCoinsWithDenoms(extractCollateralTypes(globalIndexes)...), - } + borrow := NewBorrowBuilder(claim.Owner). + WithArbitrarySourceShares(extractCollateralTypes(globalIndexes)...). + Build() suite.keeper.SynchronizeHardBorrowReward(suite.ctx, borrow) @@ -169,10 +158,9 @@ func (suite *SynchronizeHardBorrowRewardTests) TestRewardIsIncrementedWhenGlobal }, }) - borrow := hardtypes.Borrow{ - Borrower: claim.Owner, - Amount: cs(c("borrowdenom", 1e9)), - } + borrow := NewBorrowBuilder(claim.Owner). + WithSourceShares("borrowdenom", 1e9). + Build() suite.keeper.SynchronizeHardBorrowReward(suite.ctx, borrow) @@ -232,10 +220,10 @@ func (suite *SynchronizeHardBorrowRewardTests) TestRewardIsIncrementedWhenNewRew } suite.storeGlobalBorrowIndexes(globalIndexes) - borrow := hardtypes.Borrow{ - Borrower: claim.Owner, - Amount: cs(c("rewarded", 1e9), c("newlyrewarded", 1e9)), - } + borrow := NewBorrowBuilder(claim.Owner). + WithSourceShares("rewarded", 1e9). + WithSourceShares("newlyrewarded", 1e9). + Build() suite.keeper.SynchronizeHardBorrowReward(suite.ctx, borrow) @@ -290,10 +278,9 @@ func (suite *SynchronizeHardBorrowRewardTests) TestRewardIsIncrementedWhenNewRew } suite.storeGlobalBorrowIndexes(globalIndexes) - borrow := hardtypes.Borrow{ - Borrower: claim.Owner, - Amount: cs(c("borrowed", 1e9)), - } + borrow := NewBorrowBuilder(claim.Owner). + WithSourceShares("borrowed", 1e9). + Build() suite.keeper.SynchronizeHardBorrowReward(suite.ctx, borrow) @@ -306,6 +293,53 @@ func (suite *SynchronizeHardBorrowRewardTests) TestRewardIsIncrementedWhenNewRew ) } +// BorrowBuilder is a tool for creating a hard borrows. +// The builder inherits from hard.Borrow, so fields can be accessed directly if a helper method doesn't exist. +type BorrowBuilder struct { + hardtypes.Borrow +} + +// NewBorrowBuilder creates a BorrowBuilder containing an empty borrow. +func NewBorrowBuilder(borrower sdk.AccAddress) BorrowBuilder { + return BorrowBuilder{ + Borrow: hardtypes.Borrow{ + Borrower: borrower, + }} +} + +// Build assembles and returns the final borrow. +func (builder BorrowBuilder) Build() hardtypes.Borrow { return builder.Borrow } + +// WithSourceShares adds a borrow amount and factor such that the source shares for this borrow is equal to specified. +// With a factor of 1, the borrow amount is the source shares. This picks an arbitrary factor to ensure factors are accounted for in production code. +func (builder BorrowBuilder) WithSourceShares(denom string, shares int64) BorrowBuilder { + if !builder.Amount.AmountOf(denom).Equal(sdk.ZeroInt()) { + panic("adding to amount with existing denom not implemented") + } + if _, f := builder.Index.GetInterestFactor(denom); f { + panic("adding to indexes with existing denom not implemented") + } + + // pick arbitrary factor + factor := sdk.MustNewDecFromStr("2") + + // Calculate borrow amount that would equal the requested source shares given the above factor. + amt := sdk.NewInt(shares).Mul(factor.RoundInt()) + + builder.Amount = builder.Amount.Add(sdk.NewCoin(denom, amt)) + builder.Index = builder.Index.SetInterestFactor(denom, factor) + return builder +} + +// WithArbitrarySourceShares adds arbitrary borrow amounts and indexes for each specified denom. +func (builder BorrowBuilder) WithArbitrarySourceShares(denoms ...string) BorrowBuilder { + const arbitraryShares = 1e9 + for _, denom := range denoms { + builder = builder.WithSourceShares(denom, arbitraryShares) + } + return builder +} + func TestCalculateRewards(t *testing.T) { type expected struct { err error diff --git a/x/incentive/keeper/rewards_borrow_test.go b/x/incentive/keeper/rewards_borrow_test.go index 42981881..28e6a4f2 100644 --- a/x/incentive/keeper/rewards_borrow_test.go +++ b/x/incentive/keeper/rewards_borrow_test.go @@ -17,8 +17,75 @@ import ( "github.com/kava-labs/kava/x/incentive/keeper" "github.com/kava-labs/kava/x/incentive/testutil" "github.com/kava-labs/kava/x/incentive/types" + "github.com/kava-labs/kava/x/kavadist" ) +type BorrowIntegrationTests struct { + testutil.IntegrationTester + + genesisTime time.Time + addrs []sdk.AccAddress +} + +func TestBorrowIntegration(t *testing.T) { + suite.Run(t, new(BorrowIntegrationTests)) +} + +// SetupTest is run automatically before each suite test +func (suite *BorrowIntegrationTests) SetupTest() { + + _, suite.addrs = app.GeneratePrivKeyAddressPairs(5) + + suite.genesisTime = time.Date(2020, 12, 15, 14, 0, 0, 0, time.UTC) +} + +func (suite *BorrowIntegrationTests) TestSingleUserAccumulatesRewardsAfterSyncing() { + userA := suite.addrs[0] + + authBulder := app.NewAuthGenesisBuilder(). + WithSimpleModuleAccount(kavadist.ModuleName, cs(c("hard", 1e18))). // Fill kavadist with enough coins to pay out any reward + WithSimpleAccount(userA, cs(c("bnb", 1e12))) // give the user some coins + + incentBuilder := testutil.NewIncentiveGenesisBuilder(). + WithGenesisTime(suite.genesisTime). + WithMultipliers(types.Multipliers{ + types.NewMultiplier(types.MultiplierName("large"), 12, d("1.0")), // keep payout at 1.0 to make maths easier + }). + WithSimpleBorrowRewardPeriod("bnb", cs(c("hard", 1e6))) // only borrow rewards + + suite.StartChain( + suite.genesisTime, + NewPricefeedGenStateMultiFromTime(suite.genesisTime), + NewHardGenStateMulti(suite.genesisTime).BuildMarshalled(), + authBulder.BuildMarshalled(), + incentBuilder.BuildMarshalled(), + ) + + // Create a borrow (need to first deposit to allow it) + suite.NoError(suite.DeliverHardMsgDeposit(userA, cs(c("bnb", 1e11)))) + suite.NoError(suite.DeliverHardMsgBorrow(userA, cs(c("bnb", 1e10)))) + + // Let time pass to accumulate interest on the borrow + // Use one long block instead of many to reduce any rounding errors, and speed up tests. + suite.NextBlockAfter(1e6 * time.Second) // about 12 days + + // User borrows and repays just to sync their borrow. + suite.NoError(suite.DeliverHardMsgRepay(userA, cs(c("bnb", 1)))) + suite.NoError(suite.DeliverHardMsgBorrow(userA, cs(c("bnb", 1)))) + + // Accumulate more rewards. + // The user still has the same percentage of all borrows (100%) so their rewards should be the same as in the previous block. + suite.NextBlockAfter(1e6 * time.Second) // about 12 days + + // User claims all their rewards + suite.NoError(suite.DeliverIncentiveMsg(types.NewMsgClaimHardReward(userA, "large", nil))) + + // The users has always had 100% of borrows, so they should receive all rewards for the previous two blocks. + // Total rewards for each block is block duration * rewards per second + accuracy := 1e-10 // using a very high accuracy to flag future small calculation changes + suite.BalanceInEpsilon(userA, cs(c("bnb", 1e12-1e11+1e10), c("hard", 2*1e6*1e6)), accuracy) +} + // Test suite used for all keeper tests type BorrowRewardsTestSuite struct { suite.Suite @@ -62,7 +129,7 @@ func (suite *BorrowRewardsTestSuite) SetupWithGenState(authBuilder app.AuthGenes authBuilder.BuildMarshalled(), NewPricefeedGenStateMultiFromTime(suite.genesisTime), hardBuilder.BuildMarshalled(), - NewCommitteeGenesisState(suite.addrs[:2]), + NewCommitteeGenesisState(1, suite.addrs[:2]...), incentBuilder.BuildMarshalled(), ) } diff --git a/x/incentive/keeper/rewards_borrow_update_test.go b/x/incentive/keeper/rewards_borrow_update_test.go index 0cfa7fb2..06187dfa 100644 --- a/x/incentive/keeper/rewards_borrow_update_test.go +++ b/x/incentive/keeper/rewards_borrow_update_test.go @@ -5,19 +5,10 @@ import ( "github.com/stretchr/testify/suite" - hardtypes "github.com/kava-labs/kava/x/hard/types" "github.com/kava-labs/kava/x/incentive/types" ) // UpdateHardBorrowIndexDenomsTests runs unit tests for the keeper.UpdateHardBorrowIndexDenoms method -// -// inputs -// - claim in store if it exists (only claim.BorrowRewardIndexes) -// - global indexes in store -// - borrow function arg (only borrow.Amount) -// -// outputs -// - sets a claim type UpdateHardBorrowIndexDenomsTests struct { unitTester } @@ -38,10 +29,9 @@ func (suite *UpdateHardBorrowIndexDenomsTests) TestClaimIndexesAreRemovedForDeno // remove one denom from the indexes already in the borrow expectedIndexes := claim.BorrowRewardIndexes[1:] - borrow := hardtypes.Borrow{ - Borrower: claim.Owner, - Amount: arbitraryCoinsWithDenoms(extractCollateralTypes(expectedIndexes)...), - } + borrow := NewBorrowBuilder(claim.Owner). + WithArbitrarySourceShares(extractCollateralTypes(expectedIndexes)...). + Build() suite.keeper.UpdateHardBorrowIndexDenoms(suite.ctx, borrow) @@ -60,10 +50,9 @@ func (suite *UpdateHardBorrowIndexDenomsTests) TestClaimIndexesAreAddedForNewlyB globalIndexes := appendUniqueMultiRewardIndex(claim.BorrowRewardIndexes) suite.storeGlobalBorrowIndexes(globalIndexes) - borrow := hardtypes.Borrow{ - Borrower: claim.Owner, - Amount: arbitraryCoinsWithDenoms(extractCollateralTypes(globalIndexes)...), - } + borrow := NewBorrowBuilder(claim.Owner). + WithArbitrarySourceShares(extractCollateralTypes(globalIndexes)...). + Build() suite.keeper.UpdateHardBorrowIndexDenoms(suite.ctx, borrow) @@ -83,10 +72,9 @@ func (suite *UpdateHardBorrowIndexDenomsTests) TestClaimIndexesAreUnchangedWhenB // UpdateHardBorrowIndexDenoms should ignore the new values. suite.storeGlobalBorrowIndexes(increaseAllRewardFactors(claim.BorrowRewardIndexes)) - borrow := hardtypes.Borrow{ - Borrower: claim.Owner, - Amount: arbitraryCoinsWithDenoms(extractCollateralTypes(claim.BorrowRewardIndexes)...), - } + borrow := NewBorrowBuilder(claim.Owner). + WithArbitrarySourceShares(extractCollateralTypes(claim.BorrowRewardIndexes)...). + Build() suite.keeper.UpdateHardBorrowIndexDenoms(suite.ctx, borrow) @@ -107,10 +95,9 @@ func (suite *UpdateHardBorrowIndexDenomsTests) TestEmptyClaimIndexesAreAddedForN // add a denom to the borrowed amount that is not in the global or claim's indexes expectedIndexes := appendUniqueEmptyMultiRewardIndex(claim.BorrowRewardIndexes) borrowedDenoms := extractCollateralTypes(expectedIndexes) - borrow := hardtypes.Borrow{ - Borrower: claim.Owner, - Amount: arbitraryCoinsWithDenoms(borrowedDenoms...), - } + borrow := NewBorrowBuilder(claim.Owner). + WithArbitrarySourceShares(borrowedDenoms...). + Build() suite.keeper.UpdateHardBorrowIndexDenoms(suite.ctx, borrow) diff --git a/x/incentive/keeper/rewards_supply.go b/x/incentive/keeper/rewards_supply.go index 210a4d8e..d8c31ed8 100644 --- a/x/incentive/keeper/rewards_supply.go +++ b/x/incentive/keeper/rewards_supply.go @@ -38,7 +38,7 @@ func (k Keeper) AccumulateHardSupplyRewards(ctx sdk.Context, rewardPeriod types. // getHardSupplyTotalSourceShares fetches the sum of all source shares for a supply reward. // In the case of hard supply, this is the total supplied divided by the supply interest factor. -// This give the "pre interest" value of the total supplied. +// This gives the "pre interest" value of the total supplied. func (k Keeper) getHardSupplyTotalSourceShares(ctx sdk.Context, denom string) sdk.Dec { totalSuppliedCoins, found := k.hardKeeper.GetSuppliedCoins(ctx) if !found { @@ -86,39 +86,55 @@ func (k Keeper) SynchronizeHardSupplyReward(ctx sdk.Context, deposit hardtypes.D return } - for _, coin := range deposit.Amount { - globalRewardIndexes, found := k.GetHardSupplyRewardIndexes(ctx, coin.Denom) - if !found { - // The global factor is only not found if - // - the supply denom has not started accumulating rewards yet (either there is no reward specified in params, or the reward start time hasn't been hit) - // - OR it was wrongly deleted from state (factors should never be removed while unsynced claims exist) - // If not found we could either skip this sync, or assume the global factor is zero. - // Skipping will avoid storing unnecessary factors in the claim for non rewarded denoms. - // And in the event a global factor is wrongly deleted, it will avoid this function panicking when calculating rewards. - continue - } + // Source shares for hard deposits is their normalized deposit amount + normalizedDeposit, err := deposit.NormalizedDeposit() + if err != nil { + panic(fmt.Sprintf("during deposit reward sync, could not get normalized deposit for %s: %s", deposit.Depositor, err.Error())) + } - userRewardIndexes, found := claim.SupplyRewardIndexes.Get(coin.Denom) - if !found { - // Normally the reward indexes should always be found. - // But if a denom was not rewarded then becomes rewarded (ie a reward period is added to params), then the indexes will be missing from claims for that supplied denom. - // So given the reward period was just added, assume the starting value for any global reward indexes, which is an empty slice. - userRewardIndexes = types.RewardIndexes{} - } + for _, normedDeposit := range normalizedDeposit { - newRewards, err := k.CalculateRewards(userRewardIndexes, globalRewardIndexes, coin.Amount.ToDec()) - if err != nil { - // Global reward factors should never decrease, as it would lead to a negative update to claim.Rewards. - // This panics if a global reward factor decreases or disappears between the old and new indexes. - panic(fmt.Sprintf("corrupted global reward indexes found: %v", err)) - } - - claim.Reward = claim.Reward.Add(newRewards...) - claim.SupplyRewardIndexes = claim.SupplyRewardIndexes.With(coin.Denom, globalRewardIndexes) + claim = k.synchronizeSingleHardSupplyReward(ctx, claim, normedDeposit.Denom, normedDeposit.Amount) } k.SetHardLiquidityProviderClaim(ctx, claim) } +// synchronizeSingleHardSupplyReward synchronizes a single rewarded supply denom in a hard claim. +// It returns the claim without setting in the store. +// The public methods for accessing and modifying claims are preferred over this one. Direct modification of claims is easy to get wrong. +func (k Keeper) synchronizeSingleHardSupplyReward(ctx sdk.Context, claim types.HardLiquidityProviderClaim, denom string, sourceShares sdk.Dec) types.HardLiquidityProviderClaim { + globalRewardIndexes, found := k.GetHardSupplyRewardIndexes(ctx, denom) + if !found { + // The global factor is only not found if + // - the supply denom has not started accumulating rewards yet (either there is no reward specified in params, or the reward start time hasn't been hit) + // - OR it was wrongly deleted from state (factors should never be removed while unsynced claims exist) + // If not found we could either skip this sync, or assume the global factor is zero. + // Skipping will avoid storing unnecessary factors in the claim for non rewarded denoms. + // And in the event a global factor is wrongly deleted, it will avoid this function panicking when calculating rewards. + return claim + } + + userRewardIndexes, found := claim.SupplyRewardIndexes.Get(denom) + if !found { + // Normally the reward indexes should always be found. + // But if a denom was not rewarded then becomes rewarded (ie a reward period is added to params), then the indexes will be missing from claims for that supplied denom. + // So given the reward period was just added, assume the starting value for any global reward indexes, which is an empty slice. + userRewardIndexes = types.RewardIndexes{} + } + + newRewards, err := k.CalculateRewards(userRewardIndexes, globalRewardIndexes, sourceShares) + if err != nil { + // Global reward factors should never decrease, as it would lead to a negative update to claim.Rewards. + // This panics if a global reward factor decreases or disappears between the old and new indexes. + panic(fmt.Sprintf("corrupted global reward indexes found: %v", err)) + } + + claim.Reward = claim.Reward.Add(newRewards...) + claim.SupplyRewardIndexes = claim.SupplyRewardIndexes.With(denom, globalRewardIndexes) + + return claim +} + // UpdateHardSupplyIndexDenoms adds any new deposit denoms to the claim's supply reward index func (k Keeper) UpdateHardSupplyIndexDenoms(ctx sdk.Context, deposit hardtypes.Deposit) { claim, found := k.GetHardLiquidityProviderClaim(ctx, deposit.Depositor) diff --git a/x/incentive/keeper/rewards_supply_init_test.go b/x/incentive/keeper/rewards_supply_init_test.go index 0c0927d0..3e1b218c 100644 --- a/x/incentive/keeper/rewards_supply_init_test.go +++ b/x/incentive/keeper/rewards_supply_init_test.go @@ -5,19 +5,10 @@ import ( "github.com/stretchr/testify/suite" - hardtypes "github.com/kava-labs/kava/x/hard/types" "github.com/kava-labs/kava/x/incentive/types" ) // InitializeHardSupplyRewardTests runs unit tests for the keeper.InitializeHardSupplyReward method -// -// inputs -// - claim in store if it exists (only claim.SupplyRewardIndexes) -// - global indexes in store -// - deposit function arg (only deposit.Amount) -// -// outputs -// - sets or creates a claim type InitializeHardSupplyRewardTests struct { unitTester } @@ -41,10 +32,9 @@ func (suite *InitializeHardSupplyRewardTests) TestClaimIndexesAreSetWhenClaimExi globalIndexes := nonEmptyMultiRewardIndexes suite.storeGlobalSupplyIndexes(globalIndexes) - deposit := hardtypes.Deposit{ - Depositor: claim.Owner, - Amount: arbitraryCoinsWithDenoms(extractCollateralTypes(globalIndexes)...), - } + deposit := NewDepositBuilder(claim.Owner). + WithArbitrarySourceShares(extractCollateralTypes(globalIndexes)...). + Build() suite.keeper.InitializeHardSupplyReward(suite.ctx, deposit) @@ -56,10 +46,9 @@ func (suite *InitializeHardSupplyRewardTests) TestClaimIndexesAreSetWhenClaimDoe suite.storeGlobalSupplyIndexes(globalIndexes) owner := arbitraryAddress() - deposit := hardtypes.Deposit{ - Depositor: owner, - Amount: arbitraryCoinsWithDenoms(extractCollateralTypes(globalIndexes)...), - } + deposit := NewDepositBuilder(owner). + WithArbitrarySourceShares(extractCollateralTypes(globalIndexes)...). + Build() suite.keeper.InitializeHardSupplyReward(suite.ctx, deposit) @@ -77,10 +66,9 @@ func (suite *InitializeHardSupplyRewardTests) TestClaimIndexesAreSetEmptyForMiss // This happens when a deposit denom has no rewards associated with it. expectedIndexes := appendUniqueEmptyMultiRewardIndex(globalIndexes) depositedDenoms := extractCollateralTypes(expectedIndexes) - deposit := hardtypes.Deposit{ - Depositor: owner, - Amount: arbitraryCoinsWithDenoms(depositedDenoms...), - } + deposit := NewDepositBuilder(owner). + WithArbitrarySourceShares(depositedDenoms...). + Build() suite.keeper.InitializeHardSupplyReward(suite.ctx, deposit) diff --git a/x/incentive/keeper/rewards_supply_sync_test.go b/x/incentive/keeper/rewards_supply_sync_test.go index 7f12f3d4..c4444665 100644 --- a/x/incentive/keeper/rewards_supply_sync_test.go +++ b/x/incentive/keeper/rewards_supply_sync_test.go @@ -3,6 +3,7 @@ package keeper_test import ( "testing" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/suite" hardtypes "github.com/kava-labs/kava/x/hard/types" @@ -10,14 +11,6 @@ import ( ) // SynchronizeHardSupplyRewardTests runs unit tests for the keeper.SynchronizeHardSupplyReward method -// -// inputs -// - claim in store (only claim.SupplyRewardIndexes, claim.Reward) -// - global indexes in store -// - deposit function arg (only deposit.Amount) -// -// outputs -// - sets a claim type SynchronizeHardSupplyRewardTests struct { unitTester } @@ -39,10 +32,9 @@ func (suite *SynchronizeHardSupplyRewardTests) TestClaimIndexesAreUpdatedWhenGlo globalIndexes := increaseAllRewardFactors(nonEmptyMultiRewardIndexes) suite.storeGlobalSupplyIndexes(globalIndexes) - deposit := hardtypes.Deposit{ - Depositor: claim.Owner, - Amount: arbitraryCoinsWithDenoms(extractCollateralTypes(claim.SupplyRewardIndexes)...), - } + deposit := NewDepositBuilder(claim.Owner). + WithArbitrarySourceShares(extractCollateralTypes(claim.SupplyRewardIndexes)...). + Build() suite.keeper.SynchronizeHardSupplyReward(suite.ctx, deposit) @@ -64,10 +56,9 @@ func (suite *SynchronizeHardSupplyRewardTests) TestClaimIndexesAreUnchangedWhenG suite.storeGlobalSupplyIndexes(unchangingIndexes) - deposit := hardtypes.Deposit{ - Depositor: claim.Owner, - Amount: arbitraryCoinsWithDenoms(extractCollateralTypes(unchangingIndexes)...), - } + deposit := NewDepositBuilder(claim.Owner). + WithArbitrarySourceShares(extractCollateralTypes(unchangingIndexes)...). + Build() suite.keeper.SynchronizeHardSupplyReward(suite.ctx, deposit) @@ -89,10 +80,9 @@ func (suite *SynchronizeHardSupplyRewardTests) TestClaimIndexesAreUpdatedWhenNew globalIndexes := appendUniqueMultiRewardIndex(nonEmptyMultiRewardIndexes) suite.storeGlobalSupplyIndexes(globalIndexes) - deposit := hardtypes.Deposit{ - Depositor: claim.Owner, - Amount: arbitraryCoinsWithDenoms(extractCollateralTypes(globalIndexes)...), - } + deposit := NewDepositBuilder(claim.Owner). + WithArbitrarySourceShares(extractCollateralTypes(globalIndexes)...). + Build() suite.keeper.SynchronizeHardSupplyReward(suite.ctx, deposit) @@ -115,10 +105,9 @@ func (suite *SynchronizeHardSupplyRewardTests) TestClaimIndexesAreUpdatedWhenNew globalIndexes := appendUniqueRewardIndexToFirstItem(nonEmptyMultiRewardIndexes) suite.storeGlobalSupplyIndexes(globalIndexes) - deposit := hardtypes.Deposit{ - Depositor: claim.Owner, - Amount: arbitraryCoinsWithDenoms(extractCollateralTypes(globalIndexes)...), - } + deposit := NewDepositBuilder(claim.Owner). + WithArbitrarySourceShares(extractCollateralTypes(globalIndexes)...). + Build() suite.keeper.SynchronizeHardSupplyReward(suite.ctx, deposit) @@ -165,10 +154,9 @@ func (suite *SynchronizeHardSupplyRewardTests) TestRewardIsIncrementedWhenGlobal }, }) - deposit := hardtypes.Deposit{ - Depositor: claim.Owner, - Amount: cs(c("depositdenom", 1e9)), - } + deposit := NewDepositBuilder(claim.Owner). + WithSourceShares("depositdenom", 1e9). + Build() suite.keeper.SynchronizeHardSupplyReward(suite.ctx, deposit) @@ -228,10 +216,10 @@ func (suite *SynchronizeHardSupplyRewardTests) TestRewardIsIncrementedWhenNewRew } suite.storeGlobalSupplyIndexes(globalIndexes) - deposit := hardtypes.Deposit{ - Depositor: claim.Owner, - Amount: cs(c("rewarded", 1e9), c("newlyrewarded", 1e9)), - } + deposit := NewDepositBuilder(claim.Owner). + WithSourceShares("rewarded", 1e9). + WithSourceShares("newlyrewarded", 1e9). + Build() suite.keeper.SynchronizeHardSupplyReward(suite.ctx, deposit) @@ -286,10 +274,9 @@ func (suite *SynchronizeHardSupplyRewardTests) TestRewardIsIncrementedWhenNewRew } suite.storeGlobalSupplyIndexes(globalIndexes) - deposit := hardtypes.Deposit{ - Depositor: claim.Owner, - Amount: cs(c("deposited", 1e9)), - } + deposit := NewDepositBuilder(claim.Owner). + WithSourceShares("deposited", 1e9). + Build() suite.keeper.SynchronizeHardSupplyReward(suite.ctx, deposit) @@ -301,3 +288,50 @@ func (suite *SynchronizeHardSupplyRewardTests) TestRewardIsIncrementedWhenNewRew syncedClaim.Reward, ) } + +// DepositBuilder is a tool for creating a hard deposit in tests. +// The builder inherits from hard.Deposit, so fields can be accessed directly if a helper method doesn't exist. +type DepositBuilder struct { + hardtypes.Deposit +} + +// NewDepositBuilder creates a DepositBuilder containing an empty deposit. +func NewDepositBuilder(depositor sdk.AccAddress) DepositBuilder { + return DepositBuilder{ + Deposit: hardtypes.Deposit{ + Depositor: depositor, + }} +} + +// Build assembles and returns the final deposit. +func (builder DepositBuilder) Build() hardtypes.Deposit { return builder.Deposit } + +// WithSourceShares adds a deposit amount and factor such that the source shares for this deposit is equal to specified. +// With a factor of 1, the deposit amount is the source shares. This picks an arbitrary factor to ensure factors are accounted for in production code. +func (builder DepositBuilder) WithSourceShares(denom string, shares int64) DepositBuilder { + if !builder.Amount.AmountOf(denom).Equal(sdk.ZeroInt()) { + panic("adding to amount with existing denom not implemented") + } + if _, f := builder.Index.GetInterestFactor(denom); f { + panic("adding to indexes with existing denom not implemented") + } + + // pick arbitrary factor + factor := sdk.MustNewDecFromStr("2") + + // Calculate deposit amount that would equal the requested source shares given the above factor. + amt := sdk.NewInt(shares).Mul(factor.RoundInt()) + + builder.Amount = builder.Amount.Add(sdk.NewCoin(denom, amt)) + builder.Index = builder.Index.SetInterestFactor(denom, factor) + return builder +} + +// WithArbitrarySourceShares adds arbitrary deposit amounts and indexes for each specified denom. +func (builder DepositBuilder) WithArbitrarySourceShares(denoms ...string) DepositBuilder { + const arbitraryShares = 1e9 + for _, denom := range denoms { + builder = builder.WithSourceShares(denom, arbitraryShares) + } + return builder +} diff --git a/x/incentive/keeper/rewards_supply_test.go b/x/incentive/keeper/rewards_supply_test.go index 8fcc1ffe..4380daf2 100644 --- a/x/incentive/keeper/rewards_supply_test.go +++ b/x/incentive/keeper/rewards_supply_test.go @@ -17,8 +17,76 @@ import ( "github.com/kava-labs/kava/x/incentive/keeper" "github.com/kava-labs/kava/x/incentive/testutil" "github.com/kava-labs/kava/x/incentive/types" + "github.com/kava-labs/kava/x/kavadist" ) +type SupplyIntegrationTests struct { + testutil.IntegrationTester + + genesisTime time.Time + addrs []sdk.AccAddress +} + +func TestSupplyIntegration(t *testing.T) { + suite.Run(t, new(SupplyIntegrationTests)) +} + +// SetupTest is run automatically before each suite test +func (suite *SupplyIntegrationTests) SetupTest() { + + _, suite.addrs = app.GeneratePrivKeyAddressPairs(5) + + suite.genesisTime = time.Date(2020, 12, 15, 14, 0, 0, 0, time.UTC) +} + +func (suite *SupplyIntegrationTests) TestSingleUserAccumulatesRewardsAfterSyncing() { + userA := suite.addrs[0] + + authBulder := app.NewAuthGenesisBuilder(). + WithSimpleModuleAccount(kavadist.ModuleName, cs(c("hard", 1e18))). // Fill kavadist with enough coins to pay out any reward + WithSimpleAccount(userA, cs(c("bnb", 1e12))) // give the user some coins + + incentBuilder := testutil.NewIncentiveGenesisBuilder(). + WithGenesisTime(suite.genesisTime). + WithMultipliers(types.Multipliers{ + types.NewMultiplier(types.MultiplierName("large"), 12, d("1.0")), // keep payout at 1.0 to make maths easier + }). + WithSimpleSupplyRewardPeriod("bnb", cs(c("hard", 1e6))) // only borrow rewards + + suite.StartChain( + suite.genesisTime, + NewPricefeedGenStateMultiFromTime(suite.genesisTime), + NewHardGenStateMulti(suite.genesisTime).BuildMarshalled(), + authBulder.BuildMarshalled(), + incentBuilder.BuildMarshalled(), + ) + + // Create a deposit + suite.NoError(suite.DeliverHardMsgDeposit(userA, cs(c("bnb", 1e11)))) + // Also create a borrow so interest accumulates on the deposit + suite.NoError(suite.DeliverHardMsgBorrow(userA, cs(c("bnb", 1e10)))) + + // Let time pass to accumulate interest on the deposit + // Use one long block instead of many to reduce any rounding errors, and speed up tests. + suite.NextBlockAfter(1e6 * time.Second) // about 12 days + + // User withdraw and redeposits just to sync their deposit. + suite.NoError(suite.DeliverHardMsgWithdraw(userA, cs(c("bnb", 1)))) + suite.NoError(suite.DeliverHardMsgDeposit(userA, cs(c("bnb", 1)))) + + // Accumulate more rewards. + // The user still has the same percentage of all deposits (100%) so their rewards should be the same as in the previous block. + suite.NextBlockAfter(1e6 * time.Second) // about 12 days + + // User claims all their rewards + suite.NoError(suite.DeliverIncentiveMsg(types.NewMsgClaimHardReward(userA, "large", nil))) + + // The users has always had 100% of deposits, so they should receive all rewards for the previous two blocks. + // Total rewards for each block is block duration * rewards per second + accuracy := 1e-10 // using a very high accuracy to flag future small calculation changes + suite.BalanceInEpsilon(userA, cs(c("bnb", 1e12-1e11+1e10), c("hard", 2*1e6*1e6)), accuracy) +} + // Test suite used for all keeper tests type SupplyRewardsTestSuite struct { suite.Suite @@ -62,7 +130,7 @@ func (suite *SupplyRewardsTestSuite) SetupWithGenState(authBuilder app.AuthGenes authBuilder.BuildMarshalled(), NewPricefeedGenStateMultiFromTime(suite.genesisTime), hardBuilder.BuildMarshalled(), - NewCommitteeGenesisState(suite.addrs[:2]), + NewCommitteeGenesisState(1, suite.addrs[:2]...), incentBuilder.BuildMarshalled(), ) } diff --git a/x/incentive/keeper/rewards_supply_update_test.go b/x/incentive/keeper/rewards_supply_update_test.go index cf32199d..335df05e 100644 --- a/x/incentive/keeper/rewards_supply_update_test.go +++ b/x/incentive/keeper/rewards_supply_update_test.go @@ -5,19 +5,10 @@ import ( "github.com/stretchr/testify/suite" - hardtypes "github.com/kava-labs/kava/x/hard/types" "github.com/kava-labs/kava/x/incentive/types" ) // UpdateHardSupplyIndexDenomsTests runs unit tests for the keeper.UpdateHardSupplyIndexDenoms method -// -// inputs -// - claim in store if it exists (only claim.SupplyRewardIndexes) -// - global indexes in store -// - deposit function arg (only deposit.Amount) -// -// outputs -// - sets a claim type UpdateHardSupplyIndexDenomsTests struct { unitTester } @@ -38,10 +29,9 @@ func (suite *UpdateHardSupplyIndexDenomsTests) TestClaimIndexesAreRemovedForDeno // remove one denom from the indexes already in the deposit expectedIndexes := claim.SupplyRewardIndexes[1:] - deposit := hardtypes.Deposit{ - Depositor: claim.Owner, - Amount: arbitraryCoinsWithDenoms(extractCollateralTypes(expectedIndexes)...), - } + deposit := NewDepositBuilder(claim.Owner). + WithArbitrarySourceShares(extractCollateralTypes(expectedIndexes)...). + Build() suite.keeper.UpdateHardSupplyIndexDenoms(suite.ctx, deposit) @@ -60,10 +50,9 @@ func (suite *UpdateHardSupplyIndexDenomsTests) TestClaimIndexesAreAddedForNewlyS globalIndexes := appendUniqueMultiRewardIndex(claim.SupplyRewardIndexes) suite.storeGlobalSupplyIndexes(globalIndexes) - deposit := hardtypes.Deposit{ - Depositor: claim.Owner, - Amount: arbitraryCoinsWithDenoms(extractCollateralTypes(globalIndexes)...), - } + deposit := NewDepositBuilder(claim.Owner). + WithArbitrarySourceShares(extractCollateralTypes(globalIndexes)...). + Build() suite.keeper.UpdateHardSupplyIndexDenoms(suite.ctx, deposit) @@ -83,10 +72,9 @@ func (suite *UpdateHardSupplyIndexDenomsTests) TestClaimIndexesAreUnchangedWhenS // UpdateHardSupplyIndexDenoms should ignore the new values. suite.storeGlobalSupplyIndexes(increaseAllRewardFactors(claim.SupplyRewardIndexes)) - deposit := hardtypes.Deposit{ - Depositor: claim.Owner, - Amount: arbitraryCoinsWithDenoms(extractCollateralTypes(claim.SupplyRewardIndexes)...), - } + deposit := NewDepositBuilder(claim.Owner). + WithArbitrarySourceShares(extractCollateralTypes(claim.SupplyRewardIndexes)...). + Build() suite.keeper.UpdateHardSupplyIndexDenoms(suite.ctx, deposit) @@ -107,10 +95,9 @@ func (suite *UpdateHardSupplyIndexDenomsTests) TestEmptyClaimIndexesAreAddedForN // add a denom to the deposited amount that is not in the global or claim's indexes expectedIndexes := appendUniqueEmptyMultiRewardIndex(claim.SupplyRewardIndexes) depositedDenoms := extractCollateralTypes(expectedIndexes) - deposit := hardtypes.Deposit{ - Depositor: claim.Owner, - Amount: arbitraryCoinsWithDenoms(depositedDenoms...), - } + deposit := NewDepositBuilder(claim.Owner). + WithArbitrarySourceShares(depositedDenoms...). + Build() suite.keeper.UpdateHardSupplyIndexDenoms(suite.ctx, deposit) diff --git a/x/incentive/keeper/rewards_usdx.go b/x/incentive/keeper/rewards_usdx.go index 4c45f97c..927a6947 100644 --- a/x/incentive/keeper/rewards_usdx.go +++ b/x/incentive/keeper/rewards_usdx.go @@ -41,7 +41,7 @@ func (k Keeper) AccumulateUSDXMintingRewards(ctx sdk.Context, rewardPeriod types // getUSDXTotalSourceShares fetches the sum of all source shares for a usdx minting reward. // In the case of usdx minting, this is the total debt from all cdps of a particular type, divided by the cdp interest factor. -// This give the "pre interest" value of the total debt. +// This gives the "pre interest" value of the total debt. func (k Keeper) getUSDXTotalSourceShares(ctx sdk.Context, collateralType string) sdk.Dec { totalPrincipal := k.cdpKeeper.GetTotalPrincipal(ctx, collateralType, cdptypes.DefaultStableDenom) @@ -59,15 +59,11 @@ func (k Keeper) getUSDXTotalSourceShares(ctx sdk.Context, collateralType string) // accrue rewards during the period the cdp was closed. By setting the reward factor to the current global reward factor, // any unclaimed rewards are preserved, but no new rewards are added. func (k Keeper) InitializeUSDXMintingClaim(ctx sdk.Context, cdp cdptypes.CDP) { - _, found := k.GetUSDXMintingRewardPeriod(ctx, cdp.Type) - if !found { - // this collateral type is not incentivized, do nothing - return - } claim, found := k.GetUSDXMintingClaim(ctx, cdp.Owner) if !found { // this is the owner's first usdx minting reward claim claim = types.NewUSDXMintingClaim(cdp.Owner, sdk.NewCoin(types.USDXMintingRewardDenom, sdk.ZeroInt()), types.RewardIndexes{}) } + globalRewardFactor, found := k.GetUSDXMintingRewardFactor(ctx, cdp.Type) if !found { globalRewardFactor = sdk.ZeroDec() @@ -81,7 +77,27 @@ func (k Keeper) InitializeUSDXMintingClaim(ctx sdk.Context, cdp cdptypes.CDP) { // this should be called before a cdp is modified. func (k Keeper) SynchronizeUSDXMintingReward(ctx sdk.Context, cdp cdptypes.CDP) { - globalRewardFactor, found := k.GetUSDXMintingRewardFactor(ctx, cdp.Type) + claim, found := k.GetUSDXMintingClaim(ctx, cdp.Owner) + if !found { + return + } + + sourceShares, err := cdp.GetNormalizedPrincipal() + if err != nil { + panic(fmt.Sprintf("during usdx reward sync, could not get normalized principal for %s: %s", cdp.Owner, err.Error())) + } + + claim = k.synchronizeSingleUSDXMintingReward(ctx, claim, cdp.Type, sourceShares) + + k.SetUSDXMintingClaim(ctx, claim) +} + +// synchronizeSingleUSDXMintingReward synchronizes a single rewarded cdp collateral type in a usdx minting claim. +// It returns the claim without setting in the store. +// The public methods for accessing and modifying claims are preferred over this one. Direct modification of claims is easy to get wrong. +func (k Keeper) synchronizeSingleUSDXMintingReward(ctx sdk.Context, claim types.USDXMintingClaim, ctype string, sourceShares sdk.Dec) types.USDXMintingClaim { + + globalRewardFactor, found := k.GetUSDXMintingRewardFactor(ctx, ctype) if !found { // The global factor is only not found if // - the cdp collateral type has not started accumulating rewards yet (either there is no reward specified in params, or the reward start time hasn't been hit) @@ -89,18 +105,10 @@ func (k Keeper) SynchronizeUSDXMintingReward(ctx sdk.Context, cdp cdptypes.CDP) // If not found we could either skip this sync, or assume the global factor is zero. // Skipping will avoid storing unnecessary factors in the claim for non rewarded denoms. // And in the event a global factor is wrongly deleted, it will avoid this function panicking when calculating rewards. - return - } - claim, found := k.GetUSDXMintingClaim(ctx, cdp.Owner) - if !found { - claim = types.NewUSDXMintingClaim( - cdp.Owner, - sdk.NewCoin(types.USDXMintingRewardDenom, sdk.ZeroInt()), - types.RewardIndexes{}, - ) + return claim } - userRewardFactor, found := claim.RewardIndexes.Get(cdp.Type) + userRewardFactor, found := claim.RewardIndexes.Get(ctype) if !found { // Normally the factor should always be found, as it is added when the cdp is created in InitializeUSDXMintingClaim. // However if a cdp type is not rewarded then becomes rewarded (ie a reward period is added to params), existing cdps will not have the factor in their claims. @@ -108,7 +116,7 @@ func (k Keeper) SynchronizeUSDXMintingReward(ctx sdk.Context, cdp cdptypes.CDP) userRewardFactor = sdk.ZeroDec() } - newRewardsAmount, err := k.CalculateSingleReward(userRewardFactor, globalRewardFactor, cdp.GetTotalPrincipal().Amount.ToDec()) + newRewardsAmount, err := k.CalculateSingleReward(userRewardFactor, globalRewardFactor, sourceShares) if err != nil { // Global reward factors should never decrease, as it would lead to a negative update to claim.Rewards. // This panics if a global reward factor decreases or disappears between the old and new indexes. @@ -117,9 +125,9 @@ func (k Keeper) SynchronizeUSDXMintingReward(ctx sdk.Context, cdp cdptypes.CDP) newRewardsCoin := sdk.NewCoin(types.USDXMintingRewardDenom, newRewardsAmount) claim.Reward = claim.Reward.Add(newRewardsCoin) - claim.RewardIndexes = claim.RewardIndexes.With(cdp.Type, globalRewardFactor) + claim.RewardIndexes = claim.RewardIndexes.With(ctype, globalRewardFactor) - k.SetUSDXMintingClaim(ctx, claim) + return claim } // SimulateUSDXMintingSynchronization calculates a user's outstanding USDX minting rewards by simulating reward synchronization diff --git a/x/incentive/keeper/rewards_usdx_test.go b/x/incentive/keeper/rewards_usdx_test.go index 7f656db0..a7c5e8a4 100644 --- a/x/incentive/keeper/rewards_usdx_test.go +++ b/x/incentive/keeper/rewards_usdx_test.go @@ -5,16 +5,229 @@ import ( "time" sdk "github.com/cosmos/cosmos-sdk/types" + paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" "github.com/stretchr/testify/suite" abci "github.com/tendermint/tendermint/abci/types" "github.com/kava-labs/kava/app" cdpkeeper "github.com/kava-labs/kava/x/cdp/keeper" cdptypes "github.com/kava-labs/kava/x/cdp/types" + "github.com/kava-labs/kava/x/incentive" "github.com/kava-labs/kava/x/incentive/keeper" "github.com/kava-labs/kava/x/incentive/testutil" + "github.com/kava-labs/kava/x/incentive/types" + "github.com/kava-labs/kava/x/kavadist" ) +type USDXIntegrationTests struct { + testutil.IntegrationTester + + genesisTime time.Time + addrs []sdk.AccAddress +} + +func TestUSDXIntegration(t *testing.T) { + suite.Run(t, new(USDXIntegrationTests)) +} + +// SetupTest is run automatically before each suite test +func (suite *USDXIntegrationTests) SetupTest() { + + _, suite.addrs = app.GeneratePrivKeyAddressPairs(5) + + suite.genesisTime = time.Date(2020, 12, 15, 14, 0, 0, 0, time.UTC) +} + +func (suite *USDXIntegrationTests) ProposeAndVoteOnNewRewardPeriods(committeeID uint64, voter sdk.AccAddress, newPeriods types.RewardPeriods) { + suite.ProposeAndVoteOnNewParams( + voter, + committeeID, + []paramtypes.ParamChange{{ + Subspace: incentive.ModuleName, + Key: string(incentive.KeyUSDXMintingRewardPeriods), + Value: string(incentive.ModuleCdc.MustMarshalJSON(newPeriods)), + }}) +} + +func (suite *USDXIntegrationTests) TestSingleUserAccumulatesRewardsAfterSyncing() { + userA := suite.addrs[0] + + authBulder := app.NewAuthGenesisBuilder(). + WithSimpleModuleAccount(kavadist.ModuleName, cs(c(types.USDXMintingRewardDenom, 1e18))). // Fill kavadist with enough coins to pay out any reward + WithSimpleAccount(userA, cs(c("bnb", 1e12))) // give the user some coins + + incentBuilder := testutil.NewIncentiveGenesisBuilder(). + WithGenesisTime(suite.genesisTime). + WithMultipliers(types.Multipliers{ + types.NewMultiplier(types.MultiplierName("large"), 12, d("1.0")), // keep payout at 1.0 to make maths easier + }). + WithSimpleUSDXRewardPeriod("bnb-a", c(types.USDXMintingRewardDenom, 1e6)) + + suite.StartChain( + suite.genesisTime, + NewPricefeedGenStateMultiFromTime(suite.genesisTime), + NewCDPGenStateMulti(), + authBulder.BuildMarshalled(), + incentBuilder.BuildMarshalled(), + ) + + // User creates a CDP to begin earning rewards. + suite.NoError( + suite.DeliverMsgCreateCDP(userA, c("bnb", 1e10), c(cdptypes.DefaultStableDenom, 1e9), "bnb-a"), + ) + + // Let time pass to accumulate interest on the deposit + // Use one long block instead of many to reduce any rounding errors, and speed up tests. + suite.NextBlockAfter(1e6 * time.Second) // about 12 days + + // User repays and borrows just to sync their CDP + suite.NoError( + suite.DeliverCDPMsgRepay(userA, "bnb-a", c(cdptypes.DefaultStableDenom, 1)), + ) + suite.NoError( + suite.DeliverCDPMsgBorrow(userA, "bnb-a", c(cdptypes.DefaultStableDenom, 1)), + ) + + // Accumulate more rewards. + // The user still has the same percentage of all CDP debt (100%) so their rewards should be the same as in the previous block. + suite.NextBlockAfter(1e6 * time.Second) // about 12 days + + // User claims all their rewards + suite.NoError( + suite.DeliverIncentiveMsg(types.NewMsgClaimUSDXMintingReward(userA, "large")), + ) + + // The users has always had 100% of cdp debt, so they should receive all rewards for the previous two blocks. + // Total rewards for each block is block duration * rewards per second + accuracy := 1e-18 // using a very high accuracy to flag future small calculation changes + suite.BalanceInEpsilon(userA, cs(c("bnb", 1e12-1e10), c(cdptypes.DefaultStableDenom, 1e9), c(types.USDXMintingRewardDenom, 2*1e6*1e6)), accuracy) +} + +func (suite *USDXIntegrationTests) TestSingleUserAccumulatesRewardsWithoutSyncing() { + + user := suite.addrs[0] + initialCollateral := c("bnb", 1e9) + + authBuilder := app.NewAuthGenesisBuilder(). + WithSimpleModuleAccount(kavadist.ModuleName, cs(c(types.USDXMintingRewardDenom, 1e18))). // Fill kavadist with enough coins to pay out any reward + WithSimpleAccount(user, cs(initialCollateral)) + + collateralType := "bnb-a" + + incentBuilder := testutil.NewIncentiveGenesisBuilder(). + WithGenesisTime(suite.genesisTime). + WithMultipliers(types.Multipliers{ + types.NewMultiplier(types.MultiplierName("large"), 12, d("1.0")), // keep payout at 1.0 to make maths easier + }). + WithSimpleUSDXRewardPeriod(collateralType, c(types.USDXMintingRewardDenom, 1e6)) + + suite.StartChain( + suite.genesisTime, + authBuilder.BuildMarshalled(), + NewPricefeedGenStateMultiFromTime(suite.genesisTime), + NewCDPGenStateMulti(), + incentBuilder.BuildMarshalled(), + ) + + // Setup cdp state containing one CDP + suite.NoError( + suite.DeliverMsgCreateCDP(user, initialCollateral, c("usdx", 1e8), collateralType), + ) + + // Skip ahead a few blocks blocks to accumulate both interest and usdx reward for the cdp + // Don't sync the CDP between the blocks + suite.NextBlockAfter(1e6 * time.Second) // about 12 days + suite.NextBlockAfter(1e6 * time.Second) + suite.NextBlockAfter(1e6 * time.Second) + + suite.NoError( + suite.DeliverIncentiveMsg(types.NewMsgClaimUSDXMintingReward(user, "large")), + ) + + // The users has always had 100% of cdp debt, so they should receive all rewards for the previous two blocks. + // Total rewards for each block is block duration * rewards per second + accuracy := 1e-18 // using a very high accuracy to flag future small calculation changes + suite.BalanceInEpsilon(user, cs(c(cdptypes.DefaultStableDenom, 1e8), c(types.USDXMintingRewardDenom, 3*1e6*1e6)), accuracy) +} + +func (suite *USDXIntegrationTests) TestReinstatingRewardParamsDoesNotTriggerOverPayments() { + + userA := suite.addrs[0] + userB := suite.addrs[1] + + authBuilder := app.NewAuthGenesisBuilder(). + WithSimpleModuleAccount(kavadist.ModuleName, cs(c(types.USDXMintingRewardDenom, 1e18))). // Fill kavadist with enough coins to pay out any reward + WithSimpleAccount(userA, cs(c("bnb", 1e10))). + WithSimpleAccount(userB, cs(c("bnb", 1e10))) + + incentBuilder := testutil.NewIncentiveGenesisBuilder(). + WithGenesisTime(suite.genesisTime). + WithMultipliers(types.Multipliers{ + types.NewMultiplier(types.MultiplierName("large"), 12, d("1.0")), // keep payout at 1.0 to make maths easier + }). + WithSimpleUSDXRewardPeriod("bnb-a", c(types.USDXMintingRewardDenom, 1e6)) + + suite.StartChain( + suite.genesisTime, + authBuilder.BuildMarshalled(), + NewPricefeedGenStateMultiFromTime(suite.genesisTime), + NewCDPGenStateMulti(), + incentBuilder.BuildMarshalled(), + NewCommitteeGenesisState(0, userA), // create a committtee to change params + ) + + // Accumulate some CDP rewards, requires creating a cdp so the total borrowed isn't 0. + suite.NoError( + suite.DeliverMsgCreateCDP(userA, c("bnb", 1e10), c("usdx", 1e9), "bnb-a"), + ) + suite.NextBlockAfter(1e6 * time.Second) + + // Remove the USDX reward period + suite.ProposeAndVoteOnNewRewardPeriods(0, userA, types.RewardPeriods{}) + // next block so proposal is enacted + suite.NextBlockAfter(1 * time.Second) + + // Create a CDP when there is no reward periods. In a previous version the claim object would not be created, leading to the bug. + // Withdraw the same amount of usdx as the first cdp currently has. This make the reward maths easier, as rewards will be split 50:50 between each cdp. + firstCDP, f := suite.App.GetCDPKeeper().GetCdpByOwnerAndCollateralType(suite.Ctx, userA, "bnb-a") + suite.True(f) + firstCDPTotalPrincipal := firstCDP.GetTotalPrincipal() + suite.NoError( + suite.DeliverMsgCreateCDP(userB, c("bnb", 1e10), firstCDPTotalPrincipal, "bnb-a"), + ) + + // Add back the reward period + suite.ProposeAndVoteOnNewRewardPeriods(0, userA, + types.RewardPeriods{types.NewRewardPeriod( + true, + "bnb-a", + suite.Ctx.BlockTime(), // start accumulating again from this block + suite.genesisTime.Add(365*24*time.Hour), + c(types.USDXMintingRewardDenom, 1e6), + )}, + ) + // next block so proposal is enacted + suite.NextBlockAfter(1 * time.Second) + + // Sync the cdp and claim by borrowing a bit + // In a previous version this would create the cdp with incorrect indexes, leading to overpayment. + suite.NoError( + suite.DeliverCDPMsgBorrow(userB, "bnb-a", c(cdptypes.DefaultStableDenom, 1)), + ) + + // Claim rewards + suite.NoError( + suite.DeliverIncentiveMsg(types.NewMsgClaimUSDXMintingReward(userB, "large")), + ) + + // The cdp had half the total borrows for a 1s block. So should earn half the rewards for that block + suite.BalanceInEpsilon( + userB, + cs(firstCDPTotalPrincipal.Add(c(cdptypes.DefaultStableDenom, 1)), c(types.USDXMintingRewardDenom, 0.5*1e6)), + 1e-18, // using very high accuracy to catch small changes to the calculations + ) +} + // Test suite used for all keeper tests type USDXRewardsTestSuite struct { suite.Suite diff --git a/x/incentive/keeper/rewards_usdx_unit_test.go b/x/incentive/keeper/rewards_usdx_unit_test.go index 9840a5fa..048672fe 100644 --- a/x/incentive/keeper/rewards_usdx_unit_test.go +++ b/x/incentive/keeper/rewards_usdx_unit_test.go @@ -35,9 +35,6 @@ func TestInitializeUSDXMintingClaims(t *testing.T) { func (suite *InitializeUSDXMintingClaimTests) TestClaimIndexIsSetWhenClaimDoesNotExist() { collateralType := "bnb-a" - subspace := paramsWithSingleUSDXRewardPeriod(collateralType) - suite.keeper = suite.NewKeeper(subspace, nil, nil, nil, nil, nil, nil) - cdp := NewCDPBuilder(arbitraryAddress(), collateralType).Build() globalIndexes := types.RewardIndexes{{ @@ -56,9 +53,6 @@ func (suite *InitializeUSDXMintingClaimTests) TestClaimIndexIsSetWhenClaimDoesNo func (suite *InitializeUSDXMintingClaimTests) TestClaimIndexIsSetWhenClaimExists() { collateralType := "bnb-a" - subspace := paramsWithSingleUSDXRewardPeriod(collateralType) - suite.keeper = suite.NewKeeper(subspace, nil, nil, nil, nil, nil, nil) - claim := types.USDXMintingClaim{ BaseClaim: types.BaseClaim{ Owner: arbitraryAddress(), @@ -106,7 +100,7 @@ func (suite *SynchronizeUSDXMintingRewardTests) TestRewardUnchangedWhenGlobalInd suite.storeGlobalUSDXIndexes(unchangingRewardIndexes) - cdp := NewCDPBuilder(claim.Owner, collateralType).WithPrincipal(i(1e12)).Build() + cdp := NewCDPBuilder(claim.Owner, collateralType).WithSourceShares(1e12).Build() suite.keeper.SynchronizeUSDXMintingReward(suite.ctx, cdp) @@ -139,7 +133,7 @@ func (suite *SynchronizeUSDXMintingRewardTests) TestRewardIsIncrementedWhenGloba } suite.storeGlobalUSDXIndexes(globalIndexes) - cdp := NewCDPBuilder(claim.Owner, collateralType).WithPrincipal(i(1e12)).Build() + cdp := NewCDPBuilder(claim.Owner, collateralType).WithSourceShares(1e12).Build() suite.keeper.SynchronizeUSDXMintingReward(suite.ctx, cdp) @@ -148,28 +142,6 @@ func (suite *SynchronizeUSDXMintingRewardTests) TestRewardIsIncrementedWhenGloba suite.Equal(c(types.USDXMintingRewardDenom, 1e11), syncedClaim.Reward) } -func (suite *SynchronizeUSDXMintingRewardTests) TestRewardIsIncrementedWhenNewRewardAddedAndClaimDoesNotExit() { - collateralType := "bnb-a" - - globalIndexes := types.RewardIndexes{ - { - CollateralType: collateralType, - RewardFactor: d("0.2"), - }, - } - suite.storeGlobalUSDXIndexes(globalIndexes) - - cdp := NewCDPBuilder(arbitraryAddress(), collateralType).WithPrincipal(i(1e12)).Build() - - suite.keeper.SynchronizeUSDXMintingReward(suite.ctx, cdp) - - syncedClaim, _ := suite.keeper.GetUSDXMintingClaim(suite.ctx, cdp.Owner) - // The global index was not around when this cdp was created as it was not stored in a claim. - // Therefore it must have been added via params after. - // To include rewards since the params were updated, the old index should be assumed to be 0. - // reward is ( new index - old index ) * cdp.TotalPrincipal - suite.Equal(c(types.USDXMintingRewardDenom, 2e11), syncedClaim.Reward) -} func (suite *SynchronizeUSDXMintingRewardTests) TestClaimIndexIsUpdatedWhenGlobalIndexIncreased() { claimsRewardIndexes := nonEmptyRewardIndexes collateralType := extractFirstCollateralType(claimsRewardIndexes) @@ -248,7 +220,7 @@ func (suite *SynchronizeUSDXMintingRewardTests) TestClaimIsUnchangedWhenGlobalFa // don't store any reward indexes // create a cdp with collateral type that doesn't exist in the claim's indexes, and does not have a corresponding global factor - cdp := NewCDPBuilder(claim.Owner, "unrewardedcollateral").WithPrincipal(i(1e12)).Build() + cdp := NewCDPBuilder(claim.Owner, "unrewardedcollateral").WithSourceShares(1e12).Build() suite.keeper.SynchronizeUSDXMintingReward(suite.ctx, cdp) @@ -257,12 +229,15 @@ func (suite *SynchronizeUSDXMintingRewardTests) TestClaimIsUnchangedWhenGlobalFa suite.Equal(claim.Reward, syncedClaim.Reward) } -type cdpBuilder struct { +// CDPBuilder is a tool for creating a CDP in tests. +// The builder inherits from cdp.CDP, so fields can be accessed directly if a helper method doesn't exist. +type CDPBuilder struct { cdptypes.CDP } -func NewCDPBuilder(owner sdk.AccAddress, collateralType string) cdpBuilder { - return cdpBuilder{ +// NewCDPBuilder creates a CdpBuilder containing a CDP with owner and collateral type set. +func NewCDPBuilder(owner sdk.AccAddress, collateralType string) CDPBuilder { + return CDPBuilder{ CDP: cdptypes.CDP{ Owner: owner, Type: collateralType, @@ -270,12 +245,36 @@ func NewCDPBuilder(owner sdk.AccAddress, collateralType string) cdpBuilder { // Set them to the default denom, but with 0 amount. Principal: c(cdptypes.DefaultStableDenom, 0), AccumulatedFees: c(cdptypes.DefaultStableDenom, 0), + // zero value of sdk.Dec causes nil pointer panics + InterestFactor: sdk.OneDec(), }} } -func (builder cdpBuilder) Build() cdptypes.CDP { return builder.CDP } +// Build assembles and returns the final deposit. +func (builder CDPBuilder) Build() cdptypes.CDP { return builder.CDP } -func (builder cdpBuilder) WithPrincipal(principal sdk.Int) cdpBuilder { +// WithSourceShares adds a principal amount and interest factor such that the source shares for this CDP is equal to specified. +// With a factor of 1, the total principal is the source shares. This picks an arbitrary factor to ensure factors are accounted for in production code. +func (builder CDPBuilder) WithSourceShares(shares int64) CDPBuilder { + if !builder.GetTotalPrincipal().Amount.Equal(sdk.ZeroInt()) { + panic("setting source shares on cdp with existing principal or fees not implemented") + } + if !(builder.InterestFactor.IsNil() || builder.InterestFactor.Equal(sdk.OneDec())) { + panic("setting source shares on cdp with existing interest factor not implemented") + } + // pick arbitrary interest factor + factor := sdk.NewInt(2) + + // Calculate deposit amount that would equal the requested source shares given the above factor. + principal := sdk.NewInt(shares).Mul(factor) + + builder.Principal = sdk.NewCoin(cdptypes.DefaultStableDenom, principal) + builder.InterestFactor = factor.ToDec() + + return builder +} + +func (builder CDPBuilder) WithPrincipal(principal sdk.Int) CDPBuilder { builder.Principal = sdk.NewCoin(cdptypes.DefaultStableDenom, principal) return builder } @@ -291,18 +290,6 @@ var nonEmptyRewardIndexes = types.RewardIndexes{ }, } -func paramsWithSingleUSDXRewardPeriod(collateralType string) types.ParamSubspace { - return &fakeParamSubspace{ - params: types.Params{ - USDXMintingRewardPeriods: types.RewardPeriods{ - { - CollateralType: collateralType, - }, - }, - }, - } -} - func extractFirstCollateralType(indexes types.RewardIndexes) string { if len(indexes) == 0 { panic("cannot extract a collateral type from 0 length RewardIndexes") diff --git a/x/incentive/testutil/integration.go b/x/incentive/testutil/integration.go index e53dfa4d..7aee75ef 100644 --- a/x/incentive/testutil/integration.go +++ b/x/incentive/testutil/integration.go @@ -8,6 +8,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" authexported "github.com/cosmos/cosmos-sdk/x/auth/exported" "github.com/cosmos/cosmos-sdk/x/auth/vesting" + paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" "github.com/cosmos/cosmos-sdk/x/staking" supplyexported "github.com/cosmos/cosmos-sdk/x/supply/exported" "github.com/stretchr/testify/suite" @@ -16,6 +17,7 @@ import ( "github.com/kava-labs/kava/app" "github.com/kava-labs/kava/x/cdp" + "github.com/kava-labs/kava/x/committee" "github.com/kava-labs/kava/x/hard" "github.com/kava-labs/kava/x/incentive" "github.com/kava-labs/kava/x/swap" @@ -27,6 +29,22 @@ type IntegrationTester struct { Ctx sdk.Context } +func (suite *IntegrationTester) SetupSuite() { + config := sdk.GetConfig() + app.SetBech32AddressPrefixes(config) +} + +func (suite *IntegrationTester) StartChain(genesisTime time.Time, genesisStates ...app.GenesisState) { + suite.App = app.NewTestApp() + + suite.App.InitializeFromGenesisStatesWithTime( + genesisTime, + genesisStates..., + ) + + suite.Ctx = suite.App.NewContext(false, abci.Header{Height: 1, Time: genesisTime}) +} + func (suite *IntegrationTester) NextBlockAt(blockTime time.Time) { if !suite.Ctx.BlockTime().Before(blockTime) { panic(fmt.Sprintf("new block time %s must be after current %s", blockTime, suite.Ctx.BlockTime())) @@ -87,14 +105,26 @@ func (suite *IntegrationTester) DeliverSwapMsgDeposit(depositor sdk.AccAddress, return err } -func (suite *IntegrationTester) DeliverHardMsgDeposit(depositor sdk.AccAddress, deposit sdk.Coins) error { - msg := hard.NewMsgDeposit(depositor, deposit) +func (suite *IntegrationTester) DeliverHardMsgDeposit(owner sdk.AccAddress, deposit sdk.Coins) error { + msg := hard.NewMsgDeposit(owner, deposit) _, err := hard.NewHandler(suite.App.GetHardKeeper())(suite.Ctx, msg) return err } -func (suite *IntegrationTester) DeliverHardMsgBorrow(depositor sdk.AccAddress, borrow sdk.Coins) error { - msg := hard.NewMsgBorrow(depositor, borrow) +func (suite *IntegrationTester) DeliverHardMsgBorrow(owner sdk.AccAddress, borrow sdk.Coins) error { + msg := hard.NewMsgBorrow(owner, borrow) + _, err := hard.NewHandler(suite.App.GetHardKeeper())(suite.Ctx, msg) + return err +} + +func (suite *IntegrationTester) DeliverHardMsgRepay(owner sdk.AccAddress, repay sdk.Coins) error { + msg := hard.NewMsgRepay(owner, owner, repay) + _, err := hard.NewHandler(suite.App.GetHardKeeper())(suite.Ctx, msg) + return err +} + +func (suite *IntegrationTester) DeliverHardMsgWithdraw(owner sdk.AccAddress, withdraw sdk.Coins) error { + msg := hard.NewMsgRepay(owner, owner, withdraw) _, err := hard.NewHandler(suite.App.GetHardKeeper())(suite.Ctx, msg) return err } @@ -105,6 +135,42 @@ func (suite *IntegrationTester) DeliverMsgCreateCDP(owner sdk.AccAddress, collat return err } +func (suite *IntegrationTester) DeliverCDPMsgRepay(owner sdk.AccAddress, collateralType string, payment sdk.Coin) error { + msg := cdp.NewMsgRepayDebt(owner, collateralType, payment) + _, err := cdp.NewHandler(suite.App.GetCDPKeeper())(suite.Ctx, msg) + return err +} + +func (suite *IntegrationTester) DeliverCDPMsgBorrow(owner sdk.AccAddress, collateralType string, draw sdk.Coin) error { + msg := cdp.NewMsgDrawDebt(owner, collateralType, draw) + _, err := cdp.NewHandler(suite.App.GetCDPKeeper())(suite.Ctx, msg) + return err +} + +func (suite *IntegrationTester) ProposeAndVoteOnNewParams(voter sdk.AccAddress, committeeID uint64, changes []paramtypes.ParamChange) { + + propose := committee.NewMsgSubmitProposal( + paramtypes.NewParameterChangeProposal( + "test title", + "test description", + changes, + ), + voter, + committeeID, + ) + + handleMsg := committee.NewHandler(suite.App.GetCommitteeKeeper()) + + res, err := handleMsg(suite.Ctx, propose) + suite.NoError(err) + + proposalID := committee.Uint64FromBytes(res.Data) + vote := committee.NewMsgVote(voter, proposalID, committee.Yes) + + _, err = handleMsg(suite.Ctx, vote) + suite.NoError(err) +} + func (suite *IntegrationTester) GetAccount(addr sdk.AccAddress) authexported.Account { ak := suite.App.GetAccountKeeper() return ak.GetAccount(suite.Ctx, addr) @@ -134,6 +200,20 @@ func (suite *IntegrationTester) BalanceEquals(address sdk.AccAddress, expected s suite.Equalf(expected, acc.GetCoins(), "expected account balance to equal coins %s, but got %s", expected, acc.GetCoins()) } +func (suite *IntegrationTester) BalanceInEpsilon(address sdk.AccAddress, expected sdk.Coins, epsilon float64) { + actual := suite.GetBalance(address) + + allDenoms := expected.Add(actual...) + for _, coin := range allDenoms { + suite.InEpsilonf( + expected.AmountOf(coin.Denom).Int64(), + actual.AmountOf(coin.Denom).Int64(), + epsilon, + "expected balance to be within %f%% of coins %s, but got %s", epsilon*100, expected, actual, + ) + } +} + func (suite *IntegrationTester) VestingPeriodsEqual(address sdk.AccAddress, expectedPeriods vesting.Periods) { acc := suite.App.GetAccountKeeper().GetAccount(suite.Ctx, address) suite.Require().NotNil(acc, "expected vesting account not to be nil")