Incentive refactor: hard rewards (#929)

* organise testing committee gen state

* remove repeated test app initialization

* minor fixes from linter in tests

* move more setup to SetupApp

* split up KeeperTestSuite for each reward type

* simplify KeeperTestSuite

* simplify PayoutKeeperSuite

* simplify DelegatorRewardSuite

* simplify SupplyRewardsSuite

* simplify BorrowRewardsSuite

* simplify USDXRewardsSuite

* add auth genesis builder for easier test setup

* migrate all incentive tests to auth builder

* add incentive genesis builder for easier setup
migrate hard incentive tests

* migrate all tests to incentive builder

* add hard genesis builder

* small tidy ups

* deduplicate initialTime from borrow tests

* deduplicate initialtTime from supply tests

* deduplicate initialTime from usdx and keeper tests

* deduplicate initialTime in delgator tests

* deduplicate genesis time in payout test

* deduplicate test app initialization

* make authGenesisBuilder available for all modules

* remove unused pricefeed setup

* export incentive genesis builder

* remove commented out test cases

* migrate cdp test to new test state builders

* migrate vv payout tests to use new builders

* add SynchronizeHardBorrowReward unit test

* extract calculatReward method

* tidy up unit test for borrow rewards

* add helper method to RewardIndexes

* user helper to extract logic from SyncBorrowReward

* add Get methods to (Multi)RewardIndexes

* replace params.Subspace in keeper to test easier

* add unit tests for usdx minting

* refactor InitializeUSDXMintingClaim

* add unit tests for InitializeHardBorrowRewards

* refactor SynchronizeUSDXMintingReward

* add unit tests for UpdateHardBorrowIndexDenoms

* change rewardSource type to Dec
needed by delegation rewards

* fix typo in test names

* refactor UpdateHardBorrowIndexDenoms

* update genesis test TODO to use auth builder

* add skipped test for bug in usdx sync

* extract common method for calculating rewards

* doc comment tidy

* add unit tests for delegator rewards

* tidy up test files

* remove old TODOs

* reaarrange InitializeHardDelegatorReward
to fit with other init reward functions

* duplicate borrow unit tests to create supply tests

* add tests for syncing with zero rewards per second

* refactor SynchronizeHardDelegatorRewards

* refactor supply rewards in same way as borrow

* fix total delegation calculation bug

* fix new usdx reward bug

* fix new supply/borrow reward bug

* remove working comment

* standardize behaviour when global factors missing

* improve documentation for CalculateRewards

* standardize variable names

* remove panic from calculateSingleReward

* wip

* Tidy up comments

* remove wip comment

x/incentive/genesis_test.go

@@ -37,11 +37,10 @@ func (suite *GenesisTestSuite) SetupTest() {
 	suite.genesisTime = time.Date(1998, 1, 1, 0, 0, 0, 0, time.UTC)
 
 	_, addrs := app.GeneratePrivKeyAddressPairs(3)
-	coins := []sdk.Coins{}
+
-	for j := 0; j < 3; j++ {
+	authBuilder := app.NewAuthGenesisBuilder().
-		coins = append(coins, cs(c("bnb", 10_000_000_000), c("ukava", 10_000_000_000)))
+		WithSimpleAccount(addrs[0], cs(c("bnb", 1e10), c("ukava", 1e10))).
-	}
+		WithSimpleModuleAccount(kavadist.KavaDistMacc, cs(c("hard", 1e15), c("ukava", 1e15)))
-	authGS := app.NewAuthGenState(addrs, coins)
 
 	loanToValue, _ := sdk.NewDecFromStr("0.6")
 	borrowLimit := sdk.NewDec(1000000000000000)
@@ -78,7 +77,7 @@ func (suite *GenesisTestSuite) SetupTest() {
 	)
 	tApp.InitializeFromGenesisStatesWithTime(
 		suite.genesisTime,
-		authGS,
+		authBuilder.BuildMarshalled(),
 		app.GenesisState{incentive.ModuleName: incentive.ModuleCdc.MustMarshalJSON(incentiveGS)},
 		app.GenesisState{hard.ModuleName: hard.ModuleCdc.MustMarshalJSON(hardGS)},
 		NewCDPGenStateMulti(),
@@ -87,14 +86,6 @@ func (suite *GenesisTestSuite) SetupTest() {
 
 	ctx := tApp.NewContext(true, abci.Header{Height: 1, Time: suite.genesisTime})
 
-	// TODO add to auth gen state to tidy up test
-	err := tApp.GetSupplyKeeper().MintCoins(
-		ctx,
-		kavadist.KavaDistMacc,
-		cs(c("hard", 1_000_000_000_000_000), c("ukava", 1_000_000_000_000_000)),
-	)
-	suite.Require().NoError(err)
-
 	suite.addrs = addrs
 	suite.keeper = keeper
 	suite.app = tApp

x/incentive/keeper/hooks.go

@@ -81,6 +81,8 @@ When delegated tokens (to bonded validators) are changed:
   - jail: total bonded delegation decreases (tokens no longer bonded (after end blocker runs))
 - validator becomes unbonded (ie when they drop out of the top 100)
   - total bonded delegation decreases (tokens no longer bonded)
+- validator becomes bonded (ie when they're promoted into the top 100)
+  - total bonded delegation increases (tokens become bonded)
 
 */
 

x/incentive/keeper/integration_test.go

@@ -262,9 +262,6 @@ func (builder IncentiveGenesisBuilder) WithSimpleBorrowRewardPeriod(ctype string
 	))
 }
 func (builder IncentiveGenesisBuilder) WithInitializedSupplyRewardPeriod(period types.MultiRewardPeriod) IncentiveGenesisBuilder {
-	// TODO this could set the start/end times on the period according to builder.genesisTime
-	// Then they could be created by a different builder
-
 	builder.Params.HardSupplyRewardPeriods = append(builder.Params.HardSupplyRewardPeriods, period)
 
 	accumulationTimeForPeriod := types.NewGenesisAccumulationTime(period.CollateralType, builder.genesisTime)

x/incentive/keeper/keeper.go

@@ -6,7 +6,6 @@ import (
 	"github.com/cosmos/cosmos-sdk/codec"
 	"github.com/cosmos/cosmos-sdk/store/prefix"
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	"github.com/cosmos/cosmos-sdk/x/params/subspace"
 
 	"github.com/kava-labs/kava/x/incentive/types"
 )
@@ -18,24 +17,28 @@ type Keeper struct {
 	cdpKeeper     types.CdpKeeper
 	hardKeeper    types.HardKeeper
 	key           sdk.StoreKey
-	paramSubspace subspace.Subspace
+	paramSubspace types.ParamSubspace
 	supplyKeeper  types.SupplyKeeper
 	stakingKeeper types.StakingKeeper
 }
 
 // NewKeeper creates a new keeper
 func NewKeeper(
-	cdc *codec.Codec, key sdk.StoreKey, paramstore subspace.Subspace, sk types.SupplyKeeper,
+	cdc *codec.Codec, key sdk.StoreKey, paramstore types.ParamSubspace, sk types.SupplyKeeper,
 	cdpk types.CdpKeeper, hk types.HardKeeper, ak types.AccountKeeper, stk types.StakingKeeper,
 ) Keeper {
 
+	if !paramstore.HasKeyTable() {
+		paramstore = paramstore.WithKeyTable(types.ParamKeyTable())
+	}
+
 	return Keeper{
 		accountKeeper: ak,
 		cdc:           cdc,
 		cdpKeeper:     cdpk,
 		hardKeeper:    hk,
 		key:           key,
-		paramSubspace: paramstore.WithKeyTable(types.ParamKeyTable()),
+		paramSubspace: paramstore,
 		supplyKeeper:  sk,
 		stakingKeeper: stk,
 	}

x/incentive/keeper/rewards_borrow.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 
 	hardtypes "github.com/kava-labs/kava/x/hard/types"
 	"github.com/kava-labs/kava/x/incentive/types"
@@ -85,14 +86,11 @@ func (k Keeper) InitializeHardBorrowReward(ctx sdk.Context, borrow hardtypes.Bor
 
 	var borrowRewardIndexes types.MultiRewardIndexes
 	for _, coin := range borrow.Amount {
-		globalRewardIndexes, foundGlobalRewardIndexes := k.GetHardBorrowRewardIndexes(ctx, coin.Denom)
+		globalRewardIndexes, found := k.GetHardBorrowRewardIndexes(ctx, coin.Denom)
-		var multiRewardIndex types.MultiRewardIndex
+		if !found {
-		if foundGlobalRewardIndexes {
+			globalRewardIndexes = types.RewardIndexes{}
-			multiRewardIndex = types.NewMultiRewardIndex(coin.Denom, globalRewardIndexes)
-		} else {
-			multiRewardIndex = types.NewMultiRewardIndex(coin.Denom, types.RewardIndexes{})
 		}
-		borrowRewardIndexes = append(borrowRewardIndexes, multiRewardIndex)
+		borrowRewardIndexes = borrowRewardIndexes.With(coin.Denom, globalRewardIndexes)
 	}
 
 	claim.BorrowRewardIndexes = borrowRewardIndexes
@@ -108,49 +106,34 @@ func (k Keeper) SynchronizeHardBorrowReward(ctx sdk.Context, borrow hardtypes.Bo
 	}
 
 	for _, coin := range borrow.Amount {
-		globalRewardIndexes, foundGlobalRewardIndexes := k.GetHardBorrowRewardIndexes(ctx, coin.Denom)
+		globalRewardIndexes, found := k.GetHardBorrowRewardIndexes(ctx, coin.Denom)
-		if !foundGlobalRewardIndexes {
+		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
 		}
 
-		userMultiRewardIndex, foundUserMultiRewardIndex := claim.BorrowRewardIndexes.GetRewardIndex(coin.Denom)
+		userRewardIndexes, found := claim.BorrowRewardIndexes.Get(coin.Denom)
-		if !foundUserMultiRewardIndex {
+		if !found {
-			continue
+			// 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{}
 		}
 
-		userRewardIndexIndex, foundUserRewardIndexIndex := claim.BorrowRewardIndexes.GetRewardIndexIndex(coin.Denom)
+		newRewards, err := k.CalculateRewards(userRewardIndexes, globalRewardIndexes, coin.Amount.ToDec())
-		if !foundUserRewardIndexIndex {
+		if err != nil {
-			continue
+			// 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))
 		}
 
-		for _, globalRewardIndex := range globalRewardIndexes {
+		claim.Reward = claim.Reward.Add(newRewards...)
-			userRewardIndex, foundUserRewardIndex := userMultiRewardIndex.RewardIndexes.GetRewardIndex(globalRewardIndex.CollateralType)
+		claim.BorrowRewardIndexes = claim.BorrowRewardIndexes.With(coin.Denom, globalRewardIndexes)
-			if !foundUserRewardIndex {
-				// User borrowed this coin type before it had rewards. When new rewards are added, legacy borrowers
-				// should immediately begin earning rewards. Enable users to do so by updating their claim with the global
-				// reward index denom and start their reward factor at 0.0
-				userRewardIndex = types.NewRewardIndex(globalRewardIndex.CollateralType, sdk.ZeroDec())
-				userMultiRewardIndex.RewardIndexes = append(userMultiRewardIndex.RewardIndexes, userRewardIndex)
-				claim.BorrowRewardIndexes[userRewardIndexIndex] = userMultiRewardIndex
-			}
-
-			globalRewardFactor := globalRewardIndex.RewardFactor
-			userRewardFactor := userRewardIndex.RewardFactor
-			rewardsAccumulatedFactor := globalRewardFactor.Sub(userRewardFactor)
-			if rewardsAccumulatedFactor.IsNegative() {
-				panic(fmt.Sprintf("reward accumulation factor cannot be negative: %s", rewardsAccumulatedFactor))
-			}
-
-			newRewardsAmount := rewardsAccumulatedFactor.Mul(borrow.Amount.AmountOf(coin.Denom).ToDec()).RoundInt()
-
-			factorIndex, foundFactorIndex := userMultiRewardIndex.RewardIndexes.GetFactorIndex(globalRewardIndex.CollateralType)
-			if !foundFactorIndex { // should never trigger
-				continue
-			}
-			claim.BorrowRewardIndexes[userRewardIndexIndex].RewardIndexes[factorIndex].RewardFactor = globalRewardIndex.RewardFactor
-			newRewardsCoin := sdk.NewCoin(userRewardIndex.CollateralType, newRewardsAmount)
-			claim.Reward = claim.Reward.Add(newRewardsCoin)
-		}
 	}
 	k.SetHardLiquidityProviderClaim(ctx, claim)
 }
@@ -165,26 +148,22 @@ func (k Keeper) UpdateHardBorrowIndexDenoms(ctx sdk.Context, borrow hardtypes.Bo
 	borrowDenoms := getDenoms(borrow.Amount)
 	borrowRewardIndexDenoms := claim.BorrowRewardIndexes.GetCollateralTypes()
 
-	uniqueBorrowDenoms := setDifference(borrowDenoms, borrowRewardIndexDenoms)
-	uniqueBorrowRewardDenoms := setDifference(borrowRewardIndexDenoms, borrowDenoms)
-
 	borrowRewardIndexes := claim.BorrowRewardIndexes
+
 	// Create a new multi-reward index in the claim for every new borrow denom
+	uniqueBorrowDenoms := setDifference(borrowDenoms, borrowRewardIndexDenoms)
+
 	for _, denom := range uniqueBorrowDenoms {
-		_, foundUserRewardIndexes := claim.BorrowRewardIndexes.GetRewardIndex(denom)
+		globalBorrowRewardIndexes, found := k.GetHardBorrowRewardIndexes(ctx, denom)
-		if !foundUserRewardIndexes {
+		if !found {
-			globalBorrowRewardIndexes, foundGlobalBorrowRewardIndexes := k.GetHardBorrowRewardIndexes(ctx, denom)
+			globalBorrowRewardIndexes = types.RewardIndexes{}
-			var multiRewardIndex types.MultiRewardIndex
-			if foundGlobalBorrowRewardIndexes {
-				multiRewardIndex = types.NewMultiRewardIndex(denom, globalBorrowRewardIndexes)
-			} else {
-				multiRewardIndex = types.NewMultiRewardIndex(denom, types.RewardIndexes{})
-			}
-			borrowRewardIndexes = append(borrowRewardIndexes, multiRewardIndex)
 		}
+		borrowRewardIndexes = borrowRewardIndexes.With(denom, globalBorrowRewardIndexes)
 	}
 
 	// Delete multi-reward index from claim if the collateral type is no longer borrowed
+	uniqueBorrowRewardDenoms := setDifference(borrowRewardIndexDenoms, borrowDenoms)
+
 	for _, denom := range uniqueBorrowRewardDenoms {
 		borrowRewardIndexes = borrowRewardIndexes.RemoveRewardIndex(denom)
 	}
@@ -192,3 +171,52 @@ func (k Keeper) UpdateHardBorrowIndexDenoms(ctx sdk.Context, borrow hardtypes.Bo
 	claim.BorrowRewardIndexes = borrowRewardIndexes
 	k.SetHardLiquidityProviderClaim(ctx, claim)
 }
+
+// CalculateRewards computes how much rewards should have accrued to a source (eg a user's hard borrowed btc amount)
+// between two index values.
+//
+// oldIndex is normally the index stored on a claim, newIndex the current global value, and rewardSource a hard borrowed/supplied amount.
+//
+// Returns an error if newIndexes does not contain all CollateralTypes from oldIndexes, or if any value of oldIndex.RewardFactor > newIndex.RewardFactor.
+// This should never happen, as it would mean that a global reward index has decreased in value, or that a global reward index has been deleted from state.
+func (k Keeper) CalculateRewards(oldIndexes, newIndexes types.RewardIndexes, rewardSource sdk.Dec) (sdk.Coins, error) {
+	// check for missing CollateralType's
+	for _, oldIndex := range oldIndexes {
+		if newIndex, found := newIndexes.Get(oldIndex.CollateralType); !found {
+			return nil, sdkerrors.Wrapf(types.ErrDecreasingRewardFactor, "old: %v, new: %v", oldIndex, newIndex)
+		}
+	}
+	var reward sdk.Coins
+	for _, newIndex := range newIndexes {
+		oldFactor, found := oldIndexes.Get(newIndex.CollateralType)
+		if !found {
+			oldFactor = sdk.ZeroDec()
+		}
+
+		rewardAmount, err := k.CalculateSingleReward(oldFactor, newIndex.RewardFactor, rewardSource)
+		if err != nil {
+			return nil, err
+		}
+
+		reward = reward.Add(
+			sdk.NewCoin(newIndex.CollateralType, rewardAmount),
+		)
+	}
+	return reward, nil
+}
+
+// CalculateSingleReward computes how much rewards should have accrued to a source (eg a user's btcb-a cdp principal)
+// between two index values.
+//
+// oldIndex is normally the index stored on a claim, newIndex the current global value, and rewardSource a cdp principal amount.
+//
+// Returns an error if oldIndex > newIndex. This should never happen, as it would mean that a global reward index has decreased in value,
+// or that a global reward index has been deleted from state.
+func (k Keeper) CalculateSingleReward(oldIndex, newIndex, rewardSource sdk.Dec) (sdk.Int, error) {
+	increase := newIndex.Sub(oldIndex)
+	if increase.IsNegative() {
+		return sdk.Int{}, sdkerrors.Wrapf(types.ErrDecreasingRewardFactor, "old: %v, new: %v", oldIndex, newIndex)
+	}
+	reward := increase.Mul(rewardSource).RoundInt()
+	return reward, nil
+}

x/incentive/keeper/rewards_borrow_init_test.go

@@ -0,0 +1,89 @@
+package keeper_test
+
+import (
+	"testing"
+
+	"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
+}
+
+func TestInitializeHardBorrowReward(t *testing.T) {
+	suite.Run(t, new(InitializeHardBorrowRewardTests))
+}
+
+func (suite *InitializeHardBorrowRewardTests) TestClaimIndexesAreSetWhenClaimExists() {
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner: arbitraryAddress(),
+		},
+		// Indexes should always be empty when initialize is called.
+		// If initialize is called then the user must have repaid their borrow positions,
+		// which means UpdateHardBorrowIndexDenoms was called and should have remove indexes.
+		BorrowRewardIndexes: types.MultiRewardIndexes{},
+	}
+	suite.storeClaim(claim)
+
+	globalIndexes := nonEmptyMultiRewardIndexes
+	suite.storeGlobalBorrowIndexes(globalIndexes)
+
+	borrow := hardtypes.Borrow{
+		Borrower: claim.Owner,
+		Amount:   arbitraryCoinsWithDenoms(extractCollateralTypes(globalIndexes)...),
+	}
+
+	suite.keeper.InitializeHardBorrowReward(suite.ctx, borrow)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(globalIndexes, syncedClaim.BorrowRewardIndexes)
+}
+func (suite *InitializeHardBorrowRewardTests) TestClaimIndexesAreSetWhenClaimDoesNotExist() {
+	globalIndexes := nonEmptyMultiRewardIndexes
+	suite.storeGlobalBorrowIndexes(globalIndexes)
+
+	owner := arbitraryAddress()
+	borrow := hardtypes.Borrow{
+		Borrower: owner,
+		Amount:   arbitraryCoinsWithDenoms(extractCollateralTypes(globalIndexes)...),
+	}
+
+	suite.keeper.InitializeHardBorrowReward(suite.ctx, borrow)
+
+	syncedClaim, found := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, owner)
+	suite.True(found)
+	suite.Equal(globalIndexes, syncedClaim.BorrowRewardIndexes)
+}
+func (suite *InitializeHardBorrowRewardTests) TestClaimIndexesAreSetEmptyForMissingIndexes() {
+
+	globalIndexes := nonEmptyMultiRewardIndexes
+	suite.storeGlobalBorrowIndexes(globalIndexes)
+
+	owner := arbitraryAddress()
+	// Borrow a denom that is not in the global indexes.
+	// 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...),
+	}
+
+	suite.keeper.InitializeHardBorrowReward(suite.ctx, borrow)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, owner)
+	suite.Equal(expectedIndexes, syncedClaim.BorrowRewardIndexes)
+}

x/incentive/keeper/rewards_borrow_sync_test.go

@@ -0,0 +1,528 @@
+package keeper_test
+
+import (
+	"errors"
+	"testing"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/stretchr/testify/require"
+	"github.com/stretchr/testify/suite"
+
+	hardtypes "github.com/kava-labs/kava/x/hard/types"
+	"github.com/kava-labs/kava/x/incentive/keeper"
+	"github.com/kava-labs/kava/x/incentive/types"
+)
+
+// 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
+}
+
+func TestSynchronizeHardBorrowReward(t *testing.T) {
+	suite.Run(t, new(SynchronizeHardBorrowRewardTests))
+}
+
+func (suite *SynchronizeHardBorrowRewardTests) TestClaimIndexesAreUpdatedWhenGlobalIndexesHaveIncreased() {
+	// This is the normal case
+
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner: arbitraryAddress(),
+		},
+		BorrowRewardIndexes: nonEmptyMultiRewardIndexes,
+	}
+	suite.storeClaim(claim)
+
+	globalIndexes := increaseAllRewardFactors(nonEmptyMultiRewardIndexes)
+	suite.storeGlobalBorrowIndexes(globalIndexes)
+	borrow := hardtypes.Borrow{
+		Borrower: claim.Owner,
+		Amount:   arbitraryCoinsWithDenoms(extractCollateralTypes(claim.BorrowRewardIndexes)...),
+	}
+
+	suite.keeper.SynchronizeHardBorrowReward(suite.ctx, borrow)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(globalIndexes, syncedClaim.BorrowRewardIndexes)
+}
+func (suite *SynchronizeHardBorrowRewardTests) TestClaimIndexesAreUnchangedWhenGlobalIndexesUnchanged() {
+	// It should be safe to call SynchronizeHardBorrowReward multiple times
+
+	unchangingIndexes := nonEmptyMultiRewardIndexes
+
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner: arbitraryAddress(),
+		},
+		BorrowRewardIndexes: unchangingIndexes,
+	}
+	suite.storeClaim(claim)
+
+	suite.storeGlobalBorrowIndexes(unchangingIndexes)
+
+	borrow := hardtypes.Borrow{
+		Borrower: claim.Owner,
+		Amount:   arbitraryCoinsWithDenoms(extractCollateralTypes(unchangingIndexes)...),
+	}
+
+	suite.keeper.SynchronizeHardBorrowReward(suite.ctx, borrow)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(unchangingIndexes, syncedClaim.BorrowRewardIndexes)
+}
+func (suite *SynchronizeHardBorrowRewardTests) TestClaimIndexesAreUpdatedWhenNewRewardAdded() {
+	// When a new reward is added (via gov) for a hard borrow denom the user has already borrowed, and the claim is synced;
+	// Then the new reward's index should be added to the claim.
+
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner: arbitraryAddress(),
+		},
+		BorrowRewardIndexes: nonEmptyMultiRewardIndexes,
+	}
+	suite.storeClaim(claim)
+
+	globalIndexes := appendUniqueMultiRewardIndex(nonEmptyMultiRewardIndexes)
+	suite.storeGlobalBorrowIndexes(globalIndexes)
+
+	borrow := hardtypes.Borrow{
+		Borrower: claim.Owner,
+		Amount:   arbitraryCoinsWithDenoms(extractCollateralTypes(globalIndexes)...),
+	}
+
+	suite.keeper.SynchronizeHardBorrowReward(suite.ctx, borrow)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(globalIndexes, syncedClaim.BorrowRewardIndexes)
+}
+
+func (suite *SynchronizeHardBorrowRewardTests) TestClaimIndexesAreUpdatedWhenNewRewardDenomAdded() {
+	// When a new reward coin is added (via gov) to an already rewarded borrow denom (that the user has already borrowed), and the claim is synced;
+	// Then the new reward coin's index should be added to the claim.
+
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner: arbitraryAddress(),
+		},
+		BorrowRewardIndexes: nonEmptyMultiRewardIndexes,
+	}
+	suite.storeClaim(claim)
+
+	globalIndexes := appendUniqueRewardIndexToFirstItem(nonEmptyMultiRewardIndexes)
+	suite.storeGlobalBorrowIndexes(globalIndexes)
+
+	borrow := hardtypes.Borrow{
+		Borrower: claim.Owner,
+		Amount:   arbitraryCoinsWithDenoms(extractCollateralTypes(globalIndexes)...),
+	}
+
+	suite.keeper.SynchronizeHardBorrowReward(suite.ctx, borrow)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(globalIndexes, syncedClaim.BorrowRewardIndexes)
+}
+
+func (suite *SynchronizeHardBorrowRewardTests) TestRewardIsIncrementedWhenGlobalIndexesHaveIncreased() {
+	// This is the normal case
+	// Given some time has passed (meaning the global indexes have increased)
+	// When the claim is synced
+	// The user earns rewards for the time passed
+
+	originalReward := arbitraryCoins()
+
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner:  arbitraryAddress(),
+			Reward: originalReward,
+		},
+		BorrowRewardIndexes: types.MultiRewardIndexes{
+			{
+				CollateralType: "borrowdenom",
+				RewardIndexes: types.RewardIndexes{
+					{
+						CollateralType: "rewarddenom",
+						RewardFactor:   d("1000.001"),
+					},
+				},
+			},
+		},
+	}
+	suite.storeClaim(claim)
+
+	suite.storeGlobalBorrowIndexes(types.MultiRewardIndexes{
+		{
+			CollateralType: "borrowdenom",
+			RewardIndexes: types.RewardIndexes{
+				{
+					CollateralType: "rewarddenom",
+					RewardFactor:   d("2000.002"),
+				},
+			},
+		},
+	})
+
+	borrow := hardtypes.Borrow{
+		Borrower: claim.Owner,
+		Amount:   cs(c("borrowdenom", 1e9)),
+	}
+
+	suite.keeper.SynchronizeHardBorrowReward(suite.ctx, borrow)
+
+	// new reward is (new index - old index) * borrow amount
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(
+		cs(c("rewarddenom", 1_000_001_000_000)).Add(originalReward...),
+		syncedClaim.Reward,
+	)
+}
+
+func (suite *SynchronizeHardBorrowRewardTests) TestRewardIsIncrementedWhenNewRewardAdded() {
+	// When a new reward is added (via gov) for a hard borrow denom the user has already borrowed, and the claim is synced
+	// Then the user earns rewards for the time since the reward was added
+
+	originalReward := arbitraryCoins()
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner:  arbitraryAddress(),
+			Reward: originalReward,
+		},
+		BorrowRewardIndexes: types.MultiRewardIndexes{
+			{
+				CollateralType: "rewarded",
+				RewardIndexes: types.RewardIndexes{
+					{
+						CollateralType: "reward",
+						RewardFactor:   d("1000.001"),
+					},
+				},
+			},
+		},
+	}
+	suite.storeClaim(claim)
+
+	globalIndexes := types.MultiRewardIndexes{
+		{
+			CollateralType: "rewarded",
+			RewardIndexes: types.RewardIndexes{
+				{
+					CollateralType: "reward",
+					RewardFactor:   d("2000.002"),
+				},
+			},
+		},
+		{
+			CollateralType: "newlyrewarded",
+			RewardIndexes: types.RewardIndexes{
+				{
+					CollateralType: "otherreward",
+					// Indexes start at 0 when the reward is added by gov,
+					// so this represents the syncing happening some time later.
+					RewardFactor: d("1000.001"),
+				},
+			},
+		},
+	}
+	suite.storeGlobalBorrowIndexes(globalIndexes)
+
+	borrow := hardtypes.Borrow{
+		Borrower: claim.Owner,
+		Amount:   cs(c("rewarded", 1e9), c("newlyrewarded", 1e9)),
+	}
+
+	suite.keeper.SynchronizeHardBorrowReward(suite.ctx, borrow)
+
+	// new reward is (new index - old index) * borrow amount for each borrowed denom
+	// The old index for `newlyrewarded` isn't in the claim, so it's added starting at 0 for calculating the reward.
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(
+		cs(c("otherreward", 1_000_001_000_000), c("reward", 1_000_001_000_000)).Add(originalReward...),
+		syncedClaim.Reward,
+	)
+}
+func (suite *SynchronizeHardBorrowRewardTests) TestRewardIsIncrementedWhenNewRewardDenomAdded() {
+	// When a new reward coin is added (via gov) to an already rewarded borrow denom (that the user has already borrowed), and the claim is synced;
+	// Then the user earns rewards for the time since the reward was added
+
+	originalReward := arbitraryCoins()
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner:  arbitraryAddress(),
+			Reward: originalReward,
+		},
+		BorrowRewardIndexes: types.MultiRewardIndexes{
+			{
+				CollateralType: "borrowed",
+				RewardIndexes: types.RewardIndexes{
+					{
+						CollateralType: "reward",
+						RewardFactor:   d("1000.001"),
+					},
+				},
+			},
+		},
+	}
+	suite.storeClaim(claim)
+
+	globalIndexes := types.MultiRewardIndexes{
+		{
+			CollateralType: "borrowed",
+			RewardIndexes: types.RewardIndexes{
+				{
+					CollateralType: "reward",
+					RewardFactor:   d("2000.002"),
+				},
+				{
+					CollateralType: "otherreward",
+					// Indexes start at 0 when the reward is added by gov,
+					// so this represents the syncing happening some time later.
+					RewardFactor: d("1000.001"),
+				},
+			},
+		},
+	}
+	suite.storeGlobalBorrowIndexes(globalIndexes)
+
+	borrow := hardtypes.Borrow{
+		Borrower: claim.Owner,
+		Amount:   cs(c("borrowed", 1e9)),
+	}
+
+	suite.keeper.SynchronizeHardBorrowReward(suite.ctx, borrow)
+
+	// new reward is (new index - old index) * borrow amount for each borrowed denom
+	// The old index for `otherreward` isn't in the claim, so it's added starting at 0 for calculating the reward.
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(
+		cs(c("reward", 1_000_001_000_000), c("otherreward", 1_000_001_000_000)).Add(originalReward...),
+		syncedClaim.Reward,
+	)
+}
+
+func TestCalculateRewards(t *testing.T) {
+	type expected struct {
+		err   error
+		coins sdk.Coins
+	}
+	type args struct {
+		oldIndexes, newIndexes types.RewardIndexes
+		sourceAmount           sdk.Dec
+	}
+	testcases := []struct {
+		name     string
+		args     args
+		expected expected
+	}{
+		{
+			name: "when old and new indexes have same denoms, rewards are calculated correctly",
+			args: args{
+				oldIndexes: types.RewardIndexes{
+					{
+						CollateralType: "hard",
+						RewardFactor:   d("0.000000001"),
+					},
+					{
+						CollateralType: "ukava",
+						RewardFactor:   d("0.1"),
+					},
+				},
+				newIndexes: types.RewardIndexes{
+					{
+						CollateralType: "hard",
+						RewardFactor:   d("1000.0"),
+					},
+					{
+						CollateralType: "ukava",
+						RewardFactor:   d("0.100000001"),
+					},
+				},
+				sourceAmount: d("1000000000"),
+			},
+			expected: expected{
+				// for each denom: (new - old) * sourceAmount
+				coins: cs(c("hard", 999999999999), c("ukava", 1)),
+			},
+		},
+		{
+			name: "when new indexes have an extra denom, rewards are calculated as if it was 0 in old indexes",
+			args: args{
+				oldIndexes: types.RewardIndexes{
+					{
+						CollateralType: "hard",
+						RewardFactor:   d("0.000000001"),
+					},
+				},
+				newIndexes: types.RewardIndexes{
+					{
+						CollateralType: "hard",
+						RewardFactor:   d("1000.0"),
+					},
+					{
+						CollateralType: "ukava",
+						RewardFactor:   d("0.100000001"),
+					},
+				},
+				sourceAmount: d("1000000000"),
+			},
+			expected: expected{
+				// for each denom: (new - old) * sourceAmount
+				coins: cs(c("hard", 999999999999), c("ukava", 100000001)),
+			},
+		},
+		{
+			name: "when new indexes are smaller than old, an error is returned",
+			args: args{
+				oldIndexes: types.RewardIndexes{
+					{
+						CollateralType: "hard",
+						RewardFactor:   d("0.2"),
+					},
+				},
+				newIndexes: types.RewardIndexes{
+					{
+						CollateralType: "hard",
+						RewardFactor:   d("0.1"),
+					},
+				},
+				sourceAmount: d("1000000000"),
+			},
+			expected: expected{
+				err: types.ErrDecreasingRewardFactor,
+			},
+		},
+		{
+			name: "when old indexes have an extra denom, an error is returned",
+			args: args{
+				oldIndexes: types.RewardIndexes{
+					{
+						CollateralType: "hard",
+						RewardFactor:   d("0.1"),
+					},
+					{
+						CollateralType: "ukava",
+						RewardFactor:   d("0.1"),
+					},
+				},
+				newIndexes: types.RewardIndexes{
+					{
+						CollateralType: "hard",
+						RewardFactor:   d("0.2"),
+					},
+				},
+				sourceAmount: d("1000000000"),
+			},
+			expected: expected{
+				err: types.ErrDecreasingRewardFactor,
+			},
+		},
+		{
+			name: "when old and new indexes are 0, rewards are 0",
+			args: args{
+				oldIndexes: types.RewardIndexes{
+					{
+						CollateralType: "hard",
+						RewardFactor:   d("0.0"),
+					},
+				},
+				newIndexes: types.RewardIndexes{
+					{
+						CollateralType: "hard",
+						RewardFactor:   d("0.0"),
+					},
+				},
+				sourceAmount: d("1000000000"),
+			},
+			expected: expected{
+				coins: nil,
+			},
+		},
+		{
+			name: "when old and new indexes are empty, rewards are 0",
+			args: args{
+				oldIndexes:   types.RewardIndexes{},
+				newIndexes:   nil,
+				sourceAmount: d("1000000000"),
+			},
+			expected: expected{
+				coins: nil,
+			},
+		},
+	}
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			coins, err := keeper.Keeper{}.CalculateRewards(tc.args.oldIndexes, tc.args.newIndexes, tc.args.sourceAmount)
+			if tc.expected.err != nil {
+				require.True(t, errors.Is(err, tc.expected.err))
+			} else {
+				require.Equal(t, tc.expected.coins, coins)
+			}
+		})
+	}
+}
+func TestCalculateSingleReward(t *testing.T) {
+	type expected struct {
+		err    error
+		reward sdk.Int
+	}
+	type args struct {
+		oldIndex, newIndex sdk.Dec
+		sourceAmount       sdk.Dec
+	}
+	testcases := []struct {
+		name     string
+		args     args
+		expected expected
+	}{
+		{
+			name: "when new index is > old, rewards are calculated correctly",
+			args: args{
+				oldIndex:     d("0.000000001"),
+				newIndex:     d("1000.0"),
+				sourceAmount: d("1000000000"),
+			},
+			expected: expected{
+				// (new - old) * sourceAmount
+				reward: i(999999999999),
+			},
+		},
+		{
+			name: "when new index is < old, an error is returned",
+			args: args{
+				oldIndex:     d("0.000000001"),
+				newIndex:     d("0.0"),
+				sourceAmount: d("1000000000"),
+			},
+			expected: expected{
+				err: types.ErrDecreasingRewardFactor,
+			},
+		},
+		{
+			name: "when old and new indexes are 0, rewards are 0",
+			args: args{
+				oldIndex:     d("0.0"),
+				newIndex:     d("0.0"),
+				sourceAmount: d("1000000000"),
+			},
+			expected: expected{
+				reward: sdk.ZeroInt(),
+			},
+		},
+	}
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			reward, err := keeper.Keeper{}.CalculateSingleReward(tc.args.oldIndex, tc.args.newIndex, tc.args.sourceAmount)
+			if tc.expected.err != nil {
+				require.True(t, errors.Is(err, tc.expected.err))
+			} else {
+				require.Equal(t, tc.expected.reward, reward)
+			}
+		})
+	}
+}

