light-client icon indicating copy to clipboard operation
light-client copied to clipboard

Improve token information handling in the dApp

Open kelsos opened this issue 4 years ago • 10 comments

Description

Currently, the token information (decimals, name, symbol) is fetched from the blockchain every time the user starts a new session. In a testnet with 100 token networks played this means that we do roughly 300 RPC requests every time we load the page, just to fetch the decimals, name, and symbol.

This information remains potentially unchanged and in fact most of the wallet applications are actually using pre-compiled information instead

The suggestion is to create a simple cache, based on the token address:

{
  "0xaf3162a5e1b8a0b7b3bf8d98c922d0f862275c28": {
    "symbol": "ETHCC3",
    "name": "ETHCC3 Workshop Token",
    "decimals": 18
  }
}

This mapping should be persisted to local storage and the updating should be as simple as possible. The mapping should include the chainid or name in the key to ensure that only the token information for a specific token network is stored there.

For the update, we should get the token addresses for the deployed token networks and remove the ones that are not there (should not happen but it would be still good to have) and fetch the data for the missing keys.

This way the only information we need to fetch live is the balance of the tokens.

We should also evaluate if it makes sense to provide a mechanism to refresh the cache or add some temporal duration to the cache.

Ideally this mechanism could also integrate with eth-contract-metadata in the future.

Acceptance criteria

Tasks

  • [ ]

kelsos avatar May 06 '20 08:05 kelsos

@kelsos Yes, we need a good way how to handle this. I was even thinking about a token white list in the dApp. ... or maybe this would be an opportunity to use the Anyblock Index.

I was stumbling across an issue that must be related.

The sorting is correct at the start, but then the ETHCC3 and TTT token are added and it's not correct any more. cc @nephix

grafik

christianbrb avatar May 08 '20 13:05 christianbrb

The sorting is correct I think, as it was requested to sort first by balance and second by token symbol?

nephix avatar May 08 '20 13:05 nephix

That's correct. The TTT balance is higher than the ETHCC-balance, so the ETHCC token should be above the second TTT token.

But I assume, that the sorting is done before the last token is getting added which was the ETHCC token.

christianbrb avatar May 08 '20 13:05 christianbrb

I already suggested that it would make sense to cache these as they rarely change and 100 requests on mobile are a pain in the b***.

localStorage can be sufficient and is relatively easy with vuex-persists from #1476 If we want to automatically invalidate the cached tokens after a certain time, they could also get saved into a cookie which vuex-persists supports. We would then get the cache expiration for free as it's provided by Browser API

nephix avatar May 08 '20 13:05 nephix

I am not sure about what the best approach would be regarding the cache invalidation. Keep in mind that most of wallets for mainnet don't even fetch this token information (except the balance) directly from the blockchain. Instead, they use pre-compiled lists of the tokens.

Also instead of deleting the information completely, we could add a validity check after x days. this way the frontend still displays data from the cache. Even if they might be slightly wrong. (They won't probably be). Essentially they only way for the token information to change for an ERC-20 token is if the deployed smart contract is a proxy contract and it starts pointing to another contract underneath.

kelsos avatar May 11 '20 07:05 kelsos

As far as I heard, we will have exactly two tokens on main net.. so maybe it's not an issue worth fixing?

nephix avatar May 14 '20 06:05 nephix

@nephix That's correct and that's why it is still at the bottom of the product backlog

christianbrb avatar May 14 '20 06:05 christianbrb

@nephix @christianbrb this issue is main for testnets, I still want to go for #716 for mainnet. And live fetching should only happen if tokens are not on that list for mainnet. The idea is to have a metadata/caching hybrid mechanism.

kelsos avatar May 14 '20 07:05 kelsos

@andrevmatos Didn't we change and fix that already?

christianbrb avatar Jul 27 '20 07:07 christianbrb

No, not yet. We have caching in-session. This is about caching across sessions, since these values aren't expected to change. We don't have this yet, although this is only needed/used if the user goes to the Connect New Network screen.

andrevmatos avatar Jul 27 '20 14:07 andrevmatos