feat/Osmosis Chain/Connector #237
Conversation
Added cosmos definitions to definitions.yml, then copied the definition names into the $ref of the relevant schemas in amm-liquidity-routes-cosmos.yml and amm-routes-cosmos.yml
|
I'll continue to test/update our swap fee estimation logic and look into the EIP-1559-style base fee RPC query posted in Osmosis' Telegram on 11/11 |
… changed reduceLiquidity() to return array of amount/base/symbol instead of static # of coins (for pools with >2 token types). also changed swap fees to use sim() by default.
Sync gateway / staging to main gateway 1.22.0
|
Hi @chasevoorhees good day
Also when setting up the wallet on hummingbot, we are asked to add the pkey for cosmos, the passphrase we normally use does not work. Where can we get the pkey of osmosis wallet? |
|
Hi @rapcmia, I can answer those for you.
We replicated the issue with the testnet.osmosis.zone connection - according to the Osmosis Discord there is an upgrade for the osmosis-testnet-5 pending for Friday Dec. 01, so we are waiting to see if that is the source.
The Keplr wallet does not export the pkey unless you create an account linked with google (dont ask us why) so we use Leap Wallet to get a pkey. It is also possible to export the seed phrase from Keplr and use it to 'restore wallet' on Leap, then extract the pkey from Leap (if you want to use an existing cosmos wallet made with Keplr). edit: Also, the exported pkey from Leap has a '0x' at the beginning which needs to be removed. |
|
Just a heads up: In case anyone is running the tests right now, there seems to have been some breaking change introduced in the tendermint server within the last 1-2 days... I'm suddenly seeing this error a whole bunch:
Presumably related to our version of @cosmjs/tendermint-rpc I'll debug into it a bit to see what's actually happening. Some other threads with similar issues: Opened an issue with @cosmjs as well |
|
Test commit
Setup on client + gateway
|
|
Small update on my issue note from above - definitely a versioning issue with the tendermint client.
TL;DR: testnet doesn't currently work with the Osmosis connector; you'll need to use mainnet until I update it to manually select the new Tendermint37Client |
|
Just pushed an update to fix the new RPC client balances issue. However, I've noticed that the client's 'gateway balance' is only querying for the OSMO balance instead of all tokens returned returned via the getTokens() endpoint - is that expected behavior? Also, we're still seeing an issue with reading back signed transactions on testnet due to the RPC server update mentioned above. I should have that fixed on our side soon, though. FYI to run this there are 2 changes required in the client: 1. osmosis needs to be added to the native_tokens array here:
2. In the hummingbot client, please place the attached file (placeholder strategy) in this location (unzip first):
|
…eparated pool types for CL vs SWAP coming back from RPC calls
|
Thanks for the update @chasevoorhees
Pending
|
placeholder strategy added by request from gateway PR hummingbot#237 hummingbot/gateway#237
| @@ -0,0 +1,85 @@ | |||
| paths: | |||
There was a problem hiding this comment.
please don't override the default docs for these endpoints. currently, the Swagger addition causes duplicate entries in local docs when users go to localhost:8080
if you want to show users how to use cosmos-specific endpoints, I suggest doing so on the doc page for this in the https://github.com/hummingbot/hummingbot-site repo.
There was a problem hiding this comment.
hi @fengtality, we have a PR planned for the /hummingbot-site repo with pages for osmosis dex, osmosis chain, update to cosmos chain, and the change to index, as described in the contribution guidelines - just havent done the PR yet, its coming up.
src/amm/amm.requests.ts
Outdated
| @@ -40,6 +42,12 @@ export interface PoolPriceRequest extends NetworkSelectionRequest { | |||
| interval: number; | |||
| } | |||
|
|
|||
| export interface CosmosPoolPriceRequest extends NetworkSelectionRequest { | |||
There was a problem hiding this comment.
Since Gateway is intended to be chain-agnostic, I don't think there should be Cosmos-specific interfaces for requests and responses for fetching pool prices, etc. Instead, please extend and modify the existing interfaces and adapt your code to them. These interfaces should cover how users interact with various exchange types within the AMM family.
Current:
export interface PoolPriceRequest extends NetworkSelectionRequest {
token0: string;
token1: string;
fee: string;
period: number;
interval: number;
}
export interface CosmosPoolPriceRequest extends NetworkSelectionRequest {
address: string;
token0: string;
token1: string;
}
New:
export interface PoolPriceRequest extends NetworkSelectionRequest {
token0: string;
token1: string;
address?: string
fee?: string;
period?: number;
interval?: number;
}
you can perform downstream validations in the Cosmos code.
src/amm/amm.requests.ts
Outdated
| token1FinalAmount?: string; // Cosmos only | ||
| } | ||
|
|
||
| export interface CosmosAddLiquidityResponse { |
There was a problem hiding this comment.
Similarly please modify the existing addLiquidity response and request interfaces
src/amm/amm.validators.ts
Outdated
| @@ -309,6 +338,66 @@ export const validateRemoveLiquidityRequest: RequestValidator = | |||
| validateMaxPriorityFeePerGas, | |||
| ]); | |||
|
|
|||
| export const validateCosmosPriceRequest: RequestValidator = mkRequestValidator([ | |||
There was a problem hiding this comment.
Removing the cosmos specific controllers will allow you should remove these validators as well. This folder should only contain generic AMM code.
There was a problem hiding this comment.
How would you want to handle the non-required vars then, just validate if not null? Then Uniswap will need to perform not-null checks inside its controllers.
There was a problem hiding this comment.
There shouldn't be much changes needed to the validators. If you make the params optional as in:
export interface PoolPriceRequest extends NetworkSelectionRequest {
token0: string;
token1: string;
fee?: string;
period?: number;
interval?: number;
}
The mkRequestValidator function in the validators in amm.validators.ts will only test the required params:
export const validatePoolPriceRequest: RequestValidator = mkRequestValidator([
validateConnector,
validateChain,
validateNetwork,
validateToken0,
validateToken1,
validateFee,
validateInterval,
validatePeriod,
]);
This may cause validation issues for connectors that require certain arguments in this request, such as fee for Uniswap. If that's needed, the Uniswap connector should validate the presence of that argument in its dedicated controllers in its folder.
| mainnet: | ||
| nodeURL: https://rpc.osmosis.zone/ | ||
| tokenListType: URL | ||
| tokenListSource: https://raw.githubusercontent.com/osmosis-labs/assetlists/main/osmosis-1/osmosis-1.assetlist.json |
There was a problem hiding this comment.
I strongly suggest copying this locally into the lists folder and using the FILE list type - see other chains/DEXs for examples. This prevents a whole class of network-related issues that affect users in production.
|
@chasevoorhees |
|
PR update:
Steps to reproduce:
logs_test-osmolp.log Manually open a liquidity position on osmosis exchange to observe the behavior and run API tests using curl. There where no curl scripts available for osmosis so we override the existing scripts and use it. However when adding liquidity we are getting:
Steps to reproduce:
|
fix/ add XRPL testnet fix to staging
Sync Gateway / Gateway staging -> master for Hummingbot version 1.23.0
combined cosmos-specific requests and responses into the existing requests and responses
|
Any updates here? |
|
@roncan I think there's ongoing dev work here. @chasevoorhees Could you please give a comment here and resolve branch conflicts? |
|
https://github.com/pecuniafinance/gateway/tree/development I went ahead and moved back to our development branch (had to merge everything in again anyways). Are we alright moving the PR to a new one from that branch? Or would you prefer we work within this one? @nkhrs * will be doing the curl tests etc, but we're all good with yarn test again and have the changes you requested @fengtality |
You're free to create new PR or making changes on this PR. Please notify here |
|
Creating a new PR via the development branch |
|
New PR can be found here #277 |
Before submitting this PR, please make sure:
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.