x/incentive/keeper/rewards_borrow_test.go

@@ -61,7 +61,7 @@ func (suite *BorrowRewardsTestSuite) SetupWithGenState(authBuilder app.AuthGenes
 		authBuilder.BuildMarshalled(),
 		NewPricefeedGenStateMultiFromTime(suite.genesisTime),
 		hardBuilder.BuildMarshalled(),
-		NewCommitteeGenesisState(suite.addrs[:2]), // TODO add committee members to suite,
+		NewCommitteeGenesisState(suite.addrs[:2]),
 		incentBuilder.BuildMarshalled(),
 	)
 }
@@ -498,7 +498,6 @@ func (suite *BorrowRewardsTestSuite) TestSynchronizeHardBorrowReward() {
 				updatedTimeDuration: 86400,
 			},
 		},
-		// TODO test synchronize when there is a reward period with 0 rewardsPerSecond
 	}
 	for _, tc := range testCases {
 		suite.Run(tc.name, func() {

x/incentive/keeper/rewards_borrow_update_test.go

@@ -0,0 +1,119 @@
+package keeper_test
+
+import (
+	"testing"
+
+	"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
+}
+
+func TestUpdateHardBorrowIndexDenoms(t *testing.T) {
+	suite.Run(t, new(UpdateHardBorrowIndexDenomsTests))
+}
+
+func (suite *UpdateHardBorrowIndexDenomsTests) TestClaimIndexesAreRemovedForDenomsNoLongerBorrowed() {
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner: arbitraryAddress(),
+		},
+		BorrowRewardIndexes: nonEmptyMultiRewardIndexes,
+	}
+	suite.storeClaim(claim)
+	suite.storeGlobalBorrowIndexes(claim.BorrowRewardIndexes)
+
+	// remove one denom from the indexes already in the borrow
+	expectedIndexes := claim.BorrowRewardIndexes[1:]
+	borrow := hardtypes.Borrow{
+		Borrower: claim.Owner,
+		Amount:   arbitraryCoinsWithDenoms(extractCollateralTypes(expectedIndexes)...),
+	}
+
+	suite.keeper.UpdateHardBorrowIndexDenoms(suite.ctx, borrow)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(expectedIndexes, syncedClaim.BorrowRewardIndexes)
+}
+
+func (suite *UpdateHardBorrowIndexDenomsTests) TestClaimIndexesAreAddedForNewlyBorrowedDenoms() {
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner: arbitraryAddress(),
+		},
+		BorrowRewardIndexes: nonEmptyMultiRewardIndexes,
+	}
+	suite.storeClaim(claim)
+	globalIndexes := appendUniqueMultiRewardIndex(claim.BorrowRewardIndexes)
+	suite.storeGlobalBorrowIndexes(globalIndexes)
+
+	borrow := hardtypes.Borrow{
+		Borrower: claim.Owner,
+		Amount:   arbitraryCoinsWithDenoms(extractCollateralTypes(globalIndexes)...),
+	}
+
+	suite.keeper.UpdateHardBorrowIndexDenoms(suite.ctx, borrow)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(globalIndexes, syncedClaim.BorrowRewardIndexes)
+}
+
+func (suite *UpdateHardBorrowIndexDenomsTests) TestClaimIndexesAreUnchangedWhenBorrowedDenomsUnchanged() {
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner: arbitraryAddress(),
+		},
+		BorrowRewardIndexes: nonEmptyMultiRewardIndexes,
+	}
+	suite.storeClaim(claim)
+	// Set global indexes with same denoms but different values.
+	// UpdateHardBorrowIndexDenoms should ignore the new values.
+	suite.storeGlobalBorrowIndexes(increaseAllRewardFactors(claim.BorrowRewardIndexes))
+
+	borrow := hardtypes.Borrow{
+		Borrower: claim.Owner,
+		Amount:   arbitraryCoinsWithDenoms(extractCollateralTypes(claim.BorrowRewardIndexes)...),
+	}
+
+	suite.keeper.UpdateHardBorrowIndexDenoms(suite.ctx, borrow)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(claim.BorrowRewardIndexes, syncedClaim.BorrowRewardIndexes)
+}
+
+func (suite *UpdateHardBorrowIndexDenomsTests) TestEmptyClaimIndexesAreAddedForNewlyBorrowedButNotRewardedDenoms() {
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner: arbitraryAddress(),
+		},
+		BorrowRewardIndexes: nonEmptyMultiRewardIndexes,
+	}
+	suite.storeClaim(claim)
+	suite.storeGlobalBorrowIndexes(claim.BorrowRewardIndexes)
+
+	// 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...),
+	}
+
+	suite.keeper.UpdateHardBorrowIndexDenoms(suite.ctx, borrow)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(expectedIndexes, syncedClaim.BorrowRewardIndexes)
+}

