Skip to content
This repository has been archived by the owner on Feb 16, 2020. It is now read-only.

OkCoin China API Wrapper #352

Merged
merged 58 commits into from
Jul 3, 2016
Merged

OkCoin China API Wrapper #352

merged 58 commits into from
Jul 3, 2016

Conversation

xhad
Copy link
Contributor

@xhad xhad commented Jun 28, 2016

Hiya Mike!

I'm getting an error that I wasn't able to figure out. Any advice off the top of your head about what could be causing this? Could it be the getTrades date formatting?

2016-06-28 21:21:14 (DEBUG): Exchange has data spanning -8 seconds, we need at least 120 seconds.
2016-06-28 21:21:14 (INFO): Unable to use locally stored candles.

Everything else seems alright with OkCoin China. I made an npm module and everything seems to work. However, I need to test everything.

@xhad xhad force-pushed the new-bitfinex branch 2 times, most recently from b6d4066 to 3f448ad Compare July 1, 2016 08:28
@xhad
Copy link
Contributor Author

xhad commented Jul 1, 2016

Hi again Mike,

Could you please help me.. everything works well with the okcoin.js wrapper. However, sometimes a trade won't get filled. Is there a way to callback and cancel until the trade gets filled? I have seen 'result.is_live' but I'm not sure how to code it.. please advise if you have some free time.

Thank you so much for all of your help.

@askmike
Copy link
Owner

askmike commented Jul 1, 2016

Hey,

No problem, I am already very happy that you are helping out!

However, sometimes a trade won't get filled. Is there a way to callback and cancel until the trade gets filled?

Currently, whenever the portfolioManager places an order it expects that order to be placed in the orderbook. 1 minute later the portfolioManager will check if the order got (fully) filled. If it did not (sounds like what you are describing) than the portfolioManager will cancel the outstanding order and create a new one.

Shouldn't this automatically resolve the issue you describe?

@xhad
Copy link
Contributor Author

xhad commented Jul 2, 2016

^^ had some old pasted code calling for btcc in my okcoin.cancelOrder method. Embarrassing. Please take a look. Everything works for me, however I'm getting this"unable to use local candles", but not sure if that is something I have configured incorrectly or if it's an issue with my code.

@askmike
Copy link
Owner

askmike commented Jul 2, 2016

this message

Do you mean the message "unable to use local candles", yes you can safely ignore that.

I will check the code out!

@xhad xhad closed this Jul 3, 2016
@askmike askmike reopened this Jul 3, 2016
@askmike askmike merged this pull request into askmike:new-bitfinex Jul 3, 2016
@askmike
Copy link
Owner

askmike commented Jul 3, 2016

I was testing the code and damn, there are a lot of trades on okcoin...

The hard thing is that their API only gives back the last 600 trades, which is only a few seconds of trade activity. On default Gekko fetches data every 20 seconds, but I changed this too 2 seconds (for Okcoin for now)..

This discussion is relevant: #336.

@askmike
Copy link
Owner

askmike commented Jul 3, 2016

Was the real trading working for you? Or did you not have a chance to test it?

askmike added a commit that referenced this pull request Jul 3, 2016
@xhad
Copy link
Contributor Author

xhad commented Jul 4, 2016

Everything is working very well, however, I need to make some changes. OKcoin has a menu to add multiple 'child-accounts' to the main account, so you can run several bots from the same account. However, I need to update the npm module to handle this.

Secondly, What do you think:

There are two OKcoin exchanges, INTL, and CHINA. Should I change the npm module to accept a param to toggle between .com and .cn, and create okcoin-intl and okcoin-china in exchanges.js?

I closed this PR for now because the branch was getting so stale, and there are some changes that need to me made. Thank you very much for looking into the local candles issue.

@askmike
Copy link
Owner

askmike commented Jul 4, 2016

There are two OKcoin exchanges, INTL, and CHINA. Should I change the npm module to accept a param to toggle between .com and .cn, and create okcoin-intl and okcoin-china in exchanges.js?

Are the APIs the same (apart from .cn vs .com domain name)? In that case I think it would be better to create 2 different files (and keys in the exchanges file). This also makes it very clear that they are indeed very different exchanges (trades on the OKcoin China do not happen on OKcoin international).

I closed this PR for now because the branch was getting so stale

I did already pull your two commits in, but if you have more updates just create a new PR.


Also a tip on how to prevent my own commits from being added to your PR: after you create a PR Github will automatically add all commits that you push to your PR branch, so if you sync your branch with mine they all show up. What you can do is create a new branch for all commits that you want to push to Gekko:

git checkout -b okcoin-fixes
git commit -m 'my okcoin fixes'

And as soon as you want to pull in my changes you can switch to any other branch

git checkout stable
git merge askmike/stable

And if you also want your own code in your stable branch as well.

git merge okcoin-fixes

And only create a PR for your okcoin-fixes branch against my develop branch.

@xhad
Copy link
Contributor Author

xhad commented Jul 4, 2016

Thank you very much. :)

@xhad
Copy link
Contributor Author

xhad commented Jul 4, 2016

Everything was going well until I sent over a lot more btc and the orders weren't getting filled, and Gekko wasn't able to cancel them. Also, it seems that the rest API was getting Ddos'd or got overloaded during high volume and the orders wouldn't get submitted, ERCONREFUSED.. something like that. I started a mocha unit test that will go through okcoin.js and sort everything out. Good for me to learn.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants