refactor external feeds to be External Price and Order Engine + fix unit tests

[thehapax]

refactor external feeds unit tests to use a proper unittest framework.

[PermieBTS]

@bitfag would you be able to handle this?
If it can be completed without slowing down the higher priority issues it would be good to have this issue closed

[bitphage]

Yes I think this this task can be done in reasonable time.

[thehapax]

i think external feeds should be refashioned into the CEX price query engine as part of the TDD refactor proposal. save us time and cost if this is done as a whole chunk including the unit tests. This is the ccxt manual

See section in yellow for 2 separate modules 1 for price query and 1 for the order engine to ccxt.
TDD UML diagram

[thehapax]

Don't forget - Relative Orders strategy uses external feeds, so whatever you change here will need to be updated over there.

[bitphage]

Sorry I don't get it: are you proposing to not refactor tests until external feeds will be moved into "CEX price query engine"? So we're postponing this task until we do application design for a new model with all these price engines etc?

[bitphage]

As @thehapax described in the dev chat, the task is the develop Cex Price Query Engine. This raises the next question: how does this engine should fit into current project architecture?

[thehapax]

The architecture has been laid in the UML. It should be obvious.

[PermieBTS]

Specifically page 5 of the TDD

And here as octo linked: https://www.lucidchart.com/documents/view/6d5d06c4-81b2-4866-82d3-88788e1e2f6b/0

[PermieBTS]

Sorry I don't get it: are you proposing to not refactor tests until external feeds will be moved into "CEX price query engine"? So we're postponing this task until we do application design for a new model with all these price engines etc?

Not postponing.

Developing the price query engine using the existing price feed code as a starting point

• Instead of unit testing the CEX price feeds which will be replaced by the price query engine anyway

[bitphage]

Ok, basically it's should be a standalone class like dexbot/cex_engine.py. UML diagram does not specify any API how to instantiate (__init__), so I assume something like this:

:param str cex: cex name, bittrex / binance /etc
:param xxx loop: asyncio event loop (for future use)
:param str market: CEX market to operate on

There are several methods in UML and no description provided, it's unclear what these methods are supposed to do?

• getexchange
• filter_symbols
• get_consolidated_price
• set_alt_usd_pair

Also maybe I missed, what is the goal of separating "CEX price query engine" and "CEX order engine"?

[bitphage]

Ok, I see these methods are part of dexbot/strategies/external_feeds/ now.

[thehapax]

I leave the init design choice to the developer who is making the implementation.

Also maybe I missed, what is the goal of separating "CEX price query engine" and "CEX order engine"?

A class should have only one job - see SOLID principles.
TDD pg 4: Data source layer to be separate from order processing layer.

[bitphage]

@thehapax I don't think separation of orders placement methods into separate class is a good idea:

• Order engine and Price engine will have exactly the same class inheritance scheme with the same code (__init__ methods), both relying on the same APIs, like ccxt, etc. This is code doubling, which breaks DRY principle.
• I don't think single class breaks single-responsibility principle as this class is still responsible for common thing: interacting with external (CEX) API.
• I don't see how moving of order placement methods will help developers. Think about, why exchange-specific libraries like python-bittrex aren't providing separate classes for placing orders? Instead, all API calls are wrapped in single class like Bittrex?

[thehapax]

@vvk

• Does not need to be code doubling if we make one connector unit to the API.
• the pipeline of data should be : price feed -> filtering -> strategies -> order placement. In this order.
• this style is functionally driven and logic separated, instead of class object driven.

[thehapax]

Further:

• In the case of CoinGecko and say Coinmarketcap, there is only requirement for "Getting Price Feeds" and the API does not allow for order placement.
• Relative orders strategy. It gets external feed data but the orders are place on the bitshares dex only.
• Other strategies may also require only price feeds.

[bitphage]

• For external APIs like CMC there is also no methods for querying own orders and depthted orderbook buy/sell prices, so these methods will not work for CMC-like datasources
• Regarding RO strategy and others required pricefeeds: they will just call only price-related methods

I currently implementing Price Engine using OOP and class inheritance, probably I need to publish WIP version for you to review, so you can tell me more precisely if you like it or not, and what needs to be changed.

[thehapax]

ok sounds good. :)

[bitphage]

I pushed current work into https://github.com/bitfag/DEXBot/tree/524-refactor-external-feeds

[thehapax]

ok I took a look. it is a good start but we will need to rename the packages.

As per conversation with @joelvai

StrategyBase will become an Abstract Base class. Future Strategies could use more than one feed at any time and more than one dex/cex at the same time.

dexbot.strategies.(StrategyBase ......  /RO/SO, future strategies)
dexbot.pricefeeds.(BitsharesFeed.... /ccxt/gecko/eos dex 1/ eos dex 2...)
dexbot.orderengines.(BitsharesOrderEngine   /ccxt/waves.... / eos dex, other)

the flow should be

Price Feed >> Strategy >> Order Engine where Each Strategy can have N source of price feeds and M order engines and the relationship is {M:N} where N >= 1 and M >=1.

price_engine would not be switching between individual feeds, i don't think we need to develop this at this stage. It would be better to just implement gecko and waves and do interface later (if needed) when we get base refactor done.

For now, I would focus on the getting staggered orders with unit test and refactored with virtual orders logic separated.

Once the base refactor is merged into the devel branch then we can continue with this part.