DeltaBalances / DeltaBalances.github.io

The Ethereum decentralized exchange assistant. Check token balances, transaction details & trade history.
https://deltabalances.github.io/
GNU General Public License v3.0
169 stars 67 forks source link

Amount gets incorrectly reported for Ethfinex trades #69

Open kjekac opened 5 years ago

kjekac commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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.

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.