Skip to content

Commit

Permalink
feat!: refactor consumer rewards (#2323)
Browse files Browse the repository at this point in the history
* move big chunk of AllocateTokens to new AllocateConsumerRewards method

* update protos to included allowlisted denoms

* added state and main functionality

* fixed integration tests

* add changelogs

* read the returned error

* took into account comments

* remove unused state

* refactor

* fix tests

---------

Co-authored-by: insumity <karolos@informal.systems>
  • Loading branch information
sainoe and insumity authored Oct 4, 2024
1 parent 123e1e4 commit 9d224a3
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 17 deletions.
10 changes: 5 additions & 5 deletions tests/integration/distribution.go
Original file line number Diff line number Diff line change
Expand Up @@ -910,14 +910,14 @@ func (s *CCVTestSuite) TestAllocateTokensToConsumerValidators() {
}

// allocate tokens
res := providerKeeper.AllocateTokensToConsumerValidators(
err = providerKeeper.AllocateTokensToConsumerValidators(
ctx,
consumerId,
tc.tokens,
)

// check that the expected result is returned
s.Require().Equal(tc.expAllocated, res)
// check that no error is returned
s.Require().NoError(err)

if !tc.expAllocated.Empty() {
// rewards are expected to be allocated evenly between validators
Expand Down Expand Up @@ -1026,14 +1026,14 @@ func (s *CCVTestSuite) TestAllocateTokensToConsumerValidatorsWithDifferentValida
}

// allocate tokens
res := providerKeeper.AllocateTokensToConsumerValidators(
err = providerKeeper.AllocateTokensToConsumerValidators(
ctx,
consumerId,
tokens,
)

// check that the expected result is returned
s.Require().Equal(expAllocated, res)
s.Require().NoError(err)

// rewards are expected to be allocated evenly between validators 3 and 4
rewardsPerVal := expAllocated.QuoDec(math.LegacyNewDec(int64(2)))
Expand Down
8 changes: 8 additions & 0 deletions testutil/integration/debug_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,14 @@ func TestAllocateTokens(t *testing.T) {
runCCVTestByName(t, "TestAllocateTokens")
}

func TestAllocateTokensToConsumerValidatorsWithDifferentValidatorHeights(t *testing.T) {
runCCVTestByName(t, "TestAllocateTokensToConsumerValidatorsWithDifferentValidatorHeights")
}

func TestAllocateTokensToConsumerValidators(t *testing.T) {
runCCVTestByName(t, "TestAllocateTokensToConsumerValidators")
}

func TestMultiConsumerRewardsDistribution(t *testing.T) {
runCCVTestByName(t, "TestMultiConsumerRewardsDistribution")
}
Expand Down
29 changes: 17 additions & 12 deletions x/ccv/provider/keeper/distribution.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,11 +212,19 @@ func (k Keeper) AllocateConsumerRewards(ctx sdk.Context, consumerId string, allo
}

// allocate tokens to consumer validators
k.AllocateTokensToConsumerValidators(
if err := k.AllocateTokensToConsumerValidators(
ctx,
consumerId,
sdk.NewDecCoinsFromCoins(validatorsRewardsTrunc...),
)
); err != nil {
k.Logger(ctx).Error(
"fail to allocate ICS rewards to validators",
"consumerId", consumerId,
"chainId", chainId,
"error", err.Error(),
)
return types.ConsumerRewardsAllocation{}, err
}

// allocate remaining rewards to the community pool
remainingRewards, remainingChanges := remaining.TruncateDecimal()
Expand Down Expand Up @@ -333,16 +341,16 @@ func (k Keeper) AllocateTokensToConsumerValidators(
ctx sdk.Context,
consumerId string,
tokens sdk.DecCoins,
) (allocated sdk.DecCoins) {
) (err error) {
// return early if the tokens are empty
if tokens.Empty() {
return allocated
return nil
}

// get the total voting power of the consumer valset
totalPower := math.LegacyNewDec(k.ComputeConsumerTotalVotingPower(ctx, consumerId))
if totalPower.IsZero() {
return allocated
return nil
}

// Allocate tokens by iterating over the consumer validators
Expand All @@ -354,7 +362,7 @@ func (k Keeper) AllocateTokensToConsumerValidators(
"error",
err,
)
return allocated
return err
}
for _, consumerVal := range consumerVals {
// if a validator is not eligible, this means that the other eligible validators would get more rewards
Expand All @@ -379,7 +387,7 @@ func (k Keeper) AllocateTokensToConsumerValidators(
"error",
err,
)
continue
return err
}

// check if the validator set a custom commission rate for the consumer chain
Expand All @@ -397,14 +405,11 @@ func (k Keeper) AllocateTokensToConsumerValidators(
if err != nil {
k.Logger(ctx).Error("fail to allocate tokens to validator :%s while allocating rewards from consumer chain: %s",
consAddr, consumerId)
continue
return err
}

// sum the tokens allocated
allocated = allocated.Add(tokensFraction...)
}

return allocated
return nil
}

// consumer reward pools getter and setter
Expand Down

0 comments on commit 9d224a3

Please sign in to comment.