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.

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

feat/ Osmosis Chain/Connector #277

Merged
merged 32 commits into from Mar 15, 2024

Conversation

nkhrs
Copy link
Contributor

@nkhrs nkhrs commented Jan 30, 2024

Before submitting this PR, please make sure:

[x ] Your code builds clean without any errors or warnings
[x ] You are using approved title ("feat/", "fix/", "docs/", "refactor/")

A description of the changes proposed in the pull request:
Adds Osmosis (with LP) support as a chain, plus some updates to Cosmos chain.

Cosmos amm.routes.ts models don't match up well with the rest due to inherent differences in their data. This has been handled by allowing certain endpoints to receive Cosmos-specific models switched by chain, eg. AddLiquidityRequest | CosmosAddLiquidityRequest.

Docs for above endpoints are in Swagger now.

Tests performed by the developer:
Jest tests added for all Osmosis functions. Very little patch use, instead a testnet wallet is instantiated and trades/liquidity actions are performed with it.
yarn test:cov is ~80% for Osmosis, improved for Cosmos.

Tips for QA testing:
THIS LINE MUST to be added to HBOT client native_tokens[]
"osmosis": "OSMO",
in hummingbot/core/utils/gateway_config_utils.py

PR has been submitted to HBOT main for that file as well.

@fengtality
Copy link
Sponsor Contributor

Please resolve conflicts as we just merged in another DEX connector

Copy link
Sponsor Contributor

@fengtality fengtality left a comment

Choose a reason for hiding this comment

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

Please ma

src/amm/amm.requests.ts Show resolved Hide resolved
src/amm/amm.requests.ts Show resolved Hide resolved
src/amm/amm.requests.ts Show resolved Hide resolved
src/amm/amm.requests.ts Show resolved Hide resolved
src/amm/amm.requests.ts Show resolved Hide resolved
src/amm/amm.requests.ts Show resolved Hide resolved
@fengtality
Copy link
Sponsor Contributor

@nkhrs As mentioned on Discord, the additions to the LP interfaces are fine.

To wrap up this PR: please move the Osmosis chain tests to the /test-bronze folder, and address any unit test failures due to interface changes. After that, LGTM!

@chasevoorhees
Copy link
Contributor

@nkhrs As mentioned on Discord, the additions to the LP interfaces are fine.

To wrap up this PR: please move the Osmosis chain tests to the /test-bronze folder, and address any unit test failures due to interface changes. After that, LGTM!

All done

@chasevoorhees
Copy link
Contributor

I was wrong about the invalid coins error - turns out it was angry about alphabetization, not ratio. Anyway, that's fixed.
I also fixed another RPC error due to the number of significant figures that DAI has.

That should be all the issues you mentioned, but I did see that .find/undefined error once on my side as well:

And it's definitely an issue with the API:
https://api-osmosis.imperator.co/fees/v1/pools
image

It came back up pretty quickly, at least:
image

When testing this, are you able to connect to a VPN in the EU? That might help, but I've also made it just ignore fees when we can't access that endpoint (fees will all appear as 0).

@rapcmia
Copy link
Contributor

rapcmia commented Feb 28, 2024

Thanks for the update 🚀

  • Reported issue on invalid coins has been fixed
  • Ran tests on other endpoints;
    • When insufficient funds, returns insufficient amount of token {0 or 1} created
    • Position retrieve pool data successfully
    • Remove position ok
  • When running amm-lp strategy for osmosis, the Pool Price is 0
    image

@chasevoorhees
Copy link
Contributor

chasevoorhees commented Feb 28, 2024

@rapcmia
What does that pool price refer to - is it an active pool position you have?

Also - can you tunnel to Germany?

@chasevoorhees
Copy link
Contributor

I think it's fixed now - is it correct to assume that that's just the market price of the 2 coins?

Otherwise there's no way to get a price for a specific pool in Osmosis; as far as I can tell, the concept doesn't exist.

@rapcmia
Copy link
Contributor

rapcmia commented Feb 29, 2024