x/incentive/keeper/rewards_delegator.go

@@ -45,23 +45,21 @@ func (k Keeper) AccumulateHardDelegatorRewards(ctx sdk.Context, rewardPeriod typ
 
 // InitializeHardDelegatorReward initializes the delegator reward index of a hard claim
 func (k Keeper) InitializeHardDelegatorReward(ctx sdk.Context, delegator sdk.AccAddress) {
-	delegatorFactor, foundDelegatorFactor := k.GetHardDelegatorRewardFactor(ctx, types.BondDenom)
-	if !foundDelegatorFactor { // Should always be found...
-		delegatorFactor = sdk.ZeroDec()
-	}
-
-	delegatorRewardIndexes := types.NewRewardIndex(types.BondDenom, delegatorFactor)
-
 	claim, found := k.GetHardLiquidityProviderClaim(ctx, delegator)
 	if !found {
-		// Instantiate claim object
 		claim = types.NewHardLiquidityProviderClaim(delegator, sdk.Coins{}, nil, nil, nil)
 	} else {
 		k.SynchronizeHardDelegatorRewards(ctx, delegator, nil, false)
 		claim, _ = k.GetHardLiquidityProviderClaim(ctx, delegator)
 	}
 
-	claim.DelegatorRewardIndexes = types.RewardIndexes{delegatorRewardIndexes}
+	globalRewardFactor, found := k.GetHardDelegatorRewardFactor(ctx, types.BondDenom)
+	if !found {
+		// If there's no global delegator reward factor, initialize the claim with a delegator factor of zero.
+		globalRewardFactor = sdk.ZeroDec()
+	}
+
+	claim.DelegatorRewardIndexes = types.RewardIndexes{types.NewRewardIndex(types.BondDenom, globalRewardFactor)}
 	k.SetHardLiquidityProviderClaim(ctx, claim)
 }
 
@@ -74,23 +72,42 @@ func (k Keeper) SynchronizeHardDelegatorRewards(ctx sdk.Context, delegator sdk.A
 		return
 	}
 
-	delagatorFactor, found := k.GetHardDelegatorRewardFactor(ctx, types.BondDenom)
+	globalRewardFactor, found := k.GetHardDelegatorRewardFactor(ctx, types.BondDenom)
 	if !found {
+		// The global factor is only not found if
+		// - the bond 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
 	}
 
-	delegatorIndex, hasDelegatorRewardIndex := claim.HasDelegatorRewardIndex(types.BondDenom)
+	userRewardFactor, found := claim.DelegatorRewardIndexes.Get(types.BondDenom)
-	if !hasDelegatorRewardIndex {
+	if !found {
-		return
+		// Normally the factor should always be found, as it is added in InitializeHardDelegatorReward when a user delegates.
+		// However if there were no delegator rewards (ie no reward period in params) then a reward period is added, existing claims will not have the factor.
+		// So assume the factor is the starting value for any global factor: 0.
+		userRewardFactor = sdk.ZeroDec()
 	}
 
-	userRewardFactor := claim.DelegatorRewardIndexes[delegatorIndex].RewardFactor
+	totalDelegated := k.GetTotalDelegated(ctx, delegator, valAddr, shouldIncludeValidator)
-	rewardsAccumulatedFactor := delagatorFactor.Sub(userRewardFactor)
-	if rewardsAccumulatedFactor.IsNegative() {
-		panic(fmt.Sprintf("reward accumulation factor cannot be negative: %s", rewardsAccumulatedFactor))
-	}
-	claim.DelegatorRewardIndexes[delegatorIndex].RewardFactor = delagatorFactor
 
+	rewardsEarned, err := k.CalculateSingleReward(userRewardFactor, globalRewardFactor, totalDelegated)
+	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))
+	}
+	newRewardsCoin := sdk.NewCoin(types.HardLiquidityRewardDenom, rewardsEarned)
+
+	claim.Reward = claim.Reward.Add(newRewardsCoin)
+	claim.DelegatorRewardIndexes = claim.DelegatorRewardIndexes.With(types.BondDenom, globalRewardFactor)
+
+	k.SetHardLiquidityProviderClaim(ctx, claim)
+}
+
+func (k Keeper) GetTotalDelegated(ctx sdk.Context, delegator sdk.AccAddress, valAddr sdk.ValAddress, shouldIncludeValidator bool) sdk.Dec {
 	totalDelegated := sdk.ZeroDec()
 
 	delegations := k.stakingKeeper.GetDelegatorDelegations(ctx, delegator, 200)
@@ -100,14 +117,16 @@ func (k Keeper) SynchronizeHardDelegatorRewards(ctx sdk.Context, delegator sdk.A
 			continue
 		}
 
-		if valAddr == nil {
+		if validator.OperatorAddress.Equals(valAddr) {
-			// Delegators don't accumulate rewards if their validator is unbonded
+			if shouldIncludeValidator {
-			if validator.GetStatus() != sdk.Bonded {
+				// do nothing, so the validator is included regardless of bonded status
+			} else {
+				// skip this validator
 				continue
 			}
 		} else {
-			if !shouldIncludeValidator && validator.OperatorAddress.Equals(valAddr) {
+			// skip any not bonded validator
-				// ignore tokens delegated to the validator
+			if validator.GetStatus() != sdk.Bonded {
 				continue
 			}
 		}
@@ -122,10 +141,5 @@ func (k Keeper) SynchronizeHardDelegatorRewards(ctx sdk.Context, delegator sdk.A
 		}
 		totalDelegated = totalDelegated.Add(delegatedTokens)
 	}
-	rewardsEarned := rewardsAccumulatedFactor.Mul(totalDelegated).RoundInt()
+	return totalDelegated
-
-	// Add rewards to delegator's hard claim
-	newRewardsCoin := sdk.NewCoin(types.HardLiquidityRewardDenom, rewardsEarned)
-	claim.Reward = claim.Reward.Add(newRewardsCoin)
-	k.SetHardLiquidityProviderClaim(ctx, claim)
 }

x/incentive/keeper/rewards_delegator_init_test.go

@@ -0,0 +1,115 @@
+package keeper_test
+
+import (
+	"testing"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
+	"github.com/stretchr/testify/suite"
+
+	"github.com/kava-labs/kava/x/incentive/types"
+)
+
+// InitializeHardDelegatorRewardTests runs unit tests for the keeper.InitializeHardDelegatorReward method
+//
+// inputs
+// - claim in store if it exists (only claim.DelegatorRewardIndexes)
+// - global indexes in store
+// - delegator function arg
+//
+// outputs
+// - sets or creates a claim
+type InitializeHardDelegatorRewardTests struct {
+	unitTester
+}
+
+func TestInitializeHardDelegatorReward(t *testing.T) {
+	suite.Run(t, new(InitializeHardDelegatorRewardTests))
+}
+
+func (suite *InitializeHardDelegatorRewardTests) storeGlobalDelegatorFactor(rewardIndexes types.RewardIndexes) {
+	factor := rewardIndexes[0]
+	suite.keeper.SetHardDelegatorRewardFactor(suite.ctx, factor.CollateralType, factor.RewardFactor)
+}
+
+func (suite *InitializeHardDelegatorRewardTests) TestClaimIndexesAreSetWhenClaimDoesNotExist() {
+	globalIndex := arbitraryDelegatorRewardIndexes
+	suite.storeGlobalDelegatorFactor(globalIndex)
+
+	delegator := arbitraryAddress()
+	suite.keeper.InitializeHardDelegatorReward(suite.ctx, delegator)
+
+	syncedClaim, f := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, delegator)
+	suite.True(f)
+	suite.Equal(globalIndex, syncedClaim.DelegatorRewardIndexes)
+}
+
+func (suite *InitializeHardDelegatorRewardTests) TestClaimIsSyncedAndIndexesAreSetWhenClaimDoesExist() {
+	validatorAddress := arbitraryValidatorAddress()
+	sk := fakeStakingKeeper{
+		delegations: stakingtypes.Delegations{{
+			ValidatorAddress: validatorAddress,
+			Shares:           d("1000"),
+		}},
+		validators: stakingtypes.Validators{{
+			OperatorAddress: validatorAddress,
+			Status:          sdk.Bonded,
+			Tokens:          i(1000),
+			DelegatorShares: d("1000"),
+		}},
+	}
+	suite.keeper = suite.NewKeeper(&fakeParamSubspace{}, nil, nil, nil, nil, sk)
+
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner: arbitraryAddress(),
+		},
+		DelegatorRewardIndexes: arbitraryDelegatorRewardIndexes,
+	}
+	suite.storeClaim(claim)
+
+	// Set the global factor to a value different to one in claim so
+	// we can detect if it is overwritten.
+	globalIndex := increaseRewardFactors(claim.DelegatorRewardIndexes)
+	suite.storeGlobalDelegatorFactor(globalIndex)
+
+	suite.keeper.InitializeHardDelegatorReward(suite.ctx, claim.Owner)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(globalIndex, syncedClaim.DelegatorRewardIndexes)
+	suite.Truef(syncedClaim.Reward.IsAllGT(claim.Reward), "'%s' not greater than '%s'", syncedClaim.Reward, claim.Reward)
+}
+
+// arbitraryDelegatorRewardIndexes contains only one reward index as there is only every one bond denom
+var arbitraryDelegatorRewardIndexes = types.RewardIndexes{
+	types.NewRewardIndex(types.BondDenom, d("0.2")),
+}
+
+type fakeStakingKeeper struct {
+	delegations stakingtypes.Delegations
+	validators  stakingtypes.Validators
+}
+
+func (k fakeStakingKeeper) TotalBondedTokens(ctx sdk.Context) sdk.Int {
+	panic("unimplemented")
+}
+func (k fakeStakingKeeper) GetDelegatorDelegations(ctx sdk.Context, delegator sdk.AccAddress, maxRetrieve uint16) []stakingtypes.Delegation {
+	return k.delegations
+}
+func (k fakeStakingKeeper) GetValidator(ctx sdk.Context, addr sdk.ValAddress) (stakingtypes.Validator, bool) {
+	for _, val := range k.validators {
+		if val.GetOperator().Equals(addr) {
+			return val, true
+		}
+	}
+	return stakingtypes.Validator{}, false
+}
+func (k fakeStakingKeeper) GetValidatorDelegations(ctx sdk.Context, valAddr sdk.ValAddress) []stakingtypes.Delegation {
+	var delegations stakingtypes.Delegations
+	for _, d := range k.delegations {
+		if d.ValidatorAddress.Equals(valAddr) {
+			delegations = append(delegations, d)
+		}
+	}
+	return delegations
+}

