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

fix!: enable the submission of allowlisted reward denoms #2326

Merged
merged 3 commits into from
Oct 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions docs/docs/build/modules/02-provider.md
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ message MsgChangeRewardDenoms {
`MsgCreateConsumer` enables a user to create a consumer chain.

Both the `chain_id` and `metadata` fields are mandatory.
Both the `initialization_parameters` and `power_shaping_parameters` fields are optional.
The `initialization_parameters`, `power_shaping_parameters`, and `allowlisted_reward_denoms` fields are optional.
The parameters not provided are set to their zero value.

The owner of the created consumer chain is the submitter of the message.
Expand Down Expand Up @@ -507,6 +507,9 @@ message MsgCreateConsumer {
ConsumerInitializationParameters initialization_parameters = 4;

PowerShapingParameters power_shaping_parameters = 5;

// allowlisted reward denoms by the consumer chain
AllowlistedRewardDenoms allowlisted_reward_denoms = 6;
}
```

Expand All @@ -516,7 +519,7 @@ message MsgCreateConsumer {

Note that only the `owner` (i.e., signer) and `consumer_id` fields are mandatory.
The others field are optional. Not providing one of them will leave the existing values unchanged.
Providing one of `metadata`, `initialization_parameters` or `power_shaping_parameters`,
Providing one of `metadata`, `initialization_parameters`, `power_shaping_parameters`, or `allowlisted_reward_denoms`
will update all the containing fields.
If one of the containing fields is missing, it will be set to its zero value.
For example, updating the `initialization_parameters` without specifying the `spawn_time`, will set the `spawn_time` to zero.
Expand All @@ -525,7 +528,7 @@ If the `initialization_parameters` field is set and `initialization_parameters.s
Updating the `spawn_time` from a positive value to zero will remove the consumer chain from the list of scheduled to launch chains.
If the consumer chain is already launched, updating the `initialization_parameters` is no longer possible.

If the `power_shaping_parameters` field is set and `power_shaping_parameters.top_N` is possitive, then the owner needs to be the gov module account address.
If the `power_shaping_parameters` field is set and `power_shaping_parameters.top_N` is positive, then the owner needs to be the gov module account address.

If the `new_owner_address` field is set to a value different than the gov module account address, then `top_N` needs to be zero.

Expand All @@ -550,6 +553,9 @@ message MsgUpdateConsumer {

// the power-shaping parameters of the consumer when updated
PowerShapingParameters power_shaping_parameters = 6;

// allowlisted reward denoms by the consumer chain
AllowlistedRewardDenoms allowlisted_reward_denoms = 7;
}
```

Expand Down Expand Up @@ -1675,6 +1681,9 @@ where `update-consumer-msg.json` contains:
"denylist":[],
"min_stake": "1000",
"allow_inactive_vals":true
},
"allowlisted_reward_denoms": {
"denoms": ["ibc/0025F8A87464A471E66B234C4F93AEC5B4DA3D42D7986451A059273426290DD5"]
}
}
```
Expand Down
8 changes: 8 additions & 0 deletions docs/docs/consumer-development/onboarding.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,14 @@ Example of power-shaping parameters:
}
```

Example of allowlisted reward denoms:
```js
// AllowlistedRewardDenoms provided in MsgCreateConsumer or MsgUpdateConsumer
{
"denoms": ["ibc/0025F8A87464A471E66B234C4F93AEC5B4DA3D42D7986451A059273426290DD5", "ibc/054892D6BB43AF8B93AAC28AA5FD7019D2C59A15DAFD6F45C1FA2BF9BDA22454"]
}
```

## 4. Launch

The consumer chain starts after at least 66.67% of its voting power comes online.
Expand Down
2 changes: 2 additions & 0 deletions proto/interchain_security/ccv/provider/v1/query.proto
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,8 @@ message Chain {
// Corresponds to whether inactive validators are allowed to validate the consumer chain.
bool allow_inactive_vals = 12;
string consumer_id = 13;
// the reward denoms allowlisted by this consumer chain
AllowlistedRewardDenoms allowlisted_reward_denoms = 14;
}

message QueryValidatorConsumerAddrRequest {
Expand Down
2 changes: 1 addition & 1 deletion proto/interchain_security/ccv/provider/v1/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@
message MsgConsumerModificationResponse {}

// MsgCreateConsumer defines the message that creates a consumer chain
message MsgCreateConsumer {

Check failure on line 348 in proto/interchain_security/ccv/provider/v1/tx.proto

View workflow job for this annotation

GitHub Actions / break-check

Previously present field "7" with name "allowlisted_reward_denoms" on message "MsgCreateConsumer" was deleted.
option (cosmos.msg.v1.signer) = "submitter";

// Submitter address. If the message is successfully handled, the ownership of
Expand All @@ -362,7 +362,7 @@
PowerShapingParameters power_shaping_parameters = 5;

// allowlisted reward denoms of the consumer
AllowlistedRewardDenoms allowlisted_reward_denoms = 7;
AllowlistedRewardDenoms allowlisted_reward_denoms = 6;
}

// MsgCreateConsumerResponse defines response type for MsgCreateConsumer
Expand Down
18 changes: 13 additions & 5 deletions x/ccv/provider/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,11 +255,14 @@ where create_consumer.json has the following structure:
"denylist": [],
"min_stake": "0",
"allow_inactive_vals": false
},
"allowlisted_reward_denoms": {
"denoms": ["ibc/...", "ibc/..."]
}
}

Note that both 'chain_id' and 'metadata' are mandatory;
and both 'initialization_parameters' and 'power_shaping_parameters' are optional.
and 'initialization_parameters', 'power_shaping_parameters' and 'allowlisted_reward_denoms' are optional.
The parameters not provided are set to their zero value.
`, version.AppName)),
Args: cobra.ExactArgs(1),
Expand All @@ -286,7 +289,8 @@ The parameters not provided are set to their zero value.
return fmt.Errorf("consumer data unmarshalling failed: %w", err)
}

