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/kucoin exchange id is none #6999

Merged

Conversation

tomasgaudino
Copy link
Contributor

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:

  • Implement requests error parsing inside place_order and place_cancel methods from kucoin spot connector
  • Implement _is_order_not_found_during_status_update_error and _is_order_not_found_during_cancelation_error methods

Tests performed by the developer:
coverage run --source=hummingbot/connector/exchange/kucoin -m nose test/hummingbot/connector/exchange/kucoin/test_kucoin_exchange.py && coverage report --include="*/kucoin_exchange.py" -m

Tips for QA testing:

  • Start hummingbot and run start --script v2_xemm.py --conf debugging_script.yml, being debugging_script.yml
markets: {}
candles_config: []
controllers_config: []
config_update_interval: 60
script_file_name: v2_xemm.py
maker_connector: kucoin
maker_trading_pair: LOOM-USDT
taker_connector: binance
taker_trading_pair: LOOM-USDT
target_profitability: 0.001
min_profitability: 0.0007
max_profitability: 0.005
order_amount_quote: 5

@tomasgaudino
Copy link
Contributor Author

I've made some research checking other connectors standards, and this is the most suitable implementation for this bug. If there is an unexpected behaviour or you think there is a better approach to this issue, please feel free to write it down here!

Copy link

1 similar comment
Copy link

@rapcmia
Copy link
Contributor

rapcmia commented May 1, 2024

PR update:

  • Ran for 3-4hours XEMM controller config on Kucoin/Binance GAS-USDT
    • Orders created ok and cancelled when beyond threshold of min/max profitability spreads
    • So far no reported issues found specially on keyError:data
    • Had some opportunity to trade and compare results
      • All orders have exchange IDs match between logs and exchange trade history
      • Found exchange_trade_id not matching from CSV to logs and exchange trade history
        • Compare to development and previous trade data from different market and matches from the logs
        • Running a different test now again on different market and see if this same results
        • Running XEMM bot overnight pending

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.

  • Ran test on XEMM controller config and XEMM script config, all ok
    • Ran for 3-4hours with spreads enough to trigger opportunity for trade
    • Ran tests on different markets and LOOM-USDT
    • Review logs, not found keyError:data nor exchange_order_id=NaN
  • Do steps to reproduced by Vik from Kucoin lost order error #6968 using simple_pmm_example.py
    • Compare this PR and Development branch
    • Observed that there is a active order on development but not on this PR, therefore issue is fix
    • Ran for 15-30mins, all ok
  • Do tests where base or quote asset is 0 balance and start v2 strategy
    • On Development, able to reproduce but not on this PR
    • Ran dman_maker_v2 on this tests and all ok, no active order on both client and exchange
  • Build docker image ok

Copy link
Contributor

@cardosofede cardosofede left a comment

Choose a reason for hiding this comment

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

LGTM!

@nikspz nikspz merged commit 00b6ad6 into hummingbot:development May 10, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Development 1.28.0
Development

Successfully merging this pull request may close these issues.

None yet

4 participants