x/incentive/keeper/rewards_delegator_sync_test.go

@@ -0,0 +1,356 @@
+package keeper_test
+
+import (
+	"testing"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
+	"github.com/stretchr/testify/suite"
+
+	"github.com/kava-labs/kava/x/incentive/types"
+)
+
+// SynchronizeHardDelegatorRewardTests runs unit tests for the keeper.SynchronizeHardDelegatorReward method
+//
+// inputs
+// - claim in store if it exists (only claim.DelegatorRewardIndexes and claim.Reward)
+// - global index in store
+// - function args: delegator address, validator address, shouldIncludeValidator flag
+// - delegator's delegations and the corresponding validators
+//
+// outputs
+// - sets or creates a claim
+type SynchronizeHardDelegatorRewardTests struct {
+	unitTester
+}
+
+func TestSynchronizeHardDelegatorReward(t *testing.T) {
+	suite.Run(t, new(SynchronizeHardDelegatorRewardTests))
+}
+
+func (suite *SynchronizeHardDelegatorRewardTests) storeGlobalDelegatorFactor(rewardIndexes types.RewardIndexes) {
+	factor := rewardIndexes[0]
+	suite.keeper.SetHardDelegatorRewardFactor(suite.ctx, factor.CollateralType, factor.RewardFactor)
+}
+
+func (suite *SynchronizeHardDelegatorRewardTests) TestClaimIndexesAreUnchangedWhenGlobalFactorUnchanged() {
+	delegator := arbitraryAddress()
+
+	stakingKeeper := fakeStakingKeeper{} // use an empty staking keeper that returns no delegations
+	suite.keeper = suite.NewKeeper(&fakeParamSubspace{}, nil, nil, nil, nil, stakingKeeper)
+
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner: delegator,
+		},
+		DelegatorRewardIndexes: arbitraryDelegatorRewardIndexes,
+	}
+	suite.storeClaim(claim)
+
+	suite.storeGlobalDelegatorFactor(claim.DelegatorRewardIndexes)
+
+	suite.keeper.SynchronizeHardDelegatorRewards(suite.ctx, claim.Owner, nil, false)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(claim.DelegatorRewardIndexes, syncedClaim.DelegatorRewardIndexes)
+}
+
+func (suite *SynchronizeHardDelegatorRewardTests) TestClaimIndexesAreUpdatedWhenGlobalFactorIncreased() {
+	delegator := arbitraryAddress()
+
+	suite.keeper = suite.NewKeeper(&fakeParamSubspace{}, nil, nil, nil, nil, fakeStakingKeeper{})
+
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner: delegator,
+		},
+		DelegatorRewardIndexes: arbitraryDelegatorRewardIndexes,
+	}
+	suite.storeClaim(claim)
+
+	globalIndexes := increaseRewardFactors(claim.DelegatorRewardIndexes)
+	suite.storeGlobalDelegatorFactor(globalIndexes)
+
+	suite.keeper.SynchronizeHardDelegatorRewards(suite.ctx, claim.Owner, nil, false)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(globalIndexes, syncedClaim.DelegatorRewardIndexes)
+}
+func (suite *SynchronizeHardDelegatorRewardTests) TestRewardIsUnchangedWhenGlobalFactorUnchanged() {
+	delegator := arbitraryAddress()
+	validatorAddress := arbitraryValidatorAddress()
+	stakingKeeper := fakeStakingKeeper{
+		delegations: stakingtypes.Delegations{
+			{
+				DelegatorAddress: delegator,
+				ValidatorAddress: validatorAddress,
+				Shares:           d("1000"),
+			},
+		},
+		validators: stakingtypes.Validators{
+			unslashedBondedValidator(validatorAddress),
+		},
+	}
+	suite.keeper = suite.NewKeeper(&fakeParamSubspace{}, nil, nil, nil, nil, stakingKeeper)
+
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner:  delegator,
+			Reward: arbitraryCoins(),
+		},
+		DelegatorRewardIndexes: types.RewardIndexes{{
+			CollateralType: types.BondDenom,
+			RewardFactor:   d("0.1"),
+		}},
+	}
+	suite.storeClaim(claim)
+
+	suite.storeGlobalDelegatorFactor(claim.DelegatorRewardIndexes)
+
+	suite.keeper.SynchronizeHardDelegatorRewards(suite.ctx, claim.Owner, nil, false)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+
+	suite.Equal(claim.Reward, syncedClaim.Reward)
+}
+
+func (suite *SynchronizeHardDelegatorRewardTests) TestRewardIsIncreasedWhenNewRewardAdded() {
+	delegator := arbitraryAddress()
+	validatorAddress := arbitraryValidatorAddress()
+	stakingKeeper := fakeStakingKeeper{
+		delegations: stakingtypes.Delegations{
+			{
+				DelegatorAddress: delegator,
+				ValidatorAddress: validatorAddress,
+				Shares:           d("1000"),
+			},
+		},
+		validators: stakingtypes.Validators{
+			unslashedBondedValidator(validatorAddress),
+		},
+	}
+	suite.keeper = suite.NewKeeper(&fakeParamSubspace{}, nil, nil, nil, nil, stakingKeeper)
+
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner:  delegator,
+			Reward: arbitraryCoins(),
+		},
+		DelegatorRewardIndexes: types.RewardIndexes{},
+	}
+	suite.storeClaim(claim)
+
+	newGlobalIndexes := types.RewardIndexes{{
+		CollateralType: types.BondDenom,
+		RewardFactor:   d("0.1"),
+	}}
+	suite.storeGlobalDelegatorFactor(newGlobalIndexes)
+
+	suite.keeper.SynchronizeHardDelegatorRewards(suite.ctx, claim.Owner, nil, false)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+
+	suite.Equal(newGlobalIndexes, syncedClaim.DelegatorRewardIndexes)
+	suite.Equal(
+		cs(c(types.HardLiquidityRewardDenom, 100)).Add(claim.Reward...),
+		syncedClaim.Reward,
+	)
+}
+
+func (suite *SynchronizeHardDelegatorRewardTests) TestRewardIsIncreasedWhenGlobalFactorIncreased() {
+	delegator := arbitraryAddress()
+	validatorAddress := arbitraryValidatorAddress()
+	stakingKeeper := fakeStakingKeeper{
+		delegations: stakingtypes.Delegations{
+			{
+				DelegatorAddress: delegator,
+				ValidatorAddress: validatorAddress,
+				Shares:           d("1000"),
+			},
+		},
+		validators: stakingtypes.Validators{
+			unslashedBondedValidator(validatorAddress),
+		},
+	}
+	suite.keeper = suite.NewKeeper(&fakeParamSubspace{}, nil, nil, nil, nil, stakingKeeper)
+
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner:  delegator,
+			Reward: arbitraryCoins(),
+		},
+		DelegatorRewardIndexes: types.RewardIndexes{{
+			CollateralType: types.BondDenom,
+			RewardFactor:   d("0.1"),
+		}},
+	}
+	suite.storeClaim(claim)
+
+	suite.storeGlobalDelegatorFactor(
+		types.RewardIndexes{
+			types.NewRewardIndex(types.BondDenom, d("0.2")),
+		},
+	)
+
+	suite.keeper.SynchronizeHardDelegatorRewards(suite.ctx, claim.Owner, nil, false)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+
+	suite.Equal(
+		cs(c(types.HardLiquidityRewardDenom, 100)).Add(claim.Reward...),
+		syncedClaim.Reward,
+	)
+}
+
+func unslashedBondedValidator(address sdk.ValAddress) stakingtypes.Validator {
+	return stakingtypes.Validator{
+		OperatorAddress: address,
+		Status:          sdk.Bonded,
+
+		// Set the tokens and shares equal so then
+		// a _delegator's_ token amount is equal to their shares amount
+		Tokens:          i(1e12),
+		DelegatorShares: i(1e12).ToDec(),
+	}
+}
+func unslashedNotBondedValidator(address sdk.ValAddress) stakingtypes.Validator {
+	return stakingtypes.Validator{
+		OperatorAddress: address,
+		Status:          sdk.Unbonding,
+
+		// Set the tokens and shares equal so then
+		// a _delegator's_ token amount is equal to their shares amount
+		Tokens:          i(1e12),
+		DelegatorShares: i(1e12).ToDec(),
+	}
+}
+
+func (suite *SynchronizeHardDelegatorRewardTests) TestGetDelegatedWhenValAddrIsNil() {
+	// when valAddr is nil, get total delegated to bonded validators
+	delegator := arbitraryAddress()
+	validatorAddresses := generateValidatorAddresses(4)
+	stakingKeeper := fakeStakingKeeper{
+		delegations: stakingtypes.Delegations{
+			//bonded
+			{
+				DelegatorAddress: delegator,
+				ValidatorAddress: validatorAddresses[0],
+				Shares:           d("1"),
+			},
+			{
+				DelegatorAddress: delegator,
+				ValidatorAddress: validatorAddresses[1],
+				Shares:           d("10"),
+			},
+			// not bonded
+			{
+				DelegatorAddress: delegator,
+				ValidatorAddress: validatorAddresses[2],
+				Shares:           d("100"),
+			},
+			{
+				DelegatorAddress: delegator,
+				ValidatorAddress: validatorAddresses[3],
+				Shares:           d("1000"),
+			},
+		},
+		validators: stakingtypes.Validators{
+			unslashedBondedValidator(validatorAddresses[0]),
+			unslashedBondedValidator(validatorAddresses[1]),
+			unslashedNotBondedValidator(validatorAddresses[2]),
+			unslashedNotBondedValidator(validatorAddresses[3]),
+		},
+	}
+	suite.keeper = suite.NewKeeper(&fakeParamSubspace{}, nil, nil, nil, nil, stakingKeeper)
+
+	suite.Equal(
+		d("11"), // delegation to bonded validators
+		suite.keeper.GetTotalDelegated(suite.ctx, delegator, nil, false),
+	)
+}
+func (suite *SynchronizeHardDelegatorRewardTests) TestGetDelegatedWhenExcludingAValidator() {
+	// when valAddr is x, get total delegated to bonded validators excluding those to x
+	delegator := arbitraryAddress()
+	validatorAddresses := generateValidatorAddresses(4)
+	stakingKeeper := fakeStakingKeeper{
+		delegations: stakingtypes.Delegations{
+			//bonded
+			{
+				DelegatorAddress: delegator,
+				ValidatorAddress: validatorAddresses[0],
+				Shares:           d("1"),
+			},
+			{
+				DelegatorAddress: delegator,
+				ValidatorAddress: validatorAddresses[1],
+				Shares:           d("10"),
+			},
+			// not bonded
+			{
+				DelegatorAddress: delegator,
+				ValidatorAddress: validatorAddresses[2],
+				Shares:           d("100"),
+			},
+			{
+				DelegatorAddress: delegator,
+				ValidatorAddress: validatorAddresses[3],
+				Shares:           d("1000"),
+			},
+		},
+		validators: stakingtypes.Validators{
+			unslashedBondedValidator(validatorAddresses[0]),
+			unslashedBondedValidator(validatorAddresses[1]),
+			unslashedNotBondedValidator(validatorAddresses[2]),
+			unslashedNotBondedValidator(validatorAddresses[3]),
+		},
+	}
+	suite.keeper = suite.NewKeeper(&fakeParamSubspace{}, nil, nil, nil, nil, stakingKeeper)
+
+	suite.Equal(
+		d("10"),
+		suite.keeper.GetTotalDelegated(suite.ctx, delegator, validatorAddresses[0], false),
+	)
+}
+func (suite *SynchronizeHardDelegatorRewardTests) TestGetDelegatedWhenIncludingAValidator() {
+	// when valAddr is x, get total delegated to bonded validators including those to x
+	delegator := arbitraryAddress()
+	validatorAddresses := generateValidatorAddresses(4)
+	stakingKeeper := fakeStakingKeeper{
+		delegations: stakingtypes.Delegations{
+			//bonded
+			{
+				DelegatorAddress: delegator,
+				ValidatorAddress: validatorAddresses[0],
+				Shares:           d("1"),
+			},
+			{
+				DelegatorAddress: delegator,
+				ValidatorAddress: validatorAddresses[1],
+				Shares:           d("10"),
+			},
+			// not bonded
+			{
+				DelegatorAddress: delegator,
+				ValidatorAddress: validatorAddresses[2],
+				Shares:           d("100"),
+			},
+			{
+				DelegatorAddress: delegator,
+				ValidatorAddress: validatorAddresses[3],
+				Shares:           d("1000"),
+			},
+		},
+		validators: stakingtypes.Validators{
+			unslashedBondedValidator(validatorAddresses[0]),
+			unslashedBondedValidator(validatorAddresses[1]),
+			unslashedNotBondedValidator(validatorAddresses[2]),
+			unslashedNotBondedValidator(validatorAddresses[3]),
+		},
+	}
+	suite.keeper = suite.NewKeeper(&fakeParamSubspace{}, nil, nil, nil, nil, stakingKeeper)
+
+	suite.Equal(
+		d("111"),
+		suite.keeper.GetTotalDelegated(suite.ctx, delegator, validatorAddresses[2], true),
+	)
+}

x/incentive/keeper/rewards_supply.go

