Safe-react: Value incorrectly displayed in transaction list

[lukasschor]

Report by Franco #707

Problem:
In the following image the tokens REP, BAT and ZRX are showing more than they should. It should be 100 REP, 100 BAT, 90 ZRX
image.png

URL: https://safe-team.dev.gnosisdev.com/app/#/safes/0xc3214e72A53695faC3A124C92FcA3f161D193299/transactions

Steps:
Just faucet some tokens from here: https://app.compound.finance/
Then transfer them to the safe

---

User Report

A user reported that The transaction value is displayed wrong in the transaction list

Unfortunately, I was not able to reproduce the issue, as everything looked fine when I was loading the same Safe:

Details

Safe: 0x35Ae15dCEB6930756A59EfcC2169d2b834CdD371
Wallet: Metamask
Browser: Firefox
OS: Windows

User Report 2 (via Tobi)

Title/Description

Wrong decimals for token in transactions list.

A user reported via Discord that the wrong number of decimals is displayed for this Safe: https://gnosis-safe.io/app/#/safes/0x83Be46243e01139bC390CC9886f011484C78dB42/balances

Here's a screenshot:

It shows a large number of RICE tokens which is not correct, cf. here: https://etherscan.io/address/0x83Be46243e01139bC390CC9886f011484C78dB42/#tokentxns

Please get in touch with him in case you need more info from the user, happy to relay.

[fernandomg]

same safe, different browsers

not completely related (as it's not the tx list). But looks like a similar issue.

well, same result in the txs list

[lukasschor]

Just had the same issue. Maybe the error log has some insights on the source of the issue.

[fernandomg]

@lukasschor, those errors are mostly related to NFTs not with ERC-20.

And looks more like a general network problem, because the "Invalid JSON RPC response" error is for the Safe's ETH balance request.

On the other hand, I've been testing/debugging my scenarios. I'd already commented to @Agupane my findings.

To summarize, for the LINK token, I had a decimal value of 12 instead of the expected 18 (this whole thing in Rinkeby).

I couldn't find the source for that 12, except that it was persisted in the localStorage. But all the endpoints that return the decimals for the tokens were returning 18 for LINK, so by cleaning up the localStorage it was fixed.

It's complicated for me to reproduce this, as I constantly switch environments (dev and staging), accounts, branches.

@Uxio0, is there any remote chance that the /balance endpoint is returning for the token 0x01be23585060835e02b77ef475b0cc51aa1e0709 a decimal of 12?

The safe I took the screenshots from: 0x441E604Ad49602c0B9C0B08D0781eCF96740786a

[mikheevm]

I have two assumptions:

1. At some point the backend had incorrect decimals number (not very likely imo)
2. When fetching decimals, it failed and fell back to default value - 18

[mikheevm]

Also I think the logic for fetching token symbol/decimals in fetchTransactions is not needed, because getTokenInfos does it.

Also having two places where the decimals can come from: getTokenInfos and catch block, It's probably worth assuming that the error may be somewhere there

[mikheevm]

So for now I wouldn't merge #755. We have just stopped fetching the decimals from the blockchain for performance reasons and moving back to that doesn't feel good.

What I would do is
1) Contact the user and ask him to update the app (removing cache if he is ok with that)
2) Invalidate locally stored token list by changing its key

[lukasschor]

Another user reported this via our Discord chat (added details to the OP). Increasing priority to high as it seems to affect more users than previously thought. @fernandomg would be good if you could prioritize this on Monday.

[fernandomg]

When fetching decimals, it failed and fell back to default value - 18

And that's the reason I though about a problem in the endpoint. The _failing_ token has to be 18 decimals, but it's 12 instead.

Another thing, we have a mix of types for the decimal values (string and number), and the 12 hapens to be string type.

Regarding performance, we have in place a generic function for batch requests (https://github.com/gnosis/safe-react/blob/e744142bb6e793afa1472388265d2e089464eafc/src/logic/contracts/generateBatchRequests.js). So we can request decimals/balances for all the tokens in one call. Thing is, we decided to go for a solution, then we knew we can improve the performance.

[mikheevm]

@fernandomg

if (currentSavedTokens && currentSavedTokens.size === tokenList.length) {

This is the logic we have for updating tokens. Probably this should be changed to hash or something to prevent such cases.

Also notice that in the second report for transactions 1-2 it is showing the correct decimals for the same token. So given that I'm leaning more towards an error in our logic for fetching decimals.
EDIT: Actually the tokens are different, it's showing incorrect decimals for 0xEc0B6AFb3F9a609CEEd67e2CA551A4c573fd45F7 (tx 3-4) but correct for 0xDcA878B874089D7C1E572b849197E6b6b898d966 (tx 1-2)
Those tokens are not in the relay service, so I assume we fetched the decimals from the blockchain

Let's also wait for @Uxio0's input, maybe some tokens were updated

[Uxio0]

@Uxio0, is there any remote chance that the /balance endpoint is returning for the token 0x01be23585060835e02b77ef475b0cc51aa1e0709 a decimal of 12?

The safe I took the screenshots from: 0x441E604Ad49602c0B9C0B08D0781eCF96740786a

@fernandomg, that's something you can easily check yourself. It will make my life easier that you check the endpoints 😉 https://safe-transaction.rinkeby.gnosis.io/api/v1/safes/0x441E604Ad49602c0B9C0B08D0781eCF96740786a/balances/

Let's also wait for @Uxio0's input, maybe some tokens were updated

Hi @mikheevm, tokens on the balance endpoint are not updated, they are detected from blockchain, so I guess there's no chance of returning 18 (unless the contract changed 😉 ). In the relay service they are updated manually, but there's no such token on rinkeby.

https://safe-relay.rinkeby.gnosis.io/api/v1/tokens/0x01BE23585060835E02B77ef475b0Cc51aA1e0709/

[rmeissner]

As @Uxio0 mentioned: @fernandomg @mikheevm please check the service endpoints yourself (this can also be easily done by checking the developer console of the web app or by using the url that uxio mentioned). If you detect an error in the returned data, please copy the url, the request, the response and the time when you did the request and open a bug ticket on the transactions service.

[fernandomg]

@rmeissner, yes, already did check the endpoints I could with the Safe. That's why reached @Uxio0, for a _remote chance_. I thought it would be easy to dump a table from the DB or if there's another thing we're not aware of that can lead to the backend storing invalid data.

But, as @Uxio0 already explained to @mikheevm,

tokens on the balance endpoint are not updated, they are detected from blockchain, so I guess there's no chance of returning 18

There's _no chance_.

So let's keep digging.

[rmeissner]

the thing is that @Uxio0 is indirectly mentioned as a blocker to provide information that is publicly available. If you need information that is not publicly available you should clearly state what information is required and why you need that (e.g. provide an example of the public information that lets you believe that our backend is containing wrong information). Else @Uxio0 needs to investigate some basic information that could have been provided and stop working on issues that have been assigned to him. So the important part is, please be specific what @Uxio0 needs to investigate and what information the safe-react team already discovered.

[Agupane]

Please check my comment, the pr is ready for review