msg, err := types.NewMsgCreateConsumer(submitter, consCreate.ChainId, consCreate.Metadata, consCreate.InitializationParameters, consCreate.PowerShapingParameters)
msg, err := types.NewMsgCreateConsumer(submitter, consCreate.ChainId, consCreate.Metadata, consCreate.InitializationParameters,
consCreate.PowerShapingParameters, consCreate.AllowlistedRewardDenoms)
if err != nil {
return err
}
Expand Down Expand Up @@ -349,12 +353,15 @@ where update_consumer.json has the following structure:
"denylist": [],
"min_stake": "0",
"allow_inactive_vals": false
}
},
"allowlisted_reward_denoms": {
"denoms": ["ibc/...", "ibc/..."]
}
}

Note that only 'consumer_id' is mandatory. The others are optional.
Not providing one of them will leave the existing values unchanged.
Providing one of 'metadata', 'initialization_parameters' or 'power_shaping_parameters',
Providing one of 'metadata', 'initialization_parameters', 'power_shaping_parameters', or 'allowlisted_reward_denoms'
will update all the containing fields.
If one of the fields is missing, it will be set to its zero value.
`, version.AppName)),
Expand Down Expand Up @@ -387,7 +394,8 @@ If one of the fields is missing, it will be set to its zero value.
return fmt.Errorf("consumer_id can't be empty")
}

msg, err := types.NewMsgUpdateConsumer(owner, consUpdate.ConsumerId, consUpdate.NewOwnerAddress, consUpdate.Metadata, consUpdate.InitializationParameters, consUpdate.PowerShapingParameters)
msg, err := types.NewMsgUpdateConsumer(owner, consUpdate.ConsumerId, consUpdate.NewOwnerAddress, consUpdate.Metadata,
consUpdate.InitializationParameters, consUpdate.PowerShapingParameters, consUpdate.AllowlistedRewardDenoms)
if err != nil {
return err
}
Expand Down
37 changes: 23 additions & 14 deletions x/ccv/provider/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,22 +114,31 @@ func (k Keeper) GetConsumerChain(ctx sdk.Context, consumerId string) (types.Chai
strDenylist[i] = addr.String()
}

metadata, _ := k.GetConsumerMetadata(ctx, consumerId)
metadata, err := k.GetConsumerMetadata(ctx, consumerId)
if err != nil {
return types.Chain{}, fmt.Errorf("cannot find metadata (%s): %s", consumerId, err.Error())
}

allowlistedRewardDenoms, err := k.GetAllowlistedRewardDenoms(ctx, consumerId)
if err != nil {
return types.Chain{}, fmt.Errorf("cannot find allowlisted reward denoms (%s): %s", consumerId, err.Error())
}

return types.Chain{
ChainId: chainID,
ClientId: clientID,
Top_N: powerShapingParameters.Top_N,
MinPowerInTop_N: minPowerInTopN,
ValidatorSetCap: powerShapingParameters.ValidatorSetCap,
ValidatorsPowerCap: powerShapingParameters.ValidatorsPowerCap,
Allowlist: strAllowlist,
Denylist: strDenylist,
Phase: k.GetConsumerPhase(ctx, consumerId).String(),
Metadata: metadata,
AllowInactiveVals: powerShapingParameters.AllowInactiveVals,
MinStake: powerShapingParameters.MinStake,
ConsumerId: consumerId,
ChainId: chainID,
ClientId: clientID,
Top_N: powerShapingParameters.Top_N,
MinPowerInTop_N: minPowerInTopN,
ValidatorSetCap: powerShapingParameters.ValidatorSetCap,
ValidatorsPowerCap: powerShapingParameters.ValidatorsPowerCap,
Allowlist: strAllowlist,
Denylist: strDenylist,
Phase: k.GetConsumerPhase(ctx, consumerId).String(),
Metadata: metadata,
AllowInactiveVals: powerShapingParameters.AllowInactiveVals,
MinStake: powerShapingParameters.MinStake,
ConsumerId: consumerId,
AllowlistedRewardDenoms: &types.AllowlistedRewardDenoms{Denoms: allowlistedRewardDenoms},
}, nil
}

Expand Down
54 changes: 32 additions & 22 deletions x/ccv/provider/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,12 @@ func TestGetConsumerChain(t *testing.T) {

metadataLists := []types.ConsumerMetadata{}

allowlistedRewardDenoms := []*types.AllowlistedRewardDenoms{
{}, // no denoms
{Denoms: []string{"ibc/1", "ibc/2"}},
{Denoms: []string{"ibc/3", "ibc/4", "ibc/5"}},
{Denoms: []string{"ibc/6"}}}

expectedGetAllOrder := []types.Chain{}
for i, consumerID := range consumerIDs {
pk.SetConsumerChainId(ctx, consumerID, chainIDs[i])
Expand Down Expand Up @@ -493,24 +499,27 @@ func TestGetConsumerChain(t *testing.T) {
metadataLists = append(metadataLists, types.ConsumerMetadata{Name: chainIDs[i]})
pk.SetConsumerMetadata(ctx, consumerID, metadataLists[i])

pk.SetAllowlistedRewardDenoms(ctx, consumerID, allowlistedRewardDenoms[i].Denoms)

phase := types.ConsumerPhase(int32(i + 1))
pk.SetConsumerPhase(ctx, consumerID, phase)

expectedGetAllOrder = append(expectedGetAllOrder,
types.Chain{
ChainId: chainIDs[i],
ClientId: clientID,
Top_N: topN,
MinPowerInTop_N: expectedMinPowerInTopNs[i],
ValidatorSetCap: validatorSetCaps[i],
ValidatorsPowerCap: validatorPowerCaps[i],
Allowlist: strAllowlist,
Denylist: strDenylist,
Phase: phase.String(),
Metadata: metadataLists[i],
AllowInactiveVals: allowInactiveVals[i],
MinStake: minStakes[i].Uint64(),
ConsumerId: consumerIDs[i],
ChainId: chainIDs[i],
ClientId: clientID,
Top_N: topN,
MinPowerInTop_N: expectedMinPowerInTopNs[i],
ValidatorSetCap: validatorSetCaps[i],
ValidatorsPowerCap: validatorPowerCaps[i],
Allowlist: strAllowlist,
Denylist: strDenylist,
Phase: phase.String(),
Metadata: metadataLists[i],
AllowInactiveVals: allowInactiveVals[i],
MinStake: minStakes[i].Uint64(),
ConsumerId: consumerIDs[i],
AllowlistedRewardDenoms: allowlistedRewardDenoms[i],
})
}

Expand Down Expand Up @@ -647,15 +656,16 @@ func TestQueryConsumerChains(t *testing.T) {

pk.SetConsumerPhase(ctx, consumerId, phases[i])
c := types.Chain{
ChainId: chainID,
MinPowerInTop_N: -1,
ValidatorsPowerCap: 0,
ValidatorSetCap: 0,
Allowlist: []string{},
Denylist: []string{},
Phase: phases[i].String(),
Metadata: metadata,
ConsumerId: consumerId,
ChainId: chainID,
MinPowerInTop_N: -1,
ValidatorsPowerCap: 0,
ValidatorSetCap: 0,
Allowlist: []string{},
Denylist: []string{},
Phase: phases[i].String(),
Metadata: metadata,
ConsumerId: consumerId,
AllowlistedRewardDenoms: &types.AllowlistedRewardDenoms{Denoms: []string{}},
}
consumerIds[i] = consumerId
consumers[i] = &c
Expand Down
10 changes: 0 additions & 10 deletions x/ccv/provider/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -415,11 +415,6 @@ func (k msgServer) CreateConsumer(goCtx context.Context, msg *types.MsgCreateCon
}

if msg.AllowlistedRewardDenoms != nil {
if len(msg.AllowlistedRewardDenoms.Denoms) > types.MaxAllowlistedRewardDenomsPerChain {
return &resp, errorsmod.Wrapf(types.ErrInvalidAllowlistedRewardDenoms,
fmt.Sprintf("a consumer chain cannot allowlist more than %d reward denoms", types.MaxAllowlistedRewardDenomsPerChain))
}

err := k.UpdateAllowlistedRewardDenoms(ctx, consumerId, msg.AllowlistedRewardDenoms.Denoms)
if err != nil {
return &resp, errorsmod.Wrapf(types.ErrInvalidAllowlistedRewardDenoms,
Expand Down Expand Up @@ -603,11 +598,6 @@ func (k msgServer) UpdateConsumer(goCtx context.Context, msg *types.MsgUpdateCon
}

if msg.AllowlistedRewardDenoms != nil {
if len(msg.AllowlistedRewardDenoms.Denoms) > types.MaxAllowlistedRewardDenomsPerChain {
return &resp, errorsmod.Wrapf(types.ErrInvalidAllowlistedRewardDenoms,
fmt.Sprintf("a consumer chain cannot allowlist more than %d reward denoms", types.MaxAllowlistedRewardDenomsPerChain))
}

if err := k.UpdateAllowlistedRewardDenoms(ctx, consumerId, msg.AllowlistedRewardDenoms.Denoms); err != nil {
return &resp, errorsmod.Wrapf(types.ErrInvalidAllowlistedRewardDenoms,
"cannot update allowlisted reward denoms: %s", err.Error())
Expand Down
31 changes: 31 additions & 0 deletions x/ccv/provider/types/msg.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package types
import (
"encoding/json"
"fmt"
"github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
"strings"

ibctmtypes "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint"
Expand Down Expand Up @@ -280,13 +281,15 @@ func (msg MsgSetConsumerCommissionRate) ValidateBasic() error {
// NewMsgCreateConsumer creates a new MsgCreateConsumer instance
func NewMsgCreateConsumer(submitter, chainId string, metadata ConsumerMetadata,
initializationParameters *ConsumerInitializationParameters, powerShapingParameters *PowerShapingParameters,
allowlistedRewardDenoms *AllowlistedRewardDenoms,
) (*MsgCreateConsumer, error) {
return &MsgCreateConsumer{
Submitter: submitter,
ChainId: chainId,
Metadata: metadata,
InitializationParameters: initializationParameters,
PowerShapingParameters: powerShapingParameters,
AllowlistedRewardDenoms: allowlistedRewardDenoms,
insumity marked this conversation as resolved.
Show resolved Hide resolved
}, nil
}

Expand Down Expand Up @@ -326,12 +329,19 @@ func (msg MsgCreateConsumer) ValidateBasic() error {
}
}

if msg.AllowlistedRewardDenoms != nil {
if err := ValidateAllowlistedRewardDenoms(*msg.AllowlistedRewardDenoms); err != nil {
return errorsmod.Wrapf(ErrInvalidMsgCreateConsumer, "AllowlistedRewardDenoms: %s", err.Error())
}
}

return nil
}

// NewMsgUpdateConsumer creates a new MsgUpdateConsumer instance
func NewMsgUpdateConsumer(owner, consumerId, ownerAddress string, metadata *ConsumerMetadata,
initializationParameters *ConsumerInitializationParameters, powerShapingParameters *PowerShapingParameters,
allowlistedRewardDenoms *AllowlistedRewardDenoms,
) (*MsgUpdateConsumer, error) {
return &MsgUpdateConsumer{
Owner: owner,
Expand All @@ -340,6 +350,7 @@ func NewMsgUpdateConsumer(owner, consumerId, ownerAddress string, metadata *Cons
Metadata: metadata,
InitializationParameters: initializationParameters,
PowerShapingParameters: powerShapingParameters,
AllowlistedRewardDenoms: allowlistedRewardDenoms,
}, nil
}

Expand Down Expand Up @@ -369,6 +380,12 @@ func (msg MsgUpdateConsumer) ValidateBasic() error {
}
}

if msg.AllowlistedRewardDenoms != nil {
if err := ValidateAllowlistedRewardDenoms(*msg.AllowlistedRewardDenoms); err != nil {
return errorsmod.Wrapf(ErrInvalidMsgUpdateConsumer, "AllowlistedRewardDenoms: %s", err.Error())
}
}

return nil
}

Expand Down Expand Up @@ -514,6 +531,20 @@ func ValidatePowerShapingParameters(powerShapingParameters PowerShapingParameter
return nil
}

// ValidateAllowlistedRewardDenoms validates the provided allowlisted reward denoms
func ValidateAllowlistedRewardDenoms(allowlistedRewardDenoms AllowlistedRewardDenoms) error {
if len(allowlistedRewardDenoms.Denoms) > MaxAllowlistedRewardDenomsPerChain {
return errorsmod.Wrapf(ErrInvalidAllowlistedRewardDenoms, "More than %d denoms", MaxAllowlistedRewardDenomsPerChain)
}

for _, denom := range allowlistedRewardDenoms.Denoms {
if err := types.ValidateIBCDenom(denom); err != nil {
return errorsmod.Wrapf(ErrInvalidAllowlistedRewardDenoms, "Invalid denom (%s): %s", denom, err.Error())
}
}
return nil
}

// ValidateInitializationParameters validates that all the provided parameters are in the expected range
func ValidateInitializationParameters(initializationParameters ConsumerInitializationParameters) error {
if initializationParameters.InitialHeight.IsZero() {
Expand Down
4 changes: 2 additions & 2 deletions x/ccv/provider/types/msg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ func TestMsgCreateConsumerValidateBasic(t *testing.T) {

for _, tc := range testCases {
validConsumerMetadata := types.ConsumerMetadata{Name: "name", Description: "description", Metadata: "metadata"}
msg, err := types.NewMsgCreateConsumer("submitter", tc.chainId, validConsumerMetadata, nil, tc.powerShapingParameters)
msg, err := types.NewMsgCreateConsumer("submitter", tc.chainId, validConsumerMetadata, nil, tc.powerShapingParameters, nil)
require.NoError(t, err)
err = msg.ValidateBasic()
if tc.expPass {
Expand Down Expand Up @@ -546,7 +546,7 @@ func TestMsgUpdateConsumerValidateBasic(t *testing.T) {

for _, tc := range testCases {
// TODO (PERMISSIONLESS) add more tests
msg, _ := types.NewMsgUpdateConsumer("", "0", "cosmos1p3ucd3ptpw902fluyjzhq3ffgq4ntddac9sa3s", nil, nil, &tc.powerShapingParameters)
msg, _ := types.NewMsgUpdateConsumer("", "0", "cosmos1p3ucd3ptpw902fluyjzhq3ffgq4ntddac9sa3s", nil, nil, &tc.powerShapingParameters, nil)
err := msg.ValidateBasic()
if tc.expPass {
require.NoError(t, err, "valid case: %s should not return error. got %w", tc.name, err)
Expand Down
Loading
Loading