Skip to content

Commit

Permalink
feat!: allow the chain id to be updated if the chain has not yet laun…
Browse files Browse the repository at this point in the history
…ched (#2378)

* init commit

* add CHANGELOG files

* took into account comments

* removed unecessary check

* add a small comment in the CLI of UpdateConsumer that NewChainId can be empty
  • Loading branch information
insumity authored Nov 6, 2024
1 parent 00ba140 commit af5d7a4
Show file tree
Hide file tree
Showing 12 changed files with 385 additions and 142 deletions.
2 changes: 2 additions & 0 deletions .changelog/unreleased/features/2378-allow-chain-id-updates.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Allow the chain id of a consumer chain to be updated before the chain
launches. ([\#2378](https://github.com/cosmos/interchain-security/pull/2378))
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Allow the chain id of a consumer chain to be updated before the chain
launches. ([\#2378](https://github.com/cosmos/interchain-security/pull/2378))
6 changes: 6 additions & 0 deletions docs/docs/build/modules/02-provider.md
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,9 @@ If the `power_shaping_parameters` field is set and `power_shaping_parameters.top

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.

We can also update the `chain_id` of a consumer chain by using the optional `new_chain_id` field. Note that the chain id of a consumer chain
can only be updated if the chain has not yet launched. After launch, the chain id of a consumer chain cannot be updated anymore.

```proto
message MsgUpdateConsumer {
option (cosmos.msg.v1.signer) = "owner";
Expand All @@ -562,6 +565,9 @@ message MsgUpdateConsumer {
// allowlisted reward denoms by the consumer chain
AllowlistedRewardDenoms allowlisted_reward_denoms = 7;
// to update the chain id of the chain (can only be updated if the chain has not yet launched)
string new_chain_id = 8;
}
```

Expand Down
5 changes: 5 additions & 0 deletions proto/interchain_security/ccv/provider/v1/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,11 @@ message MsgUpdateConsumer {

// allowlisted reward denoms of the consumer (if provided they overwrite previously set reward denoms)
AllowlistedRewardDenoms allowlisted_reward_denoms = 7;

// (optional) If the consumer chain has NOT yet launched, the chain id can be updated. After a chain has launched
// the chain id CANNOT be updated.
// This field is optional and can remain empty (i.e., `new_chain_id = ""`) or correspond to the chain id the chain already has.
string new_chain_id = 8;
}

// MsgUpdateConsumerResponse defines response type for MsgUpdateConsumer messages
Expand Down
3 changes: 2 additions & 1 deletion x/ccv/provider/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,7 @@ where update_consumer.json has the following structure:
"allowlisted_reward_denoms": {
"denoms": ["ibc/...", "ibc/..."]
}
"new_chain_id": "newConsumer-1", // is optional and can be empty (i.e., "new_chain_id": "")
}
Note that only 'consumer_id' is mandatory. The others are optional.
Expand Down Expand Up @@ -397,7 +398,7 @@ If one of the fields is missing, it will be set to its zero value.
}

msg, err := types.NewMsgUpdateConsumer(owner, consUpdate.ConsumerId, consUpdate.NewOwnerAddress, consUpdate.Metadata,
consUpdate.InitializationParameters, consUpdate.PowerShapingParameters, consUpdate.AllowlistedRewardDenoms)
consUpdate.InitializationParameters, consUpdate.PowerShapingParameters, consUpdate.AllowlistedRewardDenoms, consUpdate.NewChainId)
if err != nil {
return err
}
Expand Down
21 changes: 18 additions & 3 deletions x/ccv/provider/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,22 @@ func (k msgServer) UpdateConsumer(goCtx context.Context, msg *types.MsgUpdateCon
return &resp, errorsmod.Wrapf(ccvtypes.ErrInvalidConsumerState, "cannot get consumer chain ID: %s", err.Error())
}

// We only validate and use `NewChainId` if it is not empty (because `NewChainId` is an optional argument)
// or `NewChainId` is different from the current chain id of the consumer chain.
if strings.TrimSpace(msg.NewChainId) != "" && msg.NewChainId != chainId {
if err = types.ValidateChainId("NewChainId", msg.NewChainId); err != nil {
return &resp, errorsmod.Wrapf(types.ErrInvalidMsgUpdateConsumer, "invalid new chain id: %s", err.Error())
}

if k.IsConsumerPrelaunched(ctx, consumerId) {
chainId = msg.NewChainId
k.SetConsumerChainId(ctx, consumerId, chainId)
} else {
// the chain id cannot be updated if the chain is NOT in a prelaunched (i.e., registered or initialized) phase
return &resp, errorsmod.Wrapf(types.ErrInvalidPhase, "cannot update chain id of a non-prelaunched chain: %s", k.GetConsumerPhase(ctx, consumerId))
}
}

// add event attributes
eventAttributes = append(eventAttributes, []sdk.Attribute{
sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName),
Expand Down Expand Up @@ -511,14 +527,13 @@ func (k msgServer) UpdateConsumer(goCtx context.Context, msg *types.MsgUpdateCon
previousSpawnTime := previousInitializationParameters.SpawnTime

if msg.InitializationParameters != nil {
phase := k.GetConsumerPhase(ctx, consumerId)

if phase == types.CONSUMER_PHASE_LAUNCHED {
if !k.IsConsumerPrelaunched(ctx, consumerId) {
return &resp, errorsmod.Wrap(types.ErrInvalidMsgUpdateConsumer,
"cannot update the initialization parameters of an an already launched chain; "+
"do not provide any initialization parameters when updating a launched chain")
}

phase := k.GetConsumerPhase(ctx, consumerId)
if msg.InitializationParameters.SpawnTime.IsZero() {
if phase == types.CONSUMER_PHASE_INITIALIZED {
// chain was previously ready to launch at `previousSpawnTime` so we remove the
Expand Down
63 changes: 62 additions & 1 deletion x/ccv/provider/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,10 @@ func TestUpdateConsumer(t *testing.T) {
require.Error(t, err, "cannot update consumer chain")

// create a chain before updating it
chainId := "chainId-1"
createConsumerResponse, err := msgServer.CreateConsumer(ctx,
&providertypes.MsgCreateConsumer{
Submitter: "submitter", ChainId: "chainId-1",
Submitter: "submitter", ChainId: chainId,
Metadata: providertypes.ConsumerMetadata{
Name: "name",
Description: "description",
Expand All @@ -106,6 +107,32 @@ func TestUpdateConsumer(t *testing.T) {
})
require.Error(t, err, "expected owner address")

// assert that we can change the chain id of a registered chain
expectedChainId := "newChainId-1"
_, err = msgServer.UpdateConsumer(ctx,
&providertypes.MsgUpdateConsumer{
Owner: "submitter", ConsumerId: consumerId,
Metadata: nil,
InitializationParameters: nil,
PowerShapingParameters: nil,
NewChainId: expectedChainId,
})
require.NoError(t, err)
chainId, err = providerKeeper.GetConsumerChainId(ctx, consumerId)
require.NoError(t, err)
require.Equal(t, expectedChainId, chainId)

// assert that we cannot change the chain to that of a reserved chain id
_, err = msgServer.UpdateConsumer(ctx,
&providertypes.MsgUpdateConsumer{
Owner: "submitter", ConsumerId: consumerId,
Metadata: nil,
InitializationParameters: nil,
PowerShapingParameters: nil,
NewChainId: "stride-1", // reversed chain id
})
require.ErrorContains(t, err, "cannot use a reserved chain id")

expectedConsumerMetadata := providertypes.ConsumerMetadata{
Name: "name2",
Description: "description2",
Expand All @@ -117,12 +144,14 @@ func TestUpdateConsumer(t *testing.T) {
expectedPowerShapingParameters := testkeeper.GetTestPowerShapingParameters()

expectedOwnerAddress := "cosmos1dkas8mu4kyhl5jrh4nzvm65qz588hy9qcz08la"
expectedChainId = "updatedChainId-1"
_, err = msgServer.UpdateConsumer(ctx,
&providertypes.MsgUpdateConsumer{
Owner: "submitter", ConsumerId: consumerId, NewOwnerAddress: expectedOwnerAddress,
Metadata: &expectedConsumerMetadata,
InitializationParameters: &expectedInitializationParameters,
PowerShapingParameters: &expectedPowerShapingParameters,
NewChainId: expectedChainId,
})
require.NoError(t, err)

Expand All @@ -146,6 +175,11 @@ func TestUpdateConsumer(t *testing.T) {
require.NoError(t, err)
require.Equal(t, expectedPowerShapingParameters, actualPowerShapingParameters)

// assert that the chain id has been updated
actualChainId, err := providerKeeper.GetConsumerChainId(ctx, consumerId)
require.NoError(t, err)
require.Equal(t, expectedChainId, actualChainId)

// assert phase
phase := providerKeeper.GetConsumerPhase(ctx, consumerId)
require.Equal(t, providertypes.CONSUMER_PHASE_INITIALIZED, phase)
Expand Down Expand Up @@ -191,6 +225,33 @@ func TestUpdateConsumer(t *testing.T) {
})
require.ErrorContains(t, err, "cannot update the initialization parameters of an an already launched chain")

// assert that we CANNOT change the chain id of a launched chain
providerKeeper.SetConsumerPhase(ctx, consumerId, providertypes.CONSUMER_PHASE_LAUNCHED)
_, err = msgServer.UpdateConsumer(ctx,
&providertypes.MsgUpdateConsumer{
Owner: expectedOwnerAddress, ConsumerId: consumerId,
Metadata: nil,
InitializationParameters: nil,
PowerShapingParameters: nil,
NewChainId: "newChainId",
})
require.ErrorContains(t, err, "cannot update chain id of a non-prelaunched chain")

// assert that we can use the chain's current chain id as `NewChainId` even if the chain has launched
// as effectively this does not change anything
chainId, err = providerKeeper.GetConsumerChainId(ctx, consumerId)
require.NoError(t, err)
providerKeeper.SetConsumerPhase(ctx, consumerId, providertypes.CONSUMER_PHASE_LAUNCHED)
_, err = msgServer.UpdateConsumer(ctx,
&providertypes.MsgUpdateConsumer{
Owner: expectedOwnerAddress, ConsumerId: consumerId,
Metadata: nil,
InitializationParameters: nil,
PowerShapingParameters: nil,
NewChainId: chainId,
})
require.NoError(t, err)

// assert that we can update the consumer metadata of a launched chain
providerKeeper.SetConsumerPhase(ctx, consumerId, providertypes.CONSUMER_PHASE_LAUNCHED)
expectedConsumerMetadata.Name = "name of a launched chain"
Expand Down
7 changes: 7 additions & 0 deletions x/ccv/provider/keeper/permissionless.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,13 @@ func (k Keeper) DeleteConsumerPhase(ctx sdk.Context, consumerId string) {
store.Delete(types.ConsumerIdToPhaseKey(consumerId))
}

// IsConsumerPrelaunched checks if a consumer chain is in its prelaunch phase
func (k Keeper) IsConsumerPrelaunched(ctx sdk.Context, consumerId string) bool {
phase := k.GetConsumerPhase(ctx, consumerId)
return phase == types.CONSUMER_PHASE_REGISTERED ||
phase == types.CONSUMER_PHASE_INITIALIZED
}

// IsConsumerActive checks if a consumer chain is either registered, initialized, or launched.
func (k Keeper) IsConsumerActive(ctx sdk.Context, consumerId string) bool {
phase := k.GetConsumerPhase(ctx, consumerId)
Expand Down
20 changes: 20 additions & 0 deletions x/ccv/provider/keeper/permissionless_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,3 +198,23 @@ func TestConsumerPhase(t *testing.T) {
phase = providerKeeper.GetConsumerPhase(ctx, CONSUMER_ID)
require.Equal(t, providertypes.CONSUMER_PHASE_LAUNCHED, phase)
}

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

providerKeeper.SetConsumerPhase(ctx, CONSUMER_ID, providertypes.CONSUMER_PHASE_REGISTERED)
require.True(t, providerKeeper.IsConsumerPrelaunched(ctx, CONSUMER_ID))

providerKeeper.SetConsumerPhase(ctx, CONSUMER_ID, providertypes.CONSUMER_PHASE_INITIALIZED)
require.True(t, providerKeeper.IsConsumerPrelaunched(ctx, CONSUMER_ID))

providerKeeper.SetConsumerPhase(ctx, CONSUMER_ID, providertypes.CONSUMER_PHASE_LAUNCHED)
require.False(t, providerKeeper.IsConsumerPrelaunched(ctx, CONSUMER_ID))

providerKeeper.SetConsumerPhase(ctx, CONSUMER_ID, providertypes.CONSUMER_PHASE_STOPPED)
require.False(t, providerKeeper.IsConsumerPrelaunched(ctx, CONSUMER_ID))

providerKeeper.SetConsumerPhase(ctx, CONSUMER_ID, providertypes.CONSUMER_PHASE_DELETED)
require.False(t, providerKeeper.IsConsumerPrelaunched(ctx, CONSUMER_ID))
}
36 changes: 26 additions & 10 deletions x/ccv/provider/types/msg.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,20 +295,35 @@ func NewMsgCreateConsumer(submitter, chainId string, metadata ConsumerMetadata,
}, nil
}

// ValidateBasic implements the sdk.HasValidateBasic interface.
func (msg MsgCreateConsumer) ValidateBasic() error {
if err := ValidateStringField("ChainId", msg.ChainId, cmttypes.MaxChainIDLen); err != nil {
return errorsmod.Wrapf(ErrInvalidMsgCreateConsumer, "ChainId: %s", err.Error())
}

// IsReservedChainId returns true if the specific chain id is reserved and cannot be used by other consumer chains
func IsReservedChainId(chainId string) bool {
// With permissionless ICS, we can have multiple consumer chains with the exact same chain id.
// However, as we already have the Neutron and Stride Top N chains running, as a first step we would like to
// prevent permissionless chains from re-using the chain ids of Neutron and Stride. Note that this is just a
// preliminary measure that will be removed later on as part of:
// TODO (#2242): find a better way of ignoring past misbehaviors
if msg.ChainId == "neutron-1" || msg.ChainId == "stride-1" {
return errorsmod.Wrapf(ErrInvalidMsgCreateConsumer,
"cannot reuse chain ids of existing Neutron and Stride Top N consumer chains")
return chainId == "neutron-1" || chainId == "stride-1"
}

// ValidateChainId validates that the chain id is valid and is not reserved.
// Can be called for the `MsgUpdateConsumer.NewChainId` field as well, so this method takes the `field` as an argument
// to return more appropriate error messages in case the validation fails.
func ValidateChainId(field string, chainId string) error {
if err := ValidateStringField(field, chainId, cmttypes.MaxChainIDLen); err != nil {
return errorsmod.Wrapf(ErrInvalidMsgCreateConsumer, "%s: %s", field, err.Error())
}

if IsReservedChainId(chainId) {
return errorsmod.Wrapf(ErrInvalidMsgCreateConsumer, "cannot use a reserved chain id")
}

return nil
}

// ValidateBasic implements the sdk.HasValidateBasic interface.
func (msg MsgCreateConsumer) ValidateBasic() error {
if err := ValidateChainId("ChainId", msg.ChainId); err != nil {
return errorsmod.Wrapf(ErrInvalidMsgCreateConsumer, "ChainId: %s", err.Error())
}

if err := ValidateConsumerMetadata(msg.Metadata); err != nil {
Expand Down Expand Up @@ -343,7 +358,7 @@ func (msg MsgCreateConsumer) ValidateBasic() error {
// NewMsgUpdateConsumer creates a new MsgUpdateConsumer instance
func NewMsgUpdateConsumer(owner, consumerId, ownerAddress string, metadata *ConsumerMetadata,
initializationParameters *ConsumerInitializationParameters, powerShapingParameters *PowerShapingParameters,
allowlistedRewardDenoms *AllowlistedRewardDenoms,
allowlistedRewardDenoms *AllowlistedRewardDenoms, newChainId string,
) (*MsgUpdateConsumer, error) {
return &MsgUpdateConsumer{
Owner: owner,
Expand All @@ -353,6 +368,7 @@ func NewMsgUpdateConsumer(owner, consumerId, ownerAddress string, metadata *Cons
InitializationParameters: initializationParameters,
PowerShapingParameters: powerShapingParameters,
AllowlistedRewardDenoms: allowlistedRewardDenoms,
NewChainId: newChainId,
}, nil
}

Expand Down
Loading

0 comments on commit af5d7a4

Please sign in to comment.