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/pancake swap v3 #247

Merged
merged 12 commits into from Dec 13, 2023
Merged

Conversation

vic-en
Copy link
Collaborator

@vic-en vic-en commented Dec 3, 2023

Before submitting this PR, please make sure:

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

A description of the changes proposed in the pull request:

  • Add LP to Pancakeswap

Tests performed by the developer:

  • Curl test
  • Unit test
  • End-to-end testing with uniswapLp strategy on the client

Tips for QA testing:

  • Test as you would test uniswapLP, but replacing chain with binance-smart-chain and network with mainnet.

@vic-en vic-en mentioned this pull request Dec 3, 2023
2 tasks
@fengtality
Copy link
Sponsor Contributor

@vic-en this PR introduces bad changes to yarn.lock like this:

"@ethersproject-xdc/address" "file:../../.cache/yarn/v6/npm-@ethersproject-xdc-abi-5.7.0-c99fb10e-6e34-4123-a945-38973cd5bd96-1701573291698/node_modules/@ethersproject-xdc/address"
    "@ethersproject-xdc/bignumber" "file:../../.cache/yarn/v6/npm-@ethersproject-xdc-abi-5.7.0-c99fb10e-6e34-4123-a945-38973cd5bd96-1701573291698/node_modules/@ethersproject-xdc/bignumber"
    "@ethersproject-xdc/bytes" "file:../../.cache/yarn/v6/npm-@ethersproject-xdc-abi-5.7.0-c99fb10e-6e34-4123-a945-38973cd5bd96-1701573291698/node_modules/@ethersproject-xdc/bytes"
    "@ethersproject-xdc/constants" "file:../../.cache/yarn/v6/npm-@ethersproject-xdc-abi-5.7.0-c99fb10e-6e34-4123-a945-38973cd5bd96-1701573291698/node_modules/@ethersproject-xdc/constants"
    "@ethersproject-xdc/hash" "file:../../.cache/yarn/v6/npm-@ethersproject-xdc-abi-5.7.0-c99fb10e-6e34-4123-a945-38973cd5bd96-1701573291698/node_modules/@ethersproject-xdc/hash"
    "@ethersproject-xdc/keccak256" "file:../../.cache/yarn/v6/npm-@ethersproject-xdc-abi-5.7.0-c99fb10e-6e34-4123-a945-38973cd5bd96-1701573291698/node_modules/@ethersproject-xdc/keccak256"
    "@ethersproject-xdc/logger" "file:../../.cache/yarn/v6/npm-@ethersproject-xdc-abi-5.7.0-c99fb10e-6e34-4123-a945-38973cd5bd96-1701573291698/node_modules/@ethersproject-xdc/logger"
    "@ethersproject-xdc/properties" "file:../../.cache/yarn/v6/npm-@ethersproject-xdc-abi-5.7.0-c99fb10e-6e34-4123-a945-38973cd5bd96-1701573291698/node_modules/@ethersproject-xdc/properties"
    "@ethersproject-xdc/strings" "file:../../.cache/yarn/v6/npm-@ethersproject-xdc-abi-5.7.0-c99fb10e-6e34-4123-a945-38973cd5bd96-1701573291698/node_modules/@ethersproject-xdc/strings"

I spent some time fixing development a few days ago for this issue, so please remove these changes

@vic-en
Copy link
Collaborator Author

vic-en commented Dec 4, 2023

@fengtality I'll do that soon.
Curious to know what issue it caused.
I noticed that change when I updated the typescript version inthe package.json file which is required by the pancake dependency.

@vic-en
Copy link
Collaborator Author

vic-en commented Dec 5, 2023

@fengtality @nikspz @rapcmia
I updated the yarn lock. Review can continue.

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.

  • Setup this PR with latest development client
  • Pancakeswap LP must be tested on BSC mainnet
  • API test using Thunderclient
    • GET/port ok
    • GET/Wallet ok
    • POST/add ok
    • GET/connectors ok
  • API test using Curl
    • curl -s -X GET -k --key $GATEWAY_KEY --cert $GATEWAY_CERT https://localhost:15888/ | jq ok
    • curl -s -X GET -k --key $GATEWAY_KEY --cert $GATEWAY_CERT https://localhost:15888/chain/status | jq ok
    • curl -s -X POST -k --key $GATEWAY_KEY --cert $GATEWAY_CERT -H "Content-Type: application/json" -d "$(envsubst < ./requests/add_bsc_key.json)" https://localhost:15888/wallet/add | jq ok
    • curl -s -X POST -k --key $GATEWAY_KEY --cert $GATEWAY_CERT -H "Content-Type: application/json" -d "$(envsubst < ./requests/bsc_balances.json)" https://localhost:15888/chain/balances | jq ok
    • curl -s -X POST -k --key $GATEWAY_KEY --cert $GATEWAY_CERT -H "Content-Type: application/json" -d "$(envsubst < ./requests/eth_uniswap_add_liquidity.json)" https://localhost:15888/amm/liquidity/add | jq ok
    • curl -s -X POST -k --key $GATEWAY_KEY --cert $GATEWAY_CERT -H "Content-Type: application/json" -d "$(envsubst < ./requests/eth_uniswap_position.json)" https://localhost:15888/amm/liquidity/remove | jq ok

