Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bleutrade, anxpro and southxchange withdraw() with Python 2.7 ccxt 1.9.175 #344

Closed
PhinixPhire opened this issue Oct 19, 2017 · 11 comments
Closed
Assignees

Comments

@PhinixPhire
Copy link

PhinixPhire commented Oct 19, 2017

It looks like the withdraw method for bleutrade is calling an unsupported dictionary key (uuid). Am I understanding correctly that it's mostly mirroring bittrex with a few exceptions?

When I directly run this code:

result = bleutrade.accountGetWithdraw(bleutrade.extend({
            'currency': 'ETH',
            'quantity': 0.1,
            'address': '0x27b8F8D78742C47979dE46862696F39427f82995',
        },))
print result

I get this result:
{u'message': u'Your withdraw has been successfully scheduled.', u'result': [], u'success': u'true'}

It looks like bleutrade doesn't provide a transaction id? So I think we should change 'uuid' to, perhaps, 'message'? Then we at least get something back when executing a withdrawal and no error.

    def withdraw(self, currency, amount, address, params={}):
        self.load_markets()
        response = self.accountGetWithdraw(self.extend({
            'currency': currency,
            'quantity': amount,
            'address': address,
        }, params))
        return {
            'info': response,
            'id': response['result']['message'],
        }

I would try the pull request stuff... except I'm not certain this is a desired change for everyone. Let me know your thoughts, Thanks!

@PhinixPhire
Copy link
Author

Here's the error from using withdraw() instead:

Traceback (most recent call last):
  File "C:\Users\Kenneth\eclipse-workspace\Chank\CCXTtests.py", line 274, in <module>
    trxid = bleutrade.withdraw('ETH', 0.1, '0x27b8F8D78742C47979dE46862696F39427f82995')
  File "C:\Python27\lib\site-packages\ccxt\exchanges.py", line 5765, in withdraw
    'id': response['result']['uuid'],
TypeError: list indices must be integers, not str

Sorry for the omission!

@PhinixPhire
Copy link
Author

While we're at it... anxpro also has a similar issue. It's output is:
{u'data': {u'transactionId': u'cfdf2b21-1830-4cdf-a3b2-8ae0ceef2dbb'}, u'result': u'success'}

So perhaps it should be changed to look like this on line 1620 of exchanges?:

        return {
            'info': response,
            'id': response['data']['transactionId'],
        }

Am I assuming correctly?

@PhinixPhire
Copy link
Author

FYI, I tried changing my exchanges.py for anxpro as described above and it is working as I intended. Would love to get the change up to GitHub if acceptable to you.

@PhinixPhire PhinixPhire changed the title bleutrade withdraw() with Python 2.7 ccxt 1.9.175 bleutrade, anxpro and southxchange withdraw() with Python 2.7 ccxt 1.9.175 Oct 19, 2017
@PhinixPhire
Copy link
Author

Got another one for you...

trxid = southxchange.withdraw('ETH', 0.1, '0x6698f83ccf814f077c8b5d44d9822bfb332f20c6')
print trxid

Results in:

Traceback (most recent call last):
  File "C:\Users\Kenneth\eclipse-workspace\Chank\CCXTtests.py", line 274, in <module>
    trxid = southxchange.withdraw('ETH', 0.2, '0x6698f83ccf814f077c8b5d44d9822bfb332f20c6').get('info')
  File "C:\Python27\lib\site-packages\ccxt\exchanges.py", line 17820, in withdraw
    }, params))
  File "C:\Python27\lib\site-packages\ccxt\exchanges.py", line 17843, in request
    response = self.fetch2(path, api, method, params, headers, body)
  File "C:\Python27\lib\site-packages\ccxt\exchange.py", line 238, in fetch2
    return self.fetch(request['url'], request['method'], request['headers'], request['body'])
  File "C:\Python27\lib\site-packages\ccxt\exchange.py", line 292, in fetch
    return self.handle_rest_response(decoded_text, url, method, headers, body)
  File "C:\Python27\lib\site-packages\ccxt\exchange.py", line 329, in handle_rest_response
    raise ExchangeError(''.join([self.id, method, url, 'returned empty response']))
ccxt.errors.ExchangeError: southxchangePOSThttps://www.southxchange.com/api/withdrawreturned empty response

I haven't been able to figure that one out... the withdrawal still executes, but doesn't return data.

@kroitor kroitor self-assigned this Oct 19, 2017
@kroitor
Copy link
Member

kroitor commented Oct 19, 2017

Hi, @PhinixPhire ! Thx for opening this issue! I see, you're making good progress in understanding the inner workings of the library. That's very helpful!

It looks like bleutrade doesn't provide a transaction id? So I think we should change 'uuid' to, perhaps, 'message'?

Yes, I guess, a message is better than nothing... Though, I would better leave id untouched (undefined), if there's no txid. The message is still there in the info nearby.

I would try the pull request stuff... except I'm not certain this is a desired change for everyone.

We welcome your pull requests! )

So perhaps it should be changed to look like this on line 1620 of exchanges?

Yes, and if it works, then it's a perfect candidate for including into your PR.

I haven't been able to figure that one out... the withdrawal still executes, but doesn't return data.

That's very strange of them to return empty responses, but, I guess, we will have to add a special case for withdraw and some other methods... This error is handled in the fetch method from base class. I'll take a deeper look into it today and will fix it, hopefully.

kroitor added a commit that referenced this issue Oct 19, 2017
@kroitor kroitor reopened this Oct 19, 2017
@kroitor
Copy link
Member

kroitor commented Oct 19, 2017

I added your fixes to Bleutrade and ANXpro, now moving on to SouthXchange...

@PhinixPhire
Copy link
Author

Yes, I guess, a message is better than nothing... Though, I would better leave id untouched (undefined), if there's no txid. The message is still there in the info nearby.

Is it a better practice to leave a return as None than to populate a default response?

@kroitor
Copy link
Member

kroitor commented Oct 19, 2017

@PhinixPhire yes, because it indicates a "no response" explicitly to the user so that he can decide on what to do with it. Many users will be confused as they expect an id there, and they will treat the message as being an id, thus, all ids will be the same (not unique) → that can lead to very difficult unexpected errors elsewhere. So, I think it's always better to catch None instead of a default.

@PhinixPhire
Copy link
Author

Cool. I will be happy to work within that paradigm then. I'm sure I can patch a workaround for my local scripts.

@PhinixPhire
Copy link
Author

PhinixPhire commented Oct 19, 2017

Here's what I did, in case it's helpful to future readers... this way the script at least carries something through to describe what may have happened with the withdrawal when reviewing the ledger (I log trxid to a database for historical reference)

import sys
try:
  trxid = y.withdraw('ETH', amount, wallet).get('id')
except:
  trxid = str(sys.exc_info()[0]) + " occured. Funds may have withdrawn anyway."

The withdrawal does still complete. I've done several tests.

@kroitor
Copy link
Member

kroitor commented Oct 20, 2017

The southxchange error should be gone as of 1.9.210+! Thx for reporting!

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

No branches or pull requests

2 participants