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!: allow consumer chains to change their PSS parameters #1932

Merged
merged 7 commits into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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
33 changes: 32 additions & 1 deletion proto/interchain_security/ccv/provider/v1/provider.proto
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,37 @@ message ConsumerRemovalProposal {
[ (gogoproto.stdtime) = true, (gogoproto.nullable) = false ];
}

// ConsumerModificationProposal is a governance proposal on the provider chain to modify parameters of a running
// consumer chain. If it passes, the consumer chain's state is updated to take into account the newest params.
message ConsumerModificationProposal {
// the title of the proposal
string title = 1;
// the description of the proposal
string description = 2;
// the chain-id of the consumer chain to be modified
string chain_id = 3;
// Corresponds to the percentage of validators that have to validate the chain under the Top N case.
// For example, 53 corresponds to a Top 53% chain, meaning that the top 53% provider validators by voting power
// have to validate the proposed consumer chain. top_N can either be 0 or any value in [50, 100].
// A chain can join with top_N == 0 as an Opt In chain, or with top_N ∈ [50, 100] as a Top N chain.
uint32 top_N = 4;
// Corresponds to the maximum power (percentage-wise) a validator can have on the consumer chain. For instance, if
// `validators_power_cap` is set to 32, it means that no validator can have more than 32% of the voting power on the
// consumer chain. Note that this might not be feasible. For example, think of a consumer chain with only
// 5 validators and with `validators_power_cap` set to 10%. In such a scenario, at least one validator would need
// to have more than 20% of the total voting power. Therefore, `validators_power_cap` operates on a best-effort basis.
uint32 validators_power_cap = 5;
// Corresponds to the maximum number of validators that can validate a consumer chain.
// Only applicable to Opt In chains. Setting `validator_set_cap` on a Top N chain is a no-op.
uint32 validator_set_cap = 6;
// Corresponds to a list of provider consensus addresses of validators that are the ONLY ones that can validate
// the consumer chain.
repeated string allowlist = 7;
// Corresponds to a list of provider consensus addresses of validators that CANNOT validate the consumer chain.
repeated string denylist = 8;
}


// EquivocationProposal is a governance proposal on the provider chain to
// punish a validator for equivocation on a consumer chain.
//
Expand All @@ -133,7 +164,7 @@ message EquivocationProposal {
option deprecated = true;
// the title of the proposal
string title = 1;
// the description of the proposal
// the description of the proposal
string description = 2;
// the list of equivocations that will be processed
repeated cosmos.evidence.v1beta1.Equivocation equivocations = 3;
Expand Down
100 changes: 97 additions & 3 deletions x/ccv/provider/client/proposal_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,10 @@ import (
)

var (
ConsumerAdditionProposalHandler = govclient.NewProposalHandler(SubmitConsumerAdditionPropTxCmd)
ConsumerRemovalProposalHandler = govclient.NewProposalHandler(SubmitConsumerRemovalProposalTxCmd)
ChangeRewardDenomsProposalHandler = govclient.NewProposalHandler(SubmitChangeRewardDenomsProposalTxCmd)
ConsumerAdditionProposalHandler = govclient.NewProposalHandler(SubmitConsumerAdditionPropTxCmd)
ConsumerRemovalProposalHandler = govclient.NewProposalHandler(SubmitConsumerRemovalProposalTxCmd)
ConsumerModificationProposalHandler = govclient.NewProposalHandler(SubmitConsumerModificationProposalTxCmd)
ChangeRewardDenomsProposalHandler = govclient.NewProposalHandler(SubmitChangeRewardDenomsProposalTxCmd)
)

// SubmitConsumerAdditionPropTxCmd returns a CLI command handler for submitting
Expand Down Expand Up @@ -172,6 +173,70 @@ Where proposal.json contains:
}
}

