fix: allow the owner of a chain to remove the chain (#2178)

fixed access in RemoveConsumer
cosmos · Aug 27, 2024 · 30c3017 · 30c3017
1 parent 771be37
commit 30c3017
Show file tree

Hide file tree

Showing 4 changed files with 154 additions and 151 deletions.
diff --git a/proto/interchain_security/ccv/provider/v1/tx.proto b/proto/interchain_security/ccv/provider/v1/tx.proto
@@ -223,7 +223,7 @@ message MsgConsumerRemovalResponse {}
 // MsgRemoveConsumer defines the message used to remove (and stop) a consumer chain.
 // If it passes, all the consumer chain's state is eventually removed from the provider chain.
 message MsgRemoveConsumer {
-  option (cosmos.msg.v1.signer) = "authority";
+  option (cosmos.msg.v1.signer) = "signer";
 
   // the consumer id of the consumer chain to be stopped
   string consumer_id = 1;
@@ -232,7 +232,7 @@ message MsgRemoveConsumer {
   google.protobuf.Timestamp stop_time = 2
       [ (gogoproto.stdtime) = true, (gogoproto.nullable) = false ];
   // signer address
-  string authority = 3 [(cosmos_proto.scalar) = "cosmos.AddressString"];
+  string signer = 3 [(cosmos_proto.scalar) = "cosmos.AddressString"];
 }
 
 // MsgRemoveConsumerResponse defines response type for MsgRemoveConsumer messages

diff --git a/x/ccv/provider/keeper/msg_server.go b/x/ccv/provider/keeper/msg_server.go
@@ -87,16 +87,18 @@ func (k msgServer) AssignConsumerKey(goCtx context.Context, msg *types.MsgAssign
 }
 
 // RemoveConsumer defines an RPC handler method for MsgRemoveConsumer
-func (k msgServer) RemoveConsumer(
-	goCtx context.Context,
-	msg *types.MsgRemoveConsumer) (*types.MsgRemoveConsumerResponse, error) {
-	if k.GetAuthority() != msg.Authority {
-		return nil, errorsmod.Wrapf(types.ErrUnauthorized, "expected %s, got %s", k.GetAuthority(), msg.Authority)
-	}
-
+func (k msgServer) RemoveConsumer(goCtx context.Context, msg *types.MsgRemoveConsumer) (*types.MsgRemoveConsumerResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
 	consumerId := msg.ConsumerId
+	ownerAddress, err := k.Keeper.GetConsumerOwnerAddress(ctx, consumerId)
+	if err != nil {
+		return &types.MsgRemoveConsumerResponse{}, errorsmod.Wrapf(types.ErrNoOwnerAddress, "cannot retrieve owner address %s", ownerAddress)
+	}
+
+	if msg.Signer != ownerAddress {
+		return &types.MsgRemoveConsumerResponse{}, errorsmod.Wrapf(types.ErrUnauthorized, "expected owner address %s, got %s", ownerAddress, msg.Signer)
+	}
 
 	phase, found := k.Keeper.GetConsumerPhase(ctx, consumerId)
 	if !found || phase != Launched {

diff --git a/x/ccv/provider/types/msg.go b/x/ccv/provider/types/msg.go
@@ -368,7 +368,7 @@ func (msg MsgUpdateConsumer) GetSigners() []sdk.AccAddress {
 // NewMsgRemoveConsumer creates a new MsgRemoveConsumer instance
 func NewMsgRemoveConsumer(signer string, consumerId string, stopTime time.Time) (*MsgRemoveConsumer, error) {
 	return &MsgRemoveConsumer{
-		Authority:  signer,
+		Signer:     signer,
 		ConsumerId: consumerId,
 		StopTime:   stopTime,
 	}, nil
@@ -399,7 +399,7 @@ func (msg MsgRemoveConsumer) GetSignBytes() []byte {
 // GetSigners implements the sdk.Msg interface. It returns the address(es) that
 // must sign over msg.GetSignBytes().
 func (msg MsgRemoveConsumer) GetSigners() []sdk.AccAddress {
-	valAddr, err := sdk.ValAddressFromBech32(msg.Authority)
+	valAddr, err := sdk.ValAddressFromBech32(msg.Signer)
 	if err != nil {
 		// same behavior as in cosmos-sdk
 		panic(err)