@@ -80,24 +80,20 @@ func (k Keeper) AccumulateHardSupplyRewards(ctx sdk.Context, rewardPeriod types.
 // InitializeHardSupplyReward initializes the supply-side of a hard liquidity provider claim
 // by creating the claim and setting the supply reward factor index
 func (k Keeper) InitializeHardSupplyReward(ctx sdk.Context, deposit hardtypes.Deposit) {
-	var supplyRewardIndexes types.MultiRewardIndexes
-	for _, coin := range deposit.Amount {
-		globalRewardIndexes, foundGlobalRewardIndexes := k.GetHardSupplyRewardIndexes(ctx, coin.Denom)
-		var multiRewardIndex types.MultiRewardIndex
-		if foundGlobalRewardIndexes {
-			multiRewardIndex = types.NewMultiRewardIndex(coin.Denom, globalRewardIndexes)
-		} else {
-			multiRewardIndex = types.NewMultiRewardIndex(coin.Denom, types.RewardIndexes{})
-		}
-		supplyRewardIndexes = append(supplyRewardIndexes, multiRewardIndex)
-	}
-
 	claim, found := k.GetHardLiquidityProviderClaim(ctx, deposit.Depositor)
 	if !found {
-		// Instantiate claim object
 		claim = types.NewHardLiquidityProviderClaim(deposit.Depositor, sdk.Coins{}, nil, nil, nil)
 	}
 
+	var supplyRewardIndexes types.MultiRewardIndexes
+	for _, coin := range deposit.Amount {
+		globalRewardIndexes, found := k.GetHardSupplyRewardIndexes(ctx, coin.Denom)
+		if !found {
+			globalRewardIndexes = types.RewardIndexes{}
+		}
+		supplyRewardIndexes = supplyRewardIndexes.With(coin.Denom, globalRewardIndexes)
+	}
+
 	claim.SupplyRewardIndexes = supplyRewardIndexes
 	k.SetHardLiquidityProviderClaim(ctx, claim)
 }
@@ -111,50 +107,34 @@ func (k Keeper) SynchronizeHardSupplyReward(ctx sdk.Context, deposit hardtypes.D
 	}
 
 	for _, coin := range deposit.Amount {
-		globalRewardIndexes, foundGlobalRewardIndexes := k.GetHardSupplyRewardIndexes(ctx, coin.Denom)
+		globalRewardIndexes, found := k.GetHardSupplyRewardIndexes(ctx, coin.Denom)
-		if !foundGlobalRewardIndexes {
+		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
 		}
 
-		userMultiRewardIndex, foundUserMultiRewardIndex := claim.SupplyRewardIndexes.GetRewardIndex(coin.Denom)
+		userRewardIndexes, found := claim.SupplyRewardIndexes.Get(coin.Denom)
-		if !foundUserMultiRewardIndex {
+		if !found {
-			continue
+			// 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{}
 		}
 
-		userRewardIndexIndex, foundUserRewardIndexIndex := claim.SupplyRewardIndexes.GetRewardIndexIndex(coin.Denom)
+		newRewards, err := k.CalculateRewards(userRewardIndexes, globalRewardIndexes, coin.Amount.ToDec())
-		if !foundUserRewardIndexIndex {
+		if err != nil {
-			continue
+			// 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))
 		}
 
-		for _, globalRewardIndex := range globalRewardIndexes {
+		claim.Reward = claim.Reward.Add(newRewards...)
-			userRewardIndex, foundUserRewardIndex := userMultiRewardIndex.RewardIndexes.GetRewardIndex(globalRewardIndex.CollateralType)
+		claim.SupplyRewardIndexes = claim.SupplyRewardIndexes.With(coin.Denom, globalRewardIndexes)
-			if !foundUserRewardIndex {
-				// User deposited this coin type before it had rewards. When new rewards are added, legacy depositors
-				// should immediately begin earning rewards. Enable users to do so by updating their claim with the global
-				// reward index denom and start their reward factor at 0.0
-				userRewardIndex = types.NewRewardIndex(globalRewardIndex.CollateralType, sdk.ZeroDec())
-				userMultiRewardIndex.RewardIndexes = append(userMultiRewardIndex.RewardIndexes, userRewardIndex)
-				claim.SupplyRewardIndexes[userRewardIndexIndex] = userMultiRewardIndex
-			}
-
-			globalRewardFactor := globalRewardIndex.RewardFactor
-			userRewardFactor := userRewardIndex.RewardFactor
-			rewardsAccumulatedFactor := globalRewardFactor.Sub(userRewardFactor)
-			if rewardsAccumulatedFactor.IsNegative() {
-				panic(fmt.Sprintf("reward accumulation factor cannot be negative: %s", rewardsAccumulatedFactor))
-			}
-
-			newRewardsAmount := rewardsAccumulatedFactor.Mul(deposit.Amount.AmountOf(coin.Denom).ToDec()).RoundInt()
-
-			factorIndex, foundFactorIndex := userMultiRewardIndex.RewardIndexes.GetFactorIndex(globalRewardIndex.CollateralType)
-			if !foundFactorIndex { // should never trigger, as we basically do this check at the start of this loop
-				continue
-			}
-			claim.SupplyRewardIndexes[userRewardIndexIndex].RewardIndexes[factorIndex].RewardFactor = globalRewardIndex.RewardFactor
-
-			newRewardsCoin := sdk.NewCoin(userRewardIndex.CollateralType, newRewardsAmount)
-			claim.Reward = claim.Reward.Add(newRewardsCoin)
-		}
 	}
 	k.SetHardLiquidityProviderClaim(ctx, claim)
 }
@@ -169,26 +149,22 @@ func (k Keeper) UpdateHardSupplyIndexDenoms(ctx sdk.Context, deposit hardtypes.D
 	depositDenoms := getDenoms(deposit.Amount)
 	supplyRewardIndexDenoms := claim.SupplyRewardIndexes.GetCollateralTypes()
 
-	uniqueDepositDenoms := setDifference(depositDenoms, supplyRewardIndexDenoms)
-	uniqueSupplyRewardDenoms := setDifference(supplyRewardIndexDenoms, depositDenoms)
-
 	supplyRewardIndexes := claim.SupplyRewardIndexes
+
 	// Create a new multi-reward index in the claim for every new deposit denom
+	uniqueDepositDenoms := setDifference(depositDenoms, supplyRewardIndexDenoms)
+
 	for _, denom := range uniqueDepositDenoms {
-		_, foundUserRewardIndexes := claim.SupplyRewardIndexes.GetRewardIndex(denom)
+		globalSupplyRewardIndexes, found := k.GetHardSupplyRewardIndexes(ctx, denom)
-		if !foundUserRewardIndexes {
+		if !found {
-			globalSupplyRewardIndexes, foundGlobalSupplyRewardIndexes := k.GetHardSupplyRewardIndexes(ctx, denom)
+			globalSupplyRewardIndexes = types.RewardIndexes{}
-			var multiRewardIndex types.MultiRewardIndex
-			if foundGlobalSupplyRewardIndexes {
-				multiRewardIndex = types.NewMultiRewardIndex(denom, globalSupplyRewardIndexes)
-			} else {
-				multiRewardIndex = types.NewMultiRewardIndex(denom, types.RewardIndexes{})
-			}
-			supplyRewardIndexes = append(supplyRewardIndexes, multiRewardIndex)
 		}
+		supplyRewardIndexes = supplyRewardIndexes.With(denom, globalSupplyRewardIndexes)
 	}
 
 	// Delete multi-reward index from claim if the collateral type is no longer deposited
+	uniqueSupplyRewardDenoms := setDifference(supplyRewardIndexDenoms, depositDenoms)
+
 	for _, denom := range uniqueSupplyRewardDenoms {
 		supplyRewardIndexes = supplyRewardIndexes.RemoveRewardIndex(denom)
 	}

x/incentive/keeper/rewards_supply_init_test.go

@@ -0,0 +1,89 @@
+package keeper_test
+
+import (
+	"testing"
+
+	"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
+}
+
+func TestInitializeHardSupplyReward(t *testing.T) {
+	suite.Run(t, new(InitializeHardSupplyRewardTests))
+}
+
+func (suite *InitializeHardSupplyRewardTests) TestClaimIndexesAreSetWhenClaimExists() {
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner: arbitraryAddress(),
+		},
+		// Indexes should always be empty when initialize is called.
+		// If initialize is called then the user must have repaid their deposit positions,
+		// which means UpdateHardSupplyIndexDenoms was called and should have remove indexes.
+		SupplyRewardIndexes: types.MultiRewardIndexes{},
+	}
+	suite.storeClaim(claim)
+
+	globalIndexes := nonEmptyMultiRewardIndexes
+	suite.storeGlobalSupplyIndexes(globalIndexes)
+
+	deposit := hardtypes.Deposit{
+		Depositor: claim.Owner,
+		Amount:    arbitraryCoinsWithDenoms(extractCollateralTypes(globalIndexes)...),
+	}
+
+	suite.keeper.InitializeHardSupplyReward(suite.ctx, deposit)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(globalIndexes, syncedClaim.SupplyRewardIndexes)
+}
+func (suite *InitializeHardSupplyRewardTests) TestClaimIndexesAreSetWhenClaimDoesNotExist() {
+	globalIndexes := nonEmptyMultiRewardIndexes
+	suite.storeGlobalSupplyIndexes(globalIndexes)
+
+	owner := arbitraryAddress()
+	deposit := hardtypes.Deposit{
+		Depositor: owner,
+		Amount:    arbitraryCoinsWithDenoms(extractCollateralTypes(globalIndexes)...),
+	}
+
+	suite.keeper.InitializeHardSupplyReward(suite.ctx, deposit)
+
+	syncedClaim, found := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, owner)
+	suite.True(found)
+	suite.Equal(globalIndexes, syncedClaim.SupplyRewardIndexes)
+}
+func (suite *InitializeHardSupplyRewardTests) TestClaimIndexesAreSetEmptyForMissingIndexes() {
+
+	globalIndexes := nonEmptyMultiRewardIndexes
+	suite.storeGlobalSupplyIndexes(globalIndexes)
+
+	owner := arbitraryAddress()
+	// Supply a denom that is not in the global indexes.
+	// 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...),
+	}
+
+	suite.keeper.InitializeHardSupplyReward(suite.ctx, deposit)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, owner)
+	suite.Equal(expectedIndexes, syncedClaim.SupplyRewardIndexes)
+}

x/incentive/keeper/rewards_supply_sync_test.go

@@ -0,0 +1,303 @@
+package keeper_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/suite"
+
+	hardtypes "github.com/kava-labs/kava/x/hard/types"
+	"github.com/kava-labs/kava/x/incentive/types"
+)
+
+// 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
+}
+
+func TestSynchronizeHardSupplyReward(t *testing.T) {
+	suite.Run(t, new(SynchronizeHardSupplyRewardTests))
+}
+
+func (suite *SynchronizeHardSupplyRewardTests) TestClaimIndexesAreUpdatedWhenGlobalIndexesHaveIncreased() {
+	// This is the normal case
+
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner: arbitraryAddress(),
+		},
+		SupplyRewardIndexes: nonEmptyMultiRewardIndexes,
+	}
+	suite.storeClaim(claim)
+
+	globalIndexes := increaseAllRewardFactors(nonEmptyMultiRewardIndexes)
+	suite.storeGlobalSupplyIndexes(globalIndexes)
+	deposit := hardtypes.Deposit{
+		Depositor: claim.Owner,
+		Amount:    arbitraryCoinsWithDenoms(extractCollateralTypes(claim.SupplyRewardIndexes)...),
+	}
+
+	suite.keeper.SynchronizeHardSupplyReward(suite.ctx, deposit)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(globalIndexes, syncedClaim.SupplyRewardIndexes)
+}
+func (suite *SynchronizeHardSupplyRewardTests) TestClaimIndexesAreUnchangedWhenGlobalIndexesUnchanged() {
+	// It should be safe to call SynchronizeHardSupplyReward multiple times
+
+	unchangingIndexes := nonEmptyMultiRewardIndexes
+
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner: arbitraryAddress(),
+		},
+		SupplyRewardIndexes: unchangingIndexes,
+	}
+	suite.storeClaim(claim)
+
+	suite.storeGlobalSupplyIndexes(unchangingIndexes)
+
+	deposit := hardtypes.Deposit{
+		Depositor: claim.Owner,
+		Amount:    arbitraryCoinsWithDenoms(extractCollateralTypes(unchangingIndexes)...),
+	}
+
+	suite.keeper.SynchronizeHardSupplyReward(suite.ctx, deposit)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(unchangingIndexes, syncedClaim.SupplyRewardIndexes)
+}
+func (suite *SynchronizeHardSupplyRewardTests) TestClaimIndexesAreUpdatedWhenNewRewardAdded() {
+	// When a new reward is added (via gov) for a hard deposit denom the user has already deposited, and the claim is synced;
+	// Then the new reward's index should be added to the claim.
+
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner: arbitraryAddress(),
+		},
+		SupplyRewardIndexes: nonEmptyMultiRewardIndexes,
+	}
+	suite.storeClaim(claim)
+
+	globalIndexes := appendUniqueMultiRewardIndex(nonEmptyMultiRewardIndexes)
+	suite.storeGlobalSupplyIndexes(globalIndexes)
+
+	deposit := hardtypes.Deposit{
+		Depositor: claim.Owner,
+		Amount:    arbitraryCoinsWithDenoms(extractCollateralTypes(globalIndexes)...),
+	}
+
+	suite.keeper.SynchronizeHardSupplyReward(suite.ctx, deposit)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(globalIndexes, syncedClaim.SupplyRewardIndexes)
+}
+
+func (suite *SynchronizeHardSupplyRewardTests) TestClaimIndexesAreUpdatedWhenNewRewardDenomAdded() {
+	// When a new reward coin is added (via gov) to an already rewarded deposit denom (that the user has already deposited), and the claim is synced;
+	// Then the new reward coin's index should be added to the claim.
+
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner: arbitraryAddress(),
+		},
+		SupplyRewardIndexes: nonEmptyMultiRewardIndexes,
+	}
+	suite.storeClaim(claim)
+
+	globalIndexes := appendUniqueRewardIndexToFirstItem(nonEmptyMultiRewardIndexes)
+	suite.storeGlobalSupplyIndexes(globalIndexes)
+
+	deposit := hardtypes.Deposit{
+		Depositor: claim.Owner,
+		Amount:    arbitraryCoinsWithDenoms(extractCollateralTypes(globalIndexes)...),
+	}
+
+	suite.keeper.SynchronizeHardSupplyReward(suite.ctx, deposit)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(globalIndexes, syncedClaim.SupplyRewardIndexes)
+}
+
+func (suite *SynchronizeHardSupplyRewardTests) TestRewardIsIncrementedWhenGlobalIndexesHaveIncreased() {
+	// This is the normal case
+	// Given some time has passed (meaning the global indexes have increased)
+	// When the claim is synced
+	// The user earns rewards for the time passed
+
+	originalReward := arbitraryCoins()
+
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner:  arbitraryAddress(),
+			Reward: originalReward,
+		},
+		SupplyRewardIndexes: types.MultiRewardIndexes{
+			{
+				CollateralType: "depositdenom",
+				RewardIndexes: types.RewardIndexes{
+					{
+						CollateralType: "rewarddenom",
+						RewardFactor:   d("1000.001"),
+					},
+				},
+			},
+		},
+	}
+	suite.storeClaim(claim)
+
+	suite.storeGlobalSupplyIndexes(types.MultiRewardIndexes{
+		{
+			CollateralType: "depositdenom",
+			RewardIndexes: types.RewardIndexes{
+				{
+					CollateralType: "rewarddenom",
+					RewardFactor:   d("2000.002"),
+				},
+			},
+		},
+	})
+
+	deposit := hardtypes.Deposit{
+		Depositor: claim.Owner,
+		Amount:    cs(c("depositdenom", 1e9)),
+	}
+
+	suite.keeper.SynchronizeHardSupplyReward(suite.ctx, deposit)
+
+	// new reward is (new index - old index) * deposit amount
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(
+		cs(c("rewarddenom", 1_000_001_000_000)).Add(originalReward...),
+		syncedClaim.Reward,
+	)
+}
+
+func (suite *SynchronizeHardSupplyRewardTests) TestRewardIsIncrementedWhenNewRewardAdded() {
+	// When a new reward is added (via gov) for a hard deposit denom the user has already deposited, and the claim is synced
+	// Then the user earns rewards for the time since the reward was added
+
+	originalReward := arbitraryCoins()
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner:  arbitraryAddress(),
+			Reward: originalReward,
+		},
+		SupplyRewardIndexes: types.MultiRewardIndexes{
+			{
+				CollateralType: "rewarded",
+				RewardIndexes: types.RewardIndexes{
+					{
+						CollateralType: "reward",
+						RewardFactor:   d("1000.001"),
+					},
+				},
+			},
+		},
+	}
+	suite.storeClaim(claim)
+
+	globalIndexes := types.MultiRewardIndexes{
+		{
+			CollateralType: "rewarded",
+			RewardIndexes: types.RewardIndexes{
+				{
+					CollateralType: "reward",
+					RewardFactor:   d("2000.002"),
+				},
+			},
+		},
+		{
+			CollateralType: "newlyrewarded",
+			RewardIndexes: types.RewardIndexes{
+				{
+					CollateralType: "otherreward",
+					// Indexes start at 0 when the reward is added by gov,
+					// so this represents the syncing happening some time later.
+					RewardFactor: d("1000.001"),
+				},
+			},
+		},
+	}
+	suite.storeGlobalSupplyIndexes(globalIndexes)
+
+	deposit := hardtypes.Deposit{
+		Depositor: claim.Owner,
+		Amount:    cs(c("rewarded", 1e9), c("newlyrewarded", 1e9)),
+	}
+
+	suite.keeper.SynchronizeHardSupplyReward(suite.ctx, deposit)
+
+	// new reward is (new index - old index) * deposit amount for each deposited denom
+	// The old index for `newlyrewarded` isn't in the claim, so it's added starting at 0 for calculating the reward.
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(
+		cs(c("otherreward", 1_000_001_000_000), c("reward", 1_000_001_000_000)).Add(originalReward...),
+		syncedClaim.Reward,
+	)
+}
+func (suite *SynchronizeHardSupplyRewardTests) TestRewardIsIncrementedWhenNewRewardDenomAdded() {
+	// When a new reward coin is added (via gov) to an already rewarded deposit denom (that the user has already deposited), and the claim is synced;
+	// Then the user earns rewards for the time since the reward was added
+
+	originalReward := arbitraryCoins()
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner:  arbitraryAddress(),
+			Reward: originalReward,
+		},
+		SupplyRewardIndexes: types.MultiRewardIndexes{
+			{
+				CollateralType: "deposited",
+				RewardIndexes: types.RewardIndexes{
+					{
+						CollateralType: "reward",
+						RewardFactor:   d("1000.001"),
+					},
+				},
+			},
+		},
+	}
+	suite.storeClaim(claim)
+
+	globalIndexes := types.MultiRewardIndexes{
+		{
+			CollateralType: "deposited",
+			RewardIndexes: types.RewardIndexes{
+				{
+					CollateralType: "reward",
+					RewardFactor:   d("2000.002"),
+				},
+				{
+					CollateralType: "otherreward",
+					// Indexes start at 0 when the reward is added by gov,
+					// so this represents the syncing happening some time later.
+					RewardFactor: d("1000.001"),
+				},
+			},
+		},
+	}
+	suite.storeGlobalSupplyIndexes(globalIndexes)
+
+	deposit := hardtypes.Deposit{
+		Depositor: claim.Owner,
+		Amount:    cs(c("deposited", 1e9)),
+	}
+
+	suite.keeper.SynchronizeHardSupplyReward(suite.ctx, deposit)
+
+	// new reward is (new index - old index) * deposit amount for each deposited denom
+	// The old index for `otherreward` isn't in the claim, so it's added starting at 0 for calculating the reward.
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(
+		cs(c("reward", 1_000_001_000_000), c("otherreward", 1_000_001_000_000)).Add(originalReward...),
+		syncedClaim.Reward,
+	)
+}