Hi @chasevoorhees thanks for the update,
The amm-lp strategy is now trying to create position for the connector however we are getting error when adding liquidity:

2024-02-29 11:27:23,144 - 7772 - hummingbot.client.hummingbot_application - INFO - start command initiated.
2024-02-29 11:27:23,356 - 7772 - GatewayOsmosisAMMLP - INFO - Network status has changed to NetworkStatus.CONNECTED. Starting networking...
2024-02-29 11:27:24,000 - 7772 - hummingbot.strategy.amm_v3_lp.amm_v3_lp - WARNING - osmosis_osmosis_mainnet connector is not ready. Please wait...
2024-02-29 11:27:25,001 - 7772 - hummingbot.strategy.amm_v3_lp.amm_v3_lp - INFO - osmosis_osmosis_mainnet connector is ready. Trading started.
2024-02-29 11:27:27,535 - 7772 - hummingbot.strategy.amm_v3_lp.amm_v3_lp - INFO - Creating new position over 1.6063094809485383414370 to 1.6224532948274181237630 price range. [clock=2024-02-29 03:27:27+00:00]
2024-02-29 11:27:27,536 - 7772 - GatewayOsmosisAMMLP - ERROR - Error submitting ADD liquidity order to osmosis on mainnet for OSMO-USDC 
Traceback (most recent call last):
  File "/Users/rapcomia/github/hummingbot/6685/hummingbot/connector/gateway/amm_lp/gateway_osmosis_amm_lp.py", line 440, in _add_liquidity
    self.address0,
AttributeError: 'GatewayOsmosisAMMLP' object has no attribute 'address0'

@chasevoorhees
Copy link
Contributor

@rapcmia Yikes - that one's just a typo. Sorry.

https://github.com/pecuniafinance/hummingbot/blob/development/hummingbot/connector/gateway/amm_lp/gateway_osmosis_amm_lp.py

Here's the updated file (also pushed to the repo above)

@chasevoorhees
Copy link
Contributor

chasevoorhees commented Feb 29, 2024

@rapcmia
Nik sent me some screenshots of the latest error
image_2024-02-29_14-49-09

This is definitely slippage related - and actually the default allowedSlippage in osmosis.yml is pretty low:

allowedSlippage: 2%

You may need to set this around 40% for that transaction to go through, however what this is actually used for is calculating the minimum amount of token you're willing to put into the pool... so it's not really slippage as used elsewhere/for trades, per se:

{
  typeUrl: "/osmosis.concentratedliquidity.v1beta1.MsgCreatePosition",
  value: {
    poolId: "1066",
    sender: "osmo1mvsg3en5ulpnpd3dset2m86zjpnzp4v4epmjh7",
    lowerTick: "400000",
    upperTick: "800000",
    tokensProvided: [
      {
        denom: "ibc/0CD3A0285E1341859B5E86B6AB7682F023D03E97607CCC1DC95706411D866DF7",
        amount: "293400000000000000",
      },
      {
        denom: "uosmo",
        amount: "180000",
      },
    ],
    tokenMinAmount0: "0",
    tokenMinAmount1: "0",
  },
}

This is my join pool message with slippage=100% (hence tokenMinAmount0/1 == 0)

Here's what I ended up putting into my pool:

token0_finalamount = "0"
token1_finalamount = "293400000000000000"

