Skip to content

Commit

Permalink
[CL][bugfix] Update reward splitting logic to only use bonded balance…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
AlpinYukseloglu authored and pysel committed Jun 6, 2023
1 parent 3da305f commit edfa5b6
Show file tree
Hide file tree
Showing 6 changed files with 154 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
29 changes: 27 additions & 2 deletions x/concentrated-liquidity/incentives.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
57 changes: 54 additions & 3 deletions x/concentrated-liquidity/incentives_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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},
Expand All @@ -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)))},
Expand All @@ -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)))},
Expand All @@ -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)))},
Expand All @@ -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)))},
Expand All @@ -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},
Expand All @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 ---
Expand Down
2 changes: 2 additions & 0 deletions x/concentrated-liquidity/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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.
Expand Down
10 changes: 10 additions & 0 deletions x/gamm/keeper/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
60 changes: 60 additions & 0 deletions x/gamm/keeper/pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}

0 comments on commit edfa5b6

Please sign in to comment.