-
-
Notifications
You must be signed in to change notification settings - Fork 126
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.
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
Osmosis - Continuously creating position on amm-LP strategy #294
Comments
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:
|
Adding bounty for this issue: Acceptance: status and amm_v3_lp strategy are working on osmosis Bounty: Sponsor: Funded by Hummingbot community sponsors |
Hey, I will take this bounty 👍 |
Assigned to @mlguys, please be informed:
Thanks for your contribution! |
@mlguys |
54k HBOT works for me 👍 |
Noted |
Progress update @nikspz @chasevoorhees
For now, I'm investigate the connector implementation on client side and gateway to see what goes wrong. |
Pushed fix for the bug:
PR: hummingbot/hummingbot#6959 Now focusing on new bug:
|
Pushed fix for:
My next task: discuss with osmosis connector team on how to deal with the new found bug. @chasevoorhees |
I've been instructed to reply here for optics - I've been helping @mlguys on Discord (or lmk if I missed anything) |
Added fix on client and gateway side:
What fixed:
Note to developer:
Screen recording: I guess this solution is what the bounty amount can cover, please review the fix for this bounty |
I think your tick fixme + '0' isn't going to be constant. Maybe someone else can comment but I'm pretty sure the tick calc's issue was with the sig fig difference btween the coins. |
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. |
I've updated the FIXME content as follow: gateway/src/chains/osmosis/osmosis.ts Lines 1363 to 1365 in 19e25b6
The calculation in 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 |
We need error logs here but I'm pretty sure this:
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 |
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:
and the calculation gave me:
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§or=message):
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 |
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 |
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:
***@***.***>
|
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 |
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 |
Describe 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 exchangeScreen.Recording.2024-03-15.at.12.12.43.PM.mov
Steps To Reproduce
Release version
dev-1.26.0
Attachments
logs_ammlp-osmo.log
The text was updated successfully, but these errors were encountered: