(Feature) Remove spam tokens / Deactivate tokens refresh fix by Agupane · Pull Request #1331 · 5afe/safe-react

Agupane · 2020-09-08T14:04:35Z

Closes #1324
Closes #1329

Description

Removes spam tokens by setting exclude_spam param in fetchTokenCurrenciesBalances, now the tokens marked as spam in the backend are not fetched and the total balance is not affected by them
The "force fetch assets" feature was still there, but it wasn't working because the list of tokens within the safe state wasn't checked on updateSafe reducer, I fixed that replacing merge with mergeDeep
Another thing solved here was the currencyBalance/currencyRate that were removed in this PR and was causing two bugs:
the first one is that the user lost the feature to store his preferred currency and avoid using always USD and the second one was related with the currency balances being deleted every time the user went to the assets tabs on this line:

    const storedCurrencies = await loadCurrencyValues()
    const storedCurrency = storedCurrencies[safeAddress]
    if (!storedCurrency) {
      return batch(() => {
        dispatch(setCurrencyBalances(safeAddress, List([])))
        dispatch(setSelectedCurrency(safeAddress, AVAILABLE_CURRENCIES.USD))
        dispatch(setCurrencyRate(safeAddress, 1))
      })
    }

Also while adding some types, I found that SerializedSessionState wasn't working so I fixed it, but seems that we aren't using it, I would suggest removing it.

Update

Now I left just the code needed, there are no fixes related to this (CC @dasanra)

I also left only the spam parameter, but seems that some tokens like Cunning or Scatter are still there, as far as I discussed with Lukas, they should be considered spam, just let you know in case you need to update the db @Uxio0

Update 14/09:

Closes #1324
Closes #1329

This PR fixes the issue with reloading the tokens and also adds the spam parameter for removing spam tokens from the balances list.

Also add types and replaces old class component to functional component

…rge, now the active tokens for the safe are not overwritten

github-actions · 2020-09-08T14:04:51Z

CLA Assistant Lite All Contributors have signed the CLA.

github-actions · 2020-09-08T14:10:02Z

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type	Occurrences	Fixable
Errors	0	0
Warnings	0	0
Ignored	0	N/A

Result: ✅ success
Annotations: 0 total

^{Report generated by eslint-plus-action}

…to 1312-fetch-assets

ghost · 2020-09-08T14:23:14Z

Travis automatic deployment:
https://pr1331--safereact.review.gnosisdev.com/app

ghost · 2020-09-08T14:33:05Z

Travis automatic deployment:
https://pr1331--safereact.review.gnosisdev.com/app

src/logic/currencyValues/api/fetchTokenCurrenciesBalances.ts

dasanra · 2020-09-08T15:20:02Z

src/store/index.ts

  [COOKIES_REDUCER_ID]: Map<string, any>
  [ADDRESS_BOOK_REDUCER_ID]: AddressBookReducerMap
-  [CURRENT_SESSION_REDUCER_ID]: Map<string, any>
+  [CURRENT_SESSION_REDUCER_ID]: SerializedSessionState


In your comment you say we are not using it and we should remove it.
But here we are using it?

Yes but it's not consumed anywhere, it's just added there

src/logic/currencyValues/store/middleware/index.ts

src/logic/currencyValues/api/fetchTokenCurrenciesBalances.ts

src/logic/currentSession/store/reducer/currentSession.ts

src/logic/safe/hooks/useFetchTokens.tsx

Uxio0 · 2020-09-10T14:19:53Z

@Uxio0 please check the updated comment on the PR description

@lukasschor is the one now updating the tokens db 😉

src/logic/tokens/store/actions/fetchTokens.ts

ghost · 2020-09-10T14:37:58Z

Travis automatic deployment:
https://pr1331--safereact.review.gnosisdev.com/app

mmv08 · 2020-09-14T11:31:05Z

Could you please update PR title and desc to what it really does?

I would drop fixes for #1312 in favor of #1333

…to 1312-fetch-assets # Conflicts: # src/routes/safe/components/Balances/Coins/index.tsx # src/routes/safe/components/Balances/Coins/styles.ts

Agupane · 2020-09-14T12:35:28Z

It's done @mikheevm

ghost · 2020-09-14T12:44:51Z

Travis automatic deployment:
https://pr1331--safereact.review.gnosisdev.com/app

mmv08

How does it fix deactivated tokens appear again? Was it an issue with mergeDeep too?

src/routes/safe/components/Balances/Coins/index.tsx

Adds types on updateBlacklistedTokens

also fix blacklisted addresses calculation

Removes actions from Tokens

Agupane · 2020-09-14T15:19:00Z

How does it fix deactivated tokens appear again? Was it an issue with mergeDeep too?

I thought that because the bug seems fixed but I tested with another safeAddress and the problem was still there, anyway, I fixed it now and refactored an old class component to functional component

ghost · 2020-09-14T15:29:59Z

Travis automatic deployment:
https://pr1331--safereact.review.gnosisdev.com/app

mmv08 · 2020-09-14T16:06:41Z

src/logic/safe/store/actions/updateActiveTokens.ts

@@ -1,4 +1,6 @@
 import updateSafe from './updateSafe'
