Epic: EIP-3770 prefixes #3015

DiogoSoaress · 2021-11-16T22:29:23Z

What it solves

Resolves #2842

How this PR fixes it

This allows the optional EIP-3770 chain prefixes across the application and to accept the chain prefixes in the Input components.

How to test it

Refer to #2896 and #2954 to test this PR

* Create hook to return the prefixed address as per settings * Apply appearence settings on Sidebar header * Bump @gnosis.pm/safe-react-components to use latest EthHashInfo * Add EthHashInfo wrapper component for shortName settings * Use PrefixedEthHashInfo component in the Appearance settings page * Replace EthHashInfo for the wrapper in the Safe owner management options * Replace EthHashInfo for the wrapper in the Balances modal's send screens * Replace EthHashInfo for its wrapper in the Advanced Settings and Spending Limit options * Replace EthHashInfo for its wrapper PrefixedEthHashInfo * Remove unnecessarily introduced hook * Implement InlineEthHashInfo from the EthHashInfo wrapper * Use lib component props and avoid duplication * Add test for PrefixedEthHashInfo component * Fix "<p> cannot appear as a descendant of <p>" warning * Unset font-weight in PrefixedEthHashInfo when it is inline. * Rename PrefixedEthHashInfo inline styled component in TxList * Do not display chain shortName in AddressBook addresses * Use PrefixedEthHashInfo displaying addresses for the sake of consistency * Pass chainName prop to the EthHashInfo in the SafeListSideBar * Remove temporarly condition to not toggle prefixed addresses in PROD

* Added network prefix to the AddressInput Component * Added network prefix to BaseAddressBookInput * reduced the complexity in AddressInput component * Added AddressInput unit tests * Added a extra test in AddressInput component * Added AddressBookInput unit tests * Added unit tests to getNetworkPrefix and getAddressWithoutNetworkPrefix utils fn * Minor changes in comments th the AddressInput unit tests * Addresses without a network prefix are set by default to the current network * added improvements in AddressInput and validators * Added Unsupported network prefix error in the checkNetworkPrefix validator Co-authored-by: Diogo Soares <diogo.soares@gnosis.pm>

github-actions · 2021-11-16T22:29:39Z

CLA Assistant Lite All Contributors have signed the CLA.

github-actions · 2021-11-16T22:33:21Z

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	7	N/A

Result: ✅ success
Annotations: 0 total

^{Report generated by eslint-plus-action}

github-actions · 2021-11-16T22:37:59Z

Safe Rinkeby
Safe Mainnet
Safe Polygon
Safe BSC
Safe Arbitrum
Unified app on dev: https://safe-team.dev.gnosisdev.com/app/

coveralls · 2021-11-16T22:39:11Z

Pull Request Test Coverage Report for Build 1526115087

51 of 91 (56.04%) changed or added relevant lines in 20 files are covered.
7 unchanged lines in 6 files lost coverage.
Overall coverage increased (+0.7%) to 32.758%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/components/CustomIconText/index.tsx	0	1	0.0%
src/components/forms/validator.ts	10	11	90.91%
src/logic/addressBook/store/reducer/index.ts	0	1	0.0%
src/routes/safe/components/Balances/SendModal/screens/SendFunds/index.tsx	0	1	0.0%
src/routes/safe/components/Transactions/TxList/MethodValue.tsx	0	1	0.0%
src/routes/safe/components/Transactions/TxList/TxSummary.tsx	0	1	0.0%
src/components/forms/AddressInput/index.tsx	18	20	90.0%
src/routes/safe/components/Settings/Appearance/index.tsx	0	2	0.0%
src/components/App/ReceiveModal.tsx	0	4	0.0%
src/components/AppLayout/Sidebar/SafeHeader/index.tsx	0	4	0.0%

Files with Coverage Reduction	New Missed Lines	%
src/components/App/ReceiveModal.tsx	1	0%
src/components/forms/validator.ts	1	88.2%
src/routes/safe/components/Balances/SendModal/screens/SendCollectible/index.tsx	1	0%
src/routes/safe/components/Balances/SendModal/screens/SendFunds/index.tsx	1	0%
src/routes/safe/components/Settings/ManageOwners/ReplaceOwnerModal/screens/OwnerForm/index.tsx	1	0%
src/utils/checksumAddress.ts	2	73.33%

Totals
Change from base Build 1525475321:	0.7%
Covered Lines:	3106
Relevant Lines:	8458

