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

Amount parameter should use BigNumber type #58

Open
msieczko opened this issue Jun 4, 2019 · 6 comments
Open

Amount parameter should use BigNumber type #58

msieczko opened this issue Jun 4, 2019 · 6 comments

Comments

@msieczko
Copy link

msieczko commented Jun 4, 2019

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 9b6bfbd) cannot correctly handle all possible token amounts.

@hems
Copy link
Contributor

hems commented Jun 4, 2019

@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
Copy link
Author

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
Copy link
Contributor

hems commented Jun 14, 2019

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
Copy link
Contributor

hems commented Jun 14, 2019

@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
Copy link
Author

@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?

amount,
price,

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
Copy link
Contributor

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.

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

3 participants