Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

Remove transaction service usage #3189

Merged
merged 9 commits into from
Dec 23, 2021
Merged

Remove transaction service usage #3189

merged 9 commits into from
Dec 23, 2021

Conversation

iamacook
Copy link
Member

@iamacook iamacook commented Dec 16, 2021

What it solves

Resolves #3114

How this PR fixes it

All reference to the /tokens endpoint has been removed: fetchTokens() and all related code was deleted as it was seemingly not used at all.

getDataDecoderUrl()/getMasterCopiesUrl() were modified to rely on the CGW.

How to test it

Master copies

  1. Open settings.
  2. Check that no error is thrown and that the Safe version is correct in the top left.

Data decoder: create a hex encoded transaction via the Transaction Builder.

  1. Create a transaction via the Transaction Builder with hex encoded data.
  2. Send the transaction and check that the data decodes correctly.

@iamacook iamacook marked this pull request as draft December 16, 2021 17:05
@github-actions
Copy link

CLA Assistant Lite All Contributors have signed the CLA.

@iamacook iamacook mentioned this pull request Dec 16, 2021
3 tasks
@github-actions
Copy link

github-actions bot commented Dec 16, 2021

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 2 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

@github-actions
Copy link

Deployment links

🟠 Safe Rinkeby Safe Mainnet 🟣 Safe Polygon 🟡 Safe BSC Safe Arbitrum 🟢 Safe xDai

@coveralls
Copy link

coveralls commented Dec 16, 2021

Pull Request Test Coverage Report for Build 1615680193

  • 3 of 7 (42.86%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.02%) to 32.21%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/utils/decodeTx.ts 2 3 66.67%
src/logic/contracts/api/masterCopies.ts 0 3 0.0%
Files with Coverage Reduction New Missed Lines %
src/config/index.ts 1 60.2%
Totals Coverage Status
Change from base Build 1614899864: 0.02%
Covered Lines: 3075
Relevant Lines: 8479

💛 - Coveralls

@github-actions
Copy link

github-actions bot commented Dec 16, 2021

E2E Tests Failed
Check the results here: https://github.com/gnosis/safe-react-e2e-tests/actions/runs/1615708700

Failed tests:

  • ❌ Safe Apps List Safe Apps List
  • ❌ Read-only transaction creation and review Read-only transaction creation and review

@iamacook
Copy link
Member Author

@DiogoSoaress, having this finished is integral to starting #2533. Feel free to take on #2533. getMasterCopiesUrl and the associated retrieval of master copies and getDataDecoderUrl() and the associated interaction need to be migrated to the SDK.

@iamacook iamacook marked this pull request as ready for review December 16, 2021 17:45
Copy link
Member

@DiogoSoaress DiogoSoaress left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good!

Good job leaving the comments in the txService leftovers

2 small things that you didn't touch but we could clean up:

  • When changing safe and we enter the reducer ADD_TOKENS we are not cleaning tokens that the current safe does not have. I.e, the reducer is keeping leftover tokens

  • The method containsMethodByHash in src/logic/tokens/store/actions/fetchTokens.ts is not being used anywhere and we can delete it

yarn.lock Outdated Show resolved Hide resolved
Copy link
Member

@DiogoSoaress DiogoSoaress left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@katspaugh katspaugh merged commit 5523831 into dev Dec 23, 2021
@katspaugh katspaugh deleted the remove-tx-service-calls branch December 23, 2021 14:05
@github-actions github-actions bot locked and limited conversation to collaborators Dec 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove Backend tx-service calls
4 participants