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

Zenbot Bitstamp extension #266

Closed
wants to merge 37 commits into from
Closed

Zenbot Bitstamp extension #266

wants to merge 37 commits into from

Conversation

tuxitor
Copy link
Contributor

@tuxitor tuxitor commented Jun 11, 2017

The increase in traffic on the exhanges we have seen lately can be quite a challenge when developing a trading bot. Traffic conguestion, DOS attacks, broken connections and what not. On the exchange side they need tu have mechanisms to conrol the traffic. One measurement the bot are facing is the risk of being blocked with rate limiting. With the use of HTTP requests a lot of connecting, closing and reconnecting are taking place, and it is easy to get into a rate limit situation.

In the interface layer for Bitstamp we have used websockets for getting data. Unfortunately Bitstamp only have implemented two public channel, tickets and orderbook. Luckily it is tickers and orderbook that generate most of the traffic from the exchange. In short it means that we have to use traditional HTTP requests for the private authenticated functions. But use of websockets functions means we can connect once per session and listen for streaming data without the fear of being locked out by a rate limit block.

The Bitstamp interface may still have some bugs and shortcomings, but hopefully such problems can be sorted out.

tuxitor and others added 19 commits June 6, 2017 00:43
Introduces a new comand:
"zenbot balance <selector>"
Sample output: 
BTC-USD Asset: 9.87654321. Currency: 1.23
…ite a challenge when developing a trading bot. Traffic conguestion, DOS attacks, broken connections and what not. On the exchange side they need tu have mechanisms to conrol the traffic. One measurement the bot are facing is the risk of being blocked with rate limiting. With the use of HTTP requests a lot of connecting, closing and reconnecting are taking place, and it is easy to get into a rate limit situation.

In the interface layer for Bitstamp we have used websockets for getting data. Unfortunately Bitstamp only have implemented two public channel, tickets and orderbook. Luckily it is tickers and orderbook that generate most of the traffic from the exchange. In short it means that we have to use traditional HTTP requests for the private authenticated functions. But use of websockets functions means we can connect once per session and listen for streaming data without the fear of being locked out by a rate limit block.

The Bitstamp interface may still have some bugs and shortcomings, but hopefully such problems can be sorted out.
Copy link
Owner

@DeviaVir DeviaVir left a comment

Choose a reason for hiding this comment

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

the update-products.sh seems to be missing, could you still add this?

Really like the websocket path you took, and it looks great. Just a few nits.

// before the first call for a trade
// As zenbot dont returns the currency pair
// before the first trade is requested
// it ha ben neccessary to get it from
Copy link
Owner

Choose a reason for hiding this comment

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

it ha ben?

  client.balance(null, function (err, body) {
body = statusErr(err,body)
var balance = {asset: 0, currency: 0}
balance.currency = body[opts.currency.toLowerCase() + '_available']
Copy link
Owner

Choose a reason for hiding this comment

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

indentation problems start here

var currencyPair = joinProduct(opts.product_id).toLowerCase()
if (typeof opts.order_type === 'undefined' ) {
opts.order_type = 'maker'
}
Copy link
Owner

Choose a reason for hiding this comment

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

indentation

@DeviaVir
Copy link
Owner

@EigilB could you still add an update-products.sh?

@tuxitor
Copy link
Contributor Author

tuxitor commented Jun 12, 2017

I have not found a function for that on Bitstamp. The products file is done manually, but thanks for asking ;-)

DeviaVir
DeviaVir previously approved these changes Jun 12, 2017
@DeviaVir
Copy link
Owner

@EigilB I see, that's a bit sad, but alright then. Rest looks good to me, I'll leave the PR open for a day and if no one else speaks up let's merge this.

@tuxitor
Copy link
Contributor Author

tuxitor commented Jun 12, 2017

@DeviaVir, will You put this on hold for a while? During testing I have found an issue that needs some work.

@DeviaVir DeviaVir dismissed their stale review June 13, 2017 18:38

putting on hold

@DeviaVir
Copy link
Owner

@EigilB you might find the changes I made here interesting: https://github.com/carlos8f/zenbot/compare/master...DeviaVir:eigilb-master?expand=1

@tuxitor
Copy link
Contributor Author

tuxitor commented Jun 14, 2017

@DeviaVir, as a new user of Bitstamp, I think you may find this infornation interesting or at least useful.

The Bitstamp API has a straight forward market order API function, but the limit order has a somewhat peculiar option. This option is called "limit_price" and it is required if we don't use another option. "daily_order" (which is optional).

The 'limit_price' simply means that another order in added in the opposite direction.. It is activated when original order is executed and and the 'limit_price' then gets executed if the the market price reaches the set 'limit price'.

The call won't sell unless we have a value in "limit_price" (tested). It is described like this in the Bitstamp docs:

"If the order gets executed, a new sell order will be placed, with 'limit_price' as its price.", meaning it will place a buy order for the price of "limit_price". That price need to be lower than the sell price and with a corresponding amount.

An example: You set a sell price of 1200. At the same time you set a buy 'limit_price' of 1100. The buy order is activated as soon as the sell order is executed. If the market price goes down to 1100, the 'limit_price' order is executed to buy back the asset.

To use a limit call, I guess some logic to cancel the "limit_price" order is needed.

There is also another option. Instead we can use the "daily_order" option, which means that the same order is automatically canceled at midnight if not executed or canceled before that. Using this option means we need to have two new prototypes where the value of "daily_order" is set to "true" because the existing bitstamp node library is missing this option.

  Bitstamp.prototype.buyDaily = function(market, amount, price, callback) {
    this._post(market, 'buy', callback, {
      amount: amount,
      price: price,
      daily_order: true
    });
  }

And the same for "sellDaily()"

@tuxitor
Copy link
Contributor Author

tuxitor commented Jun 21, 2017

@DeviaVir
I think most of the issues are resolved now

commands/buy.js Outdated
} else {
so.order_type = so.taker ? 'taker' : 'maker'
so.order_type === 'taker' ? delete so.taker : delete so.maker
}
Copy link
Owner

