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

fix: filter invalid addresses in imports #3810

Merged
merged 2 commits into from
Apr 25, 2022
Merged

fix: filter invalid addresses in imports #3810

merged 2 commits into from
Apr 25, 2022

Conversation

iamacook
Copy link
Member

What it solves

Resolves #3690

How this PR fixes it

bathLoadEntries used when loading a Safe or importing and address book now filters invalid addresses from address books. The error was being thrown from the blockie generator plugin.

How to test it

Import the corrupt csv from the issue and observe that only one entry exists in the Safe address book.

@github-actions
Copy link

CLA Assistant Lite All Contributors have signed the CLA.

@@ -7,12 +7,12 @@ describe('Test AddressBook BatchLoadEntries Reducer', () => {
it('returns an addressbook array', () => {
const addressBookEntries = [
{
address: '0x4462527986c3fD47f498eF25B4D01e6AAD7aBcb2',
address: '0x5fb582FD320ab1CBf055F65ED74D01b9DdB90A00',
Copy link
Member Author

Choose a reason for hiding this comment

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

All addresses were updated in this test file as they were invalid addresses.

Copy link
Member

Choose a reason for hiding this comment

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

Should only checksummed addresses be imported? If so, should we checksum them before saving an entry so that when they are exported, they can be imported again?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea!

@github-actions
Copy link

github-actions bot commented Apr 25, 2022

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

Report generated by eslint-plus-action

@coveralls
Copy link

coveralls commented Apr 25, 2022

Pull Request Test Coverage Report for Build 2219985031

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • 15 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.09%) to 35.665%

Files with Coverage Reduction New Missed Lines %
src/logic/tokens/utils/formatAmount.ts 1 96.23%
src/components/App/index.tsx 4 0%
src/routes/safe/components/Balances/dataFetcher.ts 10 0%
Totals Coverage Status
Change from base Build 2219676897: 0.09%
Covered Lines: 3545
Relevant Lines: 9004

💛 - Coveralls

@github-actions
Copy link

Deployment links

🟠 Rinkeby Mainnet 🟣 Polygon 🟡 BSC Arbitrum 🟢 Gnosis Chain

@katspaugh
Copy link
Member

The error was being thrown from the blockie generator plugin.

Oh. 🤬

Copy link
Member

@usame-algan usame-algan left a comment

Choose a reason for hiding this comment

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

Very nice, thanks for adding tests! 👍

@francovenica
Copy link
Contributor

Looks good. I used the csv that Lili provided. It has 2 addresses, one valid and one invalid (2 characters were changed at the end).

Uploading that file only imports the valid address.
The address is checksummed
The invalid address is not being saved in the local storage either (what it was making the app crash)

QA approved

@iamacook iamacook merged commit c7121a4 into dev Apr 25, 2022
@iamacook iamacook deleted the validate-csv-imports branch April 25, 2022 15:13
@github-actions github-actions bot locked and limited conversation to collaborators Apr 25, 2022
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.

The app is crashed on addressbook opening if the imported csv file contains not existed address
5 participants