graphprotocol / uniswap-subgraph

This is for uniswap-v1. If you are looking for the uniswap v2 subgraph, please go to https://github.com/uniswap/uniswap-v2-subgraph
104 stars 57 forks source link

Incorrect balance for some of the sETH pool providers #27

Closed Destiner closed 4 years ago

Destiner commented 4 years ago

It seems that for some providers of sETH/ETH pool subgraph shows incorrect balance.

Here are some of the affected providers:

I checked some other pools and they didn't seem to have this issue.

To reproduce:

{
  userExchangeDatas(where: {
    user: "0x0d7f2e43b77deb6e922640a4b7edc5b7e874dd70",
    exchange: "0xe9cf7887b93150d4f2da7dfc6d502b216438f244"
  }) {
    uniTokenBalance
  }
}

vs etherscan

Recent Synthetix update might be relevant but I'm not sure how.

davekaj commented 4 years ago

Hey @Destiner , thanks for bringing this up. I am going to take a look at it first thing tomorrow when I am back at work!

davekaj commented 4 years ago

This appears to be more deceiving than I would have thought. I am going to keep digging into this but I don't have a solution right now. Gonna bring in @ianlapham too and see if he can figure anything out

Destiner commented 4 years ago

Hey @davekaj any luck figuring it out?

davekaj commented 4 years ago

Hey @Destiner , I will take a look today and get back to you!

davekaj commented 4 years ago

@Destiner I looked at this for about 2 hours and I still can't figure it out. My strongest guess is that is has to do with the code here:

https://github.com/graphprotocol/uniswap-subgraph/blob/master/src/mappings/exchange.ts#L940-L999

This is the only code that really updates user.uniTokenBalance. My guess is that it does have to do with the recent update to synthetix. It is probably some sort of calculation bug that is slowly accumulating incorrect balances. This is because it isn't often that people do normal transfers of UNI liqiuidity tokens, but SNX has made this commonplace (over 700 tx to their contract for staking).

Without seeing anything obvious in the code for errors, I would start testing it. The next thing I would do is simplify the subgraph to only look at this exchange, and then start logging transfer data.

I can't promise that I can dig into this and fix it, we are working towards network launch and I don't have time allocated for maintaining this right now.

It is possible Ian from Uniswap will be able to help, but I'll leave that up to him to answer.

And if you see anything wrong yourself in the code, please let up know!

Destiner commented 4 years ago

I see. Thanks for your help. I'll try to implement this part of the subgraph from scratch and see if it will have the same problem.

Destiner commented 4 years ago

Oh well. Turns out the problem is much simpler that we thought:

https://github.com/graphprotocol/uniswap-subgraph/blob/master/src/mappings/exchange.ts#L1005-L1007

userTo should be userFrom on these lines.

davekaj commented 4 years ago

Good catch, I will push a fix in a few hours - it takes a few days for the uniswap subgraph to sync, so we might have to wait until sunday or monday to see the results working

davekaj commented 4 years ago

never version has been deployed here - https://thegraph.com/explorer/subgraph/graphprotocol/uniswap?version=pending

closing. Thanks for the catch @Destiner 👍

davekaj commented 4 years ago

If for some reason this error didn't fix it (which we will see in a few days), lets reopen