feat(exchange): add Coinmate exchange support #12264
Conversation
- Add Coinmate exchange class with spot trading - Configure rate limiting (100 req/min) - Add UID validation requirement - Support market and limit orders - Add comprehensive unit tests
There was a problem hiding this comment.
This somehow feels a ton like an AI PR if i'm honest (very low quality, pointless comments all around).
As we clearly state in the PR template - we expect that it's disclosed - and that the submitter did a proper review of it (which doesn't appear to be the case here).
If you did - please do both (disclose and PROPER review) now.
I think essentially all overrides of methods / properties are pointless, duplicating functionality that's already part of the parent class - and can set via configuration (order-type validation, trading-mode validation), leaving only the ft_has configuration as actually necessary (please remove everything that duplicates existing functionality).
As then there's no additional code in that class - i think the tests are also pointless.
Instead - you should add that exchange to the active tests (tests/exchange_online/conftest.py) - and run tests with the --longrun parameter (this will do an active test agains the exchange - confirming certain settings.
| config = super()._ccxt_config | ||
| config.update({ | ||
| "uid": self._api_key_headers.get("uid", ""), | ||
| "rateLimit": 600, # 100 requests/minute |
There was a problem hiding this comment.
we don't force options like this (applies to ALL settings here).
For the rate-limit specifically - if they're really wrong in ccxt (test without any custom rate-limit settings) - report it to ccxt.
If this is required AND ccxt refuses to fix this, add the proper settings to the documentation.
Doing it here removes people's possibility to change settings - is therefore not acceptable.
Also for all other options - remove - they're not necessary.
we won't force time-adjustments - and uid is already read from the configuration as part of the base exchange.
(in summary - this whole property is useless and has to be removed.
| with this exchange. | ||
| Coinmate is a Czech cryptocurrency exchange supporting spot trading only. | ||
| Rate limit: 100 requests/minute. |
| """ | ||
|
|
||
| _ft_has: FtHas = { | ||
| # Core trading features |
There was a problem hiding this comment.
this looks like it's been done by AI (pointless comments throughout, ...) - has it?
| "bingx", | ||
| "bitmart", | ||
| "bybit", | ||
| "coinmate", |
There was a problem hiding this comment.
This is the list of "supported" exchanges - meaning exchanges we actively support (core-team has accounts there, ...)
I don't see coinate to be added to this list if i'm honest. please remove it from here again.
| # Rate limiting | ||
| "ccxt_futures_name": "swap", # Required by base class |
There was a problem hiding this comment.
pointless? (both?)
you only add support for spot - but the comment suggests something around ratelimit ...
| "l2_limit_range_required": False, | ||
|
|
||
| # Market data features | ||
| "ohlcv_candle_limit": None, # No OHLCV support |
There was a problem hiding this comment.
This exchange has no ohlcv candle endpoint? (essentially meaning - fetch_ohlcv is not implemented).
If that's really the case (i doubt it) - then we can close this PR outright and add this exchange to the list of blocked exchanges, as then it's not supportable.
|
Well its marked as draft, if I would know that you will review it right away I would wait before submiting it. I will close this one - sorry for the inconvience but as I already said - it's a draft PR @xmatthias |
|
Well also a draft should get feedback, no? if you get no feedback, would you noticed the above points on your own? (questionable - as it requires some insights into what we already have and what already works). |
Summary
Add support for Coinmate cryptocurrency exchange with spot trading.
Quick changelog
What's new?
This PR adds support for the Coinmate exchange, a Czech exchange that supports spot trading.
Features:
Implementation:
ExchangeclassFiles Modified:
freqtrade/exchange/coinmate.py- Exchange implementationfreqtrade/exchange/__init__.py- Added importfreqtrade/exchange/common.py- Added to supported exchangestests/exchange/test_coinmate.py- Unit tests