From 0da0cd2bfb1993ed498783fdc2de5403e19c2041 Mon Sep 17 00:00:00 2001 From: oren-lava <111131399+oren-lava@users.noreply.github.com> Date: Wed, 9 Oct 2024 12:07:45 +0300 Subject: [PATCH] fix: IPRPC pool rewards distribution (#1678) * fix iprpc distribution * fix unit tests * move the check of unstaked providers to specCuMap construction func * fix * fix comments * fix bug in UnstakeEntryForce * add panic when failing to distribute IPRPC rewards and remove claimable rewards --------- Co-authored-by: Oren Co-authored-by: Yaroms <103432884+Yaroms@users.noreply.github.com> --- x/dualstaking/keeper/delegator_reward.go | 31 ++++++++++++------- x/pairing/keeper/cu_tracker_test.go | 13 +++++--- x/pairing/keeper/delegator_rewards_test.go | 6 ++-- .../grpc_query_provider_monthly_payout.go | 2 +- x/pairing/keeper/unstaking.go | 20 ++++++------ x/pairing/types/expected_keepers.go | 2 +- x/rewards/keeper/iprpc.go | 6 ++-- x/rewards/keeper/providers.go | 10 ++++-- x/rewards/types/expected_keepers.go | 2 +- x/subscription/keeper/cu_tracker.go | 2 +- x/subscription/types/expected_keepers.go | 2 +- 11 files changed, 54 insertions(+), 42 deletions(-) diff --git a/x/dualstaking/keeper/delegator_reward.go b/x/dualstaking/keeper/delegator_reward.go index a1d428ee26..5decaf9a7e 100644 --- a/x/dualstaking/keeper/delegator_reward.go +++ b/x/dualstaking/keeper/delegator_reward.go @@ -1,6 +1,7 @@ package keeper import ( + "fmt" "strconv" "cosmossdk.io/math" @@ -158,47 +159,55 @@ func (k Keeper) ClaimRewards(ctx sdk.Context, delegator string, provider string) // RewardProvidersAndDelegators is the main function handling provider rewards with delegations // it returns the provider reward amount and updates the delegatorReward map with the reward portion for each delegator -func (k Keeper) RewardProvidersAndDelegators(ctx sdk.Context, provider string, chainID string, totalReward sdk.Coins, senderModule string, calcOnlyProvider bool, calcOnlyDelegators bool, calcOnlyContributor bool) (providerReward sdk.Coins, claimableRewards sdk.Coins, err error) { +// since this function does not actually send rewards to the providers and delegator (but only allocates rewards to be claimed) +func (k Keeper) RewardProvidersAndDelegators(ctx sdk.Context, provider string, chainID string, totalReward sdk.Coins, senderModule string, calcOnlyProvider bool, calcOnlyDelegators bool, calcOnlyContributor bool) (providerReward sdk.Coins, err error) { block := uint64(ctx.BlockHeight()) zeroCoins := sdk.NewCoins() epoch, _, err := k.epochstorageKeeper.GetEpochStartForBlock(ctx, block) if err != nil { - return zeroCoins, zeroCoins, utils.LavaFormatError(types.ErrCalculatingProviderReward.Error(), err, + return zeroCoins, utils.LavaFormatError(types.ErrCalculatingProviderReward.Error(), err, utils.Attribute{Key: "block", Value: block}, ) } stakeEntry, found := k.epochstorageKeeper.GetStakeEntry(ctx, epoch, chainID, provider) if !found { - return zeroCoins, zeroCoins, err + return zeroCoins, utils.LavaFormatWarning("RewardProvidersAndDelegators: cannot send rewards to provider and delegators", fmt.Errorf("provider stake entry not found, probably unstaked"), + utils.LogAttr("epoch", epoch), + utils.LogAttr("provider", provider), + utils.LogAttr("chain_id", chainID), + utils.LogAttr("sender_module", senderModule), + utils.LogAttr("reward", totalReward.String()), + ) } delegations, err := k.GetProviderDelegators(ctx, provider, epoch) if err != nil { - return zeroCoins, zeroCoins, utils.LavaFormatError("cannot get provider's delegators", err) + return zeroCoins, utils.LavaFormatError("cannot get provider's delegators", err) } - claimableRewards = totalReward // make sure this is post boost when rewards pool is introduced contributorAddresses, contributorPart := k.specKeeper.GetContributorReward(ctx, chainID) contributorsNum := sdk.NewInt(int64(len(contributorAddresses))) + contributorReward := zeroCoins if !contributorsNum.IsZero() && contributorPart.GT(math.LegacyZeroDec()) { - contributorReward := totalReward.MulInt(contributorPart.MulInt64(spectypes.ContributorPrecision).RoundInt()).QuoInt(sdk.NewInt(spectypes.ContributorPrecision)) + contributorReward = totalReward.MulInt(contributorPart.MulInt64(spectypes.ContributorPrecision).RoundInt()).QuoInt(sdk.NewInt(spectypes.ContributorPrecision)) // make sure to round it down for the integers division contributorReward = contributorReward.QuoInt(contributorsNum).MulInt(contributorsNum) - claimableRewards = totalReward.Sub(contributorReward...) if !calcOnlyContributor { err = k.PayContributors(ctx, senderModule, contributorAddresses, contributorReward, chainID) if err != nil { - return zeroCoins, zeroCoins, err + return zeroCoins, err } } } + // delegators eligible for rewards are delegators that their delegation is at least a week old relevantDelegations := lavaslices.Filter(delegations, func(d types.Delegation) bool { return d.ChainID == chainID && d.IsFirstWeekPassed(ctx.BlockTime().UTC().Unix()) && d.Delegator != stakeEntry.Vault }) - providerReward, delegatorsReward := k.CalcRewards(ctx, stakeEntry, claimableRewards, relevantDelegations) + // calculate the rewards for the providers and delegators (total reward - contributors reward) + providerReward, delegatorsReward := k.CalcRewards(ctx, stakeEntry, totalReward.Sub(contributorReward...), relevantDelegations) leftoverRewards := k.updateDelegatorsReward(ctx, stakeEntry.DelegateTotal.Amount, relevantDelegations, delegatorsReward, senderModule, calcOnlyDelegators) fullProviderReward := providerReward.Add(leftoverRewards...) @@ -208,7 +217,7 @@ func (k Keeper) RewardProvidersAndDelegators(ctx sdk.Context, provider string, c k.rewardDelegator(ctx, types.Delegation{Provider: stakeEntry.Address, ChainID: chainID, Delegator: stakeEntry.Vault}, fullProviderReward, senderModule) } - return fullProviderReward, claimableRewards, nil + return fullProviderReward, nil } // updateDelegatorsReward updates the delegator rewards map @@ -217,11 +226,9 @@ func (k Keeper) updateDelegatorsReward(ctx sdk.Context, totalDelegations math.In for _, delegation := range delegations { delegatorReward := k.CalcDelegatorReward(ctx, delegatorsReward, totalDelegations, delegation) - if !calcOnly { k.rewardDelegator(ctx, delegation, delegatorReward, senderModule) } - usedDelegatorRewards = usedDelegatorRewards.Add(delegatorReward...) } diff --git a/x/pairing/keeper/cu_tracker_test.go b/x/pairing/keeper/cu_tracker_test.go index eef6daccc7..cc5454d765 100644 --- a/x/pairing/keeper/cu_tracker_test.go +++ b/x/pairing/keeper/cu_tracker_test.go @@ -641,13 +641,16 @@ func TestProviderMonthlyPayoutQueryWithContributor(t *testing.T) { } ts.relayPaymentWithoutPay(relayPaymentMessage, true) - // check for expected balance: planPrice*100/200 (from spec1) + planPrice*(100/200)*(2/3) (from spec, considering delegations) - // for planPrice=100, expected monthly payout is 50 (spec1 with contributor) + 33 (normal spec no contributor) - expectedContributorPay := uint64(12) // half the plan payment for spec1:25 then divided between contributors half half rounded down - expectedTotalPayout := uint64(83) - expectedContributorPay*2 + // half the plan payment for spec1 is 25. Then it's divided between 2 contributors equally rounded down + expectedContributorPay := uint64(12) + + // for planPrice=100, and equal CU usage for both specs, the expected provider monthly payout is: + // spec (delegator is 33% of stake): planPrice * specUsedCu/totalUsedCu * providerStake/totalStake = 100*0.5*(2/3) = 33 + // spec1 (contributors with commission=50%): planPrice * specUsedCu/totalUsedCu - contributorsPart = 100*0.5 - 24 = 26 + expectedTotalPayout := uint64(59) expectedPayouts := []types.SubscriptionPayout{ {Subscription: clientAcc.Addr.String(), ChainId: ts.spec.Index, Amount: 33}, - {Subscription: clientAcc.Addr.String(), ChainId: spec1.Index, Amount: 26}, // 50 - 26 for contributors (each contributor gets 12) + {Subscription: clientAcc.Addr.String(), ChainId: spec1.Index, Amount: 26}, } res, err := ts.QueryPairingProviderMonthlyPayout(provider) require.NoError(t, err) diff --git a/x/pairing/keeper/delegator_rewards_test.go b/x/pairing/keeper/delegator_rewards_test.go index fd9ba80d3c..a8049e9811 100644 --- a/x/pairing/keeper/delegator_rewards_test.go +++ b/x/pairing/keeper/delegator_rewards_test.go @@ -541,7 +541,7 @@ func TestDelegationFirstMonthReward(t *testing.T) { // this, we'll call the reward calculation function directly with a fabricated reward just to // verify that the delegator gets nothing from the total reward fakeReward := sdk.NewCoins(sdk.NewCoin(ts.TokenDenom(), sdk.NewInt(testStake))) - providerReward, _, err := ts.Keepers.Dualstaking.RewardProvidersAndDelegators(ts.Ctx, provider, ts.spec.Index, + providerReward, err := ts.Keepers.Dualstaking.RewardProvidersAndDelegators(ts.Ctx, provider, ts.spec.Index, fakeReward, subscriptiontypes.ModuleName, true, true, true) require.NoError(t, err) require.True(t, fakeReward.IsEqual(providerReward)) // if the delegator got anything, this would fail @@ -603,11 +603,11 @@ func TestRedelegationFirstMonthReward(t *testing.T) { // verify that the delegator gets nothing from the total reward from provider1 but does get // reward from provider fakeReward := sdk.NewCoins(sdk.NewCoin(ts.TokenDenom(), sdk.NewInt(testStake))) - provider1Reward, _, err := ts.Keepers.Dualstaking.RewardProvidersAndDelegators(ts.Ctx, provider1, ts.spec.Index, + provider1Reward, err := ts.Keepers.Dualstaking.RewardProvidersAndDelegators(ts.Ctx, provider1, ts.spec.Index, fakeReward, subscriptiontypes.ModuleName, true, false, true) require.NoError(t, err) require.True(t, fakeReward.IsEqual(provider1Reward)) // if the delegator got anything, this would fail - providerReward, _, err := ts.Keepers.Dualstaking.RewardProvidersAndDelegators(ts.Ctx, provider, ts.spec.Index, + providerReward, err := ts.Keepers.Dualstaking.RewardProvidersAndDelegators(ts.Ctx, provider, ts.spec.Index, fakeReward, subscriptiontypes.ModuleName, true, false, true) require.NoError(t, err) require.False(t, fakeReward.IsEqual(providerReward)) // the delegator should have rewards diff --git a/x/pairing/keeper/grpc_query_provider_monthly_payout.go b/x/pairing/keeper/grpc_query_provider_monthly_payout.go index 7d5759fffa..1268a2bd82 100644 --- a/x/pairing/keeper/grpc_query_provider_monthly_payout.go +++ b/x/pairing/keeper/grpc_query_provider_monthly_payout.go @@ -73,7 +73,7 @@ func (k Keeper) ProviderMonthlyPayout(goCtx context.Context, req *types.QueryPro providerRewardAfterFees := sdk.NewCoins(sdk.NewCoin(k.stakingKeeper.BondDenom(ctx), totalMonthlyReward.Sub(validatorsFee.AmountOf(denom)).Sub(communityFee.AmountOf(denom)))) // calculate only the provider reward - providerReward, _, err := k.dualstakingKeeper.RewardProvidersAndDelegators(ctx, req.Provider, chainID, providerRewardAfterFees, subsciptiontypes.ModuleName, true, true, true) + providerReward, err := k.dualstakingKeeper.RewardProvidersAndDelegators(ctx, req.Provider, chainID, providerRewardAfterFees, subsciptiontypes.ModuleName, true, true, true) if err != nil { return nil, err } diff --git a/x/pairing/keeper/unstaking.go b/x/pairing/keeper/unstaking.go index ff4f6eafb0..87d64d5174 100644 --- a/x/pairing/keeper/unstaking.go +++ b/x/pairing/keeper/unstaking.go @@ -68,13 +68,6 @@ func (k Keeper) UnstakeEntry(ctx sdk.Context, validator, chainID, creator, unsta } func (k Keeper) UnstakeEntryForce(ctx sdk.Context, chainID, provider, unstakeDescription string) error { - providerAddr, err := sdk.AccAddressFromBech32(provider) - if err != nil { - return utils.LavaFormatWarning("invalid address", err, - utils.Attribute{Key: "provider", Value: provider}, - ) - } - existingEntry, entryExists := k.epochStorageKeeper.GetStakeEntryCurrent(ctx, chainID, provider) if !entryExists { return utils.LavaFormatWarning("can't unstake Entry, stake entry not found for address", fmt.Errorf("stake entry not found"), @@ -83,7 +76,15 @@ func (k Keeper) UnstakeEntryForce(ctx sdk.Context, chainID, provider, unstakeDes ) } totalAmount := existingEntry.Stake.Amount - delegations := k.stakingKeeper.GetAllDelegatorDelegations(ctx, providerAddr) + vaultAcc, err := sdk.AccAddressFromBech32(existingEntry.Vault) + if err != nil { + return utils.LavaFormatError("can't unstake entry, invalid vault address", err, + utils.LogAttr("provider", provider), + utils.LogAttr("chain", chainID), + utils.LogAttr("vault", existingEntry.Vault), + ) + } + delegations := k.stakingKeeper.GetAllDelegatorDelegations(ctx, vaultAcc) for _, delegation := range delegations { validator, found := k.stakingKeeper.GetValidator(ctx, delegation.GetValidatorAddr()) @@ -105,9 +106,6 @@ func (k Keeper) UnstakeEntryForce(ctx sdk.Context, chainID, provider, unstakeDes } if totalAmount.IsZero() { - existingEntry, _ := k.epochStorageKeeper.GetStakeEntryCurrent(ctx, chainID, provider) - k.epochStorageKeeper.RemoveStakeEntryCurrent(ctx, chainID, existingEntry.Address) - details := map[string]string{ "address": existingEntry.GetAddress(), "chainID": existingEntry.GetChain(), diff --git a/x/pairing/types/expected_keepers.go b/x/pairing/types/expected_keepers.go index 6a8fb13978..b9ba3bed29 100644 --- a/x/pairing/types/expected_keepers.go +++ b/x/pairing/types/expected_keepers.go @@ -102,7 +102,7 @@ type DowntimeKeeper interface { } type DualstakingKeeper interface { - RewardProvidersAndDelegators(ctx sdk.Context, providerAddr string, chainID string, totalReward sdk.Coins, senderModule string, calcOnlyProvider bool, calcOnlyDelegators bool, calcOnlyContributor bool) (providerReward sdk.Coins, totalRewards sdk.Coins, err error) + RewardProvidersAndDelegators(ctx sdk.Context, providerAddr string, chainID string, totalReward sdk.Coins, senderModule string, calcOnlyProvider bool, calcOnlyDelegators bool, calcOnlyContributor bool) (providerReward sdk.Coins, err error) DelegateFull(ctx sdk.Context, delegator string, validator string, provider string, chainID string, amount sdk.Coin) error UnbondFull(ctx sdk.Context, delegator string, validator string, provider string, chainID string, amount sdk.Coin, unstake bool) error GetProviderDelegators(ctx sdk.Context, provider string, epoch uint64) ([]dualstakingtypes.Delegation, error) diff --git a/x/rewards/keeper/iprpc.go b/x/rewards/keeper/iprpc.go index 8ac287e452..90d94380c9 100644 --- a/x/rewards/keeper/iprpc.go +++ b/x/rewards/keeper/iprpc.go @@ -132,7 +132,6 @@ func (k Keeper) distributeIprpcRewards(ctx sdk.Context, iprpcReward types.IprpcR k.handleNoIprpcRewardToProviders(ctx, iprpcReward.SpecFunds) return } - leftovers := sdk.NewCoins() for _, specFund := range iprpcReward.SpecFunds { details := map[string]string{} @@ -178,9 +177,10 @@ func (k Keeper) distributeIprpcRewards(ctx sdk.Context, iprpcReward types.IprpcR UsedReward = UsedRewardTemp // reward the provider - _, _, err := k.dualstakingKeeper.RewardProvidersAndDelegators(ctx, providerCU.Provider, specFund.Spec, providerIprpcReward, string(types.IprpcPoolName), false, false, false) + _, err := k.dualstakingKeeper.RewardProvidersAndDelegators(ctx, providerCU.Provider, specFund.Spec, providerIprpcReward, string(types.IprpcPoolName), false, false, false) if err != nil { - utils.LavaFormatError("failed to send iprpc rewards to provider", err, utils.LogAttr("provider", providerCU)) + // failed sending the rewards, add the claimable rewards to the leftovers that will be transferred to the community pool + utils.LavaFormatPanic("failed to send iprpc rewards to provider", err, utils.LogAttr("provider", providerCU)) } details[providerCU.Provider] = fmt.Sprintf("cu: %d reward: %s", providerCU.CU, providerIprpcReward.String()) } diff --git a/x/rewards/keeper/providers.go b/x/rewards/keeper/providers.go index e9a45ab093..43dc4b42f9 100644 --- a/x/rewards/keeper/providers.go +++ b/x/rewards/keeper/providers.go @@ -88,7 +88,7 @@ func (k Keeper) distributeMonthlyBonusRewards(ctx sdk.Context) { return } // now give the reward the provider contributor and delegators - _, _, err := k.dualstakingKeeper.RewardProvidersAndDelegators(ctx, basepay.Provider, basepay.ChainId, sdk.NewCoins(sdk.NewCoin(k.stakingKeeper.BondDenom(ctx), reward)), string(types.ProviderRewardsDistributionPool), false, false, false) + _, err := k.dualstakingKeeper.RewardProvidersAndDelegators(ctx, basepay.Provider, basepay.ChainId, sdk.NewCoins(sdk.NewCoin(k.stakingKeeper.BondDenom(ctx), reward)), string(types.ProviderRewardsDistributionPool), false, false, false) if err != nil { utils.LavaFormatError("failed to send bonus rewards to provider", err, utils.LogAttr("provider", basepay.Provider)) } @@ -178,10 +178,14 @@ func (k Keeper) specProvidersBasePay(ctx sdk.Context, chainID string, pop bool) } totalBasePay := math.ZeroInt() + stakedBasePays := []types.BasePayWithIndex{} for _, basepay := range basepays { - totalBasePay = totalBasePay.Add(basepay.BasePay.Total) + if _, found := k.epochstorage.GetStakeEntryCurrent(ctx, basepay.ChainId, basepay.Provider); found { + totalBasePay = totalBasePay.Add(basepay.BasePay.Total) + stakedBasePays = append(stakedBasePays, basepay) + } } - return basepays, totalBasePay + return stakedBasePays, totalBasePay } // ContributeToValidatorsAndCommunityPool transfers some of the providers' rewards to the validators and community pool diff --git a/x/rewards/types/expected_keepers.go b/x/rewards/types/expected_keepers.go index a79582f6d4..989c7b78ea 100644 --- a/x/rewards/types/expected_keepers.go +++ b/x/rewards/types/expected_keepers.go @@ -53,7 +53,7 @@ type StakingKeeper interface { } type DualStakingKeeper interface { - RewardProvidersAndDelegators(ctx sdk.Context, providerAddr string, chainID string, totalReward sdk.Coins, senderModule string, calcOnlyProvider bool, calcOnlyDelegators bool, calcOnlyContributor bool) (providerReward sdk.Coins, totalRewards sdk.Coins, err error) + RewardProvidersAndDelegators(ctx sdk.Context, providerAddr string, chainID string, totalReward sdk.Coins, senderModule string, calcOnlyProvider bool, calcOnlyDelegators bool, calcOnlyContributor bool) (providerReward sdk.Coins, err error) // Methods imported from bank should be defined here } diff --git a/x/subscription/keeper/cu_tracker.go b/x/subscription/keeper/cu_tracker.go index a742913b99..6426cffc5b 100644 --- a/x/subscription/keeper/cu_tracker.go +++ b/x/subscription/keeper/cu_tracker.go @@ -195,7 +195,7 @@ func (k Keeper) RewardAndResetCuTracker(ctx sdk.Context, cuTrackerTimerKeyBytes // Note: if the reward function doesn't reward the provider // because he was unstaked, we only print an error and not returning - _, _, err := k.dualstakingKeeper.RewardProvidersAndDelegators(ctx, provider, chainID, sdk.NewCoins(creditToSub), types.ModuleName, false, false, false) + _, err := k.dualstakingKeeper.RewardProvidersAndDelegators(ctx, provider, chainID, sdk.NewCoins(creditToSub), types.ModuleName, false, false, false) if errors.Is(err, epochstoragetypes.ErrProviderNotStaked) || errors.Is(err, epochstoragetypes.ErrStakeStorageNotFound) { utils.LavaFormatWarning("sending provider reward with delegations failed", err, utils.Attribute{Key: "provider", Value: provider}, diff --git a/x/subscription/types/expected_keepers.go b/x/subscription/types/expected_keepers.go index 264fe6f6d1..f459e5e1bc 100644 --- a/x/subscription/types/expected_keepers.go +++ b/x/subscription/types/expected_keepers.go @@ -68,7 +68,7 @@ type TimerStoreKeeper interface { } type DualStakingKeeper interface { - RewardProvidersAndDelegators(ctx sdk.Context, providerAddr string, chainID string, totalReward sdk.Coins, senderModule string, calcOnlyProvider bool, calcOnlyDelegators bool, calcOnlyContributor bool) (providerReward sdk.Coins, totalRewards sdk.Coins, err error) + RewardProvidersAndDelegators(ctx sdk.Context, providerAddr string, chainID string, totalReward sdk.Coins, senderModule string, calcOnlyProvider bool, calcOnlyDelegators bool, calcOnlyContributor bool) (providerReward sdk.Coins, err error) GetDelegation(ctx sdk.Context, delegator, provider, chainID string, epoch uint64) (dualstakingtypes.Delegation, bool) }