ethfinex / efx-api-node

Client library to interact with the DeversiFi API and smart-contracts for non-custodial trading
https://app.deversifi.com
22 stars 18 forks source link

Amount parameter should use BigNumber type #58

Open msieczko opened 5 years ago

msieczko commented 5 years ago

amount parameter in lock, unlock and submitOrder methods should use BigNumber type.

JavaScript numbers are stored in 64-bit format whereas Ethereum uses integers up to 256-bit. Current implementation (revision 9b6bfbd3fb1cf7beaaecdd6054a9a28dbef01671) cannot correctly handle all possible token amounts.

hems commented 5 years ago

@msieczko good point, will have a closer look on this.

Do you have any examples of transactions or errors that are happening because of this specific issue?

msieczko commented 5 years ago

efx.submitSellOrder('ETHUSD', 0.1, 250) returns error:

{
    "code": 10020,
    "message": "ERR_TAKERTOKEN_AMOUNT_INVALID",
    "reason": "The signed-order taker amount did not match with the\n    amount and price specified in the API call."
}

I believe this is caused by some rounding error that could've been avoided had the amount parameter been of BigNumber type just like takerAssetAmount mentioned in the error's reason.

hems commented 5 years ago

I believe this is caused by some rounding error that could've been avoided had the amount parameter been of BigNumber type just like takerAssetAmount mentioned in the error's reason.

True, that could to be the case, we better update to BigNumber and see if that solves the problem

hems commented 5 years ago

@msieczko but if you debug your order, you will see 0.1 and 250 are

  1. sent o "create_order" function: https://github.com/ethfinex/efx-api-node/blob/master/src/api/submit_order.js

  2. Used with BigNumber: https://github.com/ethfinex/efx-api-node/blob/master/src/api/contract/create_order.js

So theoretically, it should be fine that is not the case in your example?

msieczko commented 5 years ago

@hems Yes, I'm aware that amount is converted to BigNumber when creating the 0x order object. What is the purpose of sending along the original amount and price as JavaScript numbers in the API call? https://github.com/ethfinex/efx-api-node/blob/9b6bfbd3fb1cf7beaaecdd6054a9a28dbef01671/src/api/submit_order.js#L29-L30

Strangely enough, I've sent another request to your API at 2019-06-14 10:04:25 with the same parameters and the order went through. Here's the resulting transaction on Etherscan: 0x4051ccfe6d90f1e7cb761a9d970044ce48c801f2cf53f0cb1efda27b57371e27. What could be the reason of this?

plutoegg commented 5 years ago

The purpose of sending the original amount and price again as javascript numbers in the API call is to match the API format for order submission from Bitfinex. Internally once checked and then submitted onto the order-book these orders are treated like any other. It is only after matching that the orders are then submitted on-chain and uses the 0x format order.