💛 - Coveralls

github-actions · 2021-11-16T22:45:14Z

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

Failed tests:

❌ Address book Address book
❌ Safe Apps List Safe Apps List
❌ Read-only transaction creation and review Create and review a Send Funds transaction

katspaugh · 2021-11-17T09:03:56Z

Please check the e2e tests @DiogoSoaress

francovenica · 2021-11-22T07:27:40Z

I checked in the same places as I listed in this comment:
#2896 (comment)

Checked with and without prefix enabled
Checked the copy button with and without prefix
I checked that the network introduced is validated (xdai: will be considered invalid in rinkeby for example)
Checked the QR code with/without prefix for safe

Issue:
The QR code is not being treated properly when it has the prefix.
It seems that is the the QR code has the prefix and when is pasted the system is adding the prefix again

Steps:
-Load a safe
-In the sidebar open the QR code modal
-Check the checkbox of "Copy address with prefix", so the QR changes to that
-Use a screenshot tool to copy that QR code and make it a PNG that you can pick from your folder explorer (I'll attach the PNG of a rinkeby safe I have, you can copy this one if you don't have a screenshot tool, the address should be rin:0x9913B9180C20C6b0F21B6480c84422F6ebc4B808)

-Open "Send collectibles" modal
-Use the QR code option to pick the PNG of the safe

Expected:
The QR code is converted to the safe address (with or without the prefix, depending on the Appearance setting current configuration)

Result:
The prefix is added twice

DiogoSoaress · 2021-11-23T14:05:50Z

Regarding the failing E2E tests

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

Failed tests:

❌ Add an existing safe Add an existing safe

❌ Address book Address book

❌ Safe Apps List Safe Apps List

Problem:
When we are developing a new feature in safe-react, we are running the E2E tests against the PR and the E2E tests will fail if the new feature impacts the existing tests.

The issue is that I can't update/fix the E2E tests before the safe-react feature is merged into dev.

What I propose as solution is: we first QA and merge the safe-react branch into dev. Then the following task will be update the E2E tests. Is this valid? @katspaugh @DaniSomoza

* Strip network prefix from the Address when calling EthHashInfo * Do not use chain prefix for TxHash and SafeTxHash in the Tx List

github-actions · 2021-11-24T10:48:02Z

Deployment links

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

* Strip the scanned address from the chain prefix * Replace duplicated prefix removal by util method

This reverts commit d420be4.

