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

Fix/uniswap token address #169

Merged
merged 4 commits into from
Jul 25, 2023

Conversation

vic-en
Copy link
Collaborator

@vic-en vic-en commented Jul 23, 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:
This PR enables EVM connectors to store and query tokens using checksummed addresses.

Tests performed by the developer:

  • Manual curl test on uniswap LP routes.

Tips for QA testing:

  • Test all EVM(AMM/LP/Perp) connectors and confirm nothing is broken.
    Thanks.

@vic-en
Copy link
Collaborator Author

vic-en commented Jul 23, 2023

Resolves issue reported on #154

@emusol
Copy link

emusol commented Jul 23, 2023

I was able to add monitor and remove liquidity from a live position, but when the position was out of range I was not able to get info about the position:
image

@vic-en
Copy link
Collaborator Author

vic-en commented Jul 23, 2023

@emusol
Can you send a screenshot of when the response of the request was successful?

@rapcmia rapcmia self-requested a review July 24, 2023 03:34
@rapcmia
Copy link
Contributor

rapcmia commented Jul 24, 2023

PR update:

  • Setup this PR with latest development client
  • Setup AMM arb with uniswap {arbitrum, polygon}, pancakeswap-bsc and pangolin-avax
    • Start strategy and able to display no arb opportunity on logs
    • Trade data on client and txn are ok too ✅
    • However for Uniswap-Goerli:
      image
      2023-07-24 12:45:09,001 - 14731 - hummingbot.strategy.amm_arb.amm_arb - INFO - Markets are ready. Trading started.
      2023-07-24 12:45:10,365 - 14731 - hummingbot.core.utils.async_utils - ERROR - Unhandled error in background task: [<class 'decimal.DivisionImpossible'>]
      Traceback (most recent call last):
        File "/Users/rapcomia/github/hummingbot/development/hummingbot/core/utils/async_utils.py", line 9, in safe_wrapper
          return await c
        File "/Users/rapcomia/github/hummingbot/development/hummingbot/strategy/amm_arb/amm_arb.py", line 234, in main
          await self.apply_slippage_buffers(profitable_arb_proposals)
        File "/Users/rapcomia/github/hummingbot/development/hummingbot/strategy/amm_arb/amm_arb.py", line 266, in apply_slippage_buffers
          arb_side.order_price = market.quantize_order_price(arb_side.market_info.trading_pair,
        File "hummingbot/connector/connector_base.pyx", line 471, in hummingbot.connector.connector_base.ConnectorBase.quantize_order_price
          return self.c_quantize_order_price(trading_pair, price)
        File "hummingbot/connector/connector_base.pyx", line 465, in hummingbot.connector.connector_base.ConnectorBase.c_quantize_order_price
          return (price // price_quantum) * price_quantum
      decimal.InvalidOperation: [<class 'decimal.DivisionImpossible'>]
      
      • This issue does not happen on development gateway branch 👀

Run curl test on uniswapLP

  • Add liquidity position ✅
    image
  • Remove liquidity position ✅
    image

@vic-en
Copy link
Collaborator Author

vic-en commented Jul 24, 2023

PR update:

* Setup this PR with latest development client

* Setup AMM arb with uniswap {arbitrum, polygon}, pancakeswap-bsc and pangolin-avax
  
  * Start strategy and able to display no arb opportunity on logs
  * Trade data on client and txn are ok too ✅
  * However for Uniswap-Goerli:
    ![image](https://user-images.githubusercontent.com/73840223/255502616-6b7ba320-7b41-4e0b-9a0e-38213a59158f.png)
    ```
    2023-07-24 12:45:09,001 - 14731 - hummingbot.strategy.amm_arb.amm_arb - INFO - Markets are ready. Trading started.
    2023-07-24 12:45:10,365 - 14731 - hummingbot.core.utils.async_utils - ERROR - Unhandled error in background task: [<class 'decimal.DivisionImpossible'>]
    Traceback (most recent call last):
      File "/Users/rapcomia/github/hummingbot/development/hummingbot/core/utils/async_utils.py", line 9, in safe_wrapper
        return await c
      File "/Users/rapcomia/github/hummingbot/development/hummingbot/strategy/amm_arb/amm_arb.py", line 234, in main
        await self.apply_slippage_buffers(profitable_arb_proposals)
      File "/Users/rapcomia/github/hummingbot/development/hummingbot/strategy/amm_arb/amm_arb.py", line 266, in apply_slippage_buffers
        arb_side.order_price = market.quantize_order_price(arb_side.market_info.trading_pair,
      File "hummingbot/connector/connector_base.pyx", line 471, in hummingbot.connector.connector_base.ConnectorBase.quantize_order_price
        return self.c_quantize_order_price(trading_pair, price)
      File "hummingbot/connector/connector_base.pyx", line 465, in hummingbot.connector.connector_base.ConnectorBase.c_quantize_order_price
        return (price // price_quantum) * price_quantum
    decimal.InvalidOperation: [<class 'decimal.DivisionImpossible'>]
    ```
    
    
        
          
        
    
          
        
    
        
      
    
    * This issue does not happen on development gateway branch 👀

Run curl test on uniswapLP

* Add liquidity position ✅
  ![image](https://user-images.githubusercontent.com/73840223/255503566-2079f2a0-8f8e-4f40-95d8-824719808f54.png)

* Remove liquidity position ✅
  ![image](https://user-images.githubusercontent.com/73840223/255503695-061c0e60-3403-4ead-8ff0-1837a41456f4.png)
  • Issue reported on Goerli testnet should be retested. I believe it has nothing to do with this pr.
    It's most-likely an issue with liquidity. Pls retest on a more liquid market on that testnet.

@fengtality
Copy link
Contributor

I agree with @vic-en that the Goerli issue should be unrelated to this fix

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.

LGTM

  • Setup this PR with lates development client
  • Setup DEX connectors GOLD and SILVER ok
  • Retest reporting issue on Goerli and confirm related to token used
  • Ran curl tests for uniswapLP and able to add/remove position ok

@emusol
Copy link

emusol commented Jul 25, 2023

I tried to test this a lot, and one issue keeps coming up, but not every time so I cant put my finger on it,
when monitoring a position and it goes out of range, some of the times gateway fails to fetch the position(also when retrying to get the position), but other times it get it right, for now I noticed it when watching a position for an hour before it goes out of range

2023-07-26 04:32:13 | info | 	Position info for position 744055 retrieved. | undefined
2023-07-26 04:32:14 | info | 	Position info for position 744055 retrieved. | undefined
2023-07-26 04:32:14 | info | 	Position info for position 744055 retrieved. | undefined
2023-07-26 04:32:15 | info | 	Position info for position 744055 retrieved. | undefined
2023-07-26 04:32:15 | info | 	Position info for position 744055 retrieved. | undefined
2023-07-26 04:32:16 | info | 	Position info for position 744055 retrieved. | undefined
2023-07-26 04:32:16 | info | 	Position info for position 744055 retrieved. | undefined
2023-07-26 04:32:18 | info | 	3737 request(s) sent in last 300 seconds. | undefined
2023-07-26 04:34:17 | error | 	Unable to fetch position with id 744055 | Error: Unable to fetch position with id 744055
    at UniswapLP.<anonymous> (/home/ubuntu/Downloads/gateway/dist/src/connectors/uniswap/uniswap.lp.js:69:23)
    at Generator.next (<anonymous>)
    at fulfilled (/home/ubuntu/Downloads/gateway/dist/src/connectors/uniswap/uniswap.lp.js:28:58)
2023-07-26 04:34:21 | error | 	unhandledRejection: timeout (requestBody="{\"method\":\"eth_gasPrice\",\"params\":[],\"id\":15609,\"jsonrpc\":\"2.0\"}", requestMethod="POST", timeout=120000, url="https://rpc.ankr.com/arbitrum", code=TIMEOUT, version=web/5.7.1)
Error: timeout (requestBody="{\"method\":\"eth_gasPrice\",\"params\":[],\"id\":15609,\"jsonrpc\":\"2.0\"}", requestMethod="POST", timeout=120000, url="https://rpc.ankr.com/arbitrum", code=TIMEOUT, version=web/5.7.1)
    at Logger.makeError (/home/ubuntu/Downloads/gateway/node_modules/@ethersproject/logger/lib/index.js:238:21)
    at Timeout._onTimeout (/home/ubuntu/Downloads/gateway/node_modules/@ethersproject/web/lib/index.js:187:35)
    at listOnTimeout (node:internal/timers:569:17)
    at process.processTimers (node:internal/timers:512:7) | Error: timeout (requestBody="{\"method\":\"eth_gasPrice\",\"params\":[],\"id\":15609,\"jsonrpc\":\"2.0\"}", requestMethod="POST", timeout=120000, url="https://rpc.ankr.com/arbitrum", code=TIMEOUT, version=web/5.7.1)
    at Logger.makeError (/home/ubuntu/Downloads/gateway/node_modules/@ethersproject/logger/lib/index.js:238:21)
    at Timeout._onTimeout (/home/ubuntu/Downloads/gateway/node_modules/@ethersproject/web/lib/index.js:187:35)
    at listOnTimeout (node:internal/timers:569:17)
    at process.processTimers (node:internal/timers:512:7)
2023-07-26 04:36:17 | error | 	Unable to fetch position with id 744055 | Error: Unable to fetch position with id 744055
    at UniswapLP.<anonymous> (/home/ubuntu/Downloads/gateway/dist/src/connectors/uniswap/uniswap.lp.js:69:23)
    at Generator.next (<anonymous>)
    at fulfilled (/home/ubuntu/Downloads/gateway/dist/src/connectors/uniswap/uniswap.lp.js:28:58)
    at runNextTicks (node:internal/process/task_queues:60:5)
    at process.processTimers (node:internal/timers:509:9)

@vic-en
Copy link
Collaborator Author

vic-en commented Jul 25, 2023

Looks like a node connection issue.
Request to fetch position timed out. Noticed that preceding request to update gasPrice also timedout.

Try swithing to a more stable and reliable node or package.

@fengtality fengtality merged commit d272192 into hummingbot:development Jul 25, 2023
2 of 3 checks passed
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.

4 participants