Add param to allow non lock owner to force unlock

go.mod

@@ -5,7 +5,7 @@ go 1.19
 require (
 	github.com/CosmWasm/wasmd v0.30.0
 	github.com/cosmos/cosmos-proto v1.0.0-alpha8
-	github.com/cosmos/cosmos-sdk v0.46.8
+	github.com/cosmos/cosmos-sdk v0.46.9
 	github.com/cosmos/go-bip39 v1.0.0
 	github.com/cosmos/ibc-go/v4 v4.3.0
 	github.com/gogo/protobuf v1.3.3

proto/osmosis/lockup/params.proto

@@ -8,4 +8,7 @@ option go_package = "github.com/osmosis-labs/osmosis/v14/x/lockup/types";
 message Params {
   repeated string force_unlock_allowed_addresses = 1
       [ (gogoproto.moretags) = "yaml:\"force_unlock_allowed_address\"" ];
+  repeated string non_owner_force_unlock_allowed_addresses = 2
+      [ (gogoproto.moretags) =
+            "yaml:\"non_owner_force_unlock_allowed_addresses\"" ];
 }

x/lockup/keeper/grpc_query_test.go

@@ -521,10 +521,12 @@ func (suite *KeeperTestSuite) TestParams() {
 	res, err := suite.querier.Params(sdk.WrapSDKContext(suite.Ctx), &types.QueryParamsRequest{})
 	suite.Require().NoError(err)
 	suite.Require().Equal([]string(nil), res.Params.ForceUnlockAllowedAddresses)
+	suite.Require().Equal([]string(nil), res.Params.NonOwnerForceUnlockAllowedAddresses)
 
 	// Set new params & query
-	suite.App.LockupKeeper.SetParams(suite.Ctx, types.NewParams([]string{suite.TestAccs[0].String()}))
+	suite.App.LockupKeeper.SetParams(suite.Ctx, types.NewParams([]string{suite.TestAccs[0].String()}, []string{suite.TestAccs[1].String()}))
 	res, err = suite.querier.Params(sdk.WrapSDKContext(suite.Ctx), &types.QueryParamsRequest{})
 	suite.Require().NoError(err)
 	suite.Require().Equal([]string{suite.TestAccs[0].String()}, res.Params.ForceUnlockAllowedAddresses)
+	suite.Require().Equal([]string{suite.TestAccs[1].String()}, res.Params.NonOwnerForceUnlockAllowedAddresses)
 }

x/lockup/keeper/msg_server.go

@@ -187,22 +187,37 @@ func (server msgServer) ForceUnlock(goCtx context.Context, msg *types.MsgForceUn
 		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)
+	params := server.keeper.GetParams(ctx)
+	nonOwnerForceUnlockAllowedAddresses := params.NonOwnerForceUnlockAllowedAddresses
+	nonOwnerForceUnlockAuthorized := false
+
+	for _, addr := range nonOwnerForceUnlockAllowedAddresses {
+		if addr == msg.Owner {
+			nonOwnerForceUnlockAuthorized = true
+		}
+	}
+
+	// if non owner force unlock is not authorized, check if message sender matches lock owner
+	if !nonOwnerForceUnlockAuthorized {
+		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)
+		}
 	}
 
+	// inherit non owner force unlock authorization as starting point for the next check
+	// since the address is allowed for non owner force unlock, it is also allowed for owner force unlock
+	forceUnlockAuthorized := nonOwnerForceUnlockAuthorized
+
 	// check for chain parameter that the address is allowed to force unlock
-	forceUnlockAllowedAddresses := server.keeper.GetParams(ctx).ForceUnlockAllowedAddresses
-	found := false
-	for _, addr := range forceUnlockAllowedAddresses {
+	forceUnlockAllowedAddresseses := params.ForceUnlockAllowedAddresses
+	for _, addr := range forceUnlockAllowedAddresseses {
 		// 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
+			forceUnlockAuthorized = true
 			break
 		}
 	}
-	if !found {
+	if !forceUnlockAuthorized {
 		return &types.MsgForceUnlockResponse{Success: false}, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "Sender (%s) not allowed to force unlock", lock.Owner)
 	}
 

x/lockup/keeper/msg_server_test.go