* Revert prefixed input (#2954) * Add parsePrefixedAddress and isValidPrefix * Add tests * Restore Dani's tests * Display prefix but pass unprefixed address * Accept initial input thru a hidden input * Remove magic again

JagoFigueroa · 2021-11-26T13:56:10Z

Hi all! Quick comment from my end as I have been checking this branch to do some testing on the apps side.

I've noticed that the styling for the 2 fields in 'add custom app' pop up is completely gone

Please let me know if you would need me to open a ticket about this. Cheers!

…770-prefixes

katspaugh · 2021-11-29T11:22:26Z

@francovenica I've fixed the QR code validation in Send Collectible.

francovenica · 2021-11-29T11:58:10Z

Yes, the validation for the Send collectibles form is working, but now has the same issue that Send funds has: The error is shown, but the input is empty:

katspaugh · 2021-11-29T12:22:32Z

I know, it's the same behavior this input has always had when the QR code is an invalid address.

We should fix it by significantly refactoring the inputs which I don't think we should do in this PR.

…770-prefixes

github-actions · 2021-12-01T13:52:15Z

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	7	N/A

Result: ✅ success
Annotations: 0 total

^{Report generated by eslint-plus-action}

…770-prefixes

DiogoSoaress and others added 2 commits November 16, 2021 09:41

DiogoSoaress mentioned this pull request Nov 17, 2021

[Epic] Show a chain prefix everywhere where we display an address #2842

Closed

DiogoSoaress mentioned this pull request Nov 22, 2021

Strip network prefix from the Address when calling EthHashInfo #3037

Merged

getWeb3ReadOnly to update web3 when user switch the network (#3034)

38ab408

Strip network prefix from the Address when calling EthHashInfo (#3037)

6809c9d

* Strip network prefix from the Address when calling EthHashInfo * Do not use chain prefix for TxHash and SafeTxHash in the Tx List

katspaugh changed the title ~~Feature/eip 3770 prefixes~~ Epic: EIP-3770 prefixes Nov 24, 2021

Solve conflicts; fix tests

b0c2ab9

DiogoSoaress and others added 6 commits November 24, 2021 11:50

Strip the scanned address from the chain prefix (#3053)

d420be4

* Strip the scanned address from the chain prefix * Replace duplicated prefix removal by util method

Fix: increase input sizes to fit a prefix

fc0eeaa

Revert "Strip the scanned address from the chain prefix (#3053)"

9965030

This reverts commit d420be4.

Improve QR code checkbox copy

cc6f3a8

EIP-3770: Fix inputs playing nicely with QR codes (#3060)

9d4e7a1

* Revert prefixed input (#2954) * Add parsePrefixedAddress and isValidPrefix * Add tests * Restore Dani's tests * Display prefix but pass unprefixed address * Accept initial input thru a hidden input * Remove magic again

Fix: remove memoize on mustBeEthereumAddress (#3065)

ef54db8

katspaugh added 3 commits November 29, 2021 08:57

Merge branch 'dev' of github.com:gnosis/safe-react into feature/eip-3…

89e0489

…770-prefixes

Merge with dev

b894d7b

QR code validation in Send Collectible

01d9ed7

DiogoSoaress and others added 2 commits November 29, 2021 18:03

Bump safe-react-components minor version (#3076)

26265c0

Merge branch 'dev' of github.com:gnosis/safe-react into feature/eip-3…

857d50a

…770-prefixes

katspaugh added 3 commits December 1, 2021 15:01

Fix spending limit qr code

6d34d26

Merge branch 'dev' of github.com:gnosis/safe-react into feature/eip-3…

75b882e

…770-prefixes

Rm console.log

bd74c66

katspaugh merged commit f266e0c into dev Dec 1, 2021

katspaugh deleted the feature/eip-3770-prefixes branch December 1, 2021 15:14

github-actions bot locked and limited conversation to collaborators Dec 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Epic: EIP-3770 prefixes #3015

Epic: EIP-3770 prefixes #3015

DiogoSoaress commented Nov 16, 2021 •

edited by katspaugh

Loading

github-actions bot commented Nov 16, 2021

github-actions bot commented Nov 16, 2021 •

edited

Loading

github-actions bot commented Nov 16, 2021

coveralls commented Nov 16, 2021 •

edited

Loading

github-actions bot commented Nov 16, 2021 •

edited

Loading

katspaugh commented Nov 17, 2021

francovenica commented Nov 22, 2021 •

edited

Loading

DiogoSoaress commented Nov 23, 2021

github-actions bot commented Nov 24, 2021

JagoFigueroa commented Nov 26, 2021

katspaugh commented Nov 29, 2021

francovenica commented Nov 29, 2021

katspaugh commented Nov 29, 2021

github-actions bot commented Dec 1, 2021 •

edited

Loading

Epic: EIP-3770 prefixes #3015

Epic: EIP-3770 prefixes #3015

Conversation

DiogoSoaress commented Nov 16, 2021 • edited by katspaugh Loading

What it solves

How this PR fixes it

How to test it

github-actions bot commented Nov 16, 2021

github-actions bot commented Nov 16, 2021 • edited Loading

ESLint Summary View Full Report

github-actions bot commented Nov 16, 2021

coveralls commented Nov 16, 2021 • edited Loading

Pull Request Test Coverage Report for Build 1526115087

💛 - Coveralls

github-actions bot commented Nov 16, 2021 • edited Loading

katspaugh commented Nov 17, 2021

francovenica commented Nov 22, 2021 • edited Loading

DiogoSoaress commented Nov 23, 2021

github-actions bot commented Nov 24, 2021

Deployment links

JagoFigueroa commented Nov 26, 2021

katspaugh commented Nov 29, 2021

francovenica commented Nov 29, 2021

katspaugh commented Nov 29, 2021

github-actions bot commented Dec 1, 2021 • edited Loading

ESLint Summary View Full Report

DiogoSoaress commented Nov 16, 2021 •

edited by katspaugh

Loading

github-actions bot commented Nov 16, 2021 •

edited

Loading

coveralls commented Nov 16, 2021 •

edited

Loading

github-actions bot commented Nov 16, 2021 •

edited

Loading

francovenica commented Nov 22, 2021 •

edited

Loading

github-actions bot commented Dec 1, 2021 •

edited

Loading