From 0c8fe2821a991043f50f068f6456b1b8029e2014 Mon Sep 17 00:00:00 2001
From: Adam Tucker <adam@osmosis.team>
Date: Thu, 10 Nov 2022 18:17:33 -0600
Subject: [PATCH] fix force unlock bug and cli add (#3337)

---
 x/lockup/client/cli/tx.go     | 50 +++++++++++++++++++++++++++++++++++
 x/lockup/keeper/msg_server.go | 23 ++++++++++------
 2 files changed, 65 insertions(+), 8 deletions(-)

diff --git a/x/lockup/client/cli/tx.go b/x/lockup/client/cli/tx.go
index 997a28f49b4..8c26213ea89 100644
--- a/x/lockup/client/cli/tx.go
+++ b/x/lockup/client/cli/tx.go
@@ -29,6 +29,7 @@ func GetTxCmd() *cobra.Command {
 		NewLockTokensCmd(),
 		NewBeginUnlockingCmd(),
 		NewBeginUnlockByIDCmd(),
+		NewForceUnlockByIdCmd(),
 	)
 
 	return cmd
@@ -154,3 +155,52 @@ func NewBeginUnlockByIDCmd() *cobra.Command {
 	flags.AddTxFlagsToCmd(cmd)
 	return cmd
 }
+
+// NewForceUnlockByIdCmd force unlocks individual period lock by ID if proper permissions exist.
+func NewForceUnlockByIdCmd() *cobra.Command {
+	cmd := &cobra.Command{
+		Use:   "force-unlock-by-id [id]",
+		Short: "force unlocks individual period lock by ID",
+		Long:  "force unlocks individual period lock by ID. if no amount provided, entire lock is unlocked",
+		Args:  cobra.ExactArgs(1),
+		RunE: func(cmd *cobra.Command, args []string) error {
+			clientCtx, err := client.GetClientTxContext(cmd)
+			if err != nil {
+				return err
+			}
+
+			txf := tx.NewFactoryCLI(clientCtx, cmd.Flags()).WithTxConfig(clientCtx.TxConfig).WithAccountRetriever(clientCtx.AccountRetriever)
+
+			id, err := strconv.Atoi(args[0])
+			if err != nil {
+				return err
+			}
+
+			coins := sdk.Coins(nil)
+			amountStr, err := cmd.Flags().GetString(FlagAmount)
+			if err != nil {
+				return err
+			}
+
+			if amountStr != "" {
+				coins, err = sdk.ParseCoinsNormalized(amountStr)
+				if err != nil {
+					return err
+				}
+			}
+
+			msg := types.NewMsgForceUnlock(
+				clientCtx.GetFromAddress(),
+				uint64(id),
+				coins,
+			)
+
+			return tx.GenerateOrBroadcastTxWithFactory(clientCtx, txf, msg)
+		},
+	}
+
+	cmd.Flags().AddFlagSet(FlagSetUnlockTokens())
+
+	flags.AddTxFlagsToCmd(cmd)
+	return cmd
+}
diff --git a/x/lockup/keeper/msg_server.go b/x/lockup/keeper/msg_server.go
index 4e086caef20..67b251170b1 100644
--- a/x/lockup/keeper/msg_server.go
+++ b/x/lockup/keeper/msg_server.go
@@ -182,21 +182,28 @@ func (server msgServer) ExtendLockup(goCtx context.Context, msg *types.MsgExtend
 func (server msgServer) ForceUnlock(goCtx context.Context, msg *types.MsgForceUnlock) (*types.MsgForceUnlockResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
+	lock, err := server.keeper.GetLockByID(ctx, msg.ID)
+	if err != nil {
+		return &types.MsgForceUnlockResponse{Success: false}, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, err.Error())
+	}
+
+	// check if message sender matches lock owner
+	if lock.Owner != msg.Owner {
+		return &types.MsgForceUnlockResponse{Success: false}, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "Sender (%s) does not match lock owner (%s)", msg.Owner, lock.Owner)
+	}
+
 	// check for chain parameter that the address is allowed to force unlock
 	forceUnlockAllowedAddresses := server.keeper.GetParams(ctx).ForceUnlockAllowedAddresses
 	found := false
-	for _, address := range forceUnlockAllowedAddresses {
-		if address == msg.Owner {
+	for _, addr := range forceUnlockAllowedAddresses {
+		// defense in depth, double checking the message owner and lock owner are both the same and is one of the allowed force unlock addresses
+		if addr == lock.Owner && addr == msg.Owner {
 			found = true
+			break
 		}
 	}
 	if !found {
-		return &types.MsgForceUnlockResponse{Success: false}, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "Sender (%s) not allowed to force unlock", msg.Owner)
-	}
-
-	lock, err := server.keeper.GetLockByID(ctx, msg.ID)
-	if err != nil {
-		return &types.MsgForceUnlockResponse{Success: false}, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, err.Error())
+		return &types.MsgForceUnlockResponse{Success: false}, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "Sender (%s) not allowed to force unlock", lock.Owner)
 	}
 
 	// check that given lock is not superfluid staked