Ran tests using uniswap_v3_lp

  • Token approved for WBNB and CAKE
  • Liquidity added on position successfully
    image
    • When a position is closed, strategy proceeds with creating one again
    • Stop strategy and start again, retrieve the active position from pancakeswapLP
    • Stop and exit the client. Open, import and start strategy, successfully retrieved position with exact ID

@rapcmia
Copy link
Contributor

rapcmia commented Dec 8, 2023

Hi @vic-en github checks are still failing after 3/3 reruns 🙇🏼

@vic-en
Copy link
Collaborator Author

vic-en commented Dec 11, 2023

@rapcmia I'm still finding a good solution to the issue causing the workflow to fail.
I'll let you know as soon as I have something that works.
Thanks.

@fengtality
Copy link
Sponsor Contributor

@vic-en Don't worry about the workflow issues, as they appear to be related to the XRP chain and connector. I'll exclude them from the standard tests.

@fengtality fengtality merged commit 6bbec30 into hummingbot:development Dec 13, 2023
2 of 3 checks passed
@nikspz nikspz mentioned this pull request Dec 13, 2023
@jellebuth
Copy link

jellebuth commented Dec 13, 2023

I ran a test for fetching the trading prices of a pool and the new codebase does not seem to fetch the prices from a V3 pool correctly.

Steps to replicate:

  1. Set UseRouter -> True
  2. Fetch price of ARV/WBNB pool for 1 ARV
  3. Compare price with Pancakeswap interface (this should show a difference)
  4. Customise the routing on Pancakeswap to only include V2 pools
image image 5. Fetch price of ARV/WBNB pool for 1 ARV 6. Compare prices between PCS interface and Gateway price response (this should show a minimal difference)

Am I missing any steps in retrieving V3 prices compared to only V2 prices of a token?

In addition, I was unable to fetch prices for RITE/USDT V3 pool.

https://www.geckoterminal.com/bsc/pools/0x9b36e5b07eab82cd861aeb282235b95ee029164d

@nikspz
Copy link
Contributor

nikspz commented Dec 14, 2023

@jellebuth
Changed rate_oracle_source to gate_io, seems able to get the price

gw247.zip

image

image
image

@jellebuth
Copy link

@nikspz

I was trying to get it for RITE (which only has a V3 Pool. But it returns that there is no pool for RITE.

https://www.geckoterminal.com/bsc/pools/0x9b36e5b07eab82cd861aeb282235b95ee029164d

I think that the price fetching feature does not have a V3 integration yet? If I look at the source code, it seems like the function that returns the price does not follow similar logic as the one for Uniswap (which does have a V3 integration).

But I might be wrong in this.

@nikspz
Copy link
Contributor

nikspz commented Dec 14, 2023

I think that the price fetching feature does not have a V3 integration yet? If I look at the source code, it seems like the function that returns the price does not follow similar logic as the one for Uniswap (which does have a V3 integration).

But I might be wrong in this.

@vic-en Could you please check this comment?

@nikspz
Copy link
Contributor

nikspz commented Dec 14, 2023

I was trying to get it for RITE (which only has a V3 Pool. But it returns that there is no pool for RITE.

To triage the issue, Could you confirm are you're using LP strategy /requests?
Could you please attach log/ config file as well?

@vic-en
Copy link
Collaborator Author

vic-en commented Dec 14, 2023

@jellebuth @nikspz
0x0F5D54b27bDb556823F96f2536496550f8816dC5, which is the token address for RITE isn't in the default Bsc mainnet token list(https://github.com/hummingbot/gateway/blob/main/src/templates/lists/bep20_tokens_mainnet.json).

So add the token and the issue shouldn't persist.

@jellebuth
Copy link

@vic-en @nikspz I have added this one already, but the issue persists.

@vic-en can you confirm that the V3 smart order router for price fetching is correctly integrated? And whether you are able to get the prices for the RITE-USDT V3 pool?

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

5 participants