x/incentive/keeper/rewards_supply_test.go

@@ -61,7 +61,7 @@ func (suite *SupplyRewardsTestSuite) SetupWithGenState(authBuilder app.AuthGenes
 		authBuilder.BuildMarshalled(),
 		NewPricefeedGenStateMultiFromTime(suite.genesisTime),
 		hardBuilder.BuildMarshalled(),
-		NewCommitteeGenesisState(suite.addrs[:2]), // TODO add committee members to suite
+		NewCommitteeGenesisState(suite.addrs[:2]),
 		incentBuilder.BuildMarshalled(),
 	)
 }
@@ -498,7 +498,6 @@ func (suite *SupplyRewardsTestSuite) TestSynchronizeHardSupplyReward() {
 				updatedTimeDuration: 86400,
 			},
 		},
-		// TODO test synchronize when there is a reward period with 0 rewardsPerSecond
 	}
 	for _, tc := range testCases {
 		suite.Run(tc.name, func() {

x/incentive/keeper/rewards_supply_update_test.go

@@ -0,0 +1,119 @@
+package keeper_test
+
+import (
+	"testing"
+
+	"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
+}
+
+func TestUpdateHardSupplyIndexDenoms(t *testing.T) {
+	suite.Run(t, new(UpdateHardSupplyIndexDenomsTests))
+}
+
+func (suite *UpdateHardSupplyIndexDenomsTests) TestClaimIndexesAreRemovedForDenomsNoLongerSupplied() {
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner: arbitraryAddress(),
+		},
+		SupplyRewardIndexes: nonEmptyMultiRewardIndexes,
+	}
+	suite.storeClaim(claim)
+	suite.storeGlobalSupplyIndexes(claim.SupplyRewardIndexes)
+
+	// remove one denom from the indexes already in the deposit
+	expectedIndexes := claim.SupplyRewardIndexes[1:]
+	deposit := hardtypes.Deposit{
+		Depositor: claim.Owner,
+		Amount:    arbitraryCoinsWithDenoms(extractCollateralTypes(expectedIndexes)...),
+	}
+
+	suite.keeper.UpdateHardSupplyIndexDenoms(suite.ctx, deposit)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(expectedIndexes, syncedClaim.SupplyRewardIndexes)
+}
+
+func (suite *UpdateHardSupplyIndexDenomsTests) TestClaimIndexesAreAddedForNewlySuppliedDenoms() {
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner: arbitraryAddress(),
+		},
+		SupplyRewardIndexes: nonEmptyMultiRewardIndexes,
+	}
+	suite.storeClaim(claim)
+	globalIndexes := appendUniqueMultiRewardIndex(claim.SupplyRewardIndexes)
+	suite.storeGlobalSupplyIndexes(globalIndexes)
+
+	deposit := hardtypes.Deposit{
+		Depositor: claim.Owner,
+		Amount:    arbitraryCoinsWithDenoms(extractCollateralTypes(globalIndexes)...),
+	}
+
+	suite.keeper.UpdateHardSupplyIndexDenoms(suite.ctx, deposit)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(globalIndexes, syncedClaim.SupplyRewardIndexes)
+}
+
+func (suite *UpdateHardSupplyIndexDenomsTests) TestClaimIndexesAreUnchangedWhenSuppliedDenomsUnchanged() {
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner: arbitraryAddress(),
+		},
+		SupplyRewardIndexes: nonEmptyMultiRewardIndexes,
+	}
+	suite.storeClaim(claim)
+	// Set global indexes with same denoms but different values.
+	// UpdateHardSupplyIndexDenoms should ignore the new values.
+	suite.storeGlobalSupplyIndexes(increaseAllRewardFactors(claim.SupplyRewardIndexes))
+
+	deposit := hardtypes.Deposit{
+		Depositor: claim.Owner,
+		Amount:    arbitraryCoinsWithDenoms(extractCollateralTypes(claim.SupplyRewardIndexes)...),
+	}
+
+	suite.keeper.UpdateHardSupplyIndexDenoms(suite.ctx, deposit)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(claim.SupplyRewardIndexes, syncedClaim.SupplyRewardIndexes)
+}
+
+func (suite *UpdateHardSupplyIndexDenomsTests) TestEmptyClaimIndexesAreAddedForNewlySuppliedButNotRewardedDenoms() {
+	claim := types.HardLiquidityProviderClaim{
+		BaseMultiClaim: types.BaseMultiClaim{
+			Owner: arbitraryAddress(),
+		},
+		SupplyRewardIndexes: nonEmptyMultiRewardIndexes,
+	}
+	suite.storeClaim(claim)
+	suite.storeGlobalSupplyIndexes(claim.SupplyRewardIndexes)
+
+	// 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...),
+	}
+
+	suite.keeper.UpdateHardSupplyIndexDenoms(suite.ctx, deposit)
+
+	syncedClaim, _ := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, claim.Owner)
+	suite.Equal(expectedIndexes, syncedClaim.SupplyRewardIndexes)
+}

x/incentive/keeper/rewards_usdx.go

@@ -1,6 +1,8 @@
 package keeper
 
 import (
+	"fmt"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	cdptypes "github.com/kava-labs/kava/x/cdp/types"
@@ -55,66 +57,61 @@ func (k Keeper) InitializeUSDXMintingClaim(ctx sdk.Context, cdp cdptypes.CDP) {
 		// this collateral type is not incentivized, do nothing
 		return
 	}
-	rewardFactor, found := k.GetUSDXMintingRewardFactor(ctx, cdp.Type)
-	if !found {
-		rewardFactor = sdk.ZeroDec()
-	}
 	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{types.NewRewardIndex(cdp.Type, rewardFactor)})
+		claim = types.NewUSDXMintingClaim(cdp.Owner, sdk.NewCoin(types.USDXMintingRewardDenom, sdk.ZeroInt()), types.RewardIndexes{})
-		k.SetUSDXMintingClaim(ctx, claim)
-		return
 	}
-	// the owner has an existing usdx minting reward claim
-	index, hasRewardIndex := claim.HasRewardIndex(cdp.Type)
-	if !hasRewardIndex { // this is the owner's first usdx minting reward for this collateral type
-		claim.RewardIndexes = append(claim.RewardIndexes, types.NewRewardIndex(cdp.Type, rewardFactor))
-	} else { // the owner has a previous usdx minting reward for this collateral type
-		claim.RewardIndexes[index] = types.NewRewardIndex(cdp.Type, rewardFactor)
-	}
-	k.SetUSDXMintingClaim(ctx, claim)
-}
-
-// SynchronizeUSDXMintingReward updates the claim object by adding any accumulated rewards and updating the reward index value.
-// this should be called before a cdp is modified, immediately after the 'SynchronizeInterest' method is called in the cdp module
-func (k Keeper) SynchronizeUSDXMintingReward(ctx sdk.Context, cdp cdptypes.CDP) {
-	_, found := k.GetUSDXMintingRewardPeriod(ctx, cdp.Type)
-	if !found {
-		// this collateral type is not incentivized, do nothing
-		return
-	}
-
 	globalRewardFactor, found := k.GetUSDXMintingRewardFactor(ctx, cdp.Type)
 	if !found {
 		globalRewardFactor = sdk.ZeroDec()
 	}
+	claim.RewardIndexes = claim.RewardIndexes.With(cdp.Type, globalRewardFactor)
+
+	k.SetUSDXMintingClaim(ctx, claim)
+}
+
+// SynchronizeUSDXMintingReward updates the claim object by adding any accumulated rewards and updating the reward index value.
+// 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)
+	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)
+		// - 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, found := k.GetUSDXMintingClaim(ctx, cdp.Owner)
 	if !found {
-		claim = types.NewUSDXMintingClaim(cdp.Owner, sdk.NewCoin(types.USDXMintingRewardDenom, sdk.ZeroInt()), types.RewardIndexes{types.NewRewardIndex(cdp.Type, globalRewardFactor)})
+		claim = types.NewUSDXMintingClaim(
-		k.SetUSDXMintingClaim(ctx, claim)
+			cdp.Owner,
-		return
+			sdk.NewCoin(types.USDXMintingRewardDenom, sdk.ZeroInt()),
+			types.RewardIndexes{},
+		)
 	}
 
