Make GetSuperfluidLockByLockId more cleary indicate if its found or n…

…ot (#5833) (#5838)

(cherry picked from commit b7fe65f)

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>

x/lockup/keeper/grpc_query.go

@@ -197,11 +197,15 @@ func (q Querier) SyntheticLockupsByLockupID(goCtx context.Context, req *types.Sy
 	}
 
 	ctx := sdk.UnwrapSDKContext(goCtx)
-	synthLock, err := q.Keeper.GetSyntheticLockupByUnderlyingLockId(ctx, req.LockId)
+	synthLock, found, err := q.Keeper.GetSyntheticLockupByUnderlyingLockId(ctx, req.LockId)
 	if err != nil {
 		return nil, err
 	}
-	return &types.SyntheticLockupsByLockupIDResponse{SyntheticLocks: []types.SyntheticLock{synthLock}}, nil
+	synthlocks := []types.SyntheticLock{}
+	if found {
+		synthlocks = append(synthlocks, synthLock)
+	}
+	return &types.SyntheticLockupsByLockupIDResponse{SyntheticLocks: synthlocks}, nil
 }
 
 // SyntheticLockupByLockupID returns synthetic lockup by native lockup id.
@@ -211,9 +215,9 @@ func (q Querier) SyntheticLockupByLockupID(goCtx context.Context, req *types.Syn
 	}
 
 	ctx := sdk.UnwrapSDKContext(goCtx)
-	synthLock, err := q.Keeper.GetSyntheticLockupByUnderlyingLockId(ctx, req.LockId)
-	if err != nil {
-		return nil, err
+	synthLock, found, err := q.Keeper.GetSyntheticLockupByUnderlyingLockId(ctx, req.LockId)
+	if err != nil || !found {
+		return &types.SyntheticLockupByLockupIDResponse{}, err
 	}
 	return &types.SyntheticLockupByLockupIDResponse{SyntheticLock: synthLock}, nil
 }

x/lockup/keeper/lock.go

@@ -105,7 +105,8 @@ func (k Keeper) AddTokensToLockByID(ctx sdk.Context, lockID uint64, owner sdk.Ac
 		return nil, err
 	}
 
-	synthlock, err := k.GetSyntheticLockupByUnderlyingLockId(ctx, lock.ID)
+	// TODO: Handle found case in a better way, with state breaking update
+	synthlock, _, err := k.GetSyntheticLockupByUnderlyingLockId(ctx, lock.ID)
 	if err != nil {
 		return nil, err
 	}