+import { Set } from 'immutable'
+import { Dispatch } from 'redux'


We have already spoke about Dispatch and ThunkDispatch differences, haven't we?

fernandomg

works great! 💯

ghost · 2020-09-15T10:43:27Z

Travis automatic deployment:
https://pr1331--safereact.review.gnosisdev.com/app

Agupane added 9 commits September 7, 2020 10:11

Fix load current session

3801325

Fixs useMemo usage in filteredData

79c3d49

Type fetchTokens

82c5e4e

Type useFetchTokens

8027da7

Type setCurrencyBalances

d9e0bcd

Fixs ADD_SAFE reducer for existing safe, uses mergeDeep instead of me…

5bc0b8d

…rge, now the active tokens for the safe are not overwritten

Fix save selected currency

7548e4d

Adds excludeSpamTokens param in fetchTokenCurrenciesBalances

70be14f

Adds onlyTrustedTokens param in fetchTokenCurrenciesBalances

7cde7f0

Agupane self-assigned this Sep 8, 2020

Agupane marked this pull request as ready for review September 8, 2020 14:19

Merge branch 'development' of https://github.com/gnosis/safe-react in…

9055021

…to 1312-fetch-assets

Agupane requested review from dasanra, fernandomg, mmv08 and nicosampler September 8, 2020 14:21

Agupane requested a review from francovenica September 8, 2020 14:25

dasanra suggested changes Sep 8, 2020

View reviewed changes

mmv08 suggested changes Sep 9, 2020

View reviewed changes

Agupane added 4 commits September 10, 2020 09:24

Merge with development

c5f69ca

Remove onlyTrustedTokens param

de6d3bd

Fix unnecesary changes

e74be01

Merge branch 'development' into 1312-fetch-assets

4bfe6a9

Agupane requested review from Uxio0, dasanra and mmv08 September 10, 2020 12:40

dasanra approved these changes Sep 10, 2020

View reviewed changes

mmv08 approved these changes Sep 10, 2020

View reviewed changes

src/logic/tokens/store/actions/fetchTokens.ts Outdated Show resolved Hide resolved

Replace Dispatch with ThunkDispatch

55b7148

Merge branch 'development' of https://github.com/gnosis/safe-react in…

e29ad45

…to 1312-fetch-assets # Conflicts: # src/routes/safe/components/Balances/Coins/index.tsx # src/routes/safe/components/Balances/Coins/styles.ts

Agupane changed the title ~~(Feature) Force fetch assets when accessing assets tabs~~ (Feature) Remove spam tokens / Deactivate tokens refresh fix Sep 14, 2020

mmv08 approved these changes Sep 14, 2020

View reviewed changes

src/routes/safe/components/Balances/Coins/index.tsx Outdated Show resolved Hide resolved

Agupane added 8 commits September 14, 2020 09:54

Fix import consistency

4b0280b

Type containsMethodByHash

a5021c3

Fix blacklisted addresses calculation

cd93e92

Adds types on updateActiveTokens

cf27a1b

Adds types on updateBlacklistedTokens

Refactor Tokens to TokenList, makes it functional component

06aa640

also fix blacklisted addresses calculation

Refactor Tokens to TokenList, makes it functional component

b7dc899

also fix blacklisted addresses calculation

Refactor AddCustomToken, add types

3c23d77

Removes actions from Tokens

Fix warning on useEffect

6b9032a

mmv08 reviewed Sep 14, 2020

View reviewed changes

fernandomg approved these changes Sep 14, 2020

View reviewed changes

Merge remote-tracking branch 'origin/development' into 1312-fetch-assets

e0429fd

mmv08 merged commit 3301817 into development Sep 15, 2020

mmv08 deleted the 1312-fetch-assets branch September 15, 2020 11:12

mmv08 restored the 1312-fetch-assets branch September 15, 2020 11:12

mmv08 deleted the 1312-fetch-assets branch September 15, 2020 11:12

github-actions bot locked and limited conversation to collaborators Sep 15, 2020

Conversation

Agupane commented Sep 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Update

Update 14/09:

Uh oh!

github-actions bot commented Sep 8, 2020

Uh oh!

github-actions bot commented Sep 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ESLint Summary View Full Report

Uh oh!

ghost commented Sep 8, 2020

Uh oh!

ghost commented Sep 8, 2020

Uh oh!

Uh oh!

dasanra Sep 8, 2020

Choose a reason for hiding this comment

Uh oh!

Agupane Sep 9, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uxio0 commented Sep 10, 2020

Uh oh!

Uh oh!

ghost commented Sep 10, 2020

Uh oh!

mmv08 commented Sep 14, 2020

Uh oh!

Agupane commented Sep 14, 2020

Uh oh!

ghost commented Sep 14, 2020

Uh oh!

mmv08 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Agupane commented Sep 14, 2020

Uh oh!

ghost commented Sep 14, 2020

Uh oh!

mmv08 Sep 14, 2020

Choose a reason for hiding this comment

Uh oh!

fernandomg left a comment

Choose a reason for hiding this comment

Uh oh!

ghost commented Sep 15, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Agupane commented Sep 8, 2020 •

edited

Loading

github-actions bot commented Sep 8, 2020 •

edited

Loading