-	// the owner has an existing usdx minting reward claim
+	userRewardFactor, found := claim.RewardIndexes.Get(cdp.Type)
-	index, hasRewardIndex := claim.HasRewardIndex(cdp.Type)
+	if !found {
-	if !hasRewardIndex { // this is the owner's first usdx minting reward for this collateral type
+		// Normally the factor should always be found, as it is added when the cdp is created in InitializeUSDXMintingClaim.
-		claim.RewardIndexes = append(claim.RewardIndexes, types.NewRewardIndex(cdp.Type, globalRewardFactor))
+		// 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.
-		k.SetUSDXMintingClaim(ctx, claim)
+		// So assume the factor is the starting value for any global factor: 0.
-		return
+		userRewardFactor = sdk.ZeroDec()
 	}
-	userRewardFactor := claim.RewardIndexes[index].RewardFactor
+
-	rewardsAccumulatedFactor := globalRewardFactor.Sub(userRewardFactor)
+	newRewardsAmount, err := k.CalculateSingleReward(userRewardFactor, globalRewardFactor, cdp.GetTotalPrincipal().Amount.ToDec())
-	if rewardsAccumulatedFactor.IsZero() {
+	if err != nil {
-		return
+		// 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.
-	claim.RewardIndexes[index].RewardFactor = globalRewardFactor
+		panic(fmt.Sprintf("corrupted global reward indexes found: %v", err))
-	newRewardsAmount := rewardsAccumulatedFactor.Mul(cdp.GetTotalPrincipal().Amount.ToDec()).RoundInt()
-	if newRewardsAmount.IsZero() {
-		k.SetUSDXMintingClaim(ctx, claim)
-		return
 	}
 	newRewardsCoin := sdk.NewCoin(types.USDXMintingRewardDenom, newRewardsAmount)
+
 	claim.Reward = claim.Reward.Add(newRewardsCoin)
+	claim.RewardIndexes = claim.RewardIndexes.With(cdp.Type, globalRewardFactor)
+
 	k.SetUSDXMintingClaim(ctx, claim)
 }
 

x/incentive/keeper/rewards_usdx_unit_test.go

@@ -0,0 +1,311 @@
+package keeper_test
+
+import (
+	"testing"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/stretchr/testify/suite"
+
+	cdptypes "github.com/kava-labs/kava/x/cdp/types"
+	"github.com/kava-labs/kava/x/incentive/types"
+)
+
+// usdxRewardsUnitTester contains common methods for running unit tests for keeper methods related to the USDX minting rewards
+type usdxRewardsUnitTester struct {
+	unitTester
+}
+
+func (suite *usdxRewardsUnitTester) storeGlobalUSDXIndexes(indexes types.RewardIndexes) {
+	for _, ri := range indexes {
+		suite.keeper.SetUSDXMintingRewardFactor(suite.ctx, ri.CollateralType, ri.RewardFactor)
+	}
+}
+func (suite *usdxRewardsUnitTester) storeClaim(claim types.USDXMintingClaim) {
+	suite.keeper.SetUSDXMintingClaim(suite.ctx, claim)
+}
+
+type InitializeUSDXMintingClaimTests struct {
+	usdxRewardsUnitTester
+}
+
+func TestInitializeUSDXMintingClaims(t *testing.T) {
+	suite.Run(t, new(InitializeUSDXMintingClaimTests))
+}
+
+func (suite *InitializeUSDXMintingClaimTests) TestClaimIndexIsSetWhenClaimDoesNotExist() {
+	collateralType := "bnb-a"
+
+	subspace := paramsWithSingleUSDXRewardPeriod(collateralType)
+	suite.keeper = suite.NewKeeper(subspace, nil, nil, nil, nil, nil)
+
+	cdp := NewCDPBuilder(arbitraryAddress(), collateralType).Build()
+
+	globalIndexes := types.RewardIndexes{{
+		CollateralType: collateralType,
+		RewardFactor:   d("0.2"),
+	}}
+	suite.storeGlobalUSDXIndexes(globalIndexes)
+
+	suite.keeper.InitializeUSDXMintingClaim(suite.ctx, cdp)
+
+	syncedClaim, f := suite.keeper.GetUSDXMintingClaim(suite.ctx, cdp.Owner)
+	suite.True(f)
+	suite.Equal(globalIndexes, syncedClaim.RewardIndexes)
+}
+
+func (suite *InitializeUSDXMintingClaimTests) TestClaimIndexIsSetWhenClaimExists() {
+	collateralType := "bnb-a"
+
+	subspace := paramsWithSingleUSDXRewardPeriod(collateralType)
+	suite.keeper = suite.NewKeeper(subspace, nil, nil, nil, nil, nil)
+
+	claim := types.USDXMintingClaim{
+		BaseClaim: types.BaseClaim{
+			Owner: arbitraryAddress(),
+		},
+		RewardIndexes: types.RewardIndexes{{
+			CollateralType: collateralType,
+			RewardFactor:   d("0.1"),
+		}},
+	}
+	suite.storeClaim(claim)
+
+	globalIndexes := types.RewardIndexes{{
+		CollateralType: collateralType,
+		RewardFactor:   d("0.2"),
+	}}
+	suite.storeGlobalUSDXIndexes(globalIndexes)
+
+	cdp := NewCDPBuilder(claim.Owner, collateralType).Build()
+
+	suite.keeper.InitializeUSDXMintingClaim(suite.ctx, cdp)
+
+	syncedClaim, _ := suite.keeper.GetUSDXMintingClaim(suite.ctx, cdp.Owner)
+	suite.Equal(globalIndexes, syncedClaim.RewardIndexes)
+}
+
+type SynchronizeUSDXMintingRewardTests struct {
+	usdxRewardsUnitTester
+}
+
+func TestSynchronizeUSDXMintingReward(t *testing.T) {
+	suite.Run(t, new(SynchronizeUSDXMintingRewardTests))
+}
+func (suite *SynchronizeUSDXMintingRewardTests) TestRewardUnchangedWhenGlobalIndexesUnchanged() {
+	unchangingRewardIndexes := nonEmptyRewardIndexes
+	collateralType := extractFirstCollateralType(unchangingRewardIndexes)
+
+	claim := types.USDXMintingClaim{
+		BaseClaim: types.BaseClaim{
+			Owner:  arbitraryAddress(),
+			Reward: c(types.USDXMintingRewardDenom, 0),
+		},
+		RewardIndexes: unchangingRewardIndexes,
+	}
+	suite.storeClaim(claim)
+
+	suite.storeGlobalUSDXIndexes(unchangingRewardIndexes)
+
+	cdp := NewCDPBuilder(claim.Owner, collateralType).WithPrincipal(i(1e12)).Build()
+
+	suite.keeper.SynchronizeUSDXMintingReward(suite.ctx, cdp)
+
+	syncedClaim, _ := suite.keeper.GetUSDXMintingClaim(suite.ctx, claim.Owner)
+	suite.Equal(claim.Reward, syncedClaim.Reward)
+}
+
+func (suite *SynchronizeUSDXMintingRewardTests) TestRewardIsIncrementedWhenGlobalIndexIncreased() {
+	collateralType := "bnb-a"
+
+	claim := types.USDXMintingClaim{
+		BaseClaim: types.BaseClaim{
+			Owner:  arbitraryAddress(),
+			Reward: c(types.USDXMintingRewardDenom, 0),
+		},
+		RewardIndexes: types.RewardIndexes{
+			{
+				CollateralType: collateralType,
+				RewardFactor:   d("0.1"),
+			},
+		},
+	}
+	suite.storeClaim(claim)
+
+	globalIndexes := types.RewardIndexes{
+		{
+			CollateralType: collateralType,
+			RewardFactor:   d("0.2"),
+		},
+	}
+	suite.storeGlobalUSDXIndexes(globalIndexes)
+
+	cdp := NewCDPBuilder(claim.Owner, collateralType).WithPrincipal(i(1e12)).Build()
+
+	suite.keeper.SynchronizeUSDXMintingReward(suite.ctx, cdp)
+
+	syncedClaim, _ := suite.keeper.GetUSDXMintingClaim(suite.ctx, claim.Owner)
+	// reward is ( new index - old index ) * cdp.TotalPrincipal
+	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)
+
+	claim := types.USDXMintingClaim{
+		BaseClaim: types.BaseClaim{
+			Owner:  arbitraryAddress(),
+			Reward: c(types.USDXMintingRewardDenom, 0),
+		},
+		RewardIndexes: claimsRewardIndexes,
+	}
+	suite.storeClaim(claim)
+
+	globalIndexes := increaseRewardFactors(claimsRewardIndexes)
+	suite.storeGlobalUSDXIndexes(globalIndexes)
+
+	cdp := NewCDPBuilder(claim.Owner, collateralType).Build()
+
+	suite.keeper.SynchronizeUSDXMintingReward(suite.ctx, cdp)
+
+	syncedClaim, _ := suite.keeper.GetUSDXMintingClaim(suite.ctx, claim.Owner)
+
+	// Only the claim's index for `collateralType` should have been changed
+	i, _ := globalIndexes.Get(collateralType)
+	expectedIndexes := claimsRewardIndexes.With(collateralType, i)
+	suite.Equal(expectedIndexes, syncedClaim.RewardIndexes)
+}
+
+func (suite *SynchronizeUSDXMintingRewardTests) TestClaimIndexIsUpdatedWhenNewRewardAddedAndClaimAlreadyExists() {
+	claimsRewardIndexes := types.RewardIndexes{
+		{
+			CollateralType: "bnb-a",
+			RewardFactor:   d("0.1"),
+		},
+		{
+			CollateralType: "busd-b",
+			RewardFactor:   d("0.4"),
+		},
+	}
+	newRewardIndex := types.NewRewardIndex("xrp-a", d("0.0001"))
+
+	claim := types.USDXMintingClaim{
+		BaseClaim: types.BaseClaim{
+			Owner:  arbitraryAddress(),
+			Reward: c(types.USDXMintingRewardDenom, 0),
+		},
+		RewardIndexes: claimsRewardIndexes,
+	}
+	suite.storeClaim(claim)
+
+	globalIndexes := increaseRewardFactors(claimsRewardIndexes)
+	globalIndexes = append(globalIndexes, newRewardIndex)
+	suite.storeGlobalUSDXIndexes(globalIndexes)
+
+	cdp := NewCDPBuilder(claim.Owner, newRewardIndex.CollateralType).Build()
+
+	suite.keeper.SynchronizeUSDXMintingReward(suite.ctx, cdp)
+
+	syncedClaim, _ := suite.keeper.GetUSDXMintingClaim(suite.ctx, claim.Owner)
+
+	// Only the claim's index for `collateralType` should have been changed
+	expectedIndexes := claimsRewardIndexes.With(newRewardIndex.CollateralType, newRewardIndex.RewardFactor)
+	suite.Equal(expectedIndexes, syncedClaim.RewardIndexes)
+}
+
+func (suite *SynchronizeUSDXMintingRewardTests) TestClaimIsUnchangedWhenGlobalFactorMissing() {
+	claimsRewardIndexes := nonEmptyRewardIndexes
+	claim := types.USDXMintingClaim{
+		BaseClaim: types.BaseClaim{
+			Owner:  arbitraryAddress(),
+			Reward: c(types.USDXMintingRewardDenom, 0),
+		},
+		RewardIndexes: claimsRewardIndexes,
+	}
+	suite.storeClaim(claim)
+	// 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()
+
+	suite.keeper.SynchronizeUSDXMintingReward(suite.ctx, cdp)
+
+	syncedClaim, _ := suite.keeper.GetUSDXMintingClaim(suite.ctx, claim.Owner)
+	suite.Equal(claim.RewardIndexes, syncedClaim.RewardIndexes)
+	suite.Equal(claim.Reward, syncedClaim.Reward)
+}
+
+type cdpBuilder struct {
+	cdptypes.CDP
+}
+
+func NewCDPBuilder(owner sdk.AccAddress, collateralType string) cdpBuilder {
+	return cdpBuilder{
+		CDP: cdptypes.CDP{
+			Owner: owner,
+			Type:  collateralType,
+			// The zero value of Principal and AccumulatedFees (type sdk.Coin) is invalid as the denom is ""
+			// Set them to the default denom, but with 0 amount.
+			Principal:       c(cdptypes.DefaultStableDenom, 0),
+			AccumulatedFees: c(cdptypes.DefaultStableDenom, 0),
+		}}
+}
+
+func (builder cdpBuilder) Build() cdptypes.CDP { return builder.CDP }
+
+func (builder cdpBuilder) WithPrincipal(principal sdk.Int) cdpBuilder {
+	builder.Principal = sdk.NewCoin(cdptypes.DefaultStableDenom, principal)
+	return builder
+}
+
+var nonEmptyRewardIndexes = types.RewardIndexes{
+	{
+		CollateralType: "bnb-a",
+		RewardFactor:   d("0.1"),
+	},
+	{
+		CollateralType: "busd-b",
+		RewardFactor:   d("0.4"),
+	},
+}
+
+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")
+	}
+	return indexes[0].CollateralType
+}

x/incentive/keeper/unit_test.go

@@ -0,0 +1,248 @@
+package keeper_test
+
+import (
+	"fmt"
+
+	"github.com/cosmos/cosmos-sdk/codec"
+	"github.com/cosmos/cosmos-sdk/store"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/x/params"
+	"github.com/stretchr/testify/suite"
+	abci "github.com/tendermint/tendermint/abci/types"
+	"github.com/tendermint/tendermint/libs/log"
+	db "github.com/tendermint/tm-db"
+
+	"github.com/kava-labs/kava/app"
+	"github.com/kava-labs/kava/x/incentive/keeper"
+	"github.com/kava-labs/kava/x/incentive/types"
+)
+
+// NewTestContext sets up a basic context with an in-memory db
+func NewTestContext(requiredStoreKeys ...sdk.StoreKey) sdk.Context {
+	memDB := db.NewMemDB()
+	cms := store.NewCommitMultiStore(memDB)
+
+	for _, key := range requiredStoreKeys {
+		cms.MountStoreWithDB(key, sdk.StoreTypeIAVL, nil)
+	}
+
+	cms.LoadLatestVersion()
+
+	return sdk.NewContext(cms, abci.Header{}, false, log.NewNopLogger())
+}
+
+// unitTester is a wrapper around suite.Suite, with common functionality for keeper unit tests.
+// It can be embedded in structs the same way as suite.Suite.
+type unitTester struct {
+	suite.Suite
+	keeper keeper.Keeper
+	ctx    sdk.Context
+
+	cdc               *codec.Codec
+	incentiveStoreKey sdk.StoreKey
+}
+
+func (suite *unitTester) SetupSuite() {
+	suite.cdc = app.MakeCodec()
+
+	suite.incentiveStoreKey = sdk.NewKVStoreKey(types.StoreKey)
+
+}
+
+func (suite *unitTester) SetupTest() {
+	suite.ctx = NewTestContext(suite.incentiveStoreKey)
+	suite.keeper = suite.NewKeeper(&fakeParamSubspace{}, nil, nil, nil, nil, nil)
+}
+
+func (suite *unitTester) TearDownTest() {
+	suite.keeper = keeper.Keeper{}
+	suite.ctx = sdk.Context{}
+}
+
+func (suite *unitTester) NewKeeper(paramSubspace types.ParamSubspace, sk types.SupplyKeeper, cdpk types.CdpKeeper, hk types.HardKeeper, ak types.AccountKeeper, stk types.StakingKeeper) keeper.Keeper {
+	return keeper.NewKeeper(suite.cdc, suite.incentiveStoreKey, paramSubspace, sk, cdpk, hk, ak, stk)
+}
+
+func (suite *unitTester) storeGlobalBorrowIndexes(indexes types.MultiRewardIndexes) {
+	for _, i := range indexes {
+		suite.keeper.SetHardBorrowRewardIndexes(suite.ctx, i.CollateralType, i.RewardIndexes)
+	}
+}
+func (suite *unitTester) storeGlobalSupplyIndexes(indexes types.MultiRewardIndexes) {
+	for _, i := range indexes {
+		suite.keeper.SetHardSupplyRewardIndexes(suite.ctx, i.CollateralType, i.RewardIndexes)
+	}
+}
+
+func (suite *unitTester) storeClaim(claim types.HardLiquidityProviderClaim) {
+	suite.keeper.SetHardLiquidityProviderClaim(suite.ctx, claim)
+}
+
+type fakeParamSubspace struct {
+	params types.Params
+}
+
+func (subspace *fakeParamSubspace) GetParamSet(_ sdk.Context, ps params.ParamSet) {
+	*(ps.(*types.Params)) = subspace.params
+}
+func (subspace *fakeParamSubspace) SetParamSet(_ sdk.Context, ps params.ParamSet) {
+	subspace.params = *(ps.(*types.Params))
+}
+func (subspace *fakeParamSubspace) HasKeyTable() bool {
+	// return true so the keeper does no try to set the key table, which does nothing
+	return true
+}
+func (subspace *fakeParamSubspace) WithKeyTable(params.KeyTable) params.Subspace {
+	// return an non-functional subspace to satisfy the interface
+	return params.Subspace{}
+}
+
+func arbitraryCoin() sdk.Coin {
+	return c("hard", 1e9)
+}
+
+func arbitraryCoins() sdk.Coins {
+	return cs(c("btcb", 1))
+}
+
+func arbitraryCoinsWithDenoms(denom ...string) sdk.Coins {
+	const arbitraryAmount = 1 // must be > 0 as sdk.Coins type only stores positive amounts
+	coins := sdk.NewCoins()
+	for _, d := range denom {
+		coins = coins.Add(sdk.NewInt64Coin(d, arbitraryAmount))
+	}
+	return coins
+}
+
+func arbitraryAddress() sdk.AccAddress {
+	_, addresses := app.GeneratePrivKeyAddressPairs(1)
+	return addresses[0]
+}
+func arbitraryValidatorAddress() sdk.ValAddress {
+	return generateValidatorAddresses(1)[0]
+}
+func generateValidatorAddresses(n int) []sdk.ValAddress {
+	_, addresses := app.GeneratePrivKeyAddressPairs(n)
+	var valAddresses []sdk.ValAddress
+	for _, a := range addresses {
+		valAddresses = append(valAddresses, sdk.ValAddress(a))
+	}
+	return valAddresses
+}
+
+var nonEmptyMultiRewardIndexes = types.MultiRewardIndexes{
+	{
+		CollateralType: "bnb",
+		RewardIndexes: types.RewardIndexes{
+			{
+				CollateralType: "hard",
+				RewardFactor:   d("0.02"),
+			},
+			{
+				CollateralType: "ukava",
+				RewardFactor:   d("0.04"),
+			},
+		},
+	},
+	{
+		CollateralType: "btcb",
+		RewardIndexes: types.RewardIndexes{
+			{
+				CollateralType: "hard",
+				RewardFactor:   d("0.2"),
+			},
+			{
+				CollateralType: "ukava",
+				RewardFactor:   d("0.4"),
+			},
+		},
+	},
+}
+
+func extractCollateralTypes(indexes types.MultiRewardIndexes) []string {
+	var denoms []string
+	for _, ri := range indexes {
+		denoms = append(denoms, ri.CollateralType)
+	}
+	return denoms
+}
+
+func increaseAllRewardFactors(indexes types.MultiRewardIndexes) types.MultiRewardIndexes {
+	increasedIndexes := make(types.MultiRewardIndexes, len(indexes))
+	copy(increasedIndexes, indexes)
+
+	for i := range increasedIndexes {
+		increasedIndexes[i].RewardIndexes = increaseRewardFactors(increasedIndexes[i].RewardIndexes)
+	}
+	return increasedIndexes
+}
+
+func increaseRewardFactors(indexes types.RewardIndexes) types.RewardIndexes {
+	increasedIndexes := make(types.RewardIndexes, len(indexes))
+	copy(increasedIndexes, indexes)
+
+	for i := range increasedIndexes {
+		increasedIndexes[i].RewardFactor = increasedIndexes[i].RewardFactor.MulInt64(2)
+	}
+	return increasedIndexes
+}
+
+func appendUniqueMultiRewardIndex(indexes types.MultiRewardIndexes) types.MultiRewardIndexes {
+	const uniqueDenom = "uniquedenom"
+
+	for _, mri := range indexes {
+		if mri.CollateralType == uniqueDenom {
+			panic(fmt.Sprintf("tried to add unique multi reward index with denom '%s', but denom already existed", uniqueDenom))
+		}
+	}
+
+	return append(indexes, types.NewMultiRewardIndex(
+		uniqueDenom,
+		types.RewardIndexes{
+			{
+				CollateralType: "hard",
+				RewardFactor:   d("0.02"),
+			},
+			{
+				CollateralType: "ukava",
+				RewardFactor:   d("0.04"),
+			},
+		},
+	),
+	)
+}
+
+func appendUniqueEmptyMultiRewardIndex(indexes types.MultiRewardIndexes) types.MultiRewardIndexes {
+	const uniqueDenom = "uniquedenom"
+
+	for _, mri := range indexes {
+		if mri.CollateralType == uniqueDenom {
+			panic(fmt.Sprintf("tried to add unique multi reward index with denom '%s', but denom already existed", uniqueDenom))
+		}
+	}
+
+	return append(indexes, types.NewMultiRewardIndex(uniqueDenom, nil))
+}
+
+func appendUniqueRewardIndexToFirstItem(indexes types.MultiRewardIndexes) types.MultiRewardIndexes {
+	newIndexes := make(types.MultiRewardIndexes, len(indexes))
+	copy(newIndexes, indexes)
+
+	newIndexes[0].RewardIndexes = appendUniqueRewardIndex(newIndexes[0].RewardIndexes)
+	return newIndexes
+}
+
+func appendUniqueRewardIndex(indexes types.RewardIndexes) types.RewardIndexes {
+	const uniqueDenom = "uniquereward"
+
+	for _, mri := range indexes {
+		if mri.CollateralType == uniqueDenom {
+			panic(fmt.Sprintf("tried to add unique reward index with denom '%s', but denom already existed", uniqueDenom))
+		}
+	}
+
+	return append(
+		indexes,
+		types.NewRewardIndex(uniqueDenom, d("0.02")),
+	)
+}

