Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: PSS enable per-consumer chain commission #1657

Merged
merged 16 commits into from
Mar 8, 2024
2 changes: 1 addition & 1 deletion proto/interchain_security/ccv/provider/v1/provider.proto
Original file line number Diff line number Diff line change
Expand Up @@ -325,4 +325,4 @@ message OptedInValidator {
int64 power = 3;
// public key used by the validator on the consumer
bytes public_key = 4;
}
}
23 changes: 22 additions & 1 deletion proto/interchain_security/ccv/provider/v1/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ service Msg {
rpc SubmitConsumerDoubleVoting(MsgSubmitConsumerDoubleVoting) returns (MsgSubmitConsumerDoubleVotingResponse);
rpc OptIn(MsgOptIn) returns (MsgOptInResponse);
rpc OptOut(MsgOptOut) returns (MsgOptOutResponse);
rpc SetConsumerCommissionRate(MsgSetConsumerCommissionRate) returns (MsgSetConsumerCommissionRateResponse);
}

message MsgAssignConsumerKey {
Expand Down Expand Up @@ -88,4 +89,24 @@ message MsgOptOut {
string provider_addr = 2 [ (gogoproto.moretags) = "yaml:\"address\"" ];
}

message MsgOptOutResponse {}
message MsgOptOutResponse {}

// MsgSetConsumerCommissionRate allows validators to set
// a per-consumer chain commission rate
message MsgSetConsumerCommissionRate {
option (gogoproto.equal) = false;
option (gogoproto.goproto_getters) = false;
// The chain id of the consumer chain to set a commission rate
mpoke marked this conversation as resolved.
Show resolved Hide resolved
string chain_id = 1;
sainoe marked this conversation as resolved.
Show resolved Hide resolved
// The validator on the provider
string provider_addr = 2 [ (gogoproto.moretags) = "yaml:\"address\"" ];
// The rate to charge delegators on the consumer chain, as a fraction
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to call this commission_rate_fraction?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would, but I'd prefer to keep the naming aligned with the SDK code and not too long.

string rate = 3 [
(cosmos_proto.scalar) = "cosmos.Dec",
(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Dec",
(gogoproto.nullable) = false
];
}


message MsgSetConsumerCommissionRateResponse {}
119 changes: 85 additions & 34 deletions tests/integration/distribution.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"cosmossdk.io/math"
abci "github.com/cometbft/cometbft/abci/types"
"github.com/cometbft/cometbft/libs/bytes"
"github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
distrtypes "github.com/cosmos/cosmos-sdk/x/distribution/types"
transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
Expand Down Expand Up @@ -575,7 +575,7 @@ func (s *CCVTestSuite) TestIBCTransferMiddleware() {
bankKeeper := s.providerApp.GetTestBankKeeper()
amount := sdk.NewInt(100)

data = types.NewFungibleTokenPacketData( // can be explicitly changed in setup
data = transfertypes.NewFungibleTokenPacketData( // can be explicitly changed in setup
sdk.DefaultBondDenom,
amount.String(),
authtypes.NewModuleAddress(consumertypes.ConsumerToSendToProviderName).String(),
Expand Down Expand Up @@ -734,11 +734,11 @@ func (s *CCVTestSuite) TestAllocateTokens() {
perValExpReward := validatorsExpRewards.QuoDec(sdk.NewDec(int64(valNum)))

// verify the validator tokens allocation
// note all validators have the same voting power to keep things simple
// note that the validators have the same voting power to keep things simple
for _, val := range s.providerChain.Vals.Validators {
valReward := distributionKeeper.GetValidatorOutstandingRewards(s.providerCtx(), sdk.ValAddress(val.Address))
valRewards := distributionKeeper.GetValidatorOutstandingRewards(s.providerCtx(), sdk.ValAddress(val.Address))
s.Require().Equal(
valReward.Rewards,
valRewards.Rewards,
lastValOutRewards[sdk.ValAddress(val.Address).String()].Add(perValExpReward...),
)
}
Expand Down Expand Up @@ -902,80 +902,131 @@ func (s *CCVTestSuite) prepareRewardDist() {
}

func (s *CCVTestSuite) TestAllocateTokensToValidator() {

providerkeepr := s.providerApp.GetProviderKeeper()
providerKeeper := s.providerApp.GetProviderKeeper()
distributionKeeper := s.providerApp.GetTestDistributionKeeper()
bankKeeper := s.providerApp.GetTestBankKeeper()

chainID := "consumer"

validators := []bytes.HexBytes{
s.providerChain.Vals.Validators[0].Address,
s.providerChain.Vals.Validators[1].Address,
}

votes := []abci.VoteInfo{
{Validator: abci.Validator{Address: validators[0], Power: 1}},
{Validator: abci.Validator{Address: validators[1], Power: 1}},
}

testCases := []struct {
name string
votes []abci.VoteInfo
tokens sdk.DecCoins
expCoinTransferred sdk.DecCoins
name string
votes []abci.VoteInfo
tokens sdk.DecCoins
expAllocated sdk.DecCoins
}{
{
name: "reward tokens are empty",
name: "tokens are empty",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not have expAllocated: nil similar to the next test case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Omitted fields in structs are zero-valued, so expAllocated is set to nil.

},
{
name: "total voting power is zero",
tokens: sdk.DecCoins{sdk.NewDecCoin("uatom", math.NewInt(100_000))},
name: "total voting power is zero",
tokens: sdk.DecCoins{sdk.NewDecCoin(sdk.DefaultBondDenom, math.NewInt(100_000))},
expAllocated: nil,
},
{
name: "expect all tokens to be allocated to a single validator",
votes: []abci.VoteInfo{votes[0]},
tokens: sdk.DecCoins{sdk.NewDecCoin("uatom", math.NewInt(100_000))},
expCoinTransferred: sdk.DecCoins{sdk.NewDecCoin("uatom", math.NewInt(100_000))},
name: "expect all tokens to be allocated to a single validator",
votes: []abci.VoteInfo{votes[0]},
tokens: sdk.DecCoins{sdk.NewDecCoin(sdk.DefaultBondDenom, math.NewInt(999))},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are the expAllocated different (less) from the previous expCoinTransferred? It seems that with 100% commission rate, we would expect this to be more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because both tokens and expAllocated were changed to 999.

expAllocated: sdk.DecCoins{sdk.NewDecCoin(sdk.DefaultBondDenom, math.NewInt(999))},
},
{
name: "expect tokens to be allocated evenly between validators",
votes: []abci.VoteInfo{votes[0], votes[1]},
tokens: sdk.DecCoins{sdk.NewDecCoin("uatom", math.NewInt(555_555))},
expCoinTransferred: sdk.DecCoins{sdk.NewDecCoin("uatom", math.NewInt(555_555))},
name: "expect tokens to be allocated evenly between validators",
votes: []abci.VoteInfo{votes[0], votes[1]},
tokens: sdk.DecCoins{sdk.NewDecCoinFromDec(sdk.DefaultBondDenom, math.LegacyNewDecFromIntWithPrec(math.NewInt(999), 2))},
expAllocated: sdk.DecCoins{sdk.NewDecCoinFromDec(sdk.DefaultBondDenom, math.LegacyNewDecFromIntWithPrec(math.NewInt(999), 2))},
},
}

// opt the validators in to verify that they charge
// a custom commission rate on the consumer chain
for _, v := range s.providerChain.Vals.Validators {
consAddr := sdk.ConsAddress(v.Address)
provAddr := providertypes.NewProviderConsAddress(consAddr)

val, found := s.providerApp.GetTestStakingKeeper().GetValidatorByConsAddr(s.providerCtx(), consAddr)
s.Require().True(found)

// check that no commission rate is set for the validator
s.Require().Equal(val.Commission.Rate, math.LegacyZeroDec())

// set a custom commission rate of 100%
providerKeeper.SetConsumerCommissionRate(
s.providerCtx(),
chainID,
provAddr,
sdk.OneDec(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to test something besides 100% commission rate here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Addressed here.

)
}

for _, tc := range testCases {
s.Run(tc.name, func() {
// TODO: opt validators in and verify
// that rewards are solely allocated to them

// that rewards are only allocated to them
ctx, _ := s.providerCtx().CacheContext()

// allocate tokens
res := providerkeepr.AllocateTokensToConsumerValidators(
res := providerKeeper.AllocateTokensToConsumerValidators(
ctx,
chainID,
tc.votes,
tc.tokens,
)

// check that the expect result is returned
s.Require().Equal(tc.expCoinTransferred, res)
// check that the expected result is returned
s.Require().Equal(tc.expAllocated, res)

if !tc.expCoinTransferred.Empty() {
if !tc.expAllocated.Empty() {
// rewards are expected to be allocated evenly between validators
rewardsPerVal := tc.expCoinTransferred.QuoDec(sdk.NewDec(int64(len(tc.votes))))
rewardsPerVal := tc.expAllocated.QuoDec(sdk.NewDec(int64(len(tc.votes))))

// check that the rewards are allocated to validators as expected
// check that the rewards are allocated to validators
for _, v := range tc.votes {
valAddr := sdk.ValAddress(v.Validator.Address)
rewards := s.providerApp.GetTestDistributionKeeper().GetValidatorOutstandingRewards(
ctx,
sdk.ValAddress(v.Validator.Address),
valAddr,
)
s.Require().Equal(rewardsPerVal, rewards.Rewards)

// send rewards to the distribution module
valRewardsTrunc, _ := rewards.Rewards.TruncateDecimal()
err := bankKeeper.SendCoinsFromAccountToModule(
ctx,
s.providerChain.SenderAccount.GetAddress(),
distrtypes.ModuleName,
valRewardsTrunc)
s.Require().NoError(err)

// check that validators can withdraw their rewards
withdrawnCoins, err := distributionKeeper.WithdrawValidatorCommission(
ctx,
valAddr,
)
s.Require().NoError(err)

// check that the withdrawn coins is equal to the entire reward amount
s.Require().Equal(withdrawnCoins, valRewardsTrunc)

// check that validators get rewards in their balance
s.Require().Equal(withdrawnCoins, bankKeeper.GetAllBalances(ctx, sdk.AccAddress(valAddr)))
}
} else {
for _, v := range tc.votes {
valAddr := sdk.ValAddress(v.Validator.Address)
rewards := s.providerApp.GetTestDistributionKeeper().GetValidatorOutstandingRewards(
ctx,
valAddr,
)
s.Require().Zero(rewards.Rewards)
}
}

})
}
}
1 change: 1 addition & 0 deletions testutil/integration/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ type TestDistributionKeeper interface {
GetValidatorOutstandingRewards(ctx sdk.Context,
val sdk.ValAddress) (rewards distributiontypes.ValidatorOutstandingRewards)
GetCommunityTax(ctx sdk.Context) (percent sdk.Dec)
WithdrawValidatorCommission(ctx sdk.Context, valAddr sdk.ValAddress) (sdk.Coins, error)
}

type TestMintKeeper interface {
Expand Down
128 changes: 0 additions & 128 deletions testutil/keeper/mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading