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

@@ -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
 

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))
 }

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() {

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)
+			}
+		})
+	}
+
+}

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() {

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)
+			}
+		})
+	}
+
+}

x/incentive/keeper/cdp_test.go

@@ -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)
-}

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) {
 }
 

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{
+		committeetypes.NewMemberCommittee(
-			BaseCommittee: committeetypes.BaseCommittee{
+			committeeID,
-				ID:               genState.NextProposalID,
+			"This committee is for testing.",
-				Description:      "This committee is for testing.",
+			members,
-				Members:          members,
+			[]committeetypes.Permission{committeetypes.GodPermission{}},
-				Permissions:      []committeetypes.Permission{committeetypes.GodPermission{}},
+			sdk.MustNewDecFromStr("0.666666667"),
-				VoteThreshold:    d("0.667"),
+			time.Hour*24*7,
-				ProposalDuration: time.Hour * 24 * 7,
+			committeetypes.FirstPastThePost,
-				TallyOption:      committeetypes.FirstPastThePost,
+		),
-			},
-		},
 	}
-	genState.NextProposalID += 1
 	return app.GenesisState{
 		committeetypes.ModuleName: committeetypes.ModuleCdc.MustMarshalJSON(genState),
 	}

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 {
+	// Source shares for hard borrows is their normalized borrow amount
-		globalRewardIndexes, found := k.GetHardBorrowRewardIndexes(ctx, coin.Denom)
+	normalizedBorrows, err := borrow.NormalizedBorrow()
-		if !found {
+	if err != nil {
-			// The global factor is only not found if
+		panic(fmt.Sprintf("during borrow reward sync, could not get normalized borrow for %s: %s", borrow.Borrower, err.Error()))
-			// - 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
-		}
 
-		userRewardIndexes, found := claim.BorrowRewardIndexes.Get(coin.Denom)
+	for _, normedBorrow := range normalizedBorrows {
-		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, coin.Amount.ToDec())
+		claim = k.synchronizeSingleHardBorrowReward(ctx, claim, normedBorrow.Denom, normedBorrow.Amount)
-		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)
 	}
 	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 {

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{
+	borrow := NewBorrowBuilder(claim.Owner).
-		Borrower: claim.Owner,
+		WithArbitrarySourceShares(extractCollateralTypes(globalIndexes)...).
-		Amount:   arbitraryCoinsWithDenoms(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{
+	borrow := NewBorrowBuilder(owner).
-		Borrower: owner,
+		WithArbitrarySourceShares(extractCollateralTypes(globalIndexes)...).
-		Amount:   arbitraryCoinsWithDenoms(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{
+	borrow := NewBorrowBuilder(owner).
-		Borrower: owner,
+		WithArbitrarySourceShares(borrowedDenoms...).
-		Amount:   arbitraryCoinsWithDenoms(borrowedDenoms...),
+		Build()
-	}
 
 	suite.keeper.InitializeHardBorrowReward(suite.ctx, borrow)
 

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,
+	borrow := NewBorrowBuilder(claim.Owner).
-		Amount:   arbitraryCoinsWithDenoms(extractCollateralTypes(claim.BorrowRewardIndexes)...),
+		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{
+	borrow := NewBorrowBuilder(claim.Owner).
-		Borrower: claim.Owner,
+		WithArbitrarySourceShares(extractCollateralTypes(unchangingIndexes)...).
-		Amount:   arbitraryCoinsWithDenoms(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{
+	borrow := NewBorrowBuilder(claim.Owner).
-		Borrower: claim.Owner,
+		WithArbitrarySourceShares(extractCollateralTypes(globalIndexes)...).
-		Amount:   arbitraryCoinsWithDenoms(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{
+	borrow := NewBorrowBuilder(claim.Owner).
-		Borrower: claim.Owner,
+		WithArbitrarySourceShares(extractCollateralTypes(globalIndexes)...).
-		Amount:   arbitraryCoinsWithDenoms(extractCollateralTypes(globalIndexes)...),
+		Build()
-	}
 
 	suite.keeper.SynchronizeHardBorrowReward(suite.ctx, borrow)
 
@@ -169,10 +158,9 @@ func (suite *SynchronizeHardBorrowRewardTests) TestRewardIsIncrementedWhenGlobal
 		},
 	})
 
-	borrow := hardtypes.Borrow{
+	borrow := NewBorrowBuilder(claim.Owner).
-		Borrower: claim.Owner,
+		WithSourceShares("borrowdenom", 1e9).
-		Amount:   cs(c("borrowdenom", 1e9)),
+		Build()
-	}
 
 	suite.keeper.SynchronizeHardBorrowReward(suite.ctx, borrow)
 
@@ -232,10 +220,10 @@ func (suite *SynchronizeHardBorrowRewardTests) TestRewardIsIncrementedWhenNewRew
 	}
 	suite.storeGlobalBorrowIndexes(globalIndexes)
 
-	borrow := hardtypes.Borrow{
+	borrow := NewBorrowBuilder(claim.Owner).
-		Borrower: claim.Owner,
+		WithSourceShares("rewarded", 1e9).
-		Amount:   cs(c("rewarded", 1e9), c("newlyrewarded", 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{
+	borrow := NewBorrowBuilder(claim.Owner).
-		Borrower: claim.Owner,
+		WithSourceShares("borrowed", 1e9).
-		Amount:   cs(c("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

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(),
 	)
 }

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{
+	borrow := NewBorrowBuilder(claim.Owner).
-		Borrower: claim.Owner,
+		WithArbitrarySourceShares(extractCollateralTypes(expectedIndexes)...).
-		Amount:   arbitraryCoinsWithDenoms(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{
+	borrow := NewBorrowBuilder(claim.Owner).
-		Borrower: claim.Owner,
+		WithArbitrarySourceShares(extractCollateralTypes(globalIndexes)...).
-		Amount:   arbitraryCoinsWithDenoms(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{
+	borrow := NewBorrowBuilder(claim.Owner).
-		Borrower: claim.Owner,
+		WithArbitrarySourceShares(extractCollateralTypes(claim.BorrowRewardIndexes)...).
-		Amount:   arbitraryCoinsWithDenoms(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{
+	borrow := NewBorrowBuilder(claim.Owner).
-		Borrower: claim.Owner,
+		WithArbitrarySourceShares(borrowedDenoms...).
-		Amount:   arbitraryCoinsWithDenoms(borrowedDenoms...),
+		Build()
-	}
 
 	suite.keeper.UpdateHardBorrowIndexDenoms(suite.ctx, borrow)
 

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 {
+	// Source shares for hard deposits is their normalized deposit amount
-		globalRewardIndexes, found := k.GetHardSupplyRewardIndexes(ctx, coin.Denom)
+	normalizedDeposit, err := deposit.NormalizedDeposit()
-		if !found {
+	if err != nil {
-			// The global factor is only not found if
+		panic(fmt.Sprintf("during deposit reward sync, could not get normalized deposit for %s: %s", deposit.Depositor, err.Error()))
-			// - 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
-		}
 
-		userRewardIndexes, found := claim.SupplyRewardIndexes.Get(coin.Denom)
+	for _, normedDeposit := range normalizedDeposit {
-		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, coin.Amount.ToDec())
+		claim = k.synchronizeSingleHardSupplyReward(ctx, claim, normedDeposit.Denom, normedDeposit.Amount)
-		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)
 	}
 	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)

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{
+	deposit := NewDepositBuilder(claim.Owner).
-		Depositor: claim.Owner,
+		WithArbitrarySourceShares(extractCollateralTypes(globalIndexes)...).
-		Amount:    arbitraryCoinsWithDenoms(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{
+	deposit := NewDepositBuilder(owner).
-		Depositor: owner,
+		WithArbitrarySourceShares(extractCollateralTypes(globalIndexes)...).
-		Amount:    arbitraryCoinsWithDenoms(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{
+	deposit := NewDepositBuilder(owner).
-		Depositor: owner,
+		WithArbitrarySourceShares(depositedDenoms...).
-		Amount:    arbitraryCoinsWithDenoms(depositedDenoms...),
+		Build()
-	}
 
 	suite.keeper.InitializeHardSupplyReward(suite.ctx, deposit)
 

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{
+	deposit := NewDepositBuilder(claim.Owner).
-		Depositor: claim.Owner,
+		WithArbitrarySourceShares(extractCollateralTypes(claim.SupplyRewardIndexes)...).
-		Amount:    arbitraryCoinsWithDenoms(extractCollateralTypes(claim.SupplyRewardIndexes)...),
+		Build()
-	}
 
 	suite.keeper.SynchronizeHardSupplyReward(suite.ctx, deposit)
 
@@ -64,10 +56,9 @@ func (suite *SynchronizeHardSupplyRewardTests) TestClaimIndexesAreUnchangedWhenG
 
 	suite.storeGlobalSupplyIndexes(unchangingIndexes)
 
-	deposit := hardtypes.Deposit{
+	deposit := NewDepositBuilder(claim.Owner).
-		Depositor: claim.Owner,
+		WithArbitrarySourceShares(extractCollateralTypes(unchangingIndexes)...).
-		Amount:    arbitraryCoinsWithDenoms(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{
+	deposit := NewDepositBuilder(claim.Owner).
-		Depositor: claim.Owner,
+		WithArbitrarySourceShares(extractCollateralTypes(globalIndexes)...).
-		Amount:    arbitraryCoinsWithDenoms(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{
+	deposit := NewDepositBuilder(claim.Owner).
-		Depositor: claim.Owner,
+		WithArbitrarySourceShares(extractCollateralTypes(globalIndexes)...).
-		Amount:    arbitraryCoinsWithDenoms(extractCollateralTypes(globalIndexes)...),
+		Build()
-	}
 
 	suite.keeper.SynchronizeHardSupplyReward(suite.ctx, deposit)
 
@@ -165,10 +154,9 @@ func (suite *SynchronizeHardSupplyRewardTests) TestRewardIsIncrementedWhenGlobal
 		},
 	})
 
-	deposit := hardtypes.Deposit{
+	deposit := NewDepositBuilder(claim.Owner).
-		Depositor: claim.Owner,
+		WithSourceShares("depositdenom", 1e9).
-		Amount:    cs(c("depositdenom", 1e9)),
+		Build()
-	}
 
 	suite.keeper.SynchronizeHardSupplyReward(suite.ctx, deposit)
 
@@ -228,10 +216,10 @@ func (suite *SynchronizeHardSupplyRewardTests) TestRewardIsIncrementedWhenNewRew
 	}
 	suite.storeGlobalSupplyIndexes(globalIndexes)
 
-	deposit := hardtypes.Deposit{
+	deposit := NewDepositBuilder(claim.Owner).
-		Depositor: claim.Owner,
+		WithSourceShares("rewarded", 1e9).
-		Amount:    cs(c("rewarded", 1e9), c("newlyrewarded", 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{
+	deposit := NewDepositBuilder(claim.Owner).
-		Depositor: claim.Owner,
+		WithSourceShares("deposited", 1e9).
-		Amount:    cs(c("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
+}

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(),
 	)
 }

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{
+	deposit := NewDepositBuilder(claim.Owner).
-		Depositor: claim.Owner,
+		WithArbitrarySourceShares(extractCollateralTypes(expectedIndexes)...).
-		Amount:    arbitraryCoinsWithDenoms(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{
+	deposit := NewDepositBuilder(claim.Owner).
-		Depositor: claim.Owner,
+		WithArbitrarySourceShares(extractCollateralTypes(globalIndexes)...).
-		Amount:    arbitraryCoinsWithDenoms(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{
+	deposit := NewDepositBuilder(claim.Owner).
-		Depositor: claim.Owner,
+		WithArbitrarySourceShares(extractCollateralTypes(claim.SupplyRewardIndexes)...).
-		Amount:    arbitraryCoinsWithDenoms(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{
+	deposit := NewDepositBuilder(claim.Owner).
-		Depositor: claim.Owner,
+		WithArbitrarySourceShares(depositedDenoms...).
-		Amount:    arbitraryCoinsWithDenoms(depositedDenoms...),
+		Build()
-	}
 
 	suite.keeper.UpdateHardSupplyIndexDenoms(suite.ctx, deposit)
 

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
+		return claim
-	}
-	claim, found := k.GetUSDXMintingClaim(ctx, cdp.Owner)
-	if !found {
-		claim = types.NewUSDXMintingClaim(
-			cdp.Owner,
-			sdk.NewCoin(types.USDXMintingRewardDenom, sdk.ZeroInt()),
-			types.RewardIndexes{},
-		)
 	}
 
-	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

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

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 {
+// NewCDPBuilder creates a CdpBuilder containing a CDP with owner and collateral type set.
-	return cdpBuilder{
+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")

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 {
+func (suite *IntegrationTester) DeliverHardMsgDeposit(owner sdk.AccAddress, deposit sdk.Coins) error {
-	msg := hard.NewMsgDeposit(depositor, deposit)
+	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 {
+func (suite *IntegrationTester) DeliverHardMsgBorrow(owner sdk.AccAddress, borrow sdk.Coins) error {
-	msg := hard.NewMsgBorrow(depositor, borrow)
+	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")