Key assignment related panic cleanup

tests/e2e/slashing.go

@@ -691,7 +691,8 @@ func (suite *CCVTestSuite) TestSlashUndelegation() {
 		// slash validator on consumer chain
 		consumerKey, found := providerKeeper.GetValidatorConsumerPubKey(suite.providerCtx(), suite.consumerChain.ChainID, valConsAddr)
 		suite.Require().True(found)
-		consumerAddr := utils.TMCryptoPublicKeyToConsAddr(consumerKey)
+		consumerAddr, err := utils.TMCryptoPublicKeyToConsAddr(consumerKey)
+		suite.Require().NoError(err)
 		tc.slash(consumerAddr)
 
 		// increment time so that the unbonding period ends on the provider

x/ccv/provider/keeper/grpc_query.go

@@ -102,8 +102,13 @@ func (k Keeper) QueryValidatorConsumerAddr(goCtx context.Context, req *types.Que
 		return &types.QueryValidatorConsumerAddrResponse{}, nil
 	}
 
+	consumerAddr, err := utils.TMCryptoPublicKeyToConsAddr(consumerKey)
+	if err != nil {
+		return nil, err
+	}
+
 	return &types.QueryValidatorConsumerAddrResponse{
-		ConsumerAddress: utils.TMCryptoPublicKeyToConsAddr(consumerKey).String(),
+		ConsumerAddress: consumerAddr.String(),
 	}, nil
 }
 

x/ccv/provider/keeper/hooks.go