// SubmitConsumerModificationProposalTxCmd returns a CLI command handler for submitting
// a consumer modification proposal via a transaction.
func SubmitConsumerModificationProposalTxCmd() *cobra.Command {
return &cobra.Command{
Use: "consumer-modification [proposal-file]",
Args: cobra.ExactArgs(1),
Short: "Submit a consumer modification proposal",
Long: `
Submit a consumer modification proposal along with an initial deposit.
The proposal details must be supplied via a JSON file.

Example:
$ <appd> tx gov submit-legacy-proposal consumer-modification <path/to/proposal.json> --from=<key_or_address>

Where proposal.json contains:

{
"title": "Modify FooChain",
"summary": "Make it an Opt In chain",
"chain_id": "foochain",
"top_n": 0,
"validators_power_cap": 32,
"validator_set_cap": 50,
"allowlist": [],
"denylist": ["validatorAConsensusAddress", "validatorBConsensusAddress"]
}
`,
RunE: func(cmd *cobra.Command, args []string) error {
clientCtx, err := client.GetClientTxContext(cmd)
if err != nil {
return err
}

proposal, err := ParseConsumerModificationProposalJSON(args[0])
if err != nil {
return err
}

content := types.NewConsumerModificationProposal(
proposal.Title, proposal.Summary, proposal.ChainId, proposal.TopN,
proposal.ValidatorsPowerCap, proposal.ValidatorSetCap, proposal.Allowlist, proposal.Denylist)

from := clientCtx.GetFromAddress()

deposit, err := sdk.ParseCoinsNormalized(proposal.Deposit)
if err != nil {
return err
}

msgContent, err := govv1.NewLegacyContent(content, authtypes.NewModuleAddress(govtypes.ModuleName).String())
if err != nil {
return err
}

msg, err := govv1.NewMsgSubmitProposal([]sdk.Msg{msgContent}, deposit, from.String(), "", content.GetTitle(), proposal.Summary)
if err != nil {
return err
}

return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg)
},
}
}