x/incentive/types/claims.go

@@ -402,6 +402,30 @@ func (ris RewardIndexes) GetRewardIndex(denom string) (RewardIndex, bool) {
 	return RewardIndex{}, false
 }
 
+// Get fetches a RewardFactor by it's denom
+func (ris RewardIndexes) Get(denom string) (sdk.Dec, bool) {
+	for _, ri := range ris {
+		if ri.CollateralType == denom {
+			return ri.RewardFactor, true
+		}
+	}
+	return sdk.Dec{}, false
+}
+
+// With returns a copy of the indexes with a new reward factor added
+func (ris RewardIndexes) With(denom string, factor sdk.Dec) RewardIndexes {
+	newIndexes := make(RewardIndexes, len(ris))
+	copy(newIndexes, ris)
+
+	for i, ri := range newIndexes {
+		if ri.CollateralType == denom {
+			newIndexes[i].RewardFactor = factor
+			return newIndexes
+		}
+	}
+	return append(newIndexes, NewRewardIndex(denom, factor))
+}
+
 // GetFactorIndex gets the index of a specific reward index inside the array by its index
 func (ris RewardIndexes) GetFactorIndex(denom string) (int, bool) {
 	for i, ri := range ris {
@@ -476,6 +500,16 @@ func (mris MultiRewardIndexes) GetRewardIndex(denom string) (MultiRewardIndex, b
 	return MultiRewardIndex{}, false
 }
 
+// Get fetches a RewardIndexes by it's denom
+func (mris MultiRewardIndexes) Get(denom string) (RewardIndexes, bool) {
+	for _, mri := range mris {
+		if mri.CollateralType == denom {
+			return mri.RewardIndexes, true
+		}
+	}
+	return nil, false
+}
+
 // GetRewardIndexIndex fetches a specific reward index inside the array by its denom
 func (mris MultiRewardIndexes) GetRewardIndexIndex(denom string) (int, bool) {
 	for i, ri := range mris {
@@ -486,6 +520,19 @@ func (mris MultiRewardIndexes) GetRewardIndexIndex(denom string) (int, bool) {
 	return -1, false
 }
 
+// With returns a copy of the indexes with a new RewardIndexes added
+func (mris MultiRewardIndexes) With(denom string, indexes RewardIndexes) MultiRewardIndexes {
+	newIndexes := mris.copy()
+
+	for i, mri := range newIndexes {
+		if mri.CollateralType == denom {
+			newIndexes[i].RewardIndexes = indexes
+			return newIndexes
+		}
+	}
+	return append(newIndexes, NewMultiRewardIndex(denom, indexes))
+}
+
 // GetCollateralTypes returns a slice of containing all collateral types
 func (mris MultiRewardIndexes) GetCollateralTypes() []string {
 	var collateralTypes []string
@@ -499,7 +546,9 @@ func (mris MultiRewardIndexes) GetCollateralTypes() []string {
 func (mris MultiRewardIndexes) RemoveRewardIndex(denom string) MultiRewardIndexes {
 	for i, ri := range mris {
 		if ri.CollateralType == denom {
-			return append(mris[:i], mris[i+1:]...)
+			// copy the slice and underlying array to avoid altering the original
+			copy := mris.copy()
+			return append(copy[:i], copy[i+1:]...)
 		}
 	}
 	return mris
@@ -514,3 +563,10 @@ func (mris MultiRewardIndexes) Validate() error {
 	}
 	return nil
 }
+
+// copy returns a copy of the slice and underlying array
+func (mris MultiRewardIndexes) copy() MultiRewardIndexes {
+	newIndexes := make(MultiRewardIndexes, len(mris))
+	copy(newIndexes, mris)
+	return newIndexes
+}

x/incentive/types/claims_test.go

@@ -1,13 +1,12 @@
 package types
 
 import (
+	"fmt"
 	"testing"
 
-	"github.com/stretchr/testify/require"
-
-	"github.com/tendermint/tendermint/crypto"
-
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/stretchr/testify/require"
+	"github.com/tendermint/tendermint/crypto"
 )
 
 func TestClaimsValidate(t *testing.T) {
@@ -72,3 +71,295 @@ func TestClaimsValidate(t *testing.T) {
 		}
 	}
 }
+
+func TestRewardIndexes(t *testing.T) {
+	t.Run("With", func(t *testing.T) {
+		var arbitraryDec = sdk.MustNewDecFromStr("0.1")
+
+		type args struct {
+			denom  string
+			factor sdk.Dec
+		}
+		testcases := []struct {
+			name          string
+			rewardIndexes RewardIndexes
+			args          args
+			expected      RewardIndexes
+		}{
+			{
+				name: "when index is not present, it's added and original isn't overwritten",
+				rewardIndexes: RewardIndexes{
+					NewRewardIndex("denom", arbitraryDec),
+				},
+				args: args{
+					denom:  "otherdenom",
+					factor: arbitraryDec,
+				},
+				expected: RewardIndexes{
+					NewRewardIndex("denom", arbitraryDec),
+					NewRewardIndex("otherdenom", arbitraryDec),
+				},
+			},
+			{
+				name: "when index is present, it's updated and original isn't overwritten",
+				rewardIndexes: RewardIndexes{
+					NewRewardIndex("denom", arbitraryDec),
+				},
+				args: args{
+					denom:  "denom",
+					factor: arbitraryDec.MulInt64(2),
+				},
+				expected: RewardIndexes{
+					NewRewardIndex("denom", arbitraryDec.MulInt64(2)),
+				},
+			},
+		}
+
+		for _, tc := range testcases {
+			t.Run(tc.name, func(t *testing.T) {
+				newIndexes := tc.rewardIndexes.With(tc.args.denom, tc.args.factor)
+
+				require.Equal(t, tc.expected, newIndexes)
+				require.NotEqual(t, tc.rewardIndexes, newIndexes) // check original slice not modified
+			})
+		}
+	})
+	t.Run("Get", func(t *testing.T) {
+		var arbitraryDec = sdk.MustNewDecFromStr("0.1")
+
+		type expected struct {
+			factor sdk.Dec
+			found  bool
+		}
+		testcases := []struct {
+			name          string
+			rewardIndexes RewardIndexes
+			arg_denom     string
+			expected      expected
+		}{
+			{
+				name: "when index is present, it is found and returned",
+				rewardIndexes: RewardIndexes{
+					NewRewardIndex("denom", arbitraryDec),
+				},
+				arg_denom: "denom",
+				expected: expected{
+					factor: arbitraryDec,
+					found:  true,
+				},
+			},
+			{
+				name: "when index is not present, it is not found",
+				rewardIndexes: RewardIndexes{
+					NewRewardIndex("denom", arbitraryDec),
+				},
+				arg_denom: "notpresent",
+				expected: expected{
+					found: false,
+				},
+			},
+		}
+
+		for _, tc := range testcases {
+			t.Run(tc.name, func(t *testing.T) {
+				factor, found := tc.rewardIndexes.Get(tc.arg_denom)
+
+				require.Equal(t, tc.expected.found, found)
+				require.Equal(t, tc.expected.factor, factor)
+			})
+		}
+	})
+}
+
+func TestMultiRewardIndexes(t *testing.T) {
+	arbitraryRewardIndexes := RewardIndexes{
+		{
+			CollateralType: "reward",
+			RewardFactor:   sdk.MustNewDecFromStr("0.1"),
+		},
+	}
+
+	t.Run("Get", func(t *testing.T) {
+		type expected struct {
+			rewardIndexes RewardIndexes
+			found         bool
+		}
+		testcases := []struct {
+			name               string
+			multiRewardIndexes MultiRewardIndexes
+			arg_denom          string
+			expected           expected
+		}{
+			{
+				name: "when indexes are present, they are found and returned",
+				multiRewardIndexes: MultiRewardIndexes{
+					{
+						CollateralType: "denom",
+						RewardIndexes:  arbitraryRewardIndexes,
+					},
+				},
+				arg_denom: "denom",
+				expected: expected{
+					found:         true,
+					rewardIndexes: arbitraryRewardIndexes,
+				},
+			},
+			{
+				name: "when indexes are not present, they are not found",
+				multiRewardIndexes: MultiRewardIndexes{
+					{
+						CollateralType: "denom",
+						RewardIndexes:  arbitraryRewardIndexes,
+					},
+				},
+				arg_denom: "notpresent",
+				expected: expected{
+					found: false,
+				},
+			},
+		}
+		for _, tc := range testcases {
+			t.Run(tc.name, func(t *testing.T) {
+				rewardIndexes, found := tc.multiRewardIndexes.Get(tc.arg_denom)
+
+				require.Equal(t, tc.expected.found, found)
+				require.Equal(t, tc.expected.rewardIndexes, rewardIndexes)
+			})
+		}
+	})
+	t.Run("With", func(t *testing.T) {
+		type args struct {
+			denom         string
+			rewardIndexes RewardIndexes
+		}
+		testcases := []struct {
+			name               string
+			multiRewardIndexes MultiRewardIndexes
+			args               args
+			expected           MultiRewardIndexes
+		}{
+			{
+				name: "when indexes are not present, add them and do not update original",
+				multiRewardIndexes: MultiRewardIndexes{
+					{
+						CollateralType: "denom",
+						RewardIndexes:  arbitraryRewardIndexes,
+					},
+				},
+				args: args{
+					denom:         "otherdenom",
+					rewardIndexes: arbitraryRewardIndexes,
+				},
+				expected: MultiRewardIndexes{
+					{
+						CollateralType: "denom",
+						RewardIndexes:  arbitraryRewardIndexes,
+					},
+					{
+						CollateralType: "otherdenom",
+						RewardIndexes:  arbitraryRewardIndexes,
+					},
+				},
+			},
+			{
+				name: "when indexes are present, update them and do not update original",
+				multiRewardIndexes: MultiRewardIndexes{
+					{
+						CollateralType: "denom",
+						RewardIndexes:  arbitraryRewardIndexes,
+					},
+				},
+				args: args{
+					denom:         "denom",
+					rewardIndexes: appendUniqueRewardIndex(arbitraryRewardIndexes),
+				},
+				expected: MultiRewardIndexes{
+					{
+						CollateralType: "denom",
+						RewardIndexes:  appendUniqueRewardIndex(arbitraryRewardIndexes),
+					},
+				},
+			},
+		}
+		for _, tc := range testcases {
+			t.Run(tc.name, func(t *testing.T) {
+				oldIndexes := tc.multiRewardIndexes.copy()
+
+				newIndexes := tc.multiRewardIndexes.With(tc.args.denom, tc.args.rewardIndexes)
+
+				require.Equal(t, tc.expected, newIndexes)
+				require.Equal(t, oldIndexes, tc.multiRewardIndexes)
+			})
+		}
+	})
+	t.Run("RemoveRewardIndex", func(t *testing.T) {
+		testcases := []struct {
+			name               string
+			multiRewardIndexes MultiRewardIndexes
+			arg_denom          string
+			expected           MultiRewardIndexes
+		}{
+			{
+				name: "when indexes are not present, do nothing",
+				multiRewardIndexes: MultiRewardIndexes{
+					{
+						CollateralType: "denom",
+						RewardIndexes:  arbitraryRewardIndexes,
+					},
+				},
+				arg_denom: "notpresent",
+				expected: MultiRewardIndexes{
+					{
+						CollateralType: "denom",
+						RewardIndexes:  arbitraryRewardIndexes,
+					},
+				},
+			},
+			{
+				name: "when indexes are present, remove them and do not update original",
+				multiRewardIndexes: MultiRewardIndexes{
+					{
+						CollateralType: "denom",
+						RewardIndexes:  arbitraryRewardIndexes,
+					},
+					{
+						CollateralType: "otherdenom",
+						RewardIndexes:  arbitraryRewardIndexes,
+					},
+				},
+				arg_denom: "denom",
+				expected: MultiRewardIndexes{
+					{
+						CollateralType: "otherdenom",
+						RewardIndexes:  arbitraryRewardIndexes,
+					},
+				},
+			},
+		}
+		for _, tc := range testcases {
+			t.Run(tc.name, func(t *testing.T) {
+				oldIndexes := tc.multiRewardIndexes.copy()
+
+				newIndexes := tc.multiRewardIndexes.RemoveRewardIndex(tc.arg_denom)
+
+				require.Equal(t, tc.expected, newIndexes)
+				require.Equal(t, oldIndexes, tc.multiRewardIndexes)
+			})
+		}
+	})
+}
+
+func appendUniqueRewardIndex(indexes RewardIndexes) RewardIndexes {
+	const uniqueDenom = "uniquereward"
+
+	for _, mri := range indexes {
+		if mri.CollateralType == uniqueDenom {
+			panic(fmt.Sprintf("tried to add unique reward index with denom '%s', but denom already existed", uniqueDenom))
+		}
+	}
+
+	return append(
+		indexes,
+		NewRewardIndex(uniqueDenom, sdk.MustNewDecFromStr("0.02")),
+	)
+}

x/incentive/types/errors.go

@@ -19,4 +19,5 @@ var (
 	ErrClaimExpired                  = sdkerrors.Register(ModuleName, 10, "claim has expired")
 	ErrInvalidClaimType              = sdkerrors.Register(ModuleName, 11, "invalid claim type")
 	ErrInvalidClaimOwner             = sdkerrors.Register(ModuleName, 12, "invalid claim owner")
+	ErrDecreasingRewardFactor        = sdkerrors.Register(ModuleName, 13, "found new reward factor less than an old reward factor")
 )

x/incentive/types/expected_keepers.go

@@ -3,6 +3,7 @@ package types
 import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	authexported "github.com/cosmos/cosmos-sdk/x/auth/exported"
+	"github.com/cosmos/cosmos-sdk/x/params"
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
 	supplyexported "github.com/cosmos/cosmos-sdk/x/supply/exported"
 
@@ -10,6 +11,14 @@ import (
 	hardtypes "github.com/kava-labs/kava/x/hard/types"
 )
 
+// ParamSubspace defines the expected Subspace interfacace
+type ParamSubspace interface {
+	GetParamSet(sdk.Context, params.ParamSet)
+	SetParamSet(sdk.Context, params.ParamSet)
+	WithKeyTable(params.KeyTable) params.Subspace
+	HasKeyTable() bool
+}
+
 // SupplyKeeper defines the expected supply keeper for module accounts
 type SupplyKeeper interface {
 	GetModuleAccount(ctx sdk.Context, name string) supplyexported.ModuleAccountI