removed (partially) consumer removal proposal

x/ccv/provider/keeper/msg_server.go

@@ -97,10 +97,8 @@ func (k msgServer) RemoveConsumer(
 	}
 
 	ctx := sdk.UnwrapSDKContext(goCtx)
-	err := k.Keeper.HandleConsumerRemovalProposal(ctx, msg)
-	if err != nil {
-		return nil, errorsmod.Wrapf(err, "failed handling ConsumerAddition proposal")
-	}
+
+	k.Keeper.SetConsumerIdToStopTime(ctx, msg.ConsumerId, msg.StopTime)
 
 	return &types.MsgRemoveConsumerResponse{}, nil
 }

x/ccv/provider/keeper/permissionless.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/interchain-security/v5/x/ccv/provider/types"
+	"time"
 )
 
 // ConsumerPhase captures the phases of a consumer chain according to `docs/docs/adrs/adr-018-permissionless-ics.md`
@@ -198,7 +199,7 @@ func (k Keeper) GetConsumerIdToPhase(ctx sdk.Context, consumerId string) (Consum
 }
 
 // SetConsumerIdToPhase sets the phase associated with this consumer id
-// TODO (PERMISSIONLESS): use this method when launching and when stopping a chian
+// TODO (PERMISSIONLESS): use this method when launching and when stopping a chain
 func (k Keeper) SetConsumerIdToPhase(ctx sdk.Context, consumerId string, phase ConsumerPhase) {
 	store := ctx.KVStore(k.storeKey)
 	store.Set(types.ConsumerIdToPhaseKey(consumerId), []byte{byte(phase)})
@@ -210,6 +211,36 @@ func (k Keeper) DeleteConsumerIdToPhase(ctx sdk.Context, consumerId string) {
 	store.Delete(types.ConsumerIdToPhaseKey(consumerId))
 }
 
+// GetConsumerIdToStopTime returns the stop time associated with the to-be-stopped chain with consumer id
+func (k Keeper) GetConsumerIdToStopTime(ctx sdk.Context, consumerId string) (time.Time, bool) {
+	store := ctx.KVStore(k.storeKey)
+	buf := store.Get(types.ConsumerIdToStopTimeKey(consumerId))
+	if buf == nil {
+		return time.Time{}, false
+	}
+	var time time.Time
+	if err := time.UnmarshalBinary(buf); err != nil {
+		panic(fmt.Errorf("failed to unmarshal time: %w", err))
+	}
+	return time, true
+}
+
+// SetConsumerIdToStopTime sets the stop time associated with this consumer id
+func (k Keeper) SetConsumerIdToStopTime(ctx sdk.Context, consumerId string, stopTime time.Time) {
+	store := ctx.KVStore(k.storeKey)
+	buf, err := stopTime.MarshalBinary()
+	if err != nil {
+		panic(fmt.Errorf("failed to marshal time: %w", err))
+	}
+	store.Set(types.ConsumerIdToStopTimeKey(consumerId), buf)
+}
+
+// DeleteConsumerIdToStopTime deletes the stop time associated with this consumer id
+func (k Keeper) DeleteConsumerIdToStopTime(ctx sdk.Context, consumerId string) {
+	store := ctx.KVStore(k.storeKey)
+	store.Delete(types.ConsumerIdToStopTimeKey(consumerId))
+}
+
 // GetClientIdToConsumerId returns the consumer id associated with this client id
 func (k Keeper) GetClientIdToConsumerId(ctx sdk.Context, clientId string) (string, bool) {
 	store := ctx.KVStore(k.storeKey)
@@ -242,16 +273,20 @@ func (k Keeper) GetInitializedConsumersReadyToLaunch(ctx sdk.Context) []string {
 	var consumerIds []string
 
 	for ; iterator.Valid(); iterator.Next() {
+		// the `consumerId` resides in the whole key, but we skip the first byte (because it's the `ConsumerIdKey`)
+		// plus 8 more bytes for the `uint64` in the key that contains the length of the `consumerId`
+		consumerId := string(iterator.Key()[1+8:])
+
 		var record types.ConsumerInitializationRecord
 		err := record.Unmarshal(iterator.Value())
 		if err != nil {
-			panic(fmt.Errorf("failed to unmarshal consumer record: %w for consumer id: %s", err, string(iterator.Value())))
+			panic(fmt.Errorf("failed to unmarshal consumer record: %w for consumer id: %s", err, consumerId))
 		}
 
-		if !ctx.BlockTime().Before(record.SpawnTime) {
-			// the `consumerId` resides in the whole key, but we skip the first byte (because it's the `ConsumerIdKey`)
-			// plus 8 more bytes for the `uint64` in the key that contains the length of the `consumerId`
-			consumerIds = append(consumerIds, string(iterator.Key()[1+8:]))
+		// if current block time is equal to or after spawnTime, and the chain is initialized, we can launch the chain
+		phase, found := k.GetConsumerIdToPhase(ctx, consumerId)
+		if !ctx.BlockTime().Before(record.SpawnTime) && (found && phase == Initialized) {
+			consumerIds = append(consumerIds, consumerId)
 		}
 	}
 
@@ -344,3 +379,33 @@ func (k Keeper) UpdateConsumer(ctx sdk.Context, consumerId string) error {
 
 	return nil
 }
+
+// GetLaunchedConsumersReadyToStop returns the consumer ids of the pending launched consumer chains
+// that are ready to stop
+func (k Keeper) GetLaunchedConsumersReadyToStop(ctx sdk.Context) []string {
+	store := ctx.KVStore(k.storeKey)
+	iterator := storetypes.KVStorePrefixIterator(store, types.ConsumerIdToStopTimeKeyNamePrefix())
+	defer iterator.Close()
+
+	var consumerIds []string
+
+	for ; iterator.Valid(); iterator.Next() {
+		// the `consumerId` resides in the whole key, but we skip the first byte (because it's the `ConsumerIdKey`)
+		// plus 8 more bytes for the `uint64` in the key that contains the length of the `consumerId`
+		consumerId := string(iterator.Key()[1+8:])
+
+		var stopTime time.Time
+		err := stopTime.UnmarshalBinary(iterator.Value())
+		if err != nil {
+			panic(fmt.Errorf("failed to unmarshal stop stopTime: %w for consumer id: %s", err, consumerId))
+		}
+
+		// if current block time is equal to or after stop stopTime, and the chain is launched we can stop the chain
+		phase, found := k.GetConsumerIdToPhase(ctx, consumerId)
+		if !ctx.BlockTime().Before(stopTime) && (found && phase == Launched) {
+			consumerIds = append(consumerIds, string(iterator.Key()[1+8:]))
+		}
+	}
+
+	return consumerIds
+}

x/ccv/provider/keeper/permissionless_test.go

@@ -187,6 +187,25 @@ func TestConsumerIdToPhase(t *testing.T) {
 	require.Equal(t, keeper.Launched, phase)
 }
 
+// TestConsumerIdToStopTime tests the getter, setter, and deletion methods of the consumer id to stop times
+func TestConsumerIdToStopTime(t *testing.T) {
+	providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
+	defer ctrl.Finish()
+
+	_, found := providerKeeper.GetConsumerIdToStopTime(ctx, "consumerId")
+	require.False(t, found)
+
+	expectedStopTime := time.Unix(1234, 56789)
+	providerKeeper.SetConsumerIdToStopTime(ctx, "consumerId", expectedStopTime)
+	actualStopTime, found := providerKeeper.GetConsumerIdToStopTime(ctx, "consumerId")
+	require.True(t, found)
+	require.Equal(t, actualStopTime, expectedStopTime)
+
+	providerKeeper.DeleteConsumerIdToStopTime(ctx, "consumerId")
+	_, found = providerKeeper.GetConsumerIdToStopTime(ctx, "consumerId")
+	require.False(t, found)
+}
+
 // TestGetInitializedConsumersReadyToLaunch tests that the ready to-be-launched consumer chains are returned
 func TestGetInitializedConsumersReadyToLaunch(t *testing.T) {
 	providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
@@ -196,10 +215,13 @@ func TestGetInitializedConsumersReadyToLaunch(t *testing.T) {
 	require.Empty(t, providerKeeper.GetInitializedConsumersReadyToLaunch(ctx))
 
 	// set 3 initialization records with different spawn times
+	providerKeeper.SetConsumerIdToPhase(ctx, "consumerId1", keeper.Initialized)
 	providerKeeper.SetConsumerIdToInitializationRecord(ctx, "consumerId1",
 		providertypes.ConsumerInitializationRecord{SpawnTime: time.Unix(10, 0)})
+	providerKeeper.SetConsumerIdToPhase(ctx, "consumerId2", keeper.Initialized)
 	providerKeeper.SetConsumerIdToInitializationRecord(ctx, "consumerId2",
 		providertypes.ConsumerInitializationRecord{SpawnTime: time.Unix(20, 0)})
+	providerKeeper.SetConsumerIdToPhase(ctx, "consumerId3", keeper.Initialized)
 	providerKeeper.SetConsumerIdToInitializationRecord(ctx, "consumerId3",
 		providertypes.ConsumerInitializationRecord{SpawnTime: time.Unix(30, 0)})
 
@@ -219,3 +241,35 @@ func TestGetInitializedConsumersReadyToLaunch(t *testing.T) {
 	ctx = ctx.WithBlockTime(time.Unix(30, 0))
 	require.Equal(t, []string{"consumerId1", "consumerId2", "consumerId3"}, providerKeeper.GetInitializedConsumersReadyToLaunch(ctx))
 }
+
+func TestGetLaunchedConsumersReadyToStop(t *testing.T) {
+	providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
+	defer ctrl.Finish()
+
+	// no chains to-be-stopped exist
+	require.Empty(t, providerKeeper.GetLaunchedConsumersReadyToStop(ctx))
+
+	// set 3 initialization records with different spawn times
+	providerKeeper.SetConsumerIdToPhase(ctx, "consumerId1", keeper.Launched)
+	providerKeeper.SetConsumerIdToStopTime(ctx, "consumerId1", time.Unix(10, 0))
+	providerKeeper.SetConsumerIdToPhase(ctx, "consumerId2", keeper.Launched)
+	providerKeeper.SetConsumerIdToStopTime(ctx, "consumerId2", time.Unix(20, 0))
+	providerKeeper.SetConsumerIdToPhase(ctx, "consumerId3", keeper.Launched)
+	providerKeeper.SetConsumerIdToStopTime(ctx, "consumerId3", time.Unix(30, 0))
+
+	// time has not yet reached the stop time of "consumerId1"
+	ctx = ctx.WithBlockTime(time.Unix(9, 999999999))
+	require.Empty(t, providerKeeper.GetLaunchedConsumersReadyToStop(ctx))
+
+	// time has reached the stop time of "consumerId1"
+	ctx = ctx.WithBlockTime(time.Unix(10, 0))
+	require.Equal(t, []string{"consumerId1"}, providerKeeper.GetLaunchedConsumersReadyToStop(ctx))
+
+	// time has reached the stop time of "consumerId1" and "consumerId2"
+	ctx = ctx.WithBlockTime(time.Unix(20, 0))
+	require.Equal(t, []string{"consumerId1", "consumerId2"}, providerKeeper.GetLaunchedConsumersReadyToStop(ctx))
+
+	// time has reached the stop time of all chains
+	ctx = ctx.WithBlockTime(time.Unix(30, 0))
+	require.Equal(t, []string{"consumerId1", "consumerId2", "consumerId3"}, providerKeeper.GetLaunchedConsumersReadyToStop(ctx))
+}

x/ccv/provider/keeper/proposal.go

@@ -23,17 +23,6 @@ import (
 	ccv "github.com/cosmos/interchain-security/v5/x/ccv/types"
 )
 
-// Wrapper for the new proposal message MsgConsumerRemoval
-// Will replace legacy handler HandleLegacyConsumerRemovalProposal
-func (k Keeper) HandleConsumerRemovalProposal(ctx sdk.Context, proposal *types.MsgRemoveConsumer) error {
-	p := types.ConsumerRemovalProposal{
-		ConsumerId: proposal.ConsumerId,
-		StopTime:   proposal.StopTime,
-	}
-
-	return k.HandleLegacyConsumerRemovalProposal(ctx, &p)
-}
-
 // Wrapper for the new proposal message MsgChangeRewardDenoms
 // Will replace legacy handler HandleLegacyConsumerRewardDenomProposal
 func (k Keeper) HandleConsumerRewardDenomProposal(ctx sdk.Context, proposal *types.MsgChangeRewardDenoms) error {
@@ -441,73 +430,72 @@ func (k Keeper) DeletePendingConsumerRemovalProps(ctx sdk.Context, proposals ...
 	}
 }
 
-// BeginBlockCCR iterates over the pending consumer removal proposals
-// in order and stop/removes the chain if the stop time has passed,
-// otherwise it will break out of loop and return. Executed proposals are deleted.
-//
-// See: https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/methods.md#ccv-pcf-bblock-ccr1
-// Spec tag: [CCV-PCF-BBLOCK-CCR.1]
+// BeginBlockCCR iterates over the pending consumer proposals and stop/removes the chain if the stop time has passed
 func (k Keeper) BeginBlockCCR(ctx sdk.Context) {
-	propsToExecute := k.GetConsumerRemovalPropsToExecute(ctx)
-
-	for _, prop := range propsToExecute {
+	for _, consumerId := range k.GetLaunchedConsumersReadyToStop(ctx) {
 		// stop consumer chain in a cached context to handle errors
-		cachedCtx, writeFn, err := k.StopConsumerChainInCachedCtx(ctx, prop)
+		cachedCtx, writeFn := ctx.CacheContext()
+
+		stopTime, found := k.GetConsumerIdToStopTime(ctx, consumerId)
+		if !found {
+			ctx.Logger().Info("this chain (%s) is not meant to be stopped", consumerId)
+			continue
+		}
+
+		err := k.StopConsumerChain(cachedCtx, consumerId, true)
 		if err != nil {
-			// drop the proposal
 			ctx.Logger().Info("consumer chain could not be stopped: %w", err)
 			continue
 		}
 		// The cached context is created with a new EventManager so we merge the event
 		// into the original context
+		// TODO (PERMISSIONLESS): verify this here and in the initialized chains to launch
 		ctx.EventManager().EmitEvents(cachedCtx.EventManager().Events())
-		// write cache
+
+		k.SetConsumerIdToPhase(cachedCtx, consumerId, Stopped)
 		writeFn()
 
-		k.Logger(ctx).Info("executed consumer removal proposal",
-			"consumer id", prop.ConsumerId,
-			"title", prop.Title,
-			"stop time", prop.StopTime.UTC(),
+		k.Logger(ctx).Info("executed consumer removal",
+			"consumer id", consumerId,
+			"stop time", stopTime,
 		)
 	}
-	// delete the executed proposals
-	k.DeletePendingConsumerRemovalProps(ctx, propsToExecute...)
 }
 
-// GetConsumerRemovalPropsToExecute iterates over the pending consumer removal proposals
-// and returns an ordered list of consumer removal proposals to be executed,
-// ie. consumer chains to be stopped and removed from the provider chain.
-// A prop is included in the returned list if its proposed stop time has passed.
+//// GetConsumerRemovalPropsToExecute iterates over the pending consumer removal proposals
+//// and returns an ordered list of consumer removal proposals to be executed,
+//// ie. consumer chains to be stopped and removed from the provider chain.
+//// A prop is included in the returned list if its proposed stop time has passed.
+////
+//// Note: this method is split out from BeginBlockCCR to be easily unit tested.
+//func (k Keeper) GetConsumerRemovalPropsToExecute(ctx sdk.Context) []types.ConsumerRemovalProposal {
+//	// store the (to be) executed consumer removal proposals in order
+//	propsToExecute := []types.ConsumerRemovalProposal{}
 //
-// Note: this method is split out from BeginBlockCCR to be easily unit tested.
-func (k Keeper) GetConsumerRemovalPropsToExecute(ctx sdk.Context) []types.ConsumerRemovalProposal {
-	// store the (to be) executed consumer removal proposals in order
-	propsToExecute := []types.ConsumerRemovalProposal{}
-
-	store := ctx.KVStore(k.storeKey)
-	iterator := storetypes.KVStorePrefixIterator(store, types.PendingCRPKeyPrefix())
-	defer iterator.Close()
-
-	for ; iterator.Valid(); iterator.Next() {
-		var prop types.ConsumerRemovalProposal
-		err := prop.Unmarshal(iterator.Value())
-		if err != nil {
-			// An error here would indicate something is very wrong,
-			// the ConsumerRemovalProposal is assumed to be correctly serialized in SetPendingConsumerRemovalProp.
-			panic(fmt.Errorf("failed to unmarshal consumer removal proposal: %w", err))
-		}
-
-		// If current block time is equal to or after stop time, proposal is ready to be executed
-		if !ctx.BlockTime().Before(prop.StopTime) {
-			propsToExecute = append(propsToExecute, prop)
-		} else {
-			// No more proposals to check, since they're stored/ordered by timestamp.
-			break
-		}
-	}
-
-	return propsToExecute
-}
+//	store := ctx.KVStore(k.storeKey)
+//	iterator := storetypes.KVStorePrefixIterator(store, types.PendingCRPKeyPrefix())
+//	defer iterator.Close()
+//
+//	for ; iterator.Valid(); iterator.Next() {
+//		var prop types.ConsumerRemovalProposal
+//		err := prop.Unmarshal(iterator.Value())
+//		if err != nil {
+//			// An error here would indicate something is very wrong,
+//			// the ConsumerRemovalProposal is assumed to be correctly serialized in SetPendingConsumerRemovalProp.
+//			panic(fmt.Errorf("failed to unmarshal consumer removal proposal: %w", err))
+//		}
+//
+//		// If current block time is equal to or after stop time, proposal is ready to be executed
+//		if !ctx.BlockTime().Before(prop.StopTime) {
+//			propsToExecute = append(propsToExecute, prop)
+//		} else {
+//			// No more proposals to check, since they're stored/ordered by timestamp.
+//			break
+//		}
+//	}
+//
+//	return propsToExecute
+//}
 
 // GetAllPendingConsumerRemovalProps iterates through the pending consumer removal proposals.
 //