// SubmitChangeRewardDenomsProposalTxCmd returns a CLI command handler for submitting
// a change reward denoms proposal via a transaction.
func SubmitChangeRewardDenomsProposalTxCmd() *cobra.Command {
Expand Down Expand Up @@ -326,6 +391,35 @@ func ParseConsumerRemovalProposalJSON(proposalFile string) (ConsumerRemovalPropo
return proposal, nil
}

type ConsumerModificationProposalJSON struct {
Title string `json:"title"`
Summary string `json:"summary"`
ChainId string `json:"chain_id"`

TopN uint32 `json:"top_N"`
ValidatorsPowerCap uint32 `json:"validators_power_cap"`
ValidatorSetCap uint32 `json:"validator_set_cap"`
Allowlist []string `json:"allowlist"`
Denylist []string `json:"denylist"`

Deposit string `json:"deposit"`
}

func ParseConsumerModificationProposalJSON(proposalFile string) (ConsumerModificationProposalJSON, error) {
proposal := ConsumerModificationProposalJSON{}

contents, err := os.ReadFile(filepath.Clean(proposalFile))
if err != nil {
return proposal, err
}

if err := json.Unmarshal(contents, &proposal); err != nil {
return proposal, err
}

return proposal, nil
}

type ChangeRewardDenomsProposalJSON struct {
Summary string `json:"summary"`
types.ChangeRewardDenomsProposal
Expand Down
33 changes: 33 additions & 0 deletions x/ccv/provider/keeper/proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,39 @@ func (k Keeper) HandleConsumerRemovalProposal(ctx sdk.Context, p *types.Consumer
return nil
}

// HandleConsumerModificationProposal modifies a running consumer chain
func (k Keeper) HandleConsumerModificationProposal(ctx sdk.Context, p *types.ConsumerModificationProposal) error {
if _, found := k.GetConsumerClientId(ctx, p.ChainId); !found {
return fmt.Errorf("consumer chain (%s) is not runnig", p.ChainId)
}

k.SetTopN(ctx, p.ChainId, p.Top_N)
k.SetValidatorsPowerCap(ctx, p.ChainId, p.ValidatorsPowerCap)
k.SetValidatorSetCap(ctx, p.ChainId, p.ValidatorSetCap)

k.DeleteAllowlist(ctx, p.ChainId)
for _, address := range p.Allowlist {
consAddr, err := sdk.ConsAddressFromBech32(address)
if err != nil {
continue
}

k.SetAllowlist(ctx, p.ChainId, types.NewProviderConsAddress(consAddr))
}

k.DeleteDenylist(ctx, p.ChainId)
for _, address := range p.Denylist {
consAddr, err := sdk.ConsAddressFromBech32(address)
if err != nil {
continue
}

k.SetDenylist(ctx, p.ChainId, types.NewProviderConsAddress(consAddr))
}

return nil
}
Comment on lines +147 to +178
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure error handling when setting Allowlist and Denylist in HandleConsumerModificationProposal. Currently, errors during address conversion are silently ignored.

- if err != nil {
-     continue
+ if err != nil {
+     return fmt.Errorf("failed to convert address in allowlist: %w", err)

Committable suggestion was skipped due low confidence.


// StopConsumerChain cleans up the states for the given consumer chain ID and
// completes the outstanding unbonding operations on the consumer chain.
//
Expand Down
49 changes: 49 additions & 0 deletions x/ccv/provider/keeper/proposal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,55 @@ func TestHandleConsumerRemovalProposal(t *testing.T) {
}
}

func TestHandleConsumerModificationProposal(t *testing.T) {
providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
defer ctrl.Finish()

chainID := "chainID"

// set up a consumer client, so it seems that "chainID" is running
providerKeeper.SetConsumerClientId(ctx, "chainID", "clientID")

// set PSS-related fields to update them later on
providerKeeper.SetTopN(ctx, chainID, 50)
providerKeeper.SetValidatorSetCap(ctx, chainID, 10)
providerKeeper.SetValidatorsPowerCap(ctx, chainID, 34)
providerKeeper.SetAllowlist(ctx, chainID, providertypes.NewProviderConsAddress([]byte("allowlistedAddr1")))
providerKeeper.SetAllowlist(ctx, chainID, providertypes.NewProviderConsAddress([]byte("allowlistedAddr2")))
providerKeeper.SetDenylist(ctx, chainID, providertypes.NewProviderConsAddress([]byte("denylistedAddr1")))

expectedTopN := uint32(75)
expectedValidatorsPowerCap := uint32(67)
expectedValidatorSetCap := uint32(20)
expectedAllowlistedValidator := "cosmosvalcons1wpex7anfv3jhystyv3eq20r35a"
expectedDenylistedValidator := "cosmosvalcons1nx7n5uh0ztxsynn4sje6eyq2ud6rc6klc96w39"
proposal := providertypes.NewConsumerModificationProposal("title", "description", chainID,
expectedTopN,
expectedValidatorsPowerCap,
expectedValidatorSetCap,
[]string{expectedAllowlistedValidator},
[]string{expectedDenylistedValidator},
).(*providertypes.ConsumerModificationProposal)

err := providerKeeper.HandleConsumerModificationProposal(ctx, proposal)
require.NoError(t, err)

actualTopN, _ := providerKeeper.GetTopN(ctx, chainID)
require.Equal(t, expectedTopN, actualTopN)
actualValidatorsPowerCap, _ := providerKeeper.GetValidatorsPowerCap(ctx, chainID)
require.Equal(t, expectedValidatorsPowerCap, actualValidatorsPowerCap)
actualValidatorSetCap, _ := providerKeeper.GetValidatorSetCap(ctx, chainID)
require.Equal(t, expectedValidatorSetCap, actualValidatorSetCap)

allowlistedValidator, err := sdk.ConsAddressFromBech32(expectedAllowlistedValidator)
require.Equal(t, 1, len(providerKeeper.GetAllowList(ctx, chainID)))
require.Equal(t, providertypes.NewProviderConsAddress(allowlistedValidator), providerKeeper.GetAllowList(ctx, chainID)[0])

denylistedValidator, err := sdk.ConsAddressFromBech32(expectedDenylistedValidator)
require.Equal(t, 1, len(providerKeeper.GetDenyList(ctx, chainID)))
require.Equal(t, providertypes.NewProviderConsAddress(denylistedValidator), providerKeeper.GetDenyList(ctx, chainID)[0])
}
Comment on lines +520 to +567
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider enhancing the test coverage for HandleConsumerModificationProposal.

Currently, the test primarily checks the happy path. It would be beneficial to include tests for error scenarios and edge cases, such as invalid inputs or when the chain ID does not exist. This will ensure the robustness of the implementation under various conditions.


// Tests the StopConsumerChain method against the spec,
// with more granularity than what's covered in TestHandleConsumerRemovalProposal, or integration tests.
// See: https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/methods.md#ccv-pcf-stcc1
Expand Down
2 changes: 2 additions & 0 deletions x/ccv/provider/proposal_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ func NewProviderProposalHandler(k keeper.Keeper) govv1beta1.Handler {
return k.HandleConsumerAdditionProposal(ctx, c)
case *types.ConsumerRemovalProposal:
return k.HandleConsumerRemovalProposal(ctx, c)
case *types.ConsumerModificationProposal:
return k.HandleConsumerModificationProposal(ctx, c)
case *types.ChangeRewardDenomsProposal:
return k.HandleConsumerRewardDenomProposal(ctx, c)
default:
Expand Down
4 changes: 4 additions & 0 deletions x/ccv/provider/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
(*govv1beta1.Content)(nil),
&ConsumerRemovalProposal{},
)
registry.RegisterImplementations(
(*govv1beta1.Content)(nil),
&ConsumerModificationProposal{},
)
registry.RegisterImplementations(
(*sdk.Msg)(nil),
&MsgAssignConsumerKey{},
Expand Down
43 changes: 22 additions & 21 deletions x/ccv/provider/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,26 @@ import (

// Provider sentinel errors
var (
ErrInvalidConsumerAdditionProposal = errorsmod.Register(ModuleName, 1, "invalid consumer addition proposal")
ErrInvalidConsumerRemovalProp = errorsmod.Register(ModuleName, 2, "invalid consumer removal proposal")
ErrUnknownConsumerChainId = errorsmod.Register(ModuleName, 3, "no consumer chain with this chain id")
ErrUnknownConsumerChannelId = errorsmod.Register(ModuleName, 4, "no consumer chain with this channel id")
ErrInvalidConsumerConsensusPubKey = errorsmod.Register(ModuleName, 5, "empty consumer consensus public key")
ErrInvalidConsumerChainID = errorsmod.Register(ModuleName, 6, "invalid consumer chain id")
ErrConsumerKeyNotFound = errorsmod.Register(ModuleName, 7, "consumer key not found")
ErrNoValidatorConsumerAddress = errorsmod.Register(ModuleName, 8, "error getting validator consumer address")
ErrNoValidatorProviderAddress = errorsmod.Register(ModuleName, 9, "error getting validator provider address")
ErrConsumerKeyInUse = errorsmod.Register(ModuleName, 10, "consumer key is already in use by a validator")
ErrCannotAssignDefaultKeyAssignment = errorsmod.Register(ModuleName, 11, "cannot re-assign default key assignment")
ErrInvalidConsumerParams = errorsmod.Register(ModuleName, 12, "invalid consumer params")
ErrInvalidProviderAddress = errorsmod.Register(ModuleName, 13, "invalid provider address")
ErrInvalidConsumerRewardDenom = errorsmod.Register(ModuleName, 14, "invalid consumer reward denom")
ErrInvalidDepositorAddress = errorsmod.Register(ModuleName, 15, "invalid depositor address")
ErrInvalidConsumerClient = errorsmod.Register(ModuleName, 16, "ccv channel is not built on correct client")
ErrDuplicateConsumerChain = errorsmod.Register(ModuleName, 17, "consumer chain already exists")
ErrConsumerChainNotFound = errorsmod.Register(ModuleName, 18, "consumer chain not found")
ErrInvalidConsumerCommissionRate = errorsmod.Register(ModuleName, 19, "consumer commission rate is invalid")
ErrCannotOptOutFromTopN = errorsmod.Register(ModuleName, 20, "cannot opt out from a Top N chain")
ErrNoUnconfirmedVSCPacket = errorsmod.Register(ModuleName, 21, "no unconfirmed vsc packet for this chain id")
ErrInvalidConsumerAdditionProposal = errorsmod.Register(ModuleName, 1, "invalid consumer addition proposal")
ErrInvalidConsumerRemovalProp = errorsmod.Register(ModuleName, 2, "invalid consumer removal proposal")
ErrUnknownConsumerChainId = errorsmod.Register(ModuleName, 3, "no consumer chain with this chain id")
ErrUnknownConsumerChannelId = errorsmod.Register(ModuleName, 4, "no consumer chain with this channel id")
ErrInvalidConsumerConsensusPubKey = errorsmod.Register(ModuleName, 5, "empty consumer consensus public key")
ErrInvalidConsumerChainID = errorsmod.Register(ModuleName, 6, "invalid consumer chain id")
ErrConsumerKeyNotFound = errorsmod.Register(ModuleName, 7, "consumer key not found")
ErrNoValidatorConsumerAddress = errorsmod.Register(ModuleName, 8, "error getting validator consumer address")
ErrNoValidatorProviderAddress = errorsmod.Register(ModuleName, 9, "error getting validator provider address")
ErrConsumerKeyInUse = errorsmod.Register(ModuleName, 10, "consumer key is already in use by a validator")
ErrCannotAssignDefaultKeyAssignment = errorsmod.Register(ModuleName, 11, "cannot re-assign default key assignment")
ErrInvalidConsumerParams = errorsmod.Register(ModuleName, 12, "invalid consumer params")
ErrInvalidProviderAddress = errorsmod.Register(ModuleName, 13, "invalid provider address")
ErrInvalidConsumerRewardDenom = errorsmod.Register(ModuleName, 14, "invalid consumer reward denom")
ErrInvalidDepositorAddress = errorsmod.Register(ModuleName, 15, "invalid depositor address")
ErrInvalidConsumerClient = errorsmod.Register(ModuleName, 16, "ccv channel is not built on correct client")
ErrDuplicateConsumerChain = errorsmod.Register(ModuleName, 17, "consumer chain already exists")
ErrConsumerChainNotFound = errorsmod.Register(ModuleName, 18, "consumer chain not found")
ErrInvalidConsumerCommissionRate = errorsmod.Register(ModuleName, 19, "consumer commission rate is invalid")
ErrCannotOptOutFromTopN = errorsmod.Register(ModuleName, 20, "cannot opt out from a Top N chain")
ErrNoUnconfirmedVSCPacket = errorsmod.Register(ModuleName, 21, "no unconfirmed vsc packet for this chain id")
ErrInvalidConsumerModificationProposal = errorsmod.Register(ModuleName, 22, "invalid consumer modification proposal")
)
Loading
Loading