Osmosis - Continuously creating position on amm-LP strategy about gateway

[mlguys]

Progress update @nikspz @chasevoorhees

• Can reproduced the issue
• Found new issue: Strategy always create position with wrong price range for pair OSMO-USDC making new lp position out of range

For now, I'm investigate the connector implementation on client side and gateway to see what goes wrong.

[chasevoorhees]

Updated the strategy again - will keep testing stuff on my side.

BTW this bug report may be better suited on hummingbot/hummingbot, considering this is definitely an issue with gateway_osmosis_amm_lp.py

[chasevoorhees]

Error: Bad status on response: 429
    at filterBadStatus (/home/vboxuser/hbot/pecugateway-dev/gateway/node_modules/osmojs/node_modules/@cosmjs/tendermint-rpc/src/rpcclients/http.ts:9:11)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async HttpClient.execute (/home/vboxuser/hbot/pecugateway-dev/gateway/node_modules/osmojs/node_modules/@cosmjs/tendermint-rpc/src/rpcclients/httpclient.ts:44:43)
    at async Tendermint34Client.doCall (/home/vboxuser/hbot/pecugateway-dev/gateway/node_modules/osmojs/node_modules/@cosmjs/tendermint-rpc/src/tendermint34/tendermint34client.ts:341:20)
    at async QueryClient.queryUnverified (/home/vboxuser/hbot/pecugateway-dev/gateway/node_modules/osmojs/node_modules/@cosmjs/stargate/src/queryclient/queryclient.ts:578:22)

I'm seeing a whole bunch of 429s back from the RPC server now.. making it hard to test this currently.

@rapcmia Apologies but I also made a couple small changes to the development branch again:

Osmosis: Return feeTier instead of pool.fee from poolPositions(). Wrapped getCurrentBlock() and return 0 when HTTP429 back from RPC

[nikspz]

Adding bounty for this issue:

Acceptance: status and amm_v3_lp strategy are working on osmosis

Bounty:

Sponsor: Funded by Hummingbot community sponsors
Bounty Amount: 60K HBOT
Developer Payment: 54K HBOT or 272 USDC

[mlguys]

Hey, I will take this bounty 👍

[nikspz]

Assigned to @mlguys, please be informed:

• Since you’re assigned to this bounty exclusively, please keep us informed on your progress over the next few days
• Please review and follow contribution guidelines (https://hummingbot.org/developers/contributions/)
• Please review the checklist after PR submitted (https://hummingbot.org/developers/contributions/#checklist)
• Let us know if you don’t have time, and we will unassigned and keep it open for other devs
• Only one dev can be assigned to one bounty

Thanks for your contribution!

[nikspz]

@mlguys
Currently there's a new bounty process.
Assigned developer should choose bounty amount (from advised) when dev assigned,
please specify with comment below 54K HBOT or 272 USDC

[mlguys]

54k HBOT works for me 👍

[nikspz]

Noted

[mlguys]

@nikspz @chasevoorhees

Pushed fix for the bug:

• When using Osmosis connector on AMM-Lp strategy, it kept on creating position but does not indicate that its available when looking into status command. These positions are created successfully on the exchange

PR: hummingbot/hummingbot#6959

Now focusing on new bug:

• Strategy always create position with wrong price range for pair OSMO-USDC making new lp position out of range

[mlguys]

Found new bug:

• When connector run update_nft to update the lp position status, the return result doesn't contain the required information for the lp position:

[mlguys]

Pushed fix for:

• Strategy always create position with wrong price range for pair OSMO-USDC making new lp position out of range (hummingbot/hummingbot@9f9c3bd)

My next task: discuss with osmosis connector team on how to deal with the new found bug. @chasevoorhees

[chasevoorhees]

I've been instructed to reply here for optics - I've been helping @mlguys on Discord (or lmk if I missed anything)

[mlguys]

Added fix on client and gateway side:

• #318
• hummingbot/hummingbot#6959

What fixed:

• AMM LP strategy should work with Osmosis now, from start to end
• In the event that you manually close lp position on Osmosis, the strategy will detect and remove the closed position automatically

Note to developer:

• There is issue with tick calculation:

  gateway/src/chains/osmosis/osmosis.ts

  Lines 1363 to 1365 in b8ba322

  	// FIXME: ticks are less than 1 exponent, need to check tick calculation
  	const lowerTick = findTickForPrice(req.lowerPrice!, pool.exponentAtPriceOne, pool.tickSpacing, true) + '0' // pool.currentTick,
  	const upperTick = findTickForPrice(req.upperPrice!, pool.exponentAtPriceOne, pool.tickSpacing, false) + '0'

  , the calculated tick is less than 1 exponent (for example, we expected a value of -1251000, but got -125100 instead). I made a quick fix by adding back that missing exponent.

Screen recording:
https://github.com/hummingbot/gateway/assets/9627489/ee821b1c-f569-4948-9ba3-e2e411acf7b2

I guess this solution is what the bounty amount can cover, please review the fix for this bounty
@nkhrs @chasevoorhees @nikspz

[chasevoorhees]

Added fix on client and gateway side:

* [[Fix] Osmosis - Continuously creating position on amm-LP strategy #318](https://github.com/hummingbot/gateway/pull/318)

* [[Fix] Osmosis - Continuously creating position on amm-LP strategy hummingbot#6959](https://github.com/hummingbot/hummingbot/pull/6959)

What fixed:

* AMM LP strategy should work with Osmosis now, from start to end

* In the event that you manually close lp position on Osmosis, the strategy will detect and remove the closed position automatically

Note to developer:

* There is issue with tick calculation: https://github.com/hummingbot/gateway/blob/b8ba322567bb0c01cd38355723b97937eb9b9924/src/chains/osmosis/osmosis.ts#L1363-L1365
  , the calculated tick is less than 1 exponent (for example, we expected a value of `-1251000`, but got `-125100` instead)

Screen recording: https://github.com/hummingbot/gateway/assets/9627489/ee821b1c-f569-4948-9ba3-e2e411acf7b2

I guess this solution is what the bounty amount can cover, please review the fix for this bounty @nkhrs @chasevoorhees @nikspz

I think your tick fixme + '0' isn't going to be constant.
Has this been tested with multiple coin pairs? Specifically ones with different #s of significant figures, eg. Osmo/dai and usdc/eth?

Maybe someone else can comment but I'm pretty sure the tick calc's issue was with the sig fig difference btween the coins.

[mlguys]

Right, the number of missing exponent is not constant, the fix above will work for OSMO/USDC but for other pair like ETH/WBTC, it will not work, since the missing exponent is even bigger. However, the fix is just temporary since fixing it would be out of this bounty scope, the purpose is to let the connector developer know that there is issue with the tick calculation.
@chasevoorhees @nkhrs

[mlguys]

I've updated the FIXME content as follow:

gateway/src/chains/osmosis/osmosis.ts

Lines 1363 to 1365 in 19e25b6

	// FIXME: The calculation of lowerTick and upperTick is not correct for the case where price is less than 1
	const lowerTick = findTickForPrice(req.lowerPrice!, pool.exponentAtPriceOne, pool.tickSpacing, true) // pool.currentTick,
	const upperTick = findTickForPrice(req.upperPrice!, pool.exponentAtPriceOne, pool.tickSpacing, false)

The calculation in findTickForPrice is not correct for price less than 1.

For the scope of the bounty, I think it is getting bigger and bigger, should we re-scope this bounty? The amm v3 lp strategy technically is working now but only for pools with price that is greater than 1.0 @nkhrs

[chasevoorhees]

We need error logs here but I'm pretty sure this:

// FIXME: The calculation of lowerTick and upperTick is not correct for the case where price is less than 1

This should be fixed already (somewhere...) - They took forever so I made my own NPM: https://www.npmjs.com/package/@chasevoorhees/osmonauts-math-decimal (was just an issue with using bignumber.js instead of decimal.js, and bn.js doesn't support fractional exponents).

But I believe osmonauts-math-decimal has been merged with my fix already, so either make sure yours is up to date or try using my package instead.

Little busy rn, but probably more to say on the tick calc.. again, screenshots would be very helpful

[mlguys]

I added new price to tick calculation here: ed45375

Basically it is a rewrite from https://github.com/osmosis-labs/osmosis/blob/0f9eb3c1259078035445b3e3269659469b95fd9f/x/concentrated-liquidity/math/tick.go#L160

I also check the math in https://docs.osmosis.zone/osmosis-core/modules/concentrated-liquidity/#ticks and the calculation I implemented should check out.

For example I tried to add liquidity to INJ/USDC pool using this parameters:

• upper_price: 27.078618
• lower_price: 24.499702

and the calculation gave me:

Inputs: desiredPriceString=24.499702000000000, exponentAtPriceOne=-6, tickSpacing=100, is_lowerBound=true
Initial calculations: desiredPrice=24.499702, exponent=-6, geometricExponentIncrementDistanceInTicks=9000000
Loop (desiredPrice > 1): currentPrice=10, exponentAtCurrentTick=-5, ticksPassed=9000000
Loop (desiredPrice > 1): currentPrice=100, exponentAtCurrentTick=-4, ticksPassed=18000000
Ticks to be fulfilled by current exponent: -7550029.8
Tick index: 10449970.2
Final calculations: tickIndex=10449970.2, returnTick=10449900
Inputs: desiredPriceString=27.078618000000000, exponentAtPriceOne=-6, tickSpacing=100, is_lowerBound=false
Initial calculations: desiredPrice=27.078618, exponent=-6, geometricExponentIncrementDistanceInTicks=9000000
Loop (desiredPrice > 1): currentPrice=10, exponentAtCurrentTick=-5, ticksPassed=9000000
Loop (desiredPrice > 1): currentPrice=100, exponentAtCurrentTick=-4, ticksPassed=18000000
Ticks to be fulfilled by current exponent: -7292138.2
Tick index: 10707861.8
Final calculations: tickIndex=10707861.8, returnTick=10707900
lowerTick 10449900
upperTick 10707900

However, the lp position I got back has wrong price range so I tried to create lp manually using the same price range and here is what I got from the transaction (https://www.mintscan.io/osmosis/tx/876727A12DE8EFDA789B9D2FF35D7ADF5B6C824D22915E55F4E477E921276D0E?height=15254452&sector=message):

Lower_tick:-97,549,900
Upper_tick:-97,292,000

As we can see, the actual upper and lower ticks we got from the osmosis website is completely different from the calculation we got from osmosis official repo, not sure if they updated the tick calculation elsewhere or not 🤷 @chasevoorhees

[mlguys]

Look like they updated the calculation for tick: https://github.com/osmosis-labs/osmosis/blob/main/x/concentrated-liquidity/math/tick.go#L202

Did you add this update yet? I can't find it anywhere in the development branch. @chasevoorhees

[chasevoorhees]

I'm unsure, it was a while ago. I did the nignumber.js/decimals.js fix but from there my changes were all in my own connector code. If you're truly stuck i can spare a 15min call later today if you'd like, Discord me.

…

On Sat, Apr 27, 2024, 12:03 AM Wojak ***@***.***> wrote: Look like they updated the calculation for tick: https://github.com/osmosis-labs/osmosis/blob/main/x/concentrated-liquidity/math/tick.go#L202 Did you add this update yet? @chasevoorhees <https://github.com/chasevoorhees> — Reply to this email directly, view it on GitHub <#294 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGNKHWAK3HWZ4XTEW6RP6RDY7NEVPAVCNFSM6AAAAABEXKCS52VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBQGM4TIMBRGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[chasevoorhees]

Look like they updated the calculation for tick: https://github.com/osmosis-labs/osmosis/blob/main/x/concentrated-liquidity/math/tick.go#L202

Did you add this update yet? I can't find it anywhere in the development branch. @chasevoorhees

I still can't find this in the Osmosis math package. I'm going to make my own NPM (again) unless I can find it in cosmos-sdk

[chasevoorhees]

No chance I'm getting this go project compiled for npm. Can someone from osmomath/etc get the above version re- .goreleased and push the new version to NPM? That'll solve our tick issue.

Then we can probably fix the last active position display bug pretty easily... Idk who to tag for the above though @nkhrs