Skip to content

Commit

Permalink
fix!: enable the submission of allowlisted reward denoms (#2326)
Browse files Browse the repository at this point in the history
* init commit

* fix docs

* add validation
  • Loading branch information
insumity authored Oct 4, 2024
1 parent 9d224a3 commit 47cd04e
Show file tree
Hide file tree
Showing 14 changed files with 507 additions and 352 deletions.
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 @@ -362,7 +362,7 @@ message MsgCreateConsumer {
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,
}, 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

0 comments on commit 47cd04e

Please sign in to comment.