-
Notifications
You must be signed in to change notification settings - Fork 134
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
Changes from 4 commits
0ebc050
429d5ae
a3c9c77
638d4e2
4e5f44b
98ce2b1
2039258
8361601
0cacd79
75ebf4a
a4c5754
f2e44c6
8e2111a
2951e9b
d8ecbb7
dd98446
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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" | ||
|
@@ -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(), | ||
|
@@ -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...), | ||
) | ||
} | ||
|
@@ -902,80 +902,129 @@ 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", | ||
}, | ||
{ | ||
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))}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because both |
||
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), 3))}, | ||
expAllocated: sdk.DecCoins{sdk.NewDecCoinFromDec(sdk.DefaultBondDenom, math.LegacyNewDecFromIntWithPrec(math.NewInt(999), 3))}, | ||
}, | ||
} | ||
|
||
// opt the validators in to verify that they charge | ||
// to their delegators the custom commission for 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 is set for the validator | ||
s.Require().Equal(val.Commission.Rate, math.LegacyZeroDec()) | ||
|
||
// set a custom commission of 100% | ||
providerKeeper.SetConsumerCommissionRate( | ||
s.providerCtx(), | ||
chainID, | ||
provAddr, | ||
sdk.OneDec(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it make sense to test something besides 100% commission rate here? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 tokens to the distribution module to allow the rewards withdrawing | ||
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) | ||
} | ||
} else { | ||
for _, v := range tc.votes { | ||
valAddr := sdk.ValAddress(v.Validator.Address) | ||
rewards := s.providerApp.GetTestDistributionKeeper().GetValidatorOutstandingRewards( | ||
ctx, | ||
valAddr, | ||
) | ||
s.Require().Zero(rewards.Rewards) | ||
} | ||
} | ||
|
||
}) | ||
} | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
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?There was a problem hiding this comment.
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 tonil
.