Skip to content

Commit

Permalink
Merge pull request #329 from terra-money/fix/validator-different-rewards
Browse files Browse the repository at this point in the history 
fix: rewards should be the same across validators
  • Loading branch information
@javiersuweijie
javiersuweijie authored Mar 21, 2024
2 parents 6a837a8 + 111381b commit 61faf64
Show file tree
Hide file tree
Showing 2 changed files with 150 additions and 6 deletions.
11 changes: 7 additions & 4 deletions x/alliance/keeper/reward.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,20 +149,23 @@ func (k Keeper) AddAssetsToRewardPool(ctx sdk.Context, from sdk.AccAddress, val
}
alliances := k.GetAllAssets(ctx)

// Get total reward weight to normalize weights
totalRewardWeight := sdk.NewDec(0)
totalStakedRewardWeight := sdk.NewDec(0)
// Map is used here only as a lookup table so it does not change the order of the results therefore it is consensus safe
assetStakedRewardWeights := make(map[string]sdk.Dec)
for _, asset := range alliances {
if shouldSkipRewardsToAsset(ctx, *asset, val) {
continue
}
totalRewardWeight = totalRewardWeight.Add(asset.RewardWeight)
stakedRewardWeight := asset.RewardWeight.Mul(val.TotalTokensWithAsset(*asset)).QuoInt(asset.TotalTokens)
assetStakedRewardWeights[asset.Denom] = stakedRewardWeight
totalStakedRewardWeight = totalStakedRewardWeight.Add(stakedRewardWeight)
}

for _, asset := range alliances {
if shouldSkipRewardsToAsset(ctx, *asset, val) {
continue
}
normalizedWeight := asset.RewardWeight.Quo(totalRewardWeight)
normalizedWeight := assetStakedRewardWeights[asset.Denom].Quo(totalStakedRewardWeight)
for _, c := range coins {
rewardHistory, found := rewardHistories.GetIndexByDenom(c.Denom, asset.Denom)
totalTokens := val.TotalTokensWithAsset(*asset)
Expand Down
145 changes: 143 additions & 2 deletions x/alliance/keeper/tests/reward_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1288,11 +1288,11 @@ func TestMigratedRewards(t *testing.T) {

rewards1, err := app.AllianceKeeper.ClaimDelegationRewards(ctx, user1, val1, AllianceDenom)
require.NoError(t, err)
require.Equal(t, sdk.NewInt(1_000_000+2_000_000), rewards1.AmountOf(bondDenom))
require.Equal(t, sdk.NewInt(1_000_000+4285714), rewards1.AmountOf(bondDenom))

rewards2, err := app.AllianceKeeper.ClaimDelegationRewards(ctx, user2, val1, AllianceDenomTwo)
require.NoError(t, err)
require.Equal(t, sdk.NewInt(4_000_000+8_000_000), rewards2.AmountOf(bondDenom))
require.Equal(t, sdk.NewInt(4_000_000+5714285), rewards2.AmountOf(bondDenom))

rewards3, err := app.AllianceKeeper.ClaimDelegationRewards(ctx, user1, val2, AllianceDenomTwo)
require.NoError(t, err)
Expand All @@ -1319,3 +1319,144 @@ func TestMigratedRewards(t *testing.T) {
require.NoError(t, err)
require.Equal(t, sdk.NewInt(0), rewards4.AmountOf(bondDenom))
}

func TestClaimRewardsWithDifferentValidators(t *testing.T) {
var err error
app, ctx := createTestContext(t)
app.AllianceKeeper.InitGenesis(ctx, &types.GenesisState{
Params: types.DefaultParams(),
Assets: []types.AllianceAsset{
types.NewAllianceAsset(AllianceDenom, sdk.NewDec(2), sdk.NewDec(0), sdk.NewDec(5), sdk.NewDec(0), ctx.BlockTime()),
types.NewAllianceAsset(AllianceDenomTwo, sdk.NewDec(2), sdk.NewDec(0), sdk.NewDec(5), sdk.NewDec(0), ctx.BlockTime()),
},
})

// Set tax and rewards to be zero for easier calculation
distParams := app.DistrKeeper.GetParams(ctx)
distParams.CommunityTax = sdk.ZeroDec()

err = app.DistrKeeper.SetParams(ctx, distParams)
require.NoError(t, err)

// Accounts
// mintPoolAddr := app.AccountKeeper.GetModuleAddress(minttypes.ModuleName)
// rewardsPoolAddr := app.AccountKeeper.GetModuleAddress(types.RewardsPoolName)
delegations := app.StakingKeeper.GetAllDelegations(ctx)
valAddr0, err := sdk.ValAddressFromBech32(delegations[0].ValidatorAddress)
require.NoError(t, err)
val0, found := app.StakingKeeper.GetValidator(ctx, valAddr0)
require.True(t, found)

addrs := test_helpers.AddTestAddrsIncremental(app, ctx, 4, sdk.NewCoins(
sdk.NewCoin(AllianceDenom, sdk.NewInt(10_00_000_000)),
sdk.NewCoin(AllianceDenomTwo, sdk.NewInt(10_00_000_000)),
))
pks := test_helpers.CreateTestPubKeys(2)

// Creating two validators: 1 with 0% commission, 1 with 100% commission
valAddr1 := sdk.ValAddress(addrs[0])
_val1 := teststaking.NewValidator(t, valAddr1, pks[0])
_val1.Commission = stakingtypes.Commission{
CommissionRates: stakingtypes.CommissionRates{
Rate: sdk.NewDec(0),
MaxRate: sdk.NewDec(0),
MaxChangeRate: sdk.NewDec(0),
},
UpdateTime: time.Now(),
}
test_helpers.RegisterNewValidator(t, app, ctx, _val1)

valAddr2 := sdk.ValAddress(addrs[1])
_val2 := teststaking.NewValidator(t, valAddr2, pks[1])
_val2.Commission = stakingtypes.Commission{
CommissionRates: stakingtypes.CommissionRates{
Rate: sdk.NewDec(0),
MaxRate: sdk.NewDec(0),
MaxChangeRate: sdk.NewDec(0),
},
UpdateTime: time.Now(),
}
test_helpers.RegisterNewValidator(t, app, ctx, _val2)

val1, _ := app.AllianceKeeper.GetAllianceValidator(ctx, valAddr1)
val2, _ := app.AllianceKeeper.GetAllianceValidator(ctx, valAddr2)

user1 := addrs[2]
user2 := addrs[3]

// Mint tokens
err = app.BankKeeper.MintCoins(ctx, minttypes.ModuleName, sdk.NewCoins(sdk.NewCoin("stake", sdk.NewInt(4000_000))))
require.NoError(t, err)

// New delegations from user 1
_, err = app.AllianceKeeper.Delegate(ctx, user1, val1, sdk.NewCoin(AllianceDenom, sdk.NewInt(1_000_000)))
require.NoError(t, err)
_, err = app.AllianceKeeper.Delegate(ctx, user1, val2, sdk.NewCoin(AllianceDenom, sdk.NewInt(1_000_000)))
require.NoError(t, err)

// New delegation from user 2
_, err = app.AllianceKeeper.Delegate(ctx, user2, val1, sdk.NewCoin(AllianceDenom, sdk.NewInt(2_000_000)))
require.NoError(t, err)
_, err = app.AllianceKeeper.Delegate(ctx, user2, val2, sdk.NewCoin(AllianceDenom, sdk.NewInt(1_000_000)))
require.NoError(t, err)
_, err = app.AllianceKeeper.Delegate(ctx, user2, val1, sdk.NewCoin(AllianceDenomTwo, sdk.NewInt(5_000_000)))
require.NoError(t, err)
_, err = app.AllianceKeeper.Delegate(ctx, user2, val2, sdk.NewCoin(AllianceDenomTwo, sdk.NewInt(5_000_000)))
require.NoError(t, err)
assets := app.AllianceKeeper.GetAllAssets(ctx)
err = app.AllianceKeeper.RebalanceBondTokenWeights(ctx, assets)
require.NoError(t, err)

// Total AllianceDenom staked = 5_000_000
// Total AllianceDenomTwo staked = 10_000_000
// Total rewards to vals = 4_000_000
// Normalized reward weight = 0.4
// AllianceDenom rewards per token = 4_000_000 * 0.4 / 5_000_000 = 0.32
// AllianceDenomTwo rewards per token = 4_000_000 * 0.4 / 10_000_000 = 0.16

// Transfer to rewards to fee pool to be distributed
err = app.BankKeeper.SendCoinsFromModuleToModule(ctx, minttypes.ModuleName, authtypes.FeeCollectorName, sdk.NewCoins(sdk.NewCoin("stake", sdk.NewInt(4_000_000))))
require.NoError(t, err)

ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1)
// Distribute in the next begin block
// At the next begin block, tokens will be distributed from the fee pool
cons0, _ := val0.GetConsAddr()
cons1, _ := val1.GetConsAddr()
cons2, _ := val2.GetConsAddr()
var votingPower int64 = 100
app.DistrKeeper.AllocateTokens(ctx, votingPower, []abcitypes.VoteInfo{
{
Validator: abcitypes.Validator{
Address: cons0,
Power: 20,
},
SignedLastBlock: true,
},
{
Validator: abcitypes.Validator{
Address: cons1,
Power: 44,
},
SignedLastBlock: true,
},
{
Validator: abcitypes.Validator{
Address: cons2,
Power: 36,
},
SignedLastBlock: true,
},
})

// User 2 claims rewards
// 0.16 * 5_000_000 = 800_000
coins, err := app.AllianceKeeper.ClaimDelegationRewards(ctx, user2, val1, AllianceDenomTwo)
require.NoError(t, err)
require.Equal(t, sdk.NewCoins(sdk.NewCoin("stake", sdk.NewInt(800_000))), coins)

// 0.16 * 5_000_000 = 800_000
coins, err = app.AllianceKeeper.ClaimDelegationRewards(ctx, user2, val2, AllianceDenomTwo)
require.NoError(t, err)
require.Equal(t, sdk.NewCoins(sdk.NewCoin("stake", sdk.NewInt(800_000))), coins)
}

0 comments on commit 61faf64

Please sign in to comment.