fix: address comments from permissionless review (#2237)

* AttributeConsumerSpawnTime only on InitializeConsumer

* fix error message for ValidateByteSlice

* remove unnecessary errors

* fix comments

* add comment for consumer genesis creation

* adding comment on why not removing all consumer state

* improve comments

Author: mpoke

x/ccv/provider/keeper/consumer_lifecycle.go

@@ -60,7 +60,7 @@ func (k Keeper) InitializeConsumer(ctx sdk.Context, consumerId string) (time.Tim
 	return initializationParameters.SpawnTime, true
 }
 
-// BeginBlockLaunchConsumers launches initialized consumers that are ready to launch
+// BeginBlockLaunchConsumers launches initialized consumers chains for which the spawn time has passed
 func (k Keeper) BeginBlockLaunchConsumers(ctx sdk.Context) error {
 	consumerIds, err := k.ConsumeIdsFromTimeQueue(
 		ctx,
@@ -204,10 +204,10 @@ func (k Keeper) CreateConsumerClient(ctx sdk.Context, consumerId string) error {
 	// Set minimum height for equivocation evidence from this consumer chain
 	k.SetEquivocationEvidenceMinHeight(ctx, consumerId, initializationRecord.InitialHeight.RevisionHeight)
 
-	// Consumers start out with the unbonding period from the consumer addition prop
+	// Consumers start out with the unbonding period from the initialization parameters
 	consumerUnbondingPeriod := initializationRecord.UnbondingPeriod
 
-	// Create client state by getting template client from parameters and filling in zeroed fields from proposal.
+	// Create client state by getting template client from initialization parameters
 	clientState := k.GetTemplateClient(ctx)
 	clientState.ChainId = chainId
 	clientState.LatestHeight = initializationRecord.InitialHeight
@@ -351,6 +351,8 @@ func (k Keeper) MakeConsumerGenesis(
 	}
 	hash := tmtypes.NewValidatorSet(updatesAsValSet).Hash()
 
+	// note that providerFeePoolAddrStr is sent to the consumer during the IBC Channel handshake;
+	// see HandshakeMetadata in OnChanOpenTry on the provider-side, and OnChanOpenAck on the consumer-side
 	consumerGenesisParams := ccv.NewParams(
 		true,
 		initializationRecord.BlocksPerDistributionTransmission,
@@ -399,7 +401,7 @@ func (k Keeper) StopAndPrepareForConsumerRemoval(ctx sdk.Context, consumerId str
 	return nil
 }
 
-// BeginBlockRemoveConsumers iterates over the pending consumer proposals and stop/removes the chain if the removal time has passed
+// BeginBlockRemoveConsumers removes stopped consumer chain for which the removal time has passed
 func (k Keeper) BeginBlockRemoveConsumers(ctx sdk.Context) error {
 	consumerIds, err := k.ConsumeIdsFromTimeQueue(
 		ctx,
@@ -483,8 +485,11 @@ func (k Keeper) DeleteConsumerChain(ctx sdk.Context, consumerId string) (err err
 	k.DeleteConsumerRemovalTime(ctx, consumerId)
 
 	// TODO (PERMISSIONLESS) add newly-added state to be deleted
+
 	// Note that we do not delete ConsumerIdToChainIdKey and ConsumerIdToPhase, as well
 	// as consumer metadata, initialization and power-shaping parameters.
+	// This is to enable block explorers and front ends to show information of
+	// consumer chains that were removed without needing an archive node.
 
 	k.SetConsumerPhase(ctx, consumerId, types.CONSUMER_PHASE_DELETED)
 	k.Logger(ctx).Info("consumer chain deleted from provider", "consumerId", consumerId)

x/ccv/provider/keeper/msg_server.go

@@ -386,11 +386,6 @@ func (k msgServer) CreateConsumer(goCtx context.Context, msg *types.MsgCreateCon
 		return &resp, errorsmod.Wrapf(types.ErrInvalidConsumerInitializationParameters,
 			"cannot set consumer initialization parameters: %s", err.Error())
 	}
-	if !initializationParameters.SpawnTime.IsZero() {
-		// add SpawnTime event attribute
-		eventAttributes = append(eventAttributes,
-			sdk.NewAttribute(types.AttributeConsumerSpawnTime, initializationParameters.SpawnTime.String()))
-	}
 
 	// power-shaping parameters are optional and hence could be nil;
 	// in that case, set the default
@@ -413,6 +408,10 @@ func (k msgServer) CreateConsumer(goCtx context.Context, msg *types.MsgCreateCon
 			return &resp, errorsmod.Wrapf(ccvtypes.ErrInvalidConsumerState,
 				"cannot prepare chain with consumer id (%s) for launch", consumerId)
 		}
+
+		// add SpawnTime event attribute
+		eventAttributes = append(eventAttributes,
+			sdk.NewAttribute(types.AttributeConsumerSpawnTime, initializationParameters.SpawnTime.String()))
 	}
 
 	// add Phase event attribute
@@ -508,12 +507,6 @@ func (k msgServer) UpdateConsumer(goCtx context.Context, msg *types.MsgUpdateCon
 			return &resp, errorsmod.Wrapf(types.ErrInvalidConsumerInitializationParameters,
 				"cannot set consumer initialization parameters: %s", err.Error())
 		}
-
-		if !msg.InitializationParameters.SpawnTime.IsZero() {
-			// add SpawnTime event attribute
-			eventAttributes = append(eventAttributes,
-				sdk.NewAttribute(types.AttributeConsumerSpawnTime, msg.InitializationParameters.SpawnTime.String()))
-		}
 	}
 
 	if msg.PowerShapingParameters != nil {
@@ -569,6 +562,10 @@ func (k msgServer) UpdateConsumer(goCtx context.Context, msg *types.MsgUpdateCon
 			return &resp, errorsmod.Wrapf(ccvtypes.ErrInvalidConsumerState,
 				"cannot prepare chain with consumer id (%s) for launch", consumerId)
 		}
+
+		// add SpawnTime event attribute
+		eventAttributes = append(eventAttributes,
+			sdk.NewAttribute(types.AttributeConsumerSpawnTime, msg.InitializationParameters.SpawnTime.String()))
 	}
 
 	// add Owner event attribute

x/ccv/provider/keeper/params.go

@@ -11,7 +11,7 @@ import (
 	"github.com/cosmos/interchain-security/v6/x/ccv/provider/types"
 )
 
-// GetTemplateClient returns the template client for provider proposals
+// GetTemplateClient returns the template consumer client
 func (k Keeper) GetTemplateClient(ctx sdk.Context) *ibctmtypes.ClientState {
 	params := k.GetParams(ctx)
 	return params.TemplateClient

x/ccv/provider/types/errors.go

@@ -6,21 +6,16 @@ import (
 
 // Provider sentinel errors
 var (
-	ErrInvalidConsumerAdditionProposal         = errorsmod.Register(ModuleName, 1, "invalid consumer addition proposal")
-	ErrInvalidConsumerRemovalProp              = errorsmod.Register(ModuleName, 2, "invalid consumer removal proposal")
 	ErrUnknownConsumerId                       = errorsmod.Register(ModuleName, 3, "no consumer chain with this consumer id")
 	ErrUnknownConsumerChannelId                = errorsmod.Register(ModuleName, 4, "no consumer chain with this channel id")
 	ErrInvalidConsumerId                       = errorsmod.Register(ModuleName, 6, "invalid consumer id")
 	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")
 	ErrInvalidConsumerRewardDenom              = errorsmod.Register(ModuleName, 14, "invalid consumer reward denom")
 	ErrInvalidConsumerClient                   = errorsmod.Register(ModuleName, 16, "ccv channel is not built on correct client")
-	ErrConsumerChainNotFound                   = errorsmod.Register(ModuleName, 18, "consumer chain not found")
 	ErrCannotOptOutFromTopN                    = errorsmod.Register(ModuleName, 20, "cannot opt out from a Top N chain")
-	ErrInvalidConsumerModificationProposal     = errorsmod.Register(ModuleName, 22, "invalid consumer modification proposal")
 	ErrNoUnbondingTime                         = errorsmod.Register(ModuleName, 23, "provider unbonding time not found")
 	ErrUnauthorized                            = errorsmod.Register(ModuleName, 25, "unauthorized")
-	ErrBlankConsumerChainID                    = errorsmod.Register(ModuleName, 26, "consumer chain id must not be blank")
 	ErrInvalidPhase                            = errorsmod.Register(ModuleName, 27, "cannot perform action in the current phase of consumer chain")
 	ErrInvalidConsumerMetadata                 = errorsmod.Register(ModuleName, 28, "invalid consumer metadata")
 	ErrInvalidPowerShapingParameters           = errorsmod.Register(ModuleName, 29, "invalid power shaping parameters")

x/ccv/provider/types/msg.go

@@ -577,7 +577,7 @@ func ValidateInitializationParameters(initializationParameters ConsumerInitializ
 
 func ValidateByteSlice(hash []byte, maxLength int) error {
 	if len(hash) > maxLength {
-		return fmt.Errorf("hash is too long; got: %d, max: %d", len(hash), MaxHashLength)
+		return fmt.Errorf("hash is too long; got: %d, max: %d", len(hash), maxLength)
 	}
 	return nil
 }