@@ -93,7 +93,11 @@ func (h Hooks) AfterValidatorCreated(ctx sdk.Context, valAddr sdk.ValAddress) {
 func (h Hooks) AfterValidatorRemoved(ctx sdk.Context, valConsAddr sdk.ConsAddress, valAddr sdk.ValAddress) {
 	for _, validatorConsumerPubKey := range h.k.GetAllValidatorConsumerPubKeys(ctx, nil) {
 		if sdk.ConsAddress(validatorConsumerPubKey.ProviderAddr).Equals(valConsAddr) {
-			consumerAddr := utils.TMCryptoPublicKeyToConsAddr(*validatorConsumerPubKey.ConsumerKey)
+			consumerAddr, err := utils.TMCryptoPublicKeyToConsAddr(*validatorConsumerPubKey.ConsumerKey)
+			if err != nil {
+				// An error here would indicate something is very wrong
+				panic("cannot get address of consumer key")
+			}
 			h.k.DeleteValidatorByConsumerAddr(ctx, validatorConsumerPubKey.ChainId, consumerAddr)
 			h.k.DeleteValidatorConsumerPubKey(ctx, validatorConsumerPubKey.ChainId, validatorConsumerPubKey.ProviderAddr)
 		}

x/ccv/provider/keeper/key_assignment.go

@@ -1,6 +1,8 @@
 package keeper
 
 import (
+	"fmt"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
@@ -25,7 +27,9 @@ func (k Keeper) GetValidatorConsumerPubKey(
 	}
 	err := consumerKey.Unmarshal(bz)
 	if err != nil {
-		panic(err)
+		// An error here would indicate something is very wrong,
+		// the consumer key is assumed to be correctly serialized in SetValidatorConsumerPubKey.
+		panic(fmt.Sprintf("failed to unmarshal consumer key: %v", err))
 	}
 	return consumerKey, true
 }
@@ -40,7 +44,9 @@ func (k Keeper) SetValidatorConsumerPubKey(
 	store := ctx.KVStore(k.storeKey)
 	bz, err := consumerKey.Marshal()
 	if err != nil {
-		panic(err)
+		// An error here would indicate something is very wrong,
+		// the consumer key is obtained from GetValidatorConsumerPubKey, called from
+		panic(fmt.Sprintf("failed to marshal consumer key: %v", err))
 	}
 	store.Set(types.ConsumerValidatorsKey(chainID, providerAddr), bz)
 }
@@ -68,12 +74,16 @@ func (k Keeper) GetAllValidatorConsumerPubKeys(ctx sdk.Context, chainID *string)
 	for ; iterator.Valid(); iterator.Next() {
 		chainID, providerAddr, err := types.ParseChainIdAndConsAddrKey(types.ConsumerValidatorsBytePrefix, iterator.Key())
 		if err != nil {
+			// An error here would indicate something is very wrong,
+			// the store key is assumed to be correctly serialized in SetValidatorConsumerPubKey.
 			panic(err)
 		}
 		var consumerKey tmprotocrypto.PublicKey
 		err = consumerKey.Unmarshal(iterator.Value())
 		if err != nil {
-			panic(err)
+			// An error here would indicate something is very wrong,
+			// the consumer key is assumed to be correctly serialized in SetValidatorConsumerPubKey.
+			panic(fmt.Sprintf("failed to unmarshal consumer key: %v", err))
 		}
 
 		validatorConsumerPubKeys = append(validatorConsumerPubKeys, types.ValidatorConsumerPubKey{
@@ -106,7 +116,9 @@ func (k Keeper) GetValidatorByConsumerAddr(
 	}
 	err := providerAddr.Unmarshal(bz)
 	if err != nil {
-		panic(err)
+		// An error here would indicate something is very wrong,
+		// the provider address is assumed to be correctly serialized in SetValidatorByConsumerAddr.
+		panic(fmt.Sprintf("failed to unmarshal provider address: %v", err))
 	}
 	return providerAddr, true
 }
@@ -120,10 +132,8 @@ func (k Keeper) SetValidatorByConsumerAddr(
 	providerAddr sdk.ConsAddress,
 ) {
 	store := ctx.KVStore(k.storeKey)
-	bz, err := providerAddr.Marshal()
-	if err != nil {
-		panic(err)
-	}
+	// Cons address is a type alias for a byte string, no marshaling needed
+	bz := providerAddr
 	store.Set(types.ValidatorsByConsumerAddrKey(chainID, consumerAddr), bz)
 }
 
@@ -151,12 +161,16 @@ func (k Keeper) GetAllValidatorsByConsumerAddr(ctx sdk.Context, chainID *string)
 	for ; iterator.Valid(); iterator.Next() {
 		chainID, consumerAddr, err := types.ParseChainIdAndConsAddrKey(types.ValidatorsByConsumerAddrBytePrefix, iterator.Key())
 		if err != nil {
-			panic(err)
+			// An error here would indicate something is very wrong,
+			// store keys are assumed to be correctly serialized in SetValidatorByConsumerAddr.
+			panic(fmt.Sprintf("failed to parse chainID and consumer address: %v", err))
 		}
 		var providerAddr sdk.ConsAddress
 		err = providerAddr.Unmarshal(iterator.Value())
 		if err != nil {
-			panic(err)
+			// An error here would indicate something is very wrong,
+			// the provider address is assumed to be correctly serialized in SetValidatorByConsumerAddr.
+			panic(fmt.Sprintf("failed to unmarshal provider address: %v", err))
 		}
 
 		validatorConsumerAddrs = append(validatorConsumerAddrs, types.ValidatorByConsumerAddr{
@@ -193,7 +207,9 @@ func (k Keeper) GetKeyAssignmentReplacement(
 
 	err := pubKeyAndPower.Unmarshal(bz)
 	if err != nil {
-		panic(err)
+		// An error here would indicate something is very wrong,
+		// the public key and power are assumed to be correctly serialized in SetKeyAssignmentReplacement.
+		panic(fmt.Sprintf("failed to unmarshal public key and power: %v", err))
 	}
 	return pubKeyAndPower.PubKey, pubKeyAndPower.Power, true
 }
@@ -212,7 +228,11 @@ func (k Keeper) SetKeyAssignmentReplacement(
 	pubKeyAndPower := abci.ValidatorUpdate{PubKey: prevCKey, Power: power}
 	bz, err := pubKeyAndPower.Marshal()
 	if err != nil {
-		panic(err)
+		// An error here would indicate something is very wrong,
+		// prevCKey is obtained from GetValidatorConsumerPubKey (called from AssignConsumerKey),
+		// and power is obtained from GetLastValidatorPower (called from AssignConsumerKey).
+		// Both of which are assumed to return valid values.
+		panic(fmt.Sprintf("failed to marshal public key and power: %v", err))
 	}
 	store.Set(types.KeyAssignmentReplacementsKey(chainID, providerAddr), bz)
 }
@@ -231,12 +251,16 @@ func (k Keeper) GetAllKeyAssignmentReplacements(ctx sdk.Context, chainID string)
 	for ; iterator.Valid(); iterator.Next() {
 		_, providerAddr, err := types.ParseChainIdAndConsAddrKey(types.KeyAssignmentReplacementsBytePrefix, iterator.Key())
 		if err != nil {
+			// An error here would indicate something is very wrong,
+			// store keys are assumed to be correctly serialized in SetKeyAssignmentReplacement.
 			panic(err)
 		}
 		var pubKeyAndPower abci.ValidatorUpdate
 		err = pubKeyAndPower.Unmarshal(iterator.Value())
 		if err != nil {
-			panic(err)
+			// An error here would indicate something is very wrong,
+			// the public key and power are assumed to be correctly serialized in SetKeyAssignmentReplacement.
+			panic(fmt.Sprintf("failed to unmarshal public key and power: %v", err))
 		}
 
 		replacements = append(replacements, types.KeyAssignmentReplacement{
@@ -272,12 +296,16 @@ func (k Keeper) AppendConsumerAddrsToPrune(ctx sdk.Context, chainID string, vscI
 	if bz != nil {
 		err := consumerAddrsToPrune.Unmarshal(bz)
 		if err != nil {
+			// An error here would indicate something is very wrong,
+			// the data bytes are assumed to be correctly serialized by previous calls to this method.
 			panic(err)
 		}
 	}
 	consumerAddrsToPrune.Addresses = append(consumerAddrsToPrune.Addresses, consumerAddr)
 	bz, err := consumerAddrsToPrune.Marshal()
 	if err != nil {
+		// An error here would indicate something is very wrong,
+		// consumerAddrsToPrune is instantiated in this method and should be able to be marshaled.
 		panic(err)
 	}
 	store.Set(types.ConsumerAddrsToPruneKey(chainID, vscID), bz)
@@ -297,7 +325,9 @@ func (k Keeper) GetConsumerAddrsToPrune(
 	}
 	err := consumerAddrsToPrune.Unmarshal(bz)
 	if err != nil {
-		panic(err)
+		// An error here would indicate something is very wrong,
+		// the list of consumer addresses is assumed to be correctly serialized in AppendConsumerAddrsToPrune.
+		panic(fmt.Sprintf("failed to unmarshal consumer addresses to prune: %v", err))
 	}
 	return
 }
@@ -315,11 +345,15 @@ func (k Keeper) GetAllConsumerAddrsToPrune(ctx sdk.Context, chainID string) (con
 	for ; iterator.Valid(); iterator.Next() {
 		_, vscID, err := types.ParseChainIdAndUintIdKey(types.ConsumerAddrsToPruneBytePrefix, iterator.Key())
 		if err != nil {
+			// An error here would indicate something is very wrong,
+			// store keys are assumed to be correctly serialized in AppendConsumerAddrsToPrune.
 			panic(err)
 		}
 		var addrs types.AddressList
 		err = addrs.Unmarshal(iterator.Value())
 		if err != nil {
+			// An error here would indicate something is very wrong,
+			// the list of consumer addresses is assumed to be correctly serialized in AppendConsumerAddrsToPrune.
 			panic(err)
 		}
 
@@ -348,10 +382,12 @@ func (k Keeper) AssignConsumerKey(
 	consumerKey tmprotocrypto.PublicKey,
 ) error {
 
-	consumerAddr := utils.TMCryptoPublicKeyToConsAddr(consumerKey)
+	consumerAddr, err := utils.TMCryptoPublicKeyToConsAddr(consumerKey)
+	if err != nil {
+		return err
+	}
 
 	providerAddr, err := validator.GetConsAddr()
-
 	if err != nil {
 		return err
 	}
@@ -385,11 +421,15 @@ func (k Keeper) AssignConsumerKey(
 			// mark this old consumer key as prunable once the VSCMaturedPacket
 			// for the current VSC ID is received;
 			// note: this state is removed on receiving the VSCMaturedPacket
+			oldConsumerAddr, err := utils.TMCryptoPublicKeyToConsAddr(oldConsumerKey)
+			if err != nil {
+				return err
+			}
 			k.AppendConsumerAddrsToPrune(
 				ctx,
 				chainID,
 				k.GetValidatorSetUpdateId(ctx),
-				utils.TMCryptoPublicKeyToConsAddr(oldConsumerKey),
+				oldConsumerAddr,
 			)
 		} else {
 			// the validator had no key assigned on this consumer chain
@@ -423,11 +463,11 @@ func (k Keeper) AssignConsumerKey(
 		// from the old consumer address to the provider address (if any)
 		// get the previous key assigned for this validator on this consumer chain
 		if oldConsumerKey, found := k.GetValidatorConsumerPubKey(ctx, chainID, providerAddr); found {
-			k.DeleteValidatorByConsumerAddr(
-				ctx,
-				chainID,
-				utils.TMCryptoPublicKeyToConsAddr(oldConsumerKey),
-			)
+			oldConsumerAddr, err := utils.TMCryptoPublicKeyToConsAddr(oldConsumerKey)
+			if err != nil {
+				return err
+			}
+			k.DeleteValidatorByConsumerAddr(ctx, chainID, oldConsumerAddr)
 		}
 	}
 
@@ -452,7 +492,10 @@ func (k Keeper) ApplyKeyAssignmentToValUpdates(
 	valUpdates []abci.ValidatorUpdate,
 ) (newUpdates []abci.ValidatorUpdate, err error) {
 	for _, valUpdate := range valUpdates {
-		providerAddr := utils.TMCryptoPublicKeyToConsAddr(valUpdate.PubKey)
+		providerAddr, err := utils.TMCryptoPublicKeyToConsAddr(valUpdate.PubKey)
+		if err != nil {
+			return nil, err
+		}
 
 		// If a key assignment replacement is found, we remove the valupdate with the old consumer key,
 		// create two new valupdates,

x/ccv/provider/keeper/key_assignment_test.go

@@ -241,16 +241,19 @@ func TestConsumerAddrsToPruneCRUD(t *testing.T) {
 	keeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
 	defer ctrl.Finish()
 
+	addrsToPrune := keeper.GetConsumerAddrsToPrune(ctx, chainID, vscID).Addresses
+	require.Empty(t, addrsToPrune)
+
 	keeper.AppendConsumerAddrsToPrune(ctx, chainID, vscID, consumerAddr)
 
-	addrToPrune := keeper.GetConsumerAddrsToPrune(ctx, chainID, vscID).Addresses
-	require.NotEmpty(t, addrToPrune, "address to prune is empty")
-	require.Len(t, addrToPrune, 1, "address to prune is not len 1")
-	require.Equal(t, sdk.ConsAddress(addrToPrune[0]), consumerAddr)
+	addrsToPrune = keeper.GetConsumerAddrsToPrune(ctx, chainID, vscID).Addresses
+	require.NotEmpty(t, addrsToPrune, "addresses to prune is empty")
+	require.Len(t, addrsToPrune, 1, "addresses to prune is not len 1")
+	require.Equal(t, sdk.ConsAddress(addrsToPrune[0]), consumerAddr)
 
 	keeper.DeleteConsumerAddrsToPrune(ctx, chainID, vscID)
-	addrToPrune = keeper.GetConsumerAddrsToPrune(ctx, chainID, vscID).Addresses
-	require.Empty(t, addrToPrune, "address to prune was returned")
+	addrsToPrune = keeper.GetConsumerAddrsToPrune(ctx, chainID, vscID).Addresses
+	require.Empty(t, addrsToPrune, "addresses to prune was returned")
 }
 
 func TestGetAllConsumerAddrsToPrune(t *testing.T) {
@@ -336,7 +339,8 @@ func checkCorrectPruningProperty(ctx sdk.Context, k providerkeeper.Keeper, chain
 		// Try to find a validator who has this consumer address currently assigned
 		isCurrentlyAssigned := false
 		for _, valconsPubKey := range k.GetAllValidatorConsumerPubKeys(ctx, &valByConsAddr.ChainId) {
-			if utils.TMCryptoPublicKeyToConsAddr(*valconsPubKey.ConsumerKey).Equals(sdk.ConsAddress(valByConsAddr.ConsumerAddr)) {
+			consumerAddr, _ := utils.TMCryptoPublicKeyToConsAddr(*valconsPubKey.ConsumerKey)
+			if consumerAddr.Equals(sdk.ConsAddress(valByConsAddr.ConsumerAddr)) {
 				isCurrentlyAssigned = true
 				break
 			}
@@ -619,7 +623,7 @@ func (vs *ValSet) apply(updates []abci.ValidatorUpdate) {
 	for _, u := range updates {
 		for i, id := range vs.identities { // n2 looping but n is tiny
 			// cons := sdk.ConsAddress(utils.GetChangePubKeyAddress(u))
-			cons := utils.TMCryptoPublicKeyToConsAddr(u.PubKey)
+			cons, _ := utils.TMCryptoPublicKeyToConsAddr(u.PubKey)
 			if id.SDKConsAddress().Equals(cons) {
 				vs.power[i] = u.Power
 			}
@@ -822,7 +826,8 @@ func TestSimulatedAssignmentsAndUpdateApplication(t *testing.T) {
 						// Use default if unassigned
 						ck = idP.TMProtoCryptoPublicKey()
 					}
-					consC := utils.TMCryptoPublicKeyToConsAddr(ck)
+					consC, err := utils.TMCryptoPublicKeyToConsAddr(ck)
+					require.NoError(t, err)
 					// Find the corresponding consumer validator (must always be found)
 					for j, idC := range consumerValset.identities {
 						if consC.Equals(idC.SDKConsAddress()) {

x/ccv/utils/utils.go

@@ -53,12 +53,14 @@ func GetChangePubKeyAddress(change abci.ValidatorUpdate) (addr []byte) {
 	return pk.Address()
 }
 
-func TMCryptoPublicKeyToConsAddr(k tmprotocrypto.PublicKey) sdk.ConsAddress {
+// TMCryptoPublicKeyToConsAddr converts a TM public key to an SDK public key
+// and returns the associated consensus address
+func TMCryptoPublicKeyToConsAddr(k tmprotocrypto.PublicKey) (sdk.ConsAddress, error) {
 	sdkK, err := cryptocodec.FromTmProtoPublicKey(k)
 	if err != nil {
-		panic("could not get public key from tm proto public key")
+		return nil, err
 	}
-	return sdk.GetConsAddress(sdkK)
+	return sdk.GetConsAddress(sdkK), nil
 }
 
 // SendIBCPacket sends an IBC packet with packetData