-
Notifications
You must be signed in to change notification settings - Fork 18
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
Comments
@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? |
I believe this is caused by some rounding error that could've been avoided had the amount parameter been of BigNumber type just like |
True, that could to be the case, we better update to BigNumber and see if that solves the problem |
@msieczko but if you debug your order, you will see 0.1 and 250 are
So theoretically, it should be fine that is not the case in your example? |
@hems Yes, I'm aware that amount is converted to efx-api-node/src/api/submit_order.js Lines 29 to 30 in 9b6bfbd
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. |
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. |
amount
parameter inlock
,unlock
andsubmitOrder
methods should useBigNumber
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.
The text was updated successfully, but these errors were encountered: