Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add param to allow non lock owner to force unlock #4252

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions proto/osmosis/lockup/params.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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\"" ];
}
4 changes: 3 additions & 1 deletion x/lockup/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
31 changes: 23 additions & 8 deletions x/lockup/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
77 changes: 50 additions & 27 deletions x/lockup/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)))
Expand All @@ -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
}
}
}
15 changes: 11 additions & 4 deletions x/lockup/types/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import (

// Parameter store keys.
var (
KeyForceUnlockAllowedAddresses = []byte("ForceUnlockAllowedAddresses")
KeyForceUnlockAllowedAddresses = []byte("ForceUnlockAllowedAddresses")
KeyNonOwnerForceUnlockAllowedAddresses = []byte("NonOwnerForceUnlockAllowedAddresses")

_ paramtypes.ParamSet = &Params{}
)
Expand All @@ -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{},
}
}

Expand All @@ -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),
}
}

Expand Down
Loading