search

makerdao / market-maker-keeper

Maker Keeper Framework: Market maker keepers for OasisDEX, EtherDelta, 0x (RadarRelay, ERCdEX), Paradex, DDEX, IDEX, Bibox, Ethfinex, GoPax, HitBTC, TheOcean, OKEX and Gate.io.
GNU Affero General Public License v3.0
479 stars 182 forks source link

Add okcoin #144

Closed MikeHathaway closed 4 years ago

codecov-io commented 4 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@120daee). Click here to learn what that means. The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #144   +/-   ##
=========================================
  Coverage          ?   16.23%           
=========================================
  Files             ?       45           
  Lines             ?     4675           
  Branches          ?        0           
=========================================
  Hits              ?      759           
  Misses            ?     3916           
  Partials          ?        0
Impacted Files Coverage Δ
market_maker_keeper/bitso_market_maker_keeper.py 0% <0%> (ø)
market_maker_keeper/gas.py 50% <0%> (ø)
market_maker_keeper/korbit_market_maker_keeper.py 0% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 120daee...727488c. Read the comment docs.

MikeHathaway commented 4 years ago

Is there an opportunity to subclass OkcoinMarketMakerKeeper from OkexMarketMakerKeeper? Admittedly, most of the keepers have a lot of duplicated code. But this seems like a shining opportunity for code reuse. I think youd just need a newinit` method.

I'm not sure there's a way to do that without also modifying the OkexMarketMakerKeeper - there's a number of hardcoded arguments like prog that would need to be parameterized. This would be easily doable, but it would also require changes to production keepers.

Given the similarities between all the keepers, would it instead be worthwhile to work on developing a common abstract class?

MikeHathaway commented 4 years ago

Each subclass would need their own __init__ method. @grandizzy introduced PyexApi after many pyexchange integrations were baked, so at least integrations which followed, and API updates, could make use of the interface. Perhaps for future integrations, we could develop such a base class.

I agree. Perhaps a Base class for CEX and a base for DEX?