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.

Sign up for GitHub

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

fix(ProtoRev): Updating Binary Search Range with CL Pools #5930

Merged
merged 18 commits into from
Aug 4, 2023
Merged

fix(ProtoRev): Updating Binary Search Range with CL Pools #5930

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
063602c
init
davidterpay Jul 28, 2023
e2b628e
testing update
davidterpay Jul 31, 2023
c9b13cd
changelog
davidterpay Jul 31, 2023
a706848
comments
davidterpay Jul 31, 2023
efa68d2
nit
davidterpay Jul 31, 2023
d25d53e
refactor
davidterpay Aug 1, 2023
bde5c73
clean up
davidterpay Aug 1, 2023
2f314fa
nit
davidterpay Aug 1, 2023
1ede81d
comment
davidterpay Aug 1, 2023
b0c92f3
cr
davidterpay Aug 1, 2023
1c78485
cl are not symetric
davidterpay Aug 1, 2023
64a6891
smh more symetric changes
davidterpay Aug 1, 2023
b8e3f1f
simpler safe swap
davidterpay Aug 1, 2023
da72dd0
more stuff
davidterpay Aug 1, 2023
7ef7106
more testing
davidterpay Aug 2, 2023
aadbcc7
adding comments
davidterpay Aug 2, 2023
9f1e6e4
updating max range wit pool liq
davidterpay Aug 4, 2023
b890a43
updating readme
davidterpay Aug 4, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* [#5831](https://github.com/osmosis-labs/osmosis/pull/5831) Fix superfluid_delegations query
* [#5835](https://github.com/osmosis-labs/osmosis/pull/5835) Fix println's for "amountZeroInRemainingBigDec before fee" making it into production
* [#5841] (https://github.com/osmosis-labs/osmosis/pull/5841) Fix protorev's out of gas erroring of the user's transcation.
* [#5930] (https://github.com/osmosis-labs/osmosis/pull/5930) Updating Protorev Binary Search Range Logic with CL Pools

### Misc Improvements

Expand Down
8 changes: 7 additions & 1 deletion app/keepers/keepers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,13 @@ func (appKeepers *AppKeepers) InitNormalKeepers(
protorevKeeper := protorevkeeper.NewKeeper(
appCodec, appKeepers.keys[protorevtypes.StoreKey],
appKeepers.GetSubspace(protorevtypes.ModuleName),
appKeepers.AccountKeeper, appKeepers.BankKeeper, appKeepers.GAMMKeeper, appKeepers.EpochsKeeper, appKeepers.PoolManagerKeeper)
appKeepers.AccountKeeper,
appKeepers.BankKeeper,
appKeepers.GAMMKeeper,
appKeepers.EpochsKeeper,
appKeepers.PoolManagerKeeper,
appKeepers.ConcentratedLiquidityKeeper,
)
appKeepers.ProtoRevKeeper = &protorevKeeper

txFeesKeeper := txfeeskeeper.NewKeeper(
Expand Down
6 changes: 3 additions & 3 deletions x/protorev/keeper/epoch_hook_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,8 @@ func (s *KeeperTestSuite) TestUpdateHighestLiquidityPools() {
// There are 2 pools with epochTwo and uosmo as denoms,
// One in the GAMM module and one in the Concentrated Liquidity module.
// pool with ID 48 has a liquidity value of 1,000,000
// pool with ID 49 has a liquidity value of 2,000,000
// pool with ID 49 should be returned as the highest liquidity pool
// pool with ID 50 has a liquidity value of 2,000,000
// pool with ID 50 should be returned as the highest liquidity pool
// We provide epochTwo as the input base denom, to test the method chooses the correct pool
// across the GAMM and Concentrated Liquidity modules
name: "Get highest liquidity pools for one GAMM pool and one Concentrated Liquidity pool",
Expand All @@ -162,7 +162,7 @@ func (s *KeeperTestSuite) TestUpdateHighestLiquidityPools() {
},
expectedBaseDenomPools: map[string]map[string]keeper.LiquidityPoolStruct{
"epochTwo": {
"uosmo": {Liquidity: sdk.Int(sdk.NewUintFromString("999999000000000001000000000000000000")), PoolId: 49},
"uosmo": {Liquidity: sdk.Int(sdk.NewUintFromString("999999000000000001000000000000000000")), PoolId: 50},
},
},
},
Expand Down
6 changes: 3 additions & 3 deletions x/protorev/keeper/hooks_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,14 @@ func (s *KeeperTestSuite) TestSwapping() {
param: param{
expectedTrades: []types.Trade{
{
Pool: 49,
Pool: 50,
TokenIn: "uosmo",
TokenOut: "epochTwo",
},
},
executeSwap: func() {

route := []poolmanagertypes.SwapAmountInRoute{{PoolId: 49, TokenOutDenom: "epochTwo"}}
route := []poolmanagertypes.SwapAmountInRoute{{PoolId: 50, TokenOutDenom: "epochTwo"}}

_, err := s.App.PoolManagerKeeper.RouteExactAmountIn(s.Ctx, s.TestAccs[0], route, sdk.NewCoin("uosmo", sdk.NewInt(10)), sdk.NewInt(1))
s.Require().NoError(err)
Expand Down Expand Up @@ -606,7 +606,7 @@ func (s *KeeperTestSuite) TestStoreJoinExitPoolSwaps() {
{
name: "Non-Gamm Pool, Return Early Do Not Store Any Swaps",
param: param{
poolId: 49,
poolId: 50,
denom: "uosmo",
isJoin: true,
expectedSwap: types.Trade{},
Expand Down
29 changes: 16 additions & 13 deletions x/protorev/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@ type (
storeKey storetypes.StoreKey
paramstore paramtypes.Subspace

accountKeeper types.AccountKeeper
bankKeeper types.BankKeeper
gammKeeper types.GAMMKeeper
epochKeeper types.EpochKeeper
poolmanagerKeeper types.PoolManagerKeeper
accountKeeper types.AccountKeeper
bankKeeper types.BankKeeper
gammKeeper types.GAMMKeeper
epochKeeper types.EpochKeeper
poolmanagerKeeper types.PoolManagerKeeper
concentratedLiquidityKeeper types.ConcentratedLiquidityKeeper
}
)

Expand All @@ -36,21 +37,23 @@ func NewKeeper(
gammKeeper types.GAMMKeeper,
epochKeeper types.EpochKeeper,
poolmanagerKeeper types.PoolManagerKeeper,
concentratedLiquidityKeeper types.ConcentratedLiquidityKeeper,
) Keeper {
// set KeyTable if it has not already been set
if !ps.HasKeyTable() {
ps = ps.WithKeyTable(types.ParamKeyTable())
}

return Keeper{
cdc: cdc,
storeKey: storeKey,
paramstore: ps,
accountKeeper: accountKeeper,
bankKeeper: bankKeeper,
gammKeeper: gammKeeper,
epochKeeper: epochKeeper,
poolmanagerKeeper: poolmanagerKeeper,
cdc: cdc,
storeKey: storeKey,
paramstore: ps,
accountKeeper: accountKeeper,
bankKeeper: bankKeeper,
gammKeeper: gammKeeper,
epochKeeper: epochKeeper,
poolmanagerKeeper: poolmanagerKeeper,
concentratedLiquidityKeeper: concentratedLiquidityKeeper,
}
}

Expand Down
31 changes: 29 additions & 2 deletions x/protorev/keeper/keeper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -888,20 +888,47 @@ func (s *KeeperTestSuite) setUpPools() {
},
scalingFactors: []uint64{1, 1},
},
{ // Pool 49 - Used for CL testing
initialLiquidity: sdk.NewCoins(
sdk.NewCoin("uosmo", sdk.NewInt(10_000_000_000_000)),
sdk.NewCoin("epochTwo", sdk.NewInt(8_000_000_000_000)),
),
poolParams: stableswap.PoolParams{
SwapFee: sdk.NewDecWithPrec(0, 2),
ExitFee: sdk.NewDecWithPrec(0, 2),
},
scalingFactors: []uint64{1, 1},
},
}

for _, pool := range s.stableSwapPools {
s.createStableswapPool(pool.initialLiquidity, pool.poolParams, pool.scalingFactors)
}

// Create a concentrated liquidity pool for epoch_hook testing
// Pool 49
// Pool 50
s.PrepareConcentratedPoolWithCoinsAndFullRangePosition("epochTwo", "uosmo")

// Create a cosmwasm pool for testing
// Pool 50
// Pool 51
s.PrepareCosmWasmPool()

// Create a concentrated liquidity pool for range testing
// Pool 52
// Create the CL pool
clPool := s.PrepareCustomConcentratedPool(s.TestAccs[0], "epochTwo", "uosmo", apptesting.DefaultTickSpacing, sdk.ZeroDec())
fundCoins := sdk.NewCoins(sdk.NewCoin("epochTwo", sdk.NewInt(10_000_000_000_000)), sdk.NewCoin("uosmo", sdk.NewInt(10_000_000_000_000)))
s.FundAcc(s.TestAccs[0], fundCoins)
s.CreateFullRangePosition(clPool, fundCoins)

// Create a concentrated liquidity pool for range testing
// Pool 52
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Pool 52
// Pool 53

// Create the CL pool
clPool = s.PrepareCustomConcentratedPool(s.TestAccs[0], "epochTwo", "uosmo", apptesting.DefaultTickSpacing, sdk.ZeroDec())
fundCoins = sdk.NewCoins(sdk.NewCoin("epochTwo", sdk.NewInt(2_000_000_000)), sdk.NewCoin("uosmo", sdk.NewInt(1_000_000_000)))
s.FundAcc(s.TestAccs[0], fundCoins)
s.CreateFullRangePosition(clPool, fundCoins)

// Set all of the pool info into the stores
err := s.App.ProtoRevKeeper.UpdatePools(s.Ctx)
s.Require().NoError(err)
Expand Down
6 changes: 3 additions & 3 deletions x/protorev/keeper/posthandler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,12 +340,12 @@ func (s *KeeperTestSuite) TestAnteHandle() {
params: param{
trades: []types.Trade{
{
Pool: 51,
Pool: 54,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Synced in person: So after the tick change, this test no longer tests what we want it to since this does not run out of gas. So in the follow up PR that allows maxTicks to be configurable, we'll make the maxTicks really large for this test so that the rebalance logic runs out of gas again

TokenOut: "ibc/0CD3A0285E1341859B5E86B6AB7682F023D03E97607CCC1DC95706411D866DF7",
TokenIn: "uosmo",
},
},
expectedNumOfTrades: sdk.NewInt(5),
expectedNumOfTrades: sdk.NewInt(6),
expectedProfits: []sdk.Coin{
{
Denom: "Atom",
Expand All @@ -357,7 +357,7 @@ func (s *KeeperTestSuite) TestAnteHandle() {
},
{
Denom: types.OsmosisDenomination,
Amount: sdk.NewInt(56_609_900),
Amount: sdk.NewInt(56_653_504),
},
},
expectedPoolPoints: 37,
Expand Down
Loading