fixed bug on removing previous spawn time & added tests

testutil/ibc_testing/generic_setup.go

@@ -159,7 +159,7 @@ func AddConsumer[Tp testutil.ProviderApp, Tc testutil.ConsumerApp](
 	providerKeeper.SetConsumerInitializationParameters(providerChain.GetContext(), consumerId, initializationParameters)
 	providerKeeper.SetConsumerPowerShapingParameters(providerChain.GetContext(), consumerId, powerShapingParameters)
 	providerKeeper.SetConsumerPhase(providerChain.GetContext(), consumerId, keeper.Initialized)
-	providerKeeper.AppendSpawnTimeForConsumerToBeLaunched(providerChain.GetContext(), consumerId, coordinator.CurrentTime)
+	providerKeeper.AppendConsumerToBeLaunchedOnSpawnTime(providerChain.GetContext(), consumerId, coordinator.CurrentTime)
 
 	// opt-in all validators
 	lastVals, err := providerApp.GetProviderKeeper().GetLastBondedValidators(providerChain.GetContext())

testutil/keeper/unit_test_helpers.go

@@ -283,7 +283,7 @@ func GetTestConsumerMetadata() providertypes.ConsumerMetadata {
 
 func GetTestInitializationParameters() providertypes.ConsumerInitializationParameters {
 	initialHeight := clienttypes.NewHeight(4, 5)
-	spawnTime := time.Now()
+	spawnTime := time.Now().UTC()
 	ccvTimeoutPeriod := types.DefaultCCVTimeoutPeriod
 	transferTimeoutPeriod := types.DefaultTransferTimeoutPeriod
 	unbondingPeriod := types.DefaultConsumerUnbondingPeriod

x/ccv/provider/keeper/keeper.go

@@ -750,7 +750,7 @@ func (k Keeper) GetAllActiveConsumerIds(ctx sdk.Context) []string {
 	}
 
 	consumerIds := []string{}
-	for i := uint64(0); i <= latestConsumerId; i++ {
+	for i := uint64(0); i < latestConsumerId; i++ {
 		consumerId := fmt.Sprintf("%d", i)
 		phase, foundPhase := k.GetConsumerPhase(ctx, consumerId)
 		if !foundPhase || (phase != Registered && phase != Initialized && phase != Launched) {

x/ccv/provider/keeper/msg_server.go

@@ -106,11 +106,11 @@ func (k msgServer) RemoveConsumer(
 
 	previousStopTime, err := k.Keeper.GetConsumerStopTime(ctx, consumerId)
 	if err == nil {
-		k.Keeper.RemoveConsumerFromToBeStoppedConsumers(ctx, consumerId, previousStopTime)
+		k.Keeper.RemoveConsumerToBeStoppedFromStopTime(ctx, consumerId, previousStopTime)
 	}
 
 	k.Keeper.SetConsumerStopTime(ctx, consumerId, msg.StopTime)
-	k.Keeper.AppendStopTimeForConsumerToBeStopped(ctx, consumerId, msg.StopTime)
+	k.Keeper.AppendConsumerToBeStoppedOnStopTime(ctx, consumerId, msg.StopTime)
 
 	return &types.MsgRemoveConsumerResponse{}, nil
 }
@@ -309,20 +309,20 @@ func (k msgServer) SetConsumerCommissionRate(goCtx context.Context, msg *types.M
 func (k msgServer) CreateConsumer(goCtx context.Context, msg *types.MsgCreateConsumer) (*types.MsgCreateConsumerResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
-	consumerId := k.FetchAndIncrementConsumerId(ctx)
+	consumerId := k.Keeper.FetchAndIncrementConsumerId(ctx)
 
-	k.SetConsumerOwnerAddress(ctx, consumerId, msg.Signer)
-	k.SetConsumerChainId(ctx, consumerId, msg.ChainId)
-	k.SetConsumerPhase(ctx, consumerId, Registered)
+	k.Keeper.SetConsumerOwnerAddress(ctx, consumerId, msg.Signer)
+	k.Keeper.SetConsumerChainId(ctx, consumerId, msg.ChainId)
+	k.Keeper.SetConsumerPhase(ctx, consumerId, Registered)
 
-	if err := k.SetConsumerMetadata(ctx, consumerId, msg.Metadata); err != nil {
+	if err := k.Keeper.SetConsumerMetadata(ctx, consumerId, msg.Metadata); err != nil {
 		return &types.MsgCreateConsumerResponse{}, errorsmod.Wrapf(types.ErrInvalidConsumerMetadata,
 			"cannot set consumer metadata: %s", err.Error())
 	}
 
 	// initialization parameters are optional and hence could be nil
 	if msg.InitializationParameters != nil {
-		if err := k.SetConsumerInitializationParameters(ctx, consumerId, *msg.InitializationParameters); err != nil {
+		if err := k.Keeper.SetConsumerInitializationParameters(ctx, consumerId, *msg.InitializationParameters); err != nil {
 			return &types.MsgCreateConsumerResponse{}, errorsmod.Wrapf(types.ErrInvalidConsumerInitializationParameters,
 				"cannot set consumer initialization parameters: %s", err.Error())
 		}
@@ -334,15 +334,15 @@ func (k msgServer) CreateConsumer(goCtx context.Context, msg *types.MsgCreateCon
 			return &types.MsgCreateConsumerResponse{}, errorsmod.Wrap(types.ErrCannotCreateTopNChain,
 				"cannot create a Top N chain using the `MsgCreateConsumer` message; use `MsgUpdateConsumer` instead")
 		}
-		if err := k.SetConsumerPowerShapingParameters(ctx, consumerId, *msg.PowerShapingParameters); err != nil {
+		if err := k.Keeper.SetConsumerPowerShapingParameters(ctx, consumerId, *msg.PowerShapingParameters); err != nil {
 			return &types.MsgCreateConsumerResponse{}, errorsmod.Wrapf(types.ErrInvalidPowerShapingParameters,
 				"cannot set power shaping parameters")
 		}
 	}
 
-	if spawnTime, canLaunch := k.CanLaunch(ctx, consumerId); canLaunch {
-		k.SetConsumerPhase(ctx, consumerId, Initialized)
-		k.PrepareConsumerForLaunch(ctx, consumerId, time.Time{}, spawnTime)
+	if spawnTime, canLaunch := k.Keeper.CanLaunch(ctx, consumerId); canLaunch {
+		k.Keeper.SetConsumerPhase(ctx, consumerId, Initialized)
+		k.Keeper.PrepareConsumerForLaunch(ctx, consumerId, time.Time{}, spawnTime)
 	}
 
 	return &types.MsgCreateConsumerResponse{ConsumerId: consumerId}, nil
@@ -353,20 +353,12 @@ func (k msgServer) UpdateConsumer(goCtx context.Context, msg *types.MsgUpdateCon
 	ctx := sdk.UnwrapSDKContext(goCtx)
 	consumerId := msg.ConsumerId
 
-	phase, found := k.GetConsumerPhase(ctx, consumerId)
+	phase, found := k.Keeper.GetConsumerPhase(ctx, consumerId)
 	if found && phase == Stopped {
 		return &types.MsgUpdateConsumerResponse{}, errorsmod.Wrapf(types.ErrInvalidPhase,
 			"cannot update consumer chain that is in the stopped phase: %s", consumerId)
 	}
 
-	// The new owner address can be empty, in which case the consumer chain does not change its owner.
-	// However, if the new owner address is not empty, we verify that it's a valid account address.
-	if strings.TrimSpace(msg.NewOwnerAddress) != "" {
-		if _, err := k.accountKeeper.AddressCodec().StringToBytes(msg.NewOwnerAddress); err != nil {
-			return &types.MsgUpdateConsumerResponse{}, errorsmod.Wrapf(types.ErrInvalidNewOwnerAddress, "invalid new owner address %s", msg.NewOwnerAddress)
-		}
-	}
-
 	ownerAddress, err := k.Keeper.GetConsumerOwnerAddress(ctx, consumerId)
 	if err != nil {
 		return &types.MsgUpdateConsumerResponse{}, errorsmod.Wrapf(types.ErrNoOwnerAddress, "cannot retrieve owner address %s", ownerAddress)
@@ -376,20 +368,27 @@ func (k msgServer) UpdateConsumer(goCtx context.Context, msg *types.MsgUpdateCon
 		return &types.MsgUpdateConsumerResponse{}, errorsmod.Wrapf(types.ErrUnauthorized, "expected owner address %s, got %s", ownerAddress, msg.Signer)
 	}
 
+	// The new owner address can be empty, in which case the consumer chain does not change its owner.
+	// However, if the new owner address is not empty, we verify that it's a valid account address.
 	if strings.TrimSpace(msg.NewOwnerAddress) != "" {
+		if _, err := k.accountKeeper.AddressCodec().StringToBytes(msg.NewOwnerAddress); err != nil {
+			return &types.MsgUpdateConsumerResponse{}, errorsmod.Wrapf(types.ErrInvalidNewOwnerAddress, "invalid new owner address %s", msg.NewOwnerAddress)
+		}
+
 		k.Keeper.SetConsumerOwnerAddress(ctx, consumerId, msg.NewOwnerAddress)
 	}
 
 	if msg.Metadata != nil {
-		if err := k.SetConsumerMetadata(ctx, consumerId, *msg.Metadata); err != nil {
+		if err := k.Keeper.SetConsumerMetadata(ctx, consumerId, *msg.Metadata); err != nil {
 			return &types.MsgUpdateConsumerResponse{}, errorsmod.Wrapf(types.ErrInvalidConsumerMetadata,
 				"cannot set consumer metadata: %s", err.Error())
 		}
 	}
 
 	// get the previous spawn time so that we can use it in `PrepareConsumerForLaunch`
 	var previousSpawnTime time.Time
-	if previousInitializationParameters, err := k.Keeper.GetConsumerInitializationParameters(ctx, msg.ConsumerId); err != nil {
+	previousInitializationParameters, err := k.Keeper.GetConsumerInitializationParameters(ctx, msg.ConsumerId)
+	if err == nil {
 		previousSpawnTime = previousInitializationParameters.SpawnTime
 	}
 
@@ -410,7 +409,7 @@ func (k msgServer) UpdateConsumer(goCtx context.Context, msg *types.MsgUpdateCon
 		}
 
 		oldTopN := k.Keeper.GetTopN(ctx, consumerId)
-		if err = k.SetConsumerPowerShapingParameters(ctx, consumerId, *msg.PowerShapingParameters); err != nil {
+		if err = k.Keeper.SetConsumerPowerShapingParameters(ctx, consumerId, *msg.PowerShapingParameters); err != nil {
 			return &types.MsgUpdateConsumerResponse{}, errorsmod.Wrapf(types.ErrInvalidPowerShapingParameters,
 				"cannot set power shaping parameters")
 		}
@@ -426,12 +425,12 @@ func (k msgServer) UpdateConsumer(goCtx context.Context, msg *types.MsgUpdateCon
 
 	// A Top N cannot change its owner address to something different from the gov module if the chain
 	// remains a Top N chain.
-	currentOwnerAddress, err := k.GetConsumerOwnerAddress(ctx, consumerId)
+	currentOwnerAddress, err := k.Keeper.GetConsumerOwnerAddress(ctx, consumerId)
 	if err != nil {
 		return &types.MsgUpdateConsumerResponse{}, errorsmod.Wrapf(types.ErrNoOwnerAddress, "cannot retrieve owner address %s: %s", ownerAddress, err.Error())
 	}
 
-	currentPowerShapingParameters, err := k.GetConsumerPowerShapingParameters(ctx, consumerId)
+	currentPowerShapingParameters, err := k.Keeper.GetConsumerPowerShapingParameters(ctx, consumerId)
 	if err != nil {
 		return &types.MsgUpdateConsumerResponse{}, errorsmod.Wrapf(types.ErrInvalidPowerShapingParameters, "cannot retrieve power shaping parameters: %s", err.Error())
 	}
@@ -441,9 +440,9 @@ func (k msgServer) UpdateConsumer(goCtx context.Context, msg *types.MsgUpdateCon
 			"a move to a new owner address that is not the gov module can only be done if `Top N` is set to 0")
 	}
 
-	if spawnTime, canLaunch := k.CanLaunch(ctx, consumerId); canLaunch {
-		k.SetConsumerPhase(ctx, consumerId, Initialized)
-		k.PrepareConsumerForLaunch(ctx, consumerId, previousSpawnTime, spawnTime)
+	if spawnTime, canLaunch := k.Keeper.CanLaunch(ctx, consumerId); canLaunch {
+		k.Keeper.SetConsumerPhase(ctx, consumerId, Initialized)
+		k.Keeper.PrepareConsumerForLaunch(ctx, consumerId, previousSpawnTime, spawnTime)
 	}
 
 	return &types.MsgUpdateConsumerResponse{}, nil

x/ccv/provider/keeper/msg_server_test.go

@@ -1,11 +1,13 @@
 package keeper_test
 
 import (
+	"github.com/cosmos/cosmos-sdk/codec/address"
 	testkeeper "github.com/cosmos/interchain-security/v5/testutil/keeper"
 	providerkeeper "github.com/cosmos/interchain-security/v5/x/ccv/provider/keeper"
 	providertypes "github.com/cosmos/interchain-security/v5/x/ccv/provider/types"
 	"github.com/stretchr/testify/require"
 	"testing"
+	"time"
 )
 
 func TestCreateConsumer(t *testing.T) {
@@ -53,3 +55,113 @@ func TestCreateConsumer(t *testing.T) {
 	require.True(t, found)
 	require.Equal(t, providerkeeper.Registered, phase)
 }
+
+func TestUpdateConsumer(t *testing.T) {
+	providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
+	defer ctrl.Finish()
+
+	msgServer := providerkeeper.NewMsgServerImpl(&providerKeeper)
+
+	// try to update a non-existing (i.e., no consumer id exists)
+	_, err := msgServer.UpdateConsumer(ctx,
+		&providertypes.MsgUpdateConsumer{Signer: "signer", ConsumerId: "0", NewOwnerAddress: "cosmos1dkas8mu4kyhl5jrh4nzvm65qz588hy9qcz08la",
+			Metadata:                 nil,
+			InitializationParameters: nil,
+			PowerShapingParameters:   nil,
+		})
+	require.Error(t, err, "cannot update consumer chain")
+
+	// create a chain before updating it
+	createConsumerResponse, err := msgServer.CreateConsumer(ctx,
+		&providertypes.MsgCreateConsumer{Signer: "signer", ChainId: "chainId",
+			Metadata: providertypes.ConsumerMetadata{
+				Name:        "name",
+				Description: "description",
+				Metadata:    "metadata",
+			},
+			InitializationParameters: nil,
+			PowerShapingParameters:   nil,
+		})
+	require.NoError(t, err)
+	consumerId := createConsumerResponse.ConsumerId
+
+	mocks.MockAccountKeeper.EXPECT().AddressCodec().Return(address.NewBech32Codec("cosmos")).AnyTimes()
+	_, err = msgServer.UpdateConsumer(ctx,
+		&providertypes.MsgUpdateConsumer{Signer: "wrong signer", ConsumerId: consumerId, NewOwnerAddress: "cosmos1dkas8mu4kyhl5jrh4nzvm65qz588hy9qcz08la",
+			Metadata:                 nil,
+			InitializationParameters: nil,
+			PowerShapingParameters:   nil,
+		})
+	require.Error(t, err, "expected owner address")
+
+	expectedConsumerMetadata := providertypes.ConsumerMetadata{
+		Name:        "name2",
+		Description: "description2",
+		Metadata:    "metadata2",
+	}
+
+	expectedInitializationParameters := testkeeper.GetTestInitializationParameters()
+	expectedPowerShapingParameters := testkeeper.GetTestPowerShapingParameters()
+
+	expectedOwnerAddress := "cosmos1dkas8mu4kyhl5jrh4nzvm65qz588hy9qcz08la"
+	_, err = msgServer.UpdateConsumer(ctx,
+		&providertypes.MsgUpdateConsumer{Signer: "signer", ConsumerId: consumerId, NewOwnerAddress: expectedOwnerAddress,
+			Metadata:                 &expectedConsumerMetadata,
+			InitializationParameters: &expectedInitializationParameters,
+			PowerShapingParameters:   &expectedPowerShapingParameters})
+	require.NoError(t, err)
+
+	// assert that owner address was updated
+	ownerAddress, err := providerKeeper.GetConsumerOwnerAddress(ctx, consumerId)
+	require.NoError(t, err)
+	require.Equal(t, expectedOwnerAddress, ownerAddress)
+
+	// assert that consumer metadata were updated
+	actualConsumerMetadata, err := providerKeeper.GetConsumerMetadata(ctx, consumerId)
+	require.NoError(t, err)
+	require.Equal(t, expectedConsumerMetadata, actualConsumerMetadata)
+
+	// assert that initialization parameters were updated
+	actualInitializationParameters, err := providerKeeper.GetConsumerInitializationParameters(ctx, consumerId)
+	require.NoError(t, err)
+	require.Equal(t, expectedInitializationParameters, actualInitializationParameters)
+
+	// assert that power-shaping parameters were updated
+	actualPowerShapingParameters, err := providerKeeper.GetConsumerPowerShapingParameters(ctx, consumerId)
+	require.NoError(t, err)
+	require.Equal(t, expectedPowerShapingParameters, actualPowerShapingParameters)
+
+	// assert phase
+	phase, found := providerKeeper.GetConsumerPhase(ctx, consumerId)
+	require.True(t, found)
+	require.Equal(t, providerkeeper.Initialized, phase)
+
+	// assert that chain is set to launch
+	consumerIds, err := providerKeeper.GetConsumersToBeLaunched(ctx, expectedInitializationParameters.SpawnTime)
+	require.NoError(t, err)
+	require.Equal(t, providertypes.ConsumerIds{
+		Ids: []string{consumerId},
+	}, consumerIds)
+
+	// re-update (change spawnTime) and verify that the chain is still to be launched at the new spawn time
+	previousSpawnTime := expectedInitializationParameters.SpawnTime
+	updatedSpawnTime := expectedInitializationParameters.SpawnTime.Add(time.Hour)
+	expectedInitializationParameters.SpawnTime = updatedSpawnTime
+	_, err = msgServer.UpdateConsumer(ctx,
+		&providertypes.MsgUpdateConsumer{Signer: expectedOwnerAddress, ConsumerId: consumerId,
+			Metadata:                 &expectedConsumerMetadata,
+			InitializationParameters: &expectedInitializationParameters,
+			PowerShapingParameters:   &expectedPowerShapingParameters})
+	require.NoError(t, err)
+
+	consumerIds, err = providerKeeper.GetConsumersToBeLaunched(ctx, previousSpawnTime)
+	require.NoError(t, err)
+	require.Empty(t, consumerIds)
+
+	consumerIds, err = providerKeeper.GetConsumersToBeLaunched(ctx, updatedSpawnTime)
+	require.NoError(t, err)
+	require.Equal(t, providertypes.ConsumerIds{
+		Ids: []string{consumerId},
+	}, consumerIds)
+
+}