@@ -382,7 +383,8 @@ func (k Keeper) ForceUnlock(ctx sdk.Context, lock types.PeriodLock) error {
 	// 2) If lock is bonded, move it to unlocking
 	// 3) Run logic to delete unlocking metadata, and send tokens to owner.
 
-	synthLock, err := k.GetSyntheticLockupByUnderlyingLockId(ctx, lock.ID)
+	// TODO: Use found instead of !synthLock.IsNil() later on.
+	synthLock, _, err := k.GetSyntheticLockupByUnderlyingLockId(ctx, lock.ID)
 	if err != nil {
 		return err
 	}
@@ -766,7 +768,8 @@ func (k Keeper) removeTokensFromLock(ctx sdk.Context, lock *types.PeriodLock, co
 	}
 
 	// increase synthetic lockup's accumulation store
-	synthLock, err := k.GetSyntheticLockupByUnderlyingLockId(ctx, lock.ID)
+	// TODO: In next state break, do err != nil || found == false
+	synthLock, _, err := k.GetSyntheticLockupByUnderlyingLockId(ctx, lock.ID)
 	if err != nil {
 		return err
 	}

x/lockup/keeper/lock_test.go

@@ -1042,8 +1042,9 @@ func (s *KeeperTestSuite) AddTokensToLockForSynth() {
 		}
 		// by GetSyntheticLockupByUnderlyingLockId
 		for i := uint64(1); i <= 3; i++ {
-			synthlockByLockup, err := s.App.LockupKeeper.GetSyntheticLockupByUnderlyingLockId(s.Ctx, i)
+			synthlockByLockup, found, err := s.App.LockupKeeper.GetSyntheticLockupByUnderlyingLockId(s.Ctx, i)
 			s.Require().NoError(err)
+			s.Require().True(found)
 			s.Require().Equal(synthlockByLockup, synthlocks[(int(i)-1)*3+int(i)])
 
 		}
@@ -1503,8 +1504,9 @@ func (s *KeeperTestSuite) TestForceUnlock() {
 
 		// if it was superfluid delegated lock,
 		// confirm that we don't have associated synth lock
-		synthLock, err := s.App.LockupKeeper.GetSyntheticLockupByUnderlyingLockId(s.Ctx, lock.ID)
+		synthLock, found, err := s.App.LockupKeeper.GetSyntheticLockupByUnderlyingLockId(s.Ctx, lock.ID)
 		s.Require().NoError(err)
+		s.Require().False(found)
 		s.Require().Equal((lockuptypes.SyntheticLock{}), synthLock)
 
 		// check if lock is deleted by checking trying to get lock ID

x/lockup/keeper/msg_server.go

@@ -208,7 +208,8 @@ func (server msgServer) ForceUnlock(goCtx context.Context, msg *types.MsgForceUn
 	}
 
 	// check that given lock is not superfluid staked
-	synthLock, err := server.keeper.GetSyntheticLockupByUnderlyingLockId(ctx, lock.ID)
+	// TODO: Next state break do found, instead !synthlock.IsNil()
+	synthLock, _, err := server.keeper.GetSyntheticLockupByUnderlyingLockId(ctx, lock.ID)
 	if err != nil {
 		return &types.MsgForceUnlockResponse{Success: false}, errorsmod.Wrap(sdkerrors.ErrInvalidRequest, err.Error())
 	}

x/lockup/keeper/synthetic_lock.go

@@ -39,7 +39,10 @@ func (k Keeper) GetSyntheticLockup(ctx sdk.Context, lockID uint64, synthdenom st
 // Error is returned if:
 // - there are more than one synthetic lockup objects with the same underlying lock ID.
 // - there is no synthetic lockup object with the given underlying lock ID.
-func (k Keeper) GetSyntheticLockupByUnderlyingLockId(ctx sdk.Context, lockID uint64) (types.SyntheticLock, error) {
+// Returns (syntheticLockup, found, error)
+// intended behavior for most callers is to check:
+// if !found || err != nil { handle_it }
+func (k Keeper) GetSyntheticLockupByUnderlyingLockId(ctx sdk.Context, lockID uint64) (types.SyntheticLock, bool, error) {
 	store := ctx.KVStore(k.storeKey)
 	iterator := sdk.KVStorePrefixIterator(store, combineKeys(types.KeyPrefixSyntheticLockup, sdk.Uint64ToBigEndian(lockID)))
 	defer iterator.Close()
@@ -49,29 +52,29 @@ func (k Keeper) GetSyntheticLockupByUnderlyingLockId(ctx sdk.Context, lockID uin
 		synthLock := types.SyntheticLock{}
 		err := proto.Unmarshal(iterator.Value(), &synthLock)
 		if err != nil {
-			return types.SyntheticLock{}, err
+			return types.SyntheticLock{}, true, err
 		}
 		synthLocks = append(synthLocks, synthLock)
 	}
 	if len(synthLocks) > 1 {
-		return types.SyntheticLock{}, fmt.Errorf("synthetic lockup with same lock id should not exist")
+		return types.SyntheticLock{}, true, fmt.Errorf("synthetic lockup with same lock id should not exist")
 	}
 	if len(synthLocks) == 0 {
-		return types.SyntheticLock{}, nil
+		return types.SyntheticLock{}, false, nil
 	}
-	return synthLocks[0], nil
+	return synthLocks[0], true, nil
 }
 
 // GetAllSyntheticLockupsByAddr gets all the synthetic lockups from all the locks owned by the given address.
 func (k Keeper) GetAllSyntheticLockupsByAddr(ctx sdk.Context, owner sdk.AccAddress) []types.SyntheticLock {
 	synthLocks := []types.SyntheticLock{}
 	locks := k.GetAccountPeriodLocks(ctx, owner)
 	for _, lock := range locks {
-		synthLock, err := k.GetSyntheticLockupByUnderlyingLockId(ctx, lock.ID)
+		synthLock, found, err := k.GetSyntheticLockupByUnderlyingLockId(ctx, lock.ID)
 		if err != nil {
 			panic(err)
 		}
-		if synthLock.UnderlyingLockId != 0 {
+		if found {
 			synthLocks = append(synthLocks, synthLock)
 		}
 	}
@@ -110,7 +113,8 @@ func (k Keeper) CreateSyntheticLockup(ctx sdk.Context, lockID uint64, synthDenom
 	// There is no relationship between unbonding and bonding synthetic lockup, it's managed separately
 	// A separate accumulation store is incremented with the synth denom.
 
-	synthLock, err := k.GetSyntheticLockupByUnderlyingLockId(ctx, lockID)
+	// TODO: Next state break change !synthlock.IsNil -> found
+	synthLock, _, err := k.GetSyntheticLockupByUnderlyingLockId(ctx, lockID)
 	if err != nil {
 		return err
 	}

x/lockup/keeper/synthetic_lock_test.go

@@ -139,8 +139,9 @@ func (s *KeeperTestSuite) TestSyntheticLockupCreateGetDeleteAccumulation() {
 	})
 
 	expectedSynthLock := *synthLock
-	actualSynthLock, err := s.App.LockupKeeper.GetSyntheticLockupByUnderlyingLockId(s.Ctx, 1)
+	actualSynthLock, found, err := s.App.LockupKeeper.GetSyntheticLockupByUnderlyingLockId(s.Ctx, 1)
 	s.Require().NoError(err)
+	s.Require().True(found)
 	s.Require().Equal(expectedSynthLock, actualSynthLock)
 
 	allSynthLocks := s.App.LockupKeeper.GetAllSyntheticLockups(s.Ctx)

x/superfluid/keeper/grpc_query.go

@@ -656,7 +656,7 @@ func (q Querier) filterConcentratedPositionLocks(ctx sdk.Context, positions []mo
 			return nil, err
 		}
 
-		syntheticLock, err := q.Keeper.lk.GetSyntheticLockupByUnderlyingLockId(ctx, lockId)
+		syntheticLock, _, err := q.Keeper.lk.GetSyntheticLockupByUnderlyingLockId(ctx, lockId)
 		if err != nil {
 			return nil, err
 		}

x/superfluid/keeper/migrate.go

@@ -237,11 +237,12 @@ func (k Keeper) routeMigration(ctx sdk.Context, sender sdk.AccAddress, providedL
 
 	lockId := uint64(providedLockId)
 
-	synthLockBeforeMigration, err = k.lk.GetSyntheticLockupByUnderlyingLockId(ctx, lockId)
+	synthLockBeforeMigration, _, err = k.lk.GetSyntheticLockupByUnderlyingLockId(ctx, lockId)
 	if err != nil {
 		return lockuptypes.SyntheticLock{}, Unsupported, err
 	}
 
+	// TODO: Change to if !found
 	if synthLockBeforeMigration == (lockuptypes.SyntheticLock{}) {
 		migrationType = NonSuperfluid
 	} else if strings.Contains(synthLockBeforeMigration.SynthDenom, "superbonding") {

x/superfluid/keeper/stake.go

@@ -469,10 +469,11 @@ func (k Keeper) unbondLock(ctx sdk.Context, underlyingLockId uint64, sender stri
 	if err != nil {
 		return 0, err
 	}
-	synthLock, err := k.lk.GetSyntheticLockupByUnderlyingLockId(ctx, underlyingLockId)
+	synthLock, _, err := k.lk.GetSyntheticLockupByUnderlyingLockId(ctx, underlyingLockId)
 	if err != nil {
 		return 0, err
 	}
+	// TODO: Use !found
 	if synthLock == (lockuptypes.SyntheticLock{}) {
 		return 0, types.ErrNotSuperfluidUsedLockup
 	}
@@ -496,10 +497,11 @@ func (k Keeper) alreadySuperfluidStaking(ctx sdk.Context, lockID uint64) bool {
 		return true
 	}
 
-	synthLock, err := k.lk.GetSyntheticLockupByUnderlyingLockId(ctx, lockID)
+	synthLock, _, err := k.lk.GetSyntheticLockupByUnderlyingLockId(ctx, lockID)
 	if err != nil {
 		return false
 	}
+	// TODO: return found
 	return synthLock != (lockuptypes.SyntheticLock{})
 }
 

x/superfluid/types/expected_keepers.go

@@ -42,7 +42,7 @@ type LockupKeeper interface {
 	GetAllSyntheticLockups(ctx sdk.Context) []lockuptypes.SyntheticLock
 	CreateSyntheticLockup(ctx sdk.Context, lockID uint64, suffix string, unlockDuration time.Duration, isUnlocking bool) error
 	DeleteSyntheticLockup(ctx sdk.Context, lockID uint64, suffix string) error
-	GetSyntheticLockupByUnderlyingLockId(ctx sdk.Context, lockID uint64) (lockuptypes.SyntheticLock, error)
+	GetSyntheticLockupByUnderlyingLockId(ctx sdk.Context, lockID uint64) (lockuptypes.SyntheticLock, bool, error)
 }
 
 type LockupMsgServer interface {

x/valset-pref/types/expected_interfaces.go

@@ -33,7 +33,7 @@ type DistributionKeeper interface {
 }
 type LockupKeeper interface {
 	GetLockByID(ctx sdk.Context, lockID uint64) (*lockuptypes.PeriodLock, error)
-	GetSyntheticLockupByUnderlyingLockId(ctx sdk.Context, lockID uint64) (lockuptypes.SyntheticLock, error)
+	GetSyntheticLockupByUnderlyingLockId(ctx sdk.Context, lockID uint64) (lockuptypes.SyntheticLock, bool, error)
 	ForceUnlock(ctx sdk.Context, lock lockuptypes.PeriodLock) error
 	BeginUnlock(ctx sdk.Context, lockID uint64, coins sdk.Coins) (uint64, error)
 	GetPeriodLocks(ctx sdk.Context) ([]lockuptypes.PeriodLock, error)

x/valset-pref/validator_set.go

@@ -353,10 +353,11 @@ func (k Keeper) ForceUnlockBondedOsmo(ctx sdk.Context, lockID uint64, delegatorA
 	}
 
 	// Ensured the lock has no superfluid relation by checking that there are no synthetic locks
-	synthLocks, err := k.lockupKeeper.GetSyntheticLockupByUnderlyingLockId(ctx, lockID)
+	synthLocks, _, err := k.lockupKeeper.GetSyntheticLockupByUnderlyingLockId(ctx, lockID)
 	if err != nil {
 		return sdk.Coin{}, err
 	}
+	// TODO: use found
 	if synthLocks != (lockuptypes.SyntheticLock{}) {
 		return sdk.Coin{}, fmt.Errorf("cannot use DelegateBondedTokens being used for superfluid.")
 	}