From 167a74d21d7c00f9459e4a6100ce59a13612639e Mon Sep 17 00:00:00 2001
From: Simon Noetzlin <simon.ntz@gmail.com>
Date: Thu, 23 Nov 2023 11:45:59 +0100
Subject: [PATCH] remove todos and update doc

---
 x/ccv/provider/keeper/key_assignment.go | 11 ++++++-----
 x/ccv/provider/keeper/msg_server.go     |  9 ++-------
 x/ccv/provider/types/msg.go             |  3 +--
 3 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/x/ccv/provider/keeper/key_assignment.go b/x/ccv/provider/keeper/key_assignment.go
index 49d940a947..da6aa8de94 100644
--- a/x/ccv/provider/keeper/key_assignment.go
+++ b/x/ccv/provider/keeper/key_assignment.go
@@ -370,7 +370,8 @@ func (k Keeper) DeleteConsumerAddrsToPrune(ctx sdk.Context, chainID string, vscI
 }
 
 // AssignConsumerKey assigns the consumerKey to the validator with providerAddr
-// on the consumer chain with ID chainID
+// on the consumer chain with ID chainID, if it is either registered or currently
+// voted on in a ConsumerAddition governance proposal
 func (k Keeper) AssignConsumerKey(
 	ctx sdk.Context,
 	chainID string,
@@ -398,15 +399,15 @@ func (k Keeper) AssignConsumerKey(
 	providerAddr := types.NewProviderConsAddress(consAddrTmp)
 
 	if existingVal, found := k.stakingKeeper.GetValidatorByConsAddr(ctx, consumerAddr.ToSdkConsAddr()); found {
-		// If there is a validator using the consumer key to validate on the provider
-		// we prevent assigning the consumer key, unless the validator is assigning validator.
-		// This ensures that a validator joining the active set who has not explicitly assigned
-		// a consumer key, will be able to use their provider key as consumer key (as per default).
+		// If there is already a different validator using the consumer key to validate on the provider
+		// we prevent assigning the consumer key.
 		if existingVal.OperatorAddress != validator.OperatorAddress {
 			return errorsmod.Wrapf(
 				types.ErrConsumerKeyInUse, "a different validator already uses the consumer key",
 			)
 		}
+		// We prevent a validator from assigning the default provider key as a consumer key
+		// if it has not already assigned a different consumer key
 		_, found := k.GetValidatorConsumerPubKey(ctx, chainID, providerAddr)
 		if !found {
 			return errorsmod.Wrapf(
diff --git a/x/ccv/provider/keeper/msg_server.go b/x/ccv/provider/keeper/msg_server.go
index fecbdc8c36..3808b8f0f7 100644
--- a/x/ccv/provider/keeper/msg_server.go
+++ b/x/ccv/provider/keeper/msg_server.go
@@ -26,16 +26,11 @@ func NewMsgServerImpl(keeper *Keeper) types.MsgServer {
 
 var _ types.MsgServer = msgServer{}
 
-// CreateValidator defines a method for creating a new validator
+// AssignConsumerKey defines a method to assign a consensus key on a consumer chain
+// for a given validator on the provider
 func (k msgServer) AssignConsumerKey(goCtx context.Context, msg *types.MsgAssignConsumerKey) (*types.MsgAssignConsumerKeyResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
-	// It is possible to assign keys for consumer chains that are not yet approved.
-	// TODO: In future, a mechanism will be added to limit assigning keys to chains
-	// which are approved or pending approval, only.
-	// Note that current attack potential is restricted because validators must sign
-	// the transaction, and the chainID size is limited.
-
 	providerValidatorAddr, err := sdk.ValAddressFromBech32(msg.ProviderAddr)
 	if err != nil {
 		return nil, err
diff --git a/x/ccv/provider/types/msg.go b/x/ccv/provider/types/msg.go
index 901aa03600..cb124b10b7 100644
--- a/x/ccv/provider/types/msg.go
+++ b/x/ccv/provider/types/msg.go
@@ -61,8 +61,7 @@ func (msg MsgAssignConsumerKey) ValidateBasic() error {
 	// It is possible to assign keys for consumer chains that are not yet approved.
 	// This can only be done by a signing validator, but it is still sensible
 	// to limit the chainID size to prevent abuse.
-	// TODO: In future, a mechanism will be added to limit assigning keys to chains
-	// which are approved or pending approval, only.
+
 	if 128 < len(msg.ChainId) {
 		return ErrBlankConsumerChainID
 	}