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 gets incorrectly reported for Ethfinex trades #69

Open
kjekac opened this issue Apr 12, 2019 · 4 comments
Open

Amount gets incorrectly reported for Ethfinex trades #69

kjekac opened this issue Apr 12, 2019 · 4 comments

Comments

@kjekac
Copy link

kjekac commented Apr 12, 2019

Steps to reproduce:

  1. Go to https://deltabalances.github.io/trades.html#0x694e929bc5d2cdb66183a874bd19cfd2a7edcb08 and fetch data for Ethfinex for the single block 7436018.
  2. Compare the amount 10.000 in the single output entry to the amount 9.975 reported in the transaction.

I suspect that this has something to do with the taker overpaying, as can be seen here: https://deltabalances.github.io/tx.html#0x5d2b4aee9667026cf5617cbd5f105638b617938c3707e6c55d961fa058e1b42d

@DeltaBalances
Copy link
Owner

Thanks for finding these bugs and giving easy instructions.

This one is actually kind of interesting. Ethfinex v1 uses a fork of the 0x Protocol v1 contract, so I didn't really need to change anything to parse their events and transaction input.

Well it now appears that their fork has some quirks...
I'm getting the following output from their event:


address: "0xdcdb42c9a256690bd153a7b409751adfc8dd5851"
blockNumber: 7436018
events: [
    0: {name: "maker", type: "address", value: "0x694e929bc5d2cdb66183a874bd19cfd2a7edcb08"}
    1: {name: "taker", type: "address", value: "0x61b9898c9b60a159fc91ae8026563cd226b7a0c1"}
    2: {name: "feeRecipient", type: "address", value: "0x61b9898c9b60a159fc91ae8026563cd226b7a0c1"}
    3: {name: "makerToken", type: "address", value: "0xaa7427d8f17d87a28f5e1ba3adbb270badbe1011"}
    4: {name: "takerToken", type: "address", value: "0x38ae374ecf4db50b0ff37125b591a04997106a32"}
    5: {name: "filledMakerTokenAmount", type: "uint256", value: "53300000000000000000"}
    6: {name: "filledTakerTokenAmount", type: "uint256", value: "10000000000000000000"}
    7: {name: "paidMakerFee", type: "uint256", value: "0"}
    8: {name: "paidTakerFee", type: "uint256", value: "0"}
    9: {name: "tokens", type: "bytes32", value: "0x705c89bdedebc98696a3b0daaa04c60552a7ba73cf3185af895629dd17b2ffe4"}
    10: {name: "orderHash", type: "bytes32", value: "0x0e089493a662e92e9cb586448e95ea9a596b504785c7b5f99038208f3f0d24cf"}
]
hash: "0x5d2b4aee9667026cf5617cbd5f105638b617938c3707e6c55d961fa058e1b42d"
name: "LogFill"

As you can see, the 10.0 MKRW and 53.3 ETHW are given, with maker and taker fees at 0.
So they use a hidden taker fee, that I apparently missed before.

DeltaBalances added a commit that referenced this issue Apr 12, 2019
@DeltaBalances
Copy link
Owner

I added the custom Ethfinex v1 fee calculations.

I'm just debating whether I should update the price or not.
Right now amount and total show correctly, but the price remains original (without fee) because that is what users use on the exchange. But in this way amount * price is not equal to total.

@kjekac
Copy link
Author

kjekac commented Apr 24, 2019

Thanks for finding these bugs and giving easy instructions.

I've had to track these down because my tax data contained discrepancies, so might as well provide you with all the info I have. ;) Help you help me, and so on. Although now I've solved everything I currently need to, so unfortunately you shouldn't expect more of these.

As you can see, the 10.0 MKRW and 53.3 ETHW are given, with maker and taker fees at 0.
So they use a hidden taker fee, that I apparently missed before.

Are you sure that this is it? I know that in the UI, when you try to place e.g. a sell order below the current highest bid (which will get automatically matched, making you a taker), you get told that you will simply lose the difference, which, afaik, Ethfinex keep for themselves. (The most common reason for placing suboptimal orders would probably be a bulk sell where you don't want to have to repeatedly match the dust orders at the top.)

Although I suppose you could actually think of this as a hidden fee, even though it's technically voluntary.

I'm just debating whether I should update the price or not.
Right now amount and total show correctly, but the price remains original (without fee) because that is what users use on the exchange. But in this way amount * price is not equal to total.

My suggestion would be to update the price, because I imagine that my use case (accounting, tax reporting) is the most common, and then the "actual paid price" is what you want to report, not the one you forewent. But I am of course partial and might miss some other important use of this tool. :)

@DeltaBalances
Copy link
Owner

Are you sure that this is it?

This is all outdated by now, as they switched to 0x v2, but just for clarity here is what they changed.

  • they disabled any ZRX fee calculations, makerFee and takerFee will always return 0.
  • they added a hardcoded 0.25% fee for any order, but that isn't shown in any transaction input or event.

Only their admin can fill an order, thus they always earn the spread between 2 orders. But that is separate from the 0.25% fee.

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