[CL][bugfix] Update reward splitting logic to only use bonded balance…

…r amounts (#5239)

* implement/test necessary helper and implement/test bugfix for bonded share reward splitting

* changelog for gamm changes

* clean up tests and comments

CHANGELOG.md

@@ -68,6 +68,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   * [#5129](https://github.com/osmosis-labs/osmosis/pull/5129) Relax twap record validation in init genesis to allow one of the spot prices to be non-zero when twap error is observed.
   * [#5165](https://github.com/osmosis-labs/osmosis/pull/5165) Improve error message when fully exiting from a pool.
   * [#5187](https://github.com/osmosis-labs/osmosis/pull/5187) Expand `IncentivizedPools` query to include internally incentivized CL pools.
+  * [#5239](https://github.com/osmosis-labs/osmosis/pull/5239) Implement `GetTotalPoolShares` public keeper function for GAMM.
 
 ### API breaks
 

x/concentrated-liquidity/incentives.go

@@ -12,6 +12,7 @@ import (
 	"github.com/osmosis-labs/osmosis/v15/x/concentrated-liquidity/math"
 	"github.com/osmosis-labs/osmosis/v15/x/concentrated-liquidity/model"
 	"github.com/osmosis-labs/osmosis/v15/x/concentrated-liquidity/types"
+	gammtypes "github.com/osmosis-labs/osmosis/v15/x/gamm/types"
 )
 
 // createUptimeAccumulators creates accumulator objects in store for each supported uptime for the given poolId.
@@ -126,12 +127,36 @@ func (k Keeper) prepareBalancerPoolAsFullRange(ctx sdk.Context, clPoolId uint64)
 		return 0, sdk.ZeroDec(), nil
 	}
 
-	// Get Balancer pool liquidity
-	balancerPoolLiquidity, err := k.gammKeeper.GetTotalPoolLiquidity(ctx, canonicalBalancerPoolId)
+	// Get total balancer pool liquidity (denominated in pool coins)
+	totalBalancerPoolLiquidity, err := k.gammKeeper.GetTotalPoolLiquidity(ctx, canonicalBalancerPoolId)
 	if err != nil {
 		return 0, sdk.ZeroDec(), err
 	}
 
+	// Get total balancer shares for Balancer pool
+	totalBalancerPoolShares, err := k.gammKeeper.GetTotalPoolShares(ctx, canonicalBalancerPoolId)
+	if err != nil {
+		return 0, sdk.ZeroDec(), err
+	}
+
+	// Get total shares bonded on the longest lockup duration for Balancer pool
+	longestDuration, err := k.poolIncentivesKeeper.GetLongestLockableDuration(ctx)
+	if err != nil {
+		return 0, sdk.ZeroDec(), err
+	}
+	bondedShares := k.lockupKeeper.GetLockedDenom(ctx, gammtypes.GetPoolShareDenom(canonicalBalancerPoolId), longestDuration)
+
+	// Calculate portion of Balancer pool shares that are bonded
+	bondedShareRatio := bondedShares.ToDec().Quo(totalBalancerPoolShares.ToDec())
+
+	// Calculate rough number of assets in Balancer pool that are bonded
+	balancerPoolLiquidity := sdk.NewCoins()
+	for _, liquidityToken := range totalBalancerPoolLiquidity {
+		// Rounding behavior is not critical here, but for simplicity we do bankers multiplication then truncate.
+		bondedLiquidityAmount := liquidityToken.Amount.ToDec().Mul(bondedShareRatio).TruncateInt()
+		balancerPoolLiquidity = balancerPoolLiquidity.Add(sdk.NewCoin(liquidityToken.Denom, bondedLiquidityAmount))
+	}
+
 	// Validate Balancer pool liquidity. These properties should already be guaranteed by the caller,
 	// but we check them anyway as an additional guardrail in case migration link validation is ever
 	// relaxed in the future.

x/concentrated-liquidity/incentives_test.go

@@ -909,7 +909,13 @@ func (s *KeeperTestSuite) TestUpdateUptimeAccumulatorsToNow() {
 			// If applicable, create and link a canonical balancer pool
 			balancerPoolId := uint64(0)
 			if tc.canonicalBalancerPoolAssets != nil {
+				// Create balancer pool and bond its shares
 				balancerPoolId = s.PrepareCustomBalancerPool(tc.canonicalBalancerPoolAssets, defaultBalancerPoolParams)
+				longestDuration, err := s.App.PoolIncentivesKeeper.GetLongestLockableDuration(s.Ctx)
+				s.Require().NoError(err)
+				_, err = s.App.LockupKeeper.CreateLock(s.Ctx, s.TestAccs[0], sdk.NewCoins(sdk.NewCoin(gammtypes.GetPoolShareDenom(balancerPoolId), gammtypes.InitPoolSharesSupply)), longestDuration)
+				s.Require().NoError(err)
+
 				s.App.GAMMKeeper.OverwriteMigrationRecords(s.Ctx,
 					gammtypes.MigrationRecords{
 						BalancerToConcentratedPoolLinks: []gammtypes.BalancerToConcentratedPoolLink{
@@ -3793,6 +3799,7 @@ func (s *KeeperTestSuite) TestPrepareBalancerPoolAsFullRange() {
 	tests := map[string]struct {
 		existingConcentratedLiquidity sdk.Coins
 		balancerPoolAssets            []balancer.PoolAsset
+		portionOfSharesBonded         sdk.Dec
 
 		noCanonicalBalancerPool      bool
 		noBalancerPoolWithID         bool
@@ -3806,6 +3813,7 @@ func (s *KeeperTestSuite) TestPrepareBalancerPoolAsFullRange() {
 			// 100 existing shares and 100 shares added from balancer
 			existingConcentratedLiquidity: defaultConcentratedAssets,
 			balancerPoolAssets:            defaultBalancerAssets,
+			portionOfSharesBonded:         sdk.NewDec(1),
 		},
 		"same spot price, different total share amount": {
 			// 100 existing shares and 200 shares added from balancer
@@ -3814,6 +3822,7 @@ func (s *KeeperTestSuite) TestPrepareBalancerPoolAsFullRange() {
 				{Weight: sdk.NewInt(1), Token: sdk.NewCoin("foo", sdk.NewInt(200))},
 				{Weight: sdk.NewInt(1), Token: sdk.NewCoin("bar", sdk.NewInt(200))},
 			},
+			portionOfSharesBonded: sdk.NewDec(1),
 		},
 		"different spot price between balancer and CL pools (excess asset0)": {
 			// 100 existing shares and 100 shares added from balancer. We expect only the even portion of
@@ -3823,6 +3832,7 @@ func (s *KeeperTestSuite) TestPrepareBalancerPoolAsFullRange() {
 				{Weight: sdk.NewInt(1), Token: sdk.NewCoin("foo", sdk.NewInt(150))},
 				{Weight: sdk.NewInt(1), Token: sdk.NewCoin("bar", sdk.NewInt(100))},
 			},
+			portionOfSharesBonded: sdk.NewDec(1),
 		},
 		"different spot price between balancer and CL pools (excess asset1)": {
 			// 100 existing shares and 100 shares added from balancer. We expect only the even portion of
@@ -3832,11 +3842,32 @@ func (s *KeeperTestSuite) TestPrepareBalancerPoolAsFullRange() {
 				{Weight: sdk.NewInt(1), Token: sdk.NewCoin("foo", sdk.NewInt(100))},
 				{Weight: sdk.NewInt(1), Token: sdk.NewCoin("bar", sdk.NewInt(150))},
 			},
+			portionOfSharesBonded: sdk.NewDec(1),
+		},
+		"same spot price, different total share amount, only half bonded": {
+			// 100 existing shares and 200 shares added from balancer
+			existingConcentratedLiquidity: defaultConcentratedAssets,
+			balancerPoolAssets: []balancer.PoolAsset{
+				{Weight: sdk.NewInt(1), Token: sdk.NewCoin("foo", sdk.NewInt(200))},
+				{Weight: sdk.NewInt(1), Token: sdk.NewCoin("bar", sdk.NewInt(200))},
+			},
+			portionOfSharesBonded: sdk.MustNewDecFromStr("0.5"),
+		},
+		"different spot price between balancer and CL pools (excess asset1), only partially bonded": {
+			// 100 existing shares and 100 shares added from balancer. We expect only the even portion of
+			// the Balancer pool to be joined, with the remaining 50bar not qualifying.
+			existingConcentratedLiquidity: defaultConcentratedAssets,
+			balancerPoolAssets: []balancer.PoolAsset{
+				{Weight: sdk.NewInt(1), Token: sdk.NewCoin("foo", sdk.NewInt(100))},
+				{Weight: sdk.NewInt(1), Token: sdk.NewCoin("bar", sdk.NewInt(150))},
+			},
+			portionOfSharesBonded: sdk.MustNewDecFromStr("0.1"),
 		},
 		"no canonical balancer pool": {
 			// 100 existing shares and 100 shares added from balancer
 			existingConcentratedLiquidity: defaultConcentratedAssets,
 			balancerPoolAssets:            defaultBalancerAssets,
+			portionOfSharesBonded:         sdk.NewDec(1),
 
 			// Note that we expect this to fail quietly, as most CL pools will not have linked Balancer pools
 			noCanonicalBalancerPool: true,
@@ -3848,6 +3879,7 @@ func (s *KeeperTestSuite) TestPrepareBalancerPoolAsFullRange() {
 			// 100 existing shares and 100 shares added from balancer
 			existingConcentratedLiquidity: defaultConcentratedAssets,
 			balancerPoolAssets:            defaultBalancerAssets,
+			portionOfSharesBonded:         sdk.NewDec(1),
 			noBalancerPoolWithID:          true,
 
 			expectedError: gammtypes.PoolDoesNotExistError{PoolId: invalidPoolId},
@@ -3859,6 +3891,7 @@ func (s *KeeperTestSuite) TestPrepareBalancerPoolAsFullRange() {
 				{Weight: sdk.NewInt(1), Token: sdk.NewCoin("invalid", sdk.NewInt(100))},
 				{Weight: sdk.NewInt(1), Token: sdk.NewCoin("bar", sdk.NewInt(100))},
 			},
+			portionOfSharesBonded:        sdk.NewDec(1),
 			invalidBalancerPoolLiquidity: true,
 
 			expectedError: types.ErrInvalidBalancerPoolLiquidityError{ClPoolId: 1, BalancerPoolId: 2, BalancerPoolLiquidity: sdk.NewCoins(sdk.NewCoin("invalid", sdk.NewInt(100)), sdk.NewCoin("bar", sdk.NewInt(100)))},
@@ -3870,6 +3903,7 @@ func (s *KeeperTestSuite) TestPrepareBalancerPoolAsFullRange() {
 				{Weight: sdk.NewInt(1), Token: sdk.NewCoin("foo", sdk.NewInt(100))},
 				{Weight: sdk.NewInt(1), Token: sdk.NewCoin("invalid", sdk.NewInt(100))},
 			},
+			portionOfSharesBonded:        sdk.NewDec(1),
 			invalidBalancerPoolLiquidity: true,
 
 			expectedError: types.ErrInvalidBalancerPoolLiquidityError{ClPoolId: 1, BalancerPoolId: 2, BalancerPoolLiquidity: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(100)), sdk.NewCoin("invalid", sdk.NewInt(100)))},
@@ -3881,6 +3915,7 @@ func (s *KeeperTestSuite) TestPrepareBalancerPoolAsFullRange() {
 				{Weight: sdk.NewInt(1), Token: sdk.NewCoin("invalid1", sdk.NewInt(100))},
 				{Weight: sdk.NewInt(1), Token: sdk.NewCoin("invalid2", sdk.NewInt(100))},
 			},
+			portionOfSharesBonded:        sdk.NewDec(1),
 			invalidBalancerPoolLiquidity: true,
 
 			expectedError: types.ErrInvalidBalancerPoolLiquidityError{ClPoolId: 1, BalancerPoolId: 2, BalancerPoolLiquidity: sdk.NewCoins(sdk.NewCoin("invalid1", sdk.NewInt(100)), sdk.NewCoin("invalid2", sdk.NewInt(100)))},
@@ -3893,6 +3928,7 @@ func (s *KeeperTestSuite) TestPrepareBalancerPoolAsFullRange() {
 				{Weight: sdk.NewInt(1), Token: sdk.NewCoin("bar", sdk.NewInt(100))},
 				{Weight: sdk.NewInt(1), Token: sdk.NewCoin("baz", sdk.NewInt(100))},
 			},
+			portionOfSharesBonded:        sdk.NewDec(1),
 			invalidBalancerPoolLiquidity: true,
 
 			expectedError: types.ErrInvalidBalancerPoolLiquidityError{ClPoolId: 1, BalancerPoolId: 2, BalancerPoolLiquidity: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(100)), sdk.NewCoin("bar", sdk.NewInt(100)), sdk.NewCoin("baz", sdk.NewInt(100)))},
@@ -3901,6 +3937,7 @@ func (s *KeeperTestSuite) TestPrepareBalancerPoolAsFullRange() {
 			// 100 existing shares and 100 shares added from balancer
 			existingConcentratedLiquidity: defaultConcentratedAssets,
 			balancerPoolAssets:            defaultBalancerAssets,
+			portionOfSharesBonded:         sdk.NewDec(1),
 			invalidConcentratedPoolID:     true,
 
 			expectedError: types.PoolNotFoundError{PoolId: invalidPoolId + 1},
@@ -3918,6 +3955,14 @@ func (s *KeeperTestSuite) TestPrepareBalancerPoolAsFullRange() {
 
 			// If a canonical balancer pool exists, we create it and link it with the CL pool
 			balancerPoolId := s.PrepareCustomBalancerPool(tc.balancerPoolAssets, defaultBalancerPoolParams)
+
+			// Bond the appropriate portion of total Balancer shares as defined by the current test case
+			longestDuration, err := s.App.PoolIncentivesKeeper.GetLongestLockableDuration(s.Ctx)
+			s.Require().NoError(err)
+			bondedShares := gammtypes.InitPoolSharesSupply.ToDec().Mul(tc.portionOfSharesBonded).TruncateInt()
+			_, err = s.App.LockupKeeper.CreateLock(s.Ctx, s.TestAccs[0], sdk.NewCoins(sdk.NewCoin(gammtypes.GetPoolShareDenom(balancerPoolId), bondedShares)), longestDuration)
+			s.Require().NoError(err)
+
 			if tc.noBalancerPoolWithID {
 				balancerPoolId = invalidPoolId
 			} else if tc.noCanonicalBalancerPool {
@@ -3937,7 +3982,9 @@ func (s *KeeperTestSuite) TestPrepareBalancerPoolAsFullRange() {
 			// Calculate balancer share amount for full range
 			updatedClPool, err := s.App.ConcentratedLiquidityKeeper.GetPoolById(s.Ctx, clPool.GetId())
 			s.Require().NoError(err)
-			qualifyingSharesPreDiscount := math.GetLiquidityFromAmounts(updatedClPool.GetCurrentSqrtPrice(), types.MinSqrtPrice, types.MaxSqrtPrice, tc.balancerPoolAssets[1].Token.Amount, tc.balancerPoolAssets[0].Token.Amount)
+			asset0BalancerAmount := tc.balancerPoolAssets[0].Token.Amount.ToDec().Mul(tc.portionOfSharesBonded).TruncateInt()
+			asset1BalancerAmount := tc.balancerPoolAssets[1].Token.Amount.ToDec().Mul(tc.portionOfSharesBonded).TruncateInt()
+			qualifyingSharesPreDiscount := math.GetLiquidityFromAmounts(updatedClPool.GetCurrentSqrtPrice(), types.MinSqrtPrice, types.MaxSqrtPrice, asset1BalancerAmount, asset0BalancerAmount)
 			qualifyingShares := (sdk.OneDec().Sub(types.DefaultBalancerSharesDiscount)).Mul(qualifyingSharesPreDiscount)
 
 			clearOutQualifyingShares := tc.noBalancerPoolWithID || tc.invalidBalancerPoolLiquidity || tc.invalidConcentratedPoolID || tc.invalidBalancerPoolID || tc.noCanonicalBalancerPool
@@ -4124,8 +4171,12 @@ func (s *KeeperTestSuite) TestClaimAndResetFullRangeBalancerPool() {
 			// Note that the second return value here is the position ID, not an error.
 			initialLiquidity, _ := s.SetupPosition(clPoolId, s.TestAccs[0], tc.existingConcentratedLiquidity, DefaultMinTick, DefaultMaxTick, s.Ctx.BlockTime())
 
-			// Create balancer pool to be linked with CL pool in happy path cases
+			// Create and bond shares for balancer pool to be linked with CL pool in happy path cases
 			balancerPoolId := s.PrepareCustomBalancerPool(tc.balancerPoolAssets, defaultBalancerPoolParams)
+			longestDuration, err := s.App.PoolIncentivesKeeper.GetLongestLockableDuration(s.Ctx)
+			s.Require().NoError(err)
+			_, err = s.App.LockupKeeper.CreateLock(s.Ctx, s.TestAccs[0], sdk.NewCoins(sdk.NewCoin(gammtypes.GetPoolShareDenom(balancerPoolId), gammtypes.InitPoolSharesSupply)), longestDuration)
+			s.Require().NoError(err)
 
 			// Invalidate pool IDs if needed for error cases
 			if tc.balancerPoolDoesNotExist {
@@ -4165,7 +4216,7 @@ func (s *KeeperTestSuite) TestClaimAndResetFullRangeBalancerPool() {
 					s.FundAcc(clPool.GetIncentivesAddress(), normalizedEmissions)
 				}
 			}
-			err := addToUptimeAccums(s.Ctx, clPool.GetId(), s.App.ConcentratedLiquidityKeeper, tc.uptimeGrowth)
+			err = addToUptimeAccums(s.Ctx, clPool.GetId(), s.App.ConcentratedLiquidityKeeper, tc.uptimeGrowth)
 			s.Require().NoError(err)
 
 			// --- System under test ---

x/concentrated-liquidity/types/expected_keepers.go

@@ -38,6 +38,7 @@ type PoolManagerKeeper interface {
 type GAMMKeeper interface {
 	GetTotalPoolLiquidity(ctx sdk.Context, poolId uint64) (sdk.Coins, error)
 	GetLinkedBalancerPoolID(ctx sdk.Context, poolIdEntering uint64) (uint64, error)
+	GetTotalPoolShares(ctx sdk.Context, poolId uint64) (sdk.Int, error)
 }
 
 type PoolIncentivesKeeper interface {
@@ -58,6 +59,7 @@ type LockupKeeper interface {
 	ForceUnlock(ctx sdk.Context, lock lockuptypes.PeriodLock) error
 	CreateLock(ctx sdk.Context, owner sdk.AccAddress, coins sdk.Coins, duration time.Duration) (lockuptypes.PeriodLock, error)
 	SlashTokensFromLockByID(ctx sdk.Context, lockID uint64, coins sdk.Coins) (*lockuptypes.PeriodLock, error)
+	GetLockedDenom(ctx sdk.Context, denom string, duration time.Duration) sdk.Int
 }
 
 // CommunityPoolKeeper defines the contract needed to be fulfilled for distribution keeper.

x/gamm/keeper/pool.go

@@ -298,6 +298,16 @@ func (k Keeper) GetTotalPoolLiquidity(ctx sdk.Context, poolId uint64) (sdk.Coins
 	return pool.GetTotalPoolLiquidity(ctx), nil
 }
 
+// GetTotalPoolShares returns the total number of pool shares for the given pool.
+func (k Keeper) GetTotalPoolShares(ctx sdk.Context, poolId uint64) (sdk.Int, error) {
+	pool, err := k.GetCFMMPool(ctx, poolId)
+	if err != nil {
+		return sdk.Int{}, err
+	}
+
+	return pool.GetTotalShares(), nil
+}
+
 // setStableSwapScalingFactors sets the stable swap scaling factors.
 // errors if the pool does not exist, the sender is not the scaling factor controller, or due to other
 // internal errors.

x/gamm/keeper/pool_test.go

@@ -517,3 +517,63 @@ func (s *KeeperTestSuite) TestSetStableSwapScalingFactors() {
 		})
 	}
 }
+
+func (suite *KeeperTestSuite) TestGetTotalPoolShares() {
+	tests := map[string]struct {
+		sharesJoined   sdk.Int
+		poolNotCreated bool
+
+		expectedError error
+	}{
+		"happy path: default balancer pool": {
+			sharesJoined: sdk.ZeroInt(),
+		},
+		"Multiple LPs with shares exist": {
+			sharesJoined: types.OneShare,
+		},
+		"error: pool does not exist": {
+			sharesJoined:   sdk.ZeroInt(),
+			poolNotCreated: true,
+			expectedError:  types.PoolDoesNotExistError{PoolId: uint64(0)},
+		},
+	}
+
+	for name, tc := range tests {
+		suite.Run(name, func() {
+			suite.SetupTest()
+			gammKeeper := suite.App.GAMMKeeper
+			testAccount := suite.TestAccs[0]
+
+			// --- Setup ---
+
+			// Mint some assets to the accounts.
+			balancerPoolId := uint64(0)
+			if !tc.poolNotCreated {
+				balancerPoolId = suite.PrepareBalancerPool()
+			}
+
+			sharesJoined := sdk.ZeroInt()
+			if !tc.sharesJoined.Equal(sdk.ZeroInt()) {
+				suite.FundAcc(testAccount, defaultAcctFunds)
+				_, sharesActualJoined, err := gammKeeper.JoinPoolNoSwap(suite.Ctx, testAccount, balancerPoolId, tc.sharesJoined, sdk.Coins{})
+				suite.Require().NoError(err)
+				sharesJoined = sharesActualJoined
+			}
+
+			// --- System under test ---
+
+			totalShares, err := gammKeeper.GetTotalPoolShares(suite.Ctx, balancerPoolId)
+
+			// --- Assertions ---
+
+			if tc.expectedError != nil {
+				suite.Require().Error(err)
+				suite.Require().ErrorContains(err, tc.expectedError.Error())
+				return
+			}
+
+			suite.Require().NoError(err)
+			suite.Require().Equal(types.InitPoolSharesSupply.Add(sharesJoined), totalShares)
+		})
+	}
+}