fix: partially revert key assignment type safety PR (#980)

* use bytes in place where possible

* fix tests

* add v2 to proto files, adjust protocgen scripts

* regen proto

* change protos, define custom types, fix references

* Update key_assignment_test.go

* Update key_assignment.go

* format

* Update CHANGELOG.md

* nit for better diff

CHANGELOG.md

@@ -21,6 +21,7 @@ Some PRs from v2.0.0 may reappear from other releases below. This is due to the
 
 ## PRs included in v2.0.0
 
+* (fix) partially revert key assignment type safety PR [#980](https://github.com/cosmos/interchain-security/pull/980)
 * (fix) Remove panics on failure to send IBC packets [#876](https://github.com/cosmos/interchain-security/pull/876)
 * (fix) consumer key prefix order to avoid complex migrations [#963](https://github.com/cosmos/interchain-security/pull/963)
 * (fix) Prevent denom DOS [#931](https://github.com/cosmos/interchain-security/pull/931)

proto/interchain_security/ccv/provider/v1/provider.proto

@@ -105,7 +105,7 @@ message GlobalSlashEntry {
   // This field is used to obtain validator power in HandleThrottleQueues.
   // 
   // This field is not used in the store key, but is persisted in value bytes, see QueueGlobalSlashEntry.
-  ProviderConsAddress provider_val_cons_addr = 4;
+  bytes provider_val_cons_addr = 4;
 }
 
 // Params defines the parameters for CCV Provider module
@@ -166,6 +166,11 @@ message ConsumerRemovalProposals {
   repeated ConsumerRemovalProposal pending = 1;
 }
 
+// AddressList contains a list of consensus addresses
+message AddressList {
+  repeated bytes addresses = 1;
+}
+
 message ChannelToChain {
   string channel_id = 1;
   string chain_id = 2;
@@ -200,29 +205,8 @@ message VscSendTimestamp {
 // Key assignment section
 //
 
-// A validator's assigned consensus address for a consumer chain. 
-// Note this type is for type safety within provider code, consumer code uses normal sdk.ConsAddress, 
-// since there's no notion of provider vs consumer address.
-message ConsumerConsAddress {
-  // Do not generate stringer for this type, we'll use sdk.ConsAddress.String() instead
-  option (gogoproto.goproto_stringer) = false;
-  bytes address = 1;
-}
-
-// A validator's consensus address on the provider chain
-message ProviderConsAddress {
-  // Do not generate stringer for this type, we'll use sdk.ConsAddress.String() instead
-  option (gogoproto.goproto_stringer) = false;
-  bytes address = 1;
-}
-
-// ConsumerAddressList contains a list of consumer consensus addresses
-message ConsumerAddressList {
-  repeated ConsumerConsAddress addresses = 1;
-}
-
 message KeyAssignmentReplacement {
-  ProviderConsAddress provider_addr = 1;
+  bytes provider_addr = 1;
   tendermint.crypto.PublicKey prev_c_key = 2;
   int64 power = 3;
 }
@@ -231,22 +215,22 @@ message KeyAssignmentReplacement {
 // ValidatorConsumerPubKey: (chainID, providerAddr consAddr) -> consumerKey tmprotocrypto.PublicKey
 message ValidatorConsumerPubKey {
   string chain_id = 1;
-  ProviderConsAddress provider_addr = 2;
+  bytes provider_addr = 2;
   tendermint.crypto.PublicKey consumer_key = 3;
 }
 
 // Used to serialize the ValidatorConsumerAddr index from key assignment
 // ValidatorByConsumerAddr: (chainID, consumerAddr consAddr) -> providerAddr consAddr
 message ValidatorByConsumerAddr {
   string chain_id = 1;
-  ConsumerConsAddress consumer_addr = 2;
-  ProviderConsAddress provider_addr = 3;
+  bytes consumer_addr = 2;
+  bytes provider_addr = 3;
 }
 
 // Used to serialize the ConsumerAddrsToPrune index from key assignment
 // ConsumerAddrsToPrune: (chainID, vscID uint64) -> consumerAddrs AddressList
 message ConsumerAddrsToPrune {
   string chain_id = 1;
   uint64 vsc_id = 2;
-  ConsumerAddressList consumer_addrs = 3;
+  AddressList consumer_addrs = 3;
 }

tests/integration/stop_consumer.go

@@ -83,7 +83,7 @@ func (s *CCVTestSuite) TestStopConsumerChain() {
 			func(suite *CCVTestSuite) error {
 				// Queue slash and vsc packet data for consumer 0, these queue entries will be removed
 				firstBundle := s.getFirstBundle()
-				globalEntry := types.NewGlobalSlashEntry(s.providerCtx().BlockTime(), firstBundle.Chain.ChainID, 7, types.ProviderConsAddress{})
+				globalEntry := types.NewGlobalSlashEntry(s.providerCtx().BlockTime(), firstBundle.Chain.ChainID, 7, types.ProviderConsAddress{Address: []byte{}})
 				providerKeeper.QueueGlobalSlashEntry(s.providerCtx(), globalEntry)
 				err := providerKeeper.QueueThrottledSlashPacketData(s.providerCtx(), firstBundle.Chain.ChainID, 1,
 					ccv.SlashPacketData{ValsetUpdateId: 1})
@@ -94,7 +94,7 @@ func (s *CCVTestSuite) TestStopConsumerChain() {
 
 				// Queue slash and vsc packet data for consumer 1, these queue entries will be not be removed
 				secondBundle := s.getBundleByIdx(1)
-				globalEntry = types.NewGlobalSlashEntry(s.providerCtx().BlockTime(), secondBundle.Chain.ChainID, 7, types.ProviderConsAddress{})
+				globalEntry = types.NewGlobalSlashEntry(s.providerCtx().BlockTime(), secondBundle.Chain.ChainID, 7, types.ProviderConsAddress{Address: []byte{}})
 				providerKeeper.QueueGlobalSlashEntry(s.providerCtx(), globalEntry)
 				err = providerKeeper.QueueThrottledSlashPacketData(s.providerCtx(), secondBundle.Chain.ChainID, 1,
 					ccv.SlashPacketData{ValsetUpdateId: 1})

x/ccv/provider/keeper/genesis.go

@@ -73,16 +73,20 @@ func (k Keeper) InitGenesis(ctx sdk.Context, genState *types.GenesisState) {
 
 	// Import key assignment state
 	for _, item := range genState.ValidatorConsumerPubkeys {
-		k.SetValidatorConsumerPubKey(ctx, item.ChainId, *item.ProviderAddr, *item.ConsumerKey)
+		providerAddr := types.NewProviderConsAddress(item.ProviderAddr)
+		k.SetValidatorConsumerPubKey(ctx, item.ChainId, providerAddr, *item.ConsumerKey)
 	}
 
 	for _, item := range genState.ValidatorsByConsumerAddr {
-		k.SetValidatorByConsumerAddr(ctx, item.ChainId, *item.ConsumerAddr, *item.ProviderAddr)
+		consumerAddr := types.NewConsumerConsAddress(item.ConsumerAddr)
+		providerAddr := types.NewProviderConsAddress(item.ProviderAddr)
+		k.SetValidatorByConsumerAddr(ctx, item.ChainId, consumerAddr, providerAddr)
 	}
 
 	for _, item := range genState.ConsumerAddrsToPrune {
 		for _, addr := range item.ConsumerAddrs.Addresses {
-			k.AppendConsumerAddrsToPrune(ctx, item.ChainId, item.VscId, *addr)
+			consumerAddr := types.NewConsumerConsAddress(addr)
+			k.AppendConsumerAddrsToPrune(ctx, item.ChainId, item.VscId, consumerAddr)
 		}
 	}
 

x/ccv/provider/keeper/genesis_test.go

@@ -79,22 +79,22 @@ func TestInitAndExportGenesis(t *testing.T) {
 		[]providertypes.ValidatorConsumerPubKey{
 			{
 				ChainId:      cChainIDs[0],
-				ProviderAddr: &provAddr,
+				ProviderAddr: provAddr.ToSdkConsAddr(),
 				ConsumerKey:  &consumerTmPubKey,
 			},
 		},
 		[]providertypes.ValidatorByConsumerAddr{
 			{
 				ChainId:      cChainIDs[0],
-				ProviderAddr: &provAddr,
-				ConsumerAddr: &consumerConsAddr,
+				ProviderAddr: provAddr.ToSdkConsAddr(),
+				ConsumerAddr: consumerConsAddr.ToSdkConsAddr(),
 			},
 		},
 		[]providertypes.ConsumerAddrsToPrune{
 			{
 				ChainId:       cChainIDs[0],
 				VscId:         vscID,
-				ConsumerAddrs: &providertypes.ConsumerAddressList{Addresses: []*providertypes.ConsumerConsAddress{&consumerConsAddr}},
+				ConsumerAddrs: &providertypes.AddressList{Addresses: [][]byte{consumerConsAddr.ToSdkConsAddr()}},
 			},
 		},
 	)
@@ -155,7 +155,7 @@ func TestInitAndExportGenesis(t *testing.T) {
 
 	addrs := pk.GetConsumerAddrsToPrune(ctx, cChainIDs[0], vscID)
 	// Expect same list as what was provided in provGenesis
-	expectedAddrList := providertypes.ConsumerAddressList{Addresses: []*providertypes.ConsumerConsAddress{&consumerConsAddr}}
+	expectedAddrList := providertypes.AddressList{Addresses: [][]byte{consumerConsAddr.ToSdkConsAddr()}}
 	require.Equal(t, expectedAddrList, addrs)
 
 	// check provider chain's consumer chain states

x/ccv/provider/keeper/hooks.go

@@ -85,7 +85,7 @@ func ValidatorConsensusKeyInUse(k *Keeper, ctx sdk.Context, valAddr sdk.ValAddre
 	inUse := false
 
 	for _, validatorConsumerAddrs := range k.GetAllValidatorsByConsumerAddr(ctx, nil) {
-		if validatorConsumerAddrs.ConsumerAddr.ToSdkConsAddr().Equals(consensusAddr) {
+		if sdk.ConsAddress(validatorConsumerAddrs.ConsumerAddr).Equals(consensusAddr) {
 			inUse = true
 			break
 		}
@@ -103,15 +103,16 @@ 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 validatorConsumerPubKey.ProviderAddr.ToSdkConsAddr().Equals(valConsAddr) {
+		if sdk.ConsAddress(validatorConsumerPubKey.ProviderAddr).Equals(valConsAddr) {
 			consumerAddrTmp, err := ccvtypes.TMCryptoPublicKeyToConsAddr(*validatorConsumerPubKey.ConsumerKey)
 			if err != nil {
 				// An error here would indicate something is very wrong
 				panic("cannot get address of consumer key")
 			}
 			consumerAddr := providertypes.NewConsumerConsAddress(consumerAddrTmp)
 			h.k.DeleteValidatorByConsumerAddr(ctx, validatorConsumerPubKey.ChainId, consumerAddr)
-			h.k.DeleteValidatorConsumerPubKey(ctx, validatorConsumerPubKey.ChainId, *validatorConsumerPubKey.ProviderAddr)
+			providerAddr := providertypes.NewProviderConsAddress(validatorConsumerPubKey.ProviderAddr)
+			h.k.DeleteValidatorConsumerPubKey(ctx, validatorConsumerPubKey.ChainId, providerAddr)
 		}
 	}
 }

x/ccv/provider/keeper/key_assignment.go

@@ -90,7 +90,7 @@ func (k Keeper) GetAllValidatorConsumerPubKeys(ctx sdk.Context, chainID *string)
 
 		validatorConsumerPubKeys = append(validatorConsumerPubKeys, types.ValidatorConsumerPubKey{
 			ChainId:      chainID,
-			ProviderAddr: &providerAddr,
+			ProviderAddr: providerAddr.ToSdkConsAddr(),
 			ConsumerKey:  &consumerKey,
 		})
 	}
@@ -116,12 +116,7 @@ func (k Keeper) GetValidatorByConsumerAddr(
 	if bz == nil {
 		return providerAddr, false
 	}
-	err := providerAddr.Unmarshal(bz)
-	if err != nil {
-		// 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))
-	}
+	providerAddr = types.NewProviderConsAddress(bz)
 	return providerAddr, true
 }
 
@@ -135,11 +130,7 @@ func (k Keeper) SetValidatorByConsumerAddr(
 ) {
 	store := ctx.KVStore(k.storeKey)
 	// Cons address is a type alias for a byte string, no marshaling needed
-	bz, err := providerAddr.Marshal()
-	if err != nil {
-		// An error here would indicate something is very wrong,
-		panic(fmt.Sprintf("failed to marshal provider address: %v", err))
-	}
+	bz := providerAddr.ToSdkConsAddr()
 	store.Set(types.ValidatorsByConsumerAddrKey(chainID, consumerAddr), bz)
 }
 
@@ -173,17 +164,11 @@ func (k Keeper) GetAllValidatorsByConsumerAddr(ctx sdk.Context, chainID *string)
 			panic(fmt.Sprintf("failed to parse chainID and consumer address: %v", err))
 		}
 		consumerAddr := types.NewConsumerConsAddress(consumerAddrTmp)
-		var providerAddr types.ProviderConsAddress
-		err = providerAddr.Unmarshal(iterator.Value())
-		if err != nil {
-			// 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))
-		}
+		providerAddr := types.NewProviderConsAddress(iterator.Value())
 
 		validatorConsumerAddrs = append(validatorConsumerAddrs, types.ValidatorByConsumerAddr{
-			ConsumerAddr: &consumerAddr,
-			ProviderAddr: &providerAddr,
+			ConsumerAddr: consumerAddr.ToSdkConsAddr(),
+			ProviderAddr: providerAddr.ToSdkConsAddr(),
 			ChainId:      chainID,
 		})
 	}
@@ -274,7 +259,7 @@ func (k Keeper) GetAllKeyAssignmentReplacements(ctx sdk.Context, chainID string)
 		}
 
 		replacements = append(replacements, types.KeyAssignmentReplacement{
-			ProviderAddr: &providerAddr,
+			ProviderAddr: providerAddr.ToSdkConsAddr(),
 			PrevCKey:     &pubKeyAndPower.PubKey,
 			Power:        pubKeyAndPower.Power,
 		})
@@ -302,7 +287,7 @@ func (k Keeper) DeleteKeyAssignmentReplacement(ctx sdk.Context, chainID string,
 func (k Keeper) AppendConsumerAddrsToPrune(ctx sdk.Context, chainID string, vscID uint64, consumerAddr types.ConsumerConsAddress) {
 	store := ctx.KVStore(k.storeKey)
 	bz := store.Get(types.ConsumerAddrsToPruneKey(chainID, vscID))
-	var consumerAddrsToPrune types.ConsumerAddressList
+	var consumerAddrsToPrune types.AddressList
 	if bz != nil {
 		err := consumerAddrsToPrune.Unmarshal(bz)
 		if err != nil {
@@ -311,7 +296,7 @@ func (k Keeper) AppendConsumerAddrsToPrune(ctx sdk.Context, chainID string, vscI
 			panic(err)
 		}
 	}
-	consumerAddrsToPrune.Addresses = append(consumerAddrsToPrune.Addresses, &consumerAddr)
+	consumerAddrsToPrune.Addresses = append(consumerAddrsToPrune.Addresses, consumerAddr.ToSdkConsAddr())
 	bz, err := consumerAddrsToPrune.Marshal()
 	if err != nil {
 		// An error here would indicate something is very wrong,
@@ -327,7 +312,7 @@ func (k Keeper) GetConsumerAddrsToPrune(
 	ctx sdk.Context,
 	chainID string,
 	vscID uint64,
-) (consumerAddrsToPrune types.ConsumerAddressList) {
+) (consumerAddrsToPrune types.AddressList) {
 	store := ctx.KVStore(k.storeKey)
 	bz := store.Get(types.ConsumerAddrsToPruneKey(chainID, vscID))
 	if bz == nil {
@@ -359,7 +344,7 @@ func (k Keeper) GetAllConsumerAddrsToPrune(ctx sdk.Context, chainID string) (con
 			// store keys are assumed to be correctly serialized in AppendConsumerAddrsToPrune.
 			panic(err)
 		}
-		var addrs types.ConsumerAddressList
+		var addrs types.AddressList
 		err = addrs.Unmarshal(iterator.Value())
 		if err != nil {
 			// An error here would indicate something is very wrong,
@@ -564,13 +549,14 @@ func (k Keeper) MustApplyKeyAssignmentToValUpdates(
 	// set the old consumer key's power to 0 and the new consumer key's power to the
 	// power in the pending key assignment.
 	for _, replacement := range k.GetAllKeyAssignmentReplacements(ctx, chainID) {
-		k.DeleteKeyAssignmentReplacement(ctx, chainID, *replacement.ProviderAddr)
+		providerAddr := types.NewProviderConsAddress(replacement.ProviderAddr)
+		k.DeleteKeyAssignmentReplacement(ctx, chainID, providerAddr)
 		newUpdates = append(newUpdates, abci.ValidatorUpdate{
 			PubKey: *replacement.PrevCKey,
 			Power:  0,
 		})
 
-		newConsumerKey, found := k.GetValidatorConsumerPubKey(ctx, chainID, *replacement.ProviderAddr)
+		newConsumerKey, found := k.GetValidatorConsumerPubKey(ctx, chainID, providerAddr)
 		if !found {
 			// This should never happen as for every KeyAssignmentReplacement there should
 			// be a ValidatorConsumerPubKey that was stored when AssignConsumerKey() was called.
@@ -605,11 +591,12 @@ func (k Keeper) GetProviderAddrFromConsumerAddr(
 // as they cannot be referenced in slash requests (by a correct consumer)
 func (k Keeper) PruneKeyAssignments(ctx sdk.Context, chainID string, vscID uint64) {
 	consumerAddrs := k.GetConsumerAddrsToPrune(ctx, chainID, vscID)
-	for _, addr := range consumerAddrs.Addresses {
-		k.DeleteValidatorByConsumerAddr(ctx, chainID, *addr)
+	for _, addrBz := range consumerAddrs.Addresses {
+		consumerAddr := types.NewConsumerConsAddress(addrBz)
+		k.DeleteValidatorByConsumerAddr(ctx, chainID, consumerAddr)
 		k.Logger(ctx).Info("consumer address was pruned",
 			"consumer chainID", chainID,
-			"consumer consensus addr", addr.String(),
+			"consumer consensus addr", consumerAddr.String(),
 		)
 	}
 
@@ -620,17 +607,20 @@ func (k Keeper) PruneKeyAssignments(ctx sdk.Context, chainID string, vscID uint6
 func (k Keeper) DeleteKeyAssignments(ctx sdk.Context, chainID string) {
 	// delete ValidatorConsumerPubKey
 	for _, validatorConsumerAddr := range k.GetAllValidatorConsumerPubKeys(ctx, &chainID) {
-		k.DeleteValidatorConsumerPubKey(ctx, chainID, *validatorConsumerAddr.ProviderAddr)
+		providerAddr := types.NewProviderConsAddress(validatorConsumerAddr.ProviderAddr)
+		k.DeleteValidatorConsumerPubKey(ctx, chainID, providerAddr)
 	}
 
 	// delete ValidatorsByConsumerAddr
 	for _, validatorConsumerAddr := range k.GetAllValidatorsByConsumerAddr(ctx, &chainID) {
-		k.DeleteValidatorByConsumerAddr(ctx, chainID, *validatorConsumerAddr.ConsumerAddr)
+		consumerAddr := types.NewConsumerConsAddress(validatorConsumerAddr.ConsumerAddr)
+		k.DeleteValidatorByConsumerAddr(ctx, chainID, consumerAddr)
 	}
 
 	// delete KeyAssignmentReplacements
 	for _, keyAssignmentReplacement := range k.GetAllKeyAssignmentReplacements(ctx, chainID) {
-		k.DeleteKeyAssignmentReplacement(ctx, chainID, *keyAssignmentReplacement.ProviderAddr)
+		providerAddr := types.NewProviderConsAddress(keyAssignmentReplacement.ProviderAddr)
+		k.DeleteKeyAssignmentReplacement(ctx, chainID, providerAddr)
 	}
 
 	// delete ValidatorConsumerPubKey