Choose a reason for hiding this comment

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

indenting issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm.. Have tabstops set to 2 without filling with spaces. Expands in other environments

commands/sell.js Outdated
} else {
so.order_type = so.taker ? 'taker' : 'maker'
so.order_type === 'taker' ? delete so.taker : delete so.maker
}
Copy link
Owner

Choose a reason for hiding this comment

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

indenting issue

@@ -51,6 +51,13 @@ c.bitfinex.secret = 'YOUR-SECRET'
// May use 'exchange' or 'trading' wallet balances. However margin trading may not work...read the API documentation.
c.bitfinex.wallet = 'exchange'

// to enable Bitfinex trading, enter your API credentials:
Copy link
Owner

Choose a reason for hiding this comment

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

Bitfinex => Bitstamp

// before the first trade is requested
// it has been neccessary to get it from
// t:he command line arguments
args.forEach(function(value) {
Copy link
Owner

Choose a reason for hiding this comment

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

maybe this should live in a function that is called before it is needed instead of always executed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I dont quite understand the meaning of this. The internal logic is still a bit "secret". At least I have added comments where this implementation differs from the others. Any suggestions are welcome from a node programmer. This is project is my first attempt to live in the "callback hell".

price: data.price,
side: data.type === 0 ? 'buy' : 'sell'
})
if (wstrades.length > 30) wstrades.splice(0,10)
Copy link
Owner

Choose a reason for hiding this comment

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

weird indenting

function retry (method, args) {
var to = args.wait
if (method !== 'getTrades') {
console.error(('\nBitstamp API is not answering! unable to call ' + method + ',OB retrying in ' + to + 's').red)
Copy link
Owner

Choose a reason for hiding this comment

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

OB retrying, what is OB?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Stupid typo


var exchange = {
name: 'bitstamp',
historyScan: false,
Copy link
Owner

Choose a reason for hiding this comment

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

just verifying, no history for bitstamp available?

Copy link
Contributor Author

@tuxitor tuxitor Jun 21, 2017

Choose a reason for hiding this comment

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

Confirmed. From the docs:
"The time interval from which we want the transactions to be returned. Possible values are minute, hour (default) or day."
You can hardly call that history.

package.json Outdated
@@ -7,7 +7,9 @@
"zenbot": "./zenbot.sh"
},
"dependencies": {
"autobahn": "^17.5.2",
Copy link
Owner

Choose a reason for hiding this comment

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

weird indenting

orders['~' + body.id] = body
cb(null, body)
})
} else {
Copy link
Owner

Choose a reason for hiding this comment

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

perhaps we can be more explicit here and just say
if (opts.order_type === 'taker') {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a comment here to improve clarity

@tuxitor
Copy link
Contributor Author

tuxitor commented Jun 30, 2017

@carlos8f @DeviaVir
Please tak a look at this. It is probably very interesting for a lot of members of the community:
https://github.com/tuxitor/zenbot/blob/master/docs/Zentalk.md

@DeviaVir
Copy link
Owner

DeviaVir commented Jul 3, 2017

@tuxitor Definitely looks interesting, but it has become quite a mess. I spot a few typo's and wrong indents, but a bit too many to call them all out individually.

It would also be a lot easier if the zentalk and bitstamp code would be separated in different PR's (different branches). Is this something you can still do? Otherwise I'll try to update your code from my end.

@tuxitor
Copy link
Contributor Author

tuxitor commented Jul 3, 2017

I am far away from home this week, but may have some free spots and be able to do some fix up. Ideally the Zentalk addition should have been done in cooperation with @carlos to find the best approach. In the longer term I think headless networking model is the right direction to go. In general I think it may be possible make an addition like this conditionally included

@tuxitor
Copy link
Contributor Author

tuxitor commented Jul 3, 2017

I have to add that some help from an experienced node programmer would be much appreciated

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.

2 participants