diff --git a/tests/e2e/slashing.go b/tests/e2e/slashing.go index f9769bcf6b..850efcd1ac 100644 --- a/tests/e2e/slashing.go +++ b/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 diff --git a/x/ccv/provider/keeper/grpc_query.go b/x/ccv/provider/keeper/grpc_query.go index 98418ddad6..2438bdc986 100644 --- a/x/ccv/provider/keeper/grpc_query.go +++ b/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 } diff --git a/x/ccv/provider/keeper/hooks.go b/x/ccv/provider/keeper/hooks.go index 65e3a1c86f..5146efaf58 100644 --- a/x/ccv/provider/keeper/hooks.go +++ b/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) } diff --git a/x/ccv/provider/keeper/key_assignment.go b/x/ccv/provider/keeper/key_assignment.go index 5cb5bc439e..a1b6b7259b 100644 --- a/x/ccv/provider/keeper/key_assignment.go +++ b/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, diff --git a/x/ccv/provider/keeper/key_assignment_test.go b/x/ccv/provider/keeper/key_assignment_test.go index e441d60fe2..392290ddaa 100644 --- a/x/ccv/provider/keeper/key_assignment_test.go +++ b/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()) { diff --git a/x/ccv/utils/utils.go b/x/ccv/utils/utils.go index d1b3f6f38b..d35a6ff5ed 100644 --- a/x/ccv/utils/utils.go +++ b/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