@@ -332,74 +332,98 @@ func (suite *KeeperTestSuite) TestMsgEditLockup() {
 }
 
 func (suite *KeeperTestSuite) TestMsgForceUnlock() {
-	addr1 := sdk.AccAddress([]byte("addr1---------------"))
-	addr2 := sdk.AccAddress([]byte("addr2---------------"))
+	unlocker := sdk.AccAddress([]byte("addr1---------------"))
+	nonUnlocker := sdk.AccAddress([]byte("addr2---------------"))
 	defaultPoolID, defaultLockID := uint64(1), uint64(1)
 	defaultLockAmount := sdk.NewInt(1000000000)
 
 	tests := []struct {
-		name                      string
-		forceUnlockAllowedAddress types.Params
-		postLockSetup             func()
-		forceUnlockAmount         sdk.Int
-		expectPass                bool
+		name              string
+		lockupParams      types.Params
+		poolOwner         sdk.AccAddress
+		postLockSetup     func()
+		forceUnlockAmount sdk.Int
+		expectPass        bool
 	}{
 		{
 			"happy path",
-			types.Params{ForceUnlockAllowedAddresses: []string{addr1.String()}},
+			types.Params{ForceUnlockAllowedAddresses: []string{unlocker.String()}},
+			unlocker,
 			func() {},
 			defaultLockAmount,
 			true,
 		},
 		{
 			"force unlock superfluid delegated lock",
-			types.Params{ForceUnlockAllowedAddresses: []string{addr1.String()}},
+			types.Params{ForceUnlockAllowedAddresses: []string{unlocker.String()}},
+			unlocker,
 			func() {
-				err := suite.SuperfluidDelegateToDefaultVal(addr1, defaultPoolID, defaultLockID)
+				err := suite.SuperfluidDelegateToDefaultVal(unlocker, defaultPoolID, defaultLockID)
 				suite.Require().NoError(err)
 			},
 			defaultLockAmount,
 			false,
 		},
 		{
 			"superfluid undelegating lock",
-			types.Params{ForceUnlockAllowedAddresses: []string{addr1.String()}},
+			types.Params{ForceUnlockAllowedAddresses: []string{unlocker.String()}},
+			unlocker,
 			func() {
-				err := suite.SuperfluidDelegateToDefaultVal(addr1, defaultPoolID, defaultLockID)
+				err := suite.SuperfluidDelegateToDefaultVal(unlocker, defaultPoolID, defaultLockID)
 				suite.Require().NoError(err)
 
-				err = suite.App.SuperfluidKeeper.SuperfluidUndelegate(suite.Ctx, addr1.String(), defaultLockID)
+				err = suite.App.SuperfluidKeeper.SuperfluidUndelegate(suite.Ctx, unlocker.String(), defaultLockID)
 				suite.Require().NoError(err)
 			},
 			defaultLockAmount,
 			false,
 		},
 		{
 			"partial unlock",
-			types.Params{ForceUnlockAllowedAddresses: []string{addr1.String()}},
+			types.Params{ForceUnlockAllowedAddresses: []string{unlocker.String()}},
+			unlocker,
 			func() {},
 			// try force unlocking half of locked amount
 			defaultLockAmount.Quo(sdk.NewInt(2)),
 			true,
 		},
 		{
 			"force unlock more than what we have locked",
-			types.Params{ForceUnlockAllowedAddresses: []string{addr1.String()}},
+			types.Params{ForceUnlockAllowedAddresses: []string{unlocker.String()}},
+			unlocker,
 			func() {},
 			// try force more than the locked amount
 			defaultLockAmount.Add(sdk.NewInt(1)),
 			false,
 		},
 		{
-			"params with different address",
-			types.Params{ForceUnlockAllowedAddresses: []string{addr2.String()}},
+			"param with multiple addresses ",
+			types.Params{ForceUnlockAllowedAddresses: []string{unlocker.String(), nonUnlocker.String()}},
+			unlocker,
+			func() {},
+			defaultLockAmount,
+			true,
+		},
+		{
+			"force unlock lock that is not owned by the unlocker",
+			types.Params{ForceUnlockAllowedAddresses: []string{unlocker.String()}},
+			nonUnlocker,
 			func() {},
 			defaultLockAmount,
 			false,
 		},
 		{
-			"param with multiple addresses ",
-			types.Params{ForceUnlockAllowedAddresses: []string{addr1.String(), addr2.String()}},
+			"force unlock lock that is not owned by the unlocker but are allowed by the params",
+			types.Params{NonOwnerForceUnlockAllowedAddresses: []string{unlocker.String()}},
+			nonUnlocker,
+			func() {},
+			defaultLockAmount,
+			true,
+		},
+		{
+			"force unlock lock that is owned by the unlocker while msg owner is only allowed in NonOwnerForceUnlockAllowedAddresses",
+			types.Params{NonOwnerForceUnlockAllowedAddresses: []string{unlocker.String()}},
+			unlocker,
 			func() {},
 			defaultLockAmount,
 			true,
@@ -409,7 +433,7 @@ func (suite *KeeperTestSuite) TestMsgForceUnlock() {
 	for _, test := range tests {
 		// set up test
 		suite.SetupTest()
-		suite.App.LockupKeeper.SetParams(suite.Ctx, test.forceUnlockAllowedAddress)
+		suite.App.LockupKeeper.SetParams(suite.Ctx, test.lockupParams)
 
 		// prepare pool for superfluid staking cases
 		poolId := suite.PrepareBalancerPoolWithCoins(sdk.NewCoin("stake", sdk.NewInt(1000000000000)), sdk.NewCoin("foo", sdk.NewInt(5000)))
@@ -420,30 +444,29 @@ func (suite *KeeperTestSuite) TestMsgForceUnlock() {
 
 		poolDenom := gammtypes.GetPoolShareDenom(poolId)
 		coinsToLock := sdk.Coins{sdk.NewCoin(poolDenom, defaultLockAmount)}
-		suite.FundAcc(addr1, coinsToLock)
+		suite.FundAcc(test.poolOwner, coinsToLock)
 
 		unbondingDuration := suite.App.StakingKeeper.GetParams(suite.Ctx).UnbondingTime
-		resp, err := msgServer.LockTokens(c, types.NewMsgLockTokens(addr1, unbondingDuration, coinsToLock))
+		resp, err := msgServer.LockTokens(c, types.NewMsgLockTokens(test.poolOwner, unbondingDuration, coinsToLock))
 		suite.Require().NoError(err)
 
 		// setup env after lock tokens
 		test.postLockSetup()
 
 		// test force unlock
-		_, err = msgServer.ForceUnlock(c, types.NewMsgForceUnlock(addr1, resp.ID, sdk.Coins{sdk.NewCoin(poolDenom, test.forceUnlockAmount)}))
+		_, err = msgServer.ForceUnlock(c, types.NewMsgForceUnlock(unlocker, resp.ID, sdk.Coins{sdk.NewCoin(poolDenom, test.forceUnlockAmount)}))
 		if test.expectPass {
-			suite.Require().NoError(err)
+			suite.Require().NoError(err, "test: %s", test.name)
 
 			// check that we have successfully force unlocked
-			balanceAfterForceUnlock := suite.App.BankKeeper.GetBalance(suite.Ctx, addr1, poolDenom)
+			balanceAfterForceUnlock := suite.App.BankKeeper.GetBalance(suite.Ctx, test.poolOwner, poolDenom)
 			suite.Require().Equal(test.forceUnlockAmount, balanceAfterForceUnlock.Amount)
 		} else {
 			suite.Require().Error(err)
 
 			// check that we have successfully force unlocked
-			balanceAfterForceUnlock := suite.App.BankKeeper.GetBalance(suite.Ctx, addr1, poolDenom)
+			balanceAfterForceUnlock := suite.App.BankKeeper.GetBalance(suite.Ctx, test.poolOwner, poolDenom)
 			suite.Require().NotEqual(test.forceUnlockAmount, balanceAfterForceUnlock.Amount)
-			return
 		}
 	}
 }

x/lockup/types/params.go

@@ -9,7 +9,8 @@ import (
 
 // Parameter store keys.
 var (
-	KeyForceUnlockAllowedAddresses = []byte("ForceUnlockAllowedAddresses")
+	KeyForceUnlockAllowedAddresses         = []byte("ForceUnlockAllowedAddresses")
+	KeyNonOwnerForceUnlockAllowedAddresses = []byte("NonOwnerForceUnlockAllowedAddresses")
 
 	_ paramtypes.ParamSet = &Params{}
 )
@@ -19,16 +20,18 @@ func ParamKeyTable() paramtypes.KeyTable {
 	return paramtypes.NewKeyTable().RegisterParamSet(&Params{})
 }
 
-func NewParams(forceUnlockAllowedAddresses []string) Params {
+func NewParams(forceUnlockAllowedAddresses, nonOwnerForceUnlockAllowedAddresses []string) Params {
 	return Params{
-		ForceUnlockAllowedAddresses: forceUnlockAllowedAddresses,
+		ForceUnlockAllowedAddresses:         forceUnlockAllowedAddresses,
+		NonOwnerForceUnlockAllowedAddresses: nonOwnerForceUnlockAllowedAddresses,
 	}
 }
 
 // DefaultParams returns default lockup module parameters.
 func DefaultParams() Params {
 	return Params{
-		ForceUnlockAllowedAddresses: []string{},
+		ForceUnlockAllowedAddresses:         []string{},
+		NonOwnerForceUnlockAllowedAddresses: []string{},
 	}
 }
 
@@ -37,13 +40,17 @@ func (p Params) Validate() error {
 	if err := validateAddresses(p.ForceUnlockAllowedAddresses); err != nil {
 		return err
 	}
+	if err := validateAddresses(p.NonOwnerForceUnlockAllowedAddresses); err != nil {
+		return err
+	}
 	return nil
 }
 
 // Implements params.ParamSet.
 func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs {
 	return paramtypes.ParamSetPairs{
 		paramtypes.NewParamSetPair(KeyForceUnlockAllowedAddresses, &p.ForceUnlockAllowedAddresses, validateAddresses),
+		paramtypes.NewParamSetPair(KeyNonOwnerForceUnlockAllowedAddresses, &p.NonOwnerForceUnlockAllowedAddresses, validateAddresses),
 	}
 }