I believe this implies a value imbalance in the pool (in my case, there was already too much of token0 so it won't accept any more - previously I was getting this: "slippage bound: insufficient amount of token 0 created. Actual: (0). Minimum estimated: (234720000000000000)")

Would you suggest that I set the minAmount to 0 for all cases and disregard slippage as used elsewhere?
As I said, it's just the minimum amount you're willing to join the pool with.. And it's rather problematic if it's sometimes going to be 0 for some tokens.

Anyway, if you set your allowedSlippage='100%' then you should be able to get past this.

I asked the Osmosis team about this one as well to confirm.

@chasevoorhees
Copy link
Contributor

I also read that you're having issues setting values through the client:
gateway config osmosis.feeTier
I'll test this on my side as well.. not sure where this value is stored, currently.

@chasevoorhees
Copy link
Contributor

image
I'm able to update that config var without issue.

@rapcmia
Copy link
Contributor

rapcmia commented Mar 1, 2024

This is definitely slippage related - and actually the default allowedSlippage in osmosis.yml is pretty low:
allowedSlippage: 2%

Hi @chasevoorhees thanks for checking, I change the allowSlippage from 2% to 50 and 100. Both ok on amm-lp strategy and postman, we are able to add liquidity however its returning an error:
image
image


2024-03-01 14:13:33,182 - 55688 - hummingbot.core.gateway.gateway_http_client - WARNING - Call to https://localhost:15888/amm/liquidity/add failed. See logs for more details.
2024-03-01 14:13:33,183 - 55688 - GatewayOsmosisAMMLP - ERROR - Error submitting ADD liquidity order to osmosis on mainnet for OSMO-USDC 
Traceback (most recent call last):
  File "/Users/rapcomia/github/hummingbot/6685/hummingbot/connector/gateway/amm_lp/gateway_osmosis_amm_lp.py", line 436, in _add_liquidity
    order_result: Dict[str, Any] = await self._get_gateway_instance().amm_lp_add(
  File "/Users/rapcomia/github/hummingbot/6685/hummingbot/core/gateway/gateway_http_client.py", line 729, in amm_lp_add
    return await self.api_request("post", "amm/liquidity/add", request_payload)
  File "/Users/rapcomia/github/hummingbot/6685/hummingbot/core/gateway/gateway_http_client.py", line 222, in api_request
    raise e
  File "/Users/rapcomia/github/hummingbot/6685/hummingbot/core/gateway/gateway_http_client.py", line 210, in api_request
    raise ValueError(f"Error on {method.upper()} {url} Error: {parsed_response}")
ValueError: Error on POST https://localhost:15888/amm/liquidity/add Error: {'message': 'Unknown error.', 'httpErrorCode': 503, 'errorCode': 1099, 'stack': 'TypeError: Do not know how to serialize a BigInt\n    at JSON.stringify (<anonymous>)\n    at stringify (/Users/rapcomia/github/gateway/277/node_modules/express/lib/response.js:1150:12)\n    at ServerResponse.json (/Users/rapcomia/github/gateway/277/node_modules/express/lib/response.js:271:14)\n    at /Users/rapcomia/github/gateway/277/dist/src/amm/amm.routes.js:42:25\n    at Generator.next (<anonymous>)\n    at fulfilled (/Users/rapcomia/github/gateway/277/dist/src/amm/amm.routes.js:5:58)\n    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)'}
2024-03-01 14:13:33,186 - 55688 - hummingbot.core.event.event_reporter - EVENT_LOG - {"timestamp": 1709273613.0, "order_id": "add-OSMO-USDC-1709273594617378", "order_action": "LPType.ADD", "event_name": "RangePositionUpdateFailureEvent", "event_source": "osmosis_osmosis_mainnet"}

logs_ammlp-osmo.log

@chasevoorhees
Copy link
Contributor

@rapcmia

Good(ish) news, Osmo guys backed me up on the slippage issue:
image

Re: The BigInt error.. I have no idea how this one suddenly appeared again but it should be fixed now.
Sorry about this... we've basically offloaded testing to you at this point. @nkhrs owes you a beer.

@rapcmia
Copy link
Contributor

rapcmia commented Mar 4, 2024

  • TypeError on BigInt has been fixed ✅
  • Endpoints adding and removing works ok
  • Ran connector on amm-lp strategy, created position successfully however this time we are getting KeyError: 'txStatus'
    2024-03-04 11:14:22,775 - 9162 - hummingbot.client.hummingbot_application - INFO - start command initiated.
    2024-03-04 11:14:22,807 - 9162 - GatewayOsmosisAMMLP - INFO - Network status has changed to NetworkStatus.CONNECTED. Starting networking...
    2024-03-04 11:14:23,001 - 9162 - hummingbot.strategy.amm_v3_lp.amm_v3_lp - WARNING - osmosis_osmosis_mainnet connector is not ready. Please wait...
    2024-03-04 11:14:24,001 - 9162 - hummingbot.strategy.amm_v3_lp.amm_v3_lp - WARNING - osmosis_osmosis_mainnet connector is not ready. Please wait...
    2024-03-04 11:14:25,001 - 9162 - hummingbot.strategy.amm_v3_lp.amm_v3_lp - INFO - osmosis_osmosis_mainnet connector is ready. Trading started.
    2024-03-04 11:14:26,976 - 9162 - hummingbot.strategy.amm_v3_lp.amm_v3_lp - INFO - Creating new position over 1.59867495286045253462125 to 1.61474203781382391687875 price range. [clock=2024-03-04 03:14:26+00:00]
    2024-03-04 11:14:28,315 - 9162 - hummingbot.core.rate_oracle.rate_oracle - INFO - Network status has changed to NetworkStatus.CONNECTED. Starting networking...
    2024-03-04 11:14:45,434 - 9162 - GatewayOsmosisAMMLP - INFO - Created ADD liquidity order add-OSMO-USDC-1709522066976880 txHash: C22897750551744A141409CC4AE0B9A8AB9638FBB5622FC22CA1B4018FB9BAD3 on mainnet. Estimated Gas Cost: 372220  (gas limit: 3000000, gas price: 0.025000000000000001387778780781445675529539585113525390625)
    2024-03-04 11:14:45,435 - 9162 - hummingbot.core.event.event_reporter - EVENT_LOG - {"timestamp": 1709522085.0, "order_id": "add-OSMO-USDC-1709522066976880", "exchange_order_id": "C22897750551744A141409CC4AE0B9A8AB9638FBB5622FC22CA1B4018FB9BAD3", "order_action": "LPType.ADD", "trading_pair": "OSMO-USDC", "fee_tier": "MEDIUM", "lower_price": "1.598674952860452", "upper_price": "1.614742037813823", "amount": "0.500000", "creation_timestamp": 1709522066.0, "token_id": 0, "event_name": "RangePositionUpdateEvent", "event_source": "osmosis_osmosis_mainnet"}
    2024-03-04 11:14:46,417 - 9162 - GatewayOsmosisAMMLP - ERROR - 'txStatus'
    Traceback (most recent call last):
      File "/Users/rapcomia/github/hummingbot/6685/hummingbot/connector/gateway/amm_lp/gateway_osmosis_amm_lp.py", line 1119, in _status_polling_loop
        await safe_gather(
      File "/Users/rapcomia/github/hummingbot/6685/hummingbot/core/utils/async_utils.py", line 22, in safe_gather
        return await asyncio.gather(*args, **kwargs)
      File "/Users/rapcomia/github/hummingbot/6685/hummingbot/connector/gateway/amm_lp/gateway_osmosis_amm_lp.py", line 880, in update_order_status
        if update_result["txStatus"] == 1:
    KeyError: 'txStatus'
    2024-03-04 11:14:48,041 - 9162 - GatewayOsmosisAMMLP - ERROR - 'txStatus'
    Traceback (most recent call last):
      File "/Users/rapcomia/github/hummingbot/6685/hummingbot/connector/gateway/amm_lp/gateway_osmosis_amm_lp.py", line 1119, in _status_polling_loop
        await safe_gather(
      File "/Users/rapcomia/github/hummingbot/6685/hummingbot/core/utils/async_utils.py", line 22, in safe_gather
        return await asyncio.gather(*args, **kwargs)
      File "/Users/rapcomia/github/hummingbot/6685/hummingbot/connector/gateway/amm_lp/gateway_osmosis_amm_lp.py", line 880, in update_order_status
        if update_result["txStatus"] == 1:
    KeyError: 'txStatus'
    

logs_ammlp-osmo.log

@chasevoorhees
Copy link
Contributor

Sorry this one is taking bit longer than expected; the good news is that I finally reconciled the tokenId == poolId || positionId issue.

Basically, tokenId can be either poolId || positionId and I just figure out which on the backend once reduceLiquidity() is called.

This means we'll support multiple CL positions for the same pool (GAMM positions are all strictly 1 per pool though).

I'll update later tonight and will hopefully have finished re-testing everything by then.

@rapcmia
Copy link
Contributor

rapcmia commented Mar 7, 2024

Thanks for the update @chasevoorhees,

  • Issue reported for KeyError: 'txStatus' has been fixed 🚀
  • Ran another test on endpoints, all ok
  • Ran another amm-lp strategy:
    • Successfully created liqudity but getting KeyError: 'lowerPrice'
      image
      2024-03-07 19:28:05,359 - 30085 - hummingbot.client.hummingbot_application - INFO - start command initiated.
      2024-03-07 19:28:05,505 - 30085 - GatewayOsmosisAMMLP - INFO - Network status has changed to NetworkStatus.CONNECTED. Starting networking...
      2024-03-07 19:28:06,002 - 30085 - hummingbot.strategy.amm_v3_lp.amm_v3_lp - WARNING - osmosis_osmosis_mainnet connector is not ready. Please wait...
      2024-03-07 19:28:07,001 - 30085 - hummingbot.strategy.amm_v3_lp.amm_v3_lp - WARNING - osmosis_osmosis_mainnet connector is not ready. Please wait...
      2024-03-07 19:28:08,001 - 30085 - hummingbot.strategy.amm_v3_lp.amm_v3_lp - INFO - osmosis_osmosis_mainnet connector is ready. Trading started.
      2024-03-07 19:28:11,236 - 30085 - hummingbot.core.rate_oracle.rate_oracle - INFO - Network status has changed to NetworkStatus.CONNECTED. Starting networking...
      2024-03-07 19:28:12,880 - 30085 - hummingbot.strategy.amm_v3_lp.amm_v3_lp - INFO - Creating new position over 1.74170867230967679248090 to 1.75921328208163334315910 price range. [clock=2024-03-07 11:28:12+00:00]
      2024-03-07 19:28:33,453 - 30085 - GatewayOsmosisAMMLP - INFO - Created ADD liquidity order add-OSMO-USDC-1709810892880608 txHash: 144F05F523831F9B9ED30336A03EEDF13CA4A0B4353F067DD73D84ABAB2CA67A on mainnet. Estimated Gas Cost: 372031  (gas limit: 2000000, gas price: 0.025000000000000001387778780781445675529539585113525390625)
      2024-03-07 19:28:33,454 - 30085 - hummingbot.core.event.event_reporter - EVENT_LOG - {"timestamp": 1709810913.0, "order_id": "add-OSMO-USDC-1709810892880608", "exchange_order_id": "144F05F523831F9B9ED30336A03EEDF13CA4A0B4353F067DD73D84ABAB2CA67A", "order_action": "LPType.ADD", "trading_pair": "OSMO-USDC", "fee_tier": "MEDIUM", "lower_price": "1.741708672309676", "upper_price": "1.759213282081633", "amount": "0.500000", "creation_timestamp": 1709810892.0, "token_id": 0, "event_name": "RangePositionUpdateEvent", "event_source": "osmosis_osmosis_mainnet"}
      2024-03-07 19:28:34,829 - 30085 - GatewayOsmosisAMMLP - INFO - Liquidity added for position with ID 1.
      2024-03-07 19:28:34,835 - 30085 - hummingbot.core.event.event_reporter - EVENT_LOG - {"timestamp": 1709810914.0, "order_id": "add-OSMO-USDC-1709810892880608", "exchange_order_id": "144F05F523831F9B9ED30336A03EEDF13CA4A0B4353F067DD73D84ABAB2CA67A", "trading_pair": "OSMO-USDC", "lower_price": "1.741708672309676", "upper_price": "1.759213282081633", "amount": "0", "fee_tier": "MEDIUM", "creation_timestamp": 1709810892.0, "trade_fee": {"percent": "0", "percent_token": null, "flat_fees": [{"token": "OSMO", "amount": "0.000009300775000000000516296727593"}]}, "token_id": 1, "event_name": "RangePositionLiquidityAddedEvent", "event_source": "osmosis_osmosis_mainnet"}
      2024-03-07 19:28:39,962 - 30085 - GatewayOsmosisAMMLP - ERROR - 'lowerPrice'
      Traceback (most recent call last):
        File "/Users/rapcomia/github/hummingbot/6685/hummingbot/connector/gateway/amm_lp/gateway_osmosis_amm_lp.py", line 1079, in _status_polling_loop
          await safe_gather(
        File "/Users/rapcomia/github/hummingbot/6685/hummingbot/core/utils/async_utils.py", line 22, in safe_gather
          return await asyncio.gather(*args, **kwargs)
        File "/Users/rapcomia/github/hummingbot/6685/hummingbot/connector/gateway/amm_lp/gateway_osmosis_amm_lp.py", line 955, in update_nft
          lower_price = Decimal(nft_update_result["lowerPrice"])
      KeyError: 'lowerPrice'
      

logs_ammlp-osmo.log

@chasevoorhees
Copy link
Contributor

@rapcmia

The above has been fixed. I fear there still may be testing needed on the strat (collectFees and removeLiquidity specifically) but I'll have @nkhrs help me test those tonight.

@rapcmia
Copy link
Contributor

rapcmia commented Mar 12, 2024

Thanks for the update @chasevoorhees

  • Issue for KeyError: 'lowerPrice' has been fixed ✅
  • Observed that after created/added liquidity
    • There are no active positions on status
    • It kept on creating/addng positions on pool to Osmosis exchange
      image
      image

@chasevoorhees
Copy link
Contributor

@rapcmia

Been running it through debug and I'm somewhat confused on how the order tracking with the client works

Here's what gateway_evm_amm_lp is doing:

...
            if update_result["txStatus"] == 1:
...
                self.stop_tracking_order(tracked_order.client_order_id)

I'm assuming that txStatus==1 means the transaction was confirmed. I'm copying this behavior, and thus we're ending our order tracking right after Osmosis accepts the position.

So - when EVM positions run, every time we hit this:
self.logger().info(f"Liquidity added for position with ID...
they call stop_tracking_order()

Consequently, the position is never picked up by status since it's removed right after it's confirmed...

Anyway - still looking at this. Will try to get some more advice.

Copy link
Contributor

@rapcmia rapcmia left a comment

Choose a reason for hiding this comment

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

@rapcmia rapcmia merged commit 9d36722 into hummingbot:development Mar 15, 2024
3 checks passed
@david-hummingbot
Copy link
Contributor

Hi @nkhrs please add the docs for the Osmosis chain/connector in our docs site - https://github.com/hummingbot/hummingbot-site
Thanks!

@nkhrs
Copy link
Contributor Author

nkhrs commented Mar 15, 2024

Hi @david-hummingbot sure thing. Quick question, our current fork and branch (https://github.com/pecuniafinance/hummingbot-site/tree/doc/osmosis) contains a Connector Guide for Osmosis as well as the osmosis chain page, osmosis connector page, and updated cosmos chain page. Should I keep the guide in there or remove it and do a separate PR at a later date?

@david-hummingbot
Copy link
Contributor

If you already have a copy in your fork you can just submit a PR upstream to update our docs site ideally before the release this month as people will be looking to the docs for information on the connector, how to connect etc.

@chasevoorhees
Copy link
Contributor

Thanks for all the help @rapcmia

Let me know if there are any issues with the backend/chain/connector (well, stuff I wrote) going forward.

@rapcmia rapcmia mentioned this pull request Mar 18, 2024
2 tasks
@nkhrs
Copy link
Contributor Author

nkhrs commented Mar 20, 2024

Hi @david-hummingbot PR is opened for the osmosis docs hummingbot/hummingbot-site#355

Cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants