feat: Auto approve verified WC sessions #3455

usame-algan · 2024-03-19T10:59:42Z

What it solves

Resolves #3395

How this PR fixes it

Stores approvals for verified dApps per chain and auto approves future sessions

How to test it

Open a Safe
Open a verified dApp (e.g. CowSwap)
Connect CowSwap to your Safe
A green message should be visible on the Approve screen
Press Approve
Disconnect the session and connect to CowSwap again
Observe no Approve screen and automatically connecting
Switch to a different chain and connect with CowSwap again
Observe the session is not auto approved
Oberve that the dialog closes on approval

Screenshots

Screen.Recording.2024-03-19.at.11.58.10.mov

Checklist

I've tested the branch on mobile 📱
I've documented how it affects the analytics (if at all) 📊
I've written a unit/e2e test for it (if applicable) 🧑‍💻

github-actions · 2024-03-19T10:59:57Z

Branch preview

✅ Deploy successful!

https://wc_auto_approve--walletweb.review-wallet-web.5afe.dev/home?safe=eth:0xA77DE01e157f9f57C7c4A326eeE9C4874D0598b6

usame-algan · 2024-03-19T11:00:29Z

Two more ideas:

We could also auto approve for unverified domains or display a checkbox on the approval screen to "Skip this step next time"
We could close the WC dialog on successful approvals

github-actions · 2024-03-19T11:00:43Z

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}

github-actions · 2024-03-19T11:03:28Z

📦 Next.js Bundle Analysis for safe-wallet-web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

🎉 Global Bundle Size Decreased

Page	Size (compressed)
`global`	`994.24 KB` (-1 B)

Details

The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!

github-actions · 2024-03-19T11:06:56Z

Coverage report

St.^❔	Category	Percentage	Covered / Total
🟡	Statements	79.38% (-0.02% 🔻)	11124/14013
🔴	Branches	58.44% (-0.06% 🔻)	2613/4471
🟡	Functions	66.09% (+0.11% 🔼)	1795/2716
🟢	Lines	80.64% (-0.07% 🔻)	10020/12425

Show new covered files 🐣

St.^❔	File	Statements	Branches	Functions	Lines
🟡	`...` / index.tsx	73.08%	62.96%	53.33%	73.74%
🟢	`...` / index.tsx	100%	100%	100%	100%

Show files with reduced coverage 🔻

St.^❔	File	Statements	Branches	Functions	Lines
🟡	`...` / sdk.ts	69.05% (-7.14% 🔻)	35.29% (-9.15% 🔻)	72.73% (-2.27% 🔻)	68% (-9.03% 🔻)
🟡	`...` / wallets.ts	62.86% (-3.81% 🔻)	25%	75% (-2.78% 🔻)	65.52% (-3.23% 🔻)
🔴	`...` / useOnboard.ts	50.38% (-0.4% 🔻)	23.81%	15%	52.99% (-0.52% 🔻)
🟢	`...` / index.tsx	82.35% (-8.82% 🔻)	66.67% (-16.67% 🔻)	50% (-12.5% 🔻)	83.87% (-9.68% 🔻)
🟢	`...` / WalletConnectWallet.ts	90.48% (-0.22% 🔻)	90.48% (-1.19% 🔻)	84%	92.5% (-0.18% 🔻)
🔴	`...` / GlobalPushNotifications.tsx	61.76% (-0.44% 🔻)	38.18% (-0.39% 🔻)	57.14% (-1.59% 🔻)	58.56% (-0.68% 🔻)
🟡	`...` / SocialWalletService.ts	60% (-9.47% 🔻)	50% (-16.67% 🔻)	46.15% (-7.18% 🔻)	61.11% (-9.22% 🔻)
🟢	`...` / index.tsx	87.76% (-0.24% 🔻)	73.33%	100%	90.91% (-0.2% 🔻)

Test suite run success

1412 tests passing in 195 suites.

Report generated by 🧪jest coverage report action from 4217765

katspaugh

👍

katspaugh · 2024-03-19T16:15:02Z

src/features/walletconnect/components/WcProposalForm/index.tsx

@@ -162,7 +141,7 @@ const WcProposalForm = ({ proposal, setProposal }: ProposalFormProps): ReactElem
          {isLoading === WCLoadingState.REJECT ? <CircularProgress size={20} /> : 'Reject'}
        </Button>

-        <Button variant="contained" onClick={onApprove} className={css.button} disabled={disabled}>
+        <Button variant="contained" onClick={() => onApprove()} className={css.button} disabled={disabled}>


Suggested change

<Button variant="contained" onClick={() => onApprove()} className={css.button} disabled={disabled}>

<Button variant="contained" onClick={onApprove} className={css.button} disabled={disabled}>

onApprove takes an optional parameter and in this case we don't want to pass it but onClick passes an event object so we have to wrap the onApprove call.

github-actions · 2024-03-21T08:33:19Z

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	1	0
Ignored	0	N/A

Result: ✅ success
Annotations: 1 total

[warning] react-hooks/exhaustive-deps

verifies the list of dependencies for Hooks like useEffect and similar

src/features/walletconnect/components/WcSessionMananger/index.tsx Line 67 - React Hook useCallback has a missing dependency: 'setOpen'. Either include it or remove the dependency array.

^{Report generated by eslint-plus-action}

francovenica · 2024-03-22T03:01:56Z

Looks good. It works fine but I still have a question on this

Question:
I noticed that CowSwap has the green squre with the checkmark which I assume means is a trusted one.
Which apps are trusted and which ones are not? I tried with Uniswap and it seems to not be a trusted one, since it doesn't have that green checkbox, but Uniswap I think is a very well known app

Another question is, should we tell the user that "trusted apps will autoconnect after the first approve"? Should we let the user wonder why this app connects automatically and that other app does not. I don't think that the green checkbox conveys that message clearly

Comparison between Cowswap and Uniswap

…or isScam

github-actions · 2024-03-25T10:58:27Z

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}

usame-algan · 2024-03-25T10:59:36Z

I noticed that CowSwap has the green squre with the checkmark which I assume means is a trusted one.
Which apps are trusted and which ones are not? I tried with Uniswap and it seems to not be a trusted one, since it doesn't have that green checkbox, but Uniswap I think is a very well known app

Another question is, should we tell the user that "trusted apps will autoconnect after the first approve"? Should we let the user wonder why this app connects automatically and that other app does not. I don't think that the green checkbox conveys that message clearly

I think a lot of legit dApps are not verified unfortunately. We discussed it this morning and decided to auto approve all dApps (verified and unverified). The only dApps we don't auto-approve are either scams or where there is a domain mismatch.

usame-algan requested a review from katspaugh March 19, 2024 10:59

Base automatically changed from wc-loading to dev March 19, 2024 12:58

usame-algan force-pushed the wc-auto-approve branch from 0c370ae to d5f41dc Compare March 19, 2024 13:08

feat: [WC] Auto approve sessions for verified dApps

e443fb9

usame-algan force-pushed the wc-auto-approve branch from d5f41dc to e443fb9 Compare March 19, 2024 13:46

katspaugh approved these changes Mar 19, 2024

View reviewed changes

katspaugh reviewed Mar 19, 2024

View reviewed changes

fix: Store approvals per chain, close dialog on successful approval

f95b5b7

usame-algan requested a review from katspaugh March 21, 2024 08:34

fix: Auto-approve for unknown dApps as well, add condition to check f…

4217765

…or isScam

usame-algan merged commit 9a743d1 into dev Mar 25, 2024
14 checks passed

usame-algan deleted the wc-auto-approve branch March 25, 2024 14:39

github-actions bot locked and limited conversation to collaborators Mar 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Auto approve verified WC sessions #3455

feat: Auto approve verified WC sessions #3455

usame-algan commented Mar 19, 2024 •

edited

github-actions bot commented Mar 19, 2024 •

edited

usame-algan commented Mar 19, 2024

github-actions bot commented Mar 19, 2024 •

edited

github-actions bot commented Mar 19, 2024 •

edited

github-actions bot commented Mar 19, 2024 •

edited

katspaugh left a comment

katspaugh Mar 19, 2024

usame-algan Mar 21, 2024

github-actions bot commented Mar 21, 2024

francovenica commented Mar 22, 2024

github-actions bot commented Mar 25, 2024

usame-algan commented Mar 25, 2024

	<Button variant="contained" onClick={() => onApprove()} className={css.button} disabled={disabled}>
	<Button variant="contained" onClick={onApprove} className={css.button} disabled={disabled}>

feat: Auto approve verified WC sessions #3455

feat: Auto approve verified WC sessions #3455

Conversation

usame-algan commented Mar 19, 2024 • edited

What it solves

How this PR fixes it

How to test it

Screenshots

Checklist

github-actions bot commented Mar 19, 2024 • edited

Branch preview

usame-algan commented Mar 19, 2024

github-actions bot commented Mar 19, 2024 • edited

ESLint Summary View Full Report

github-actions bot commented Mar 19, 2024 • edited

📦 Next.js Bundle Analysis for safe-wallet-web

🎉 Global Bundle Size Decreased

github-actions bot commented Mar 19, 2024 • edited

Coverage report

Test suite run success

katspaugh left a comment

Choose a reason for hiding this comment

katspaugh Mar 19, 2024

Choose a reason for hiding this comment

usame-algan Mar 21, 2024

Choose a reason for hiding this comment

github-actions bot commented Mar 21, 2024

ESLint Summary View Full Report

[warning] react-hooks/exhaustive-deps

francovenica commented Mar 22, 2024

github-actions bot commented Mar 25, 2024

ESLint Summary View Full Report

usame-algan commented Mar 25, 2024

usame-algan commented Mar 19, 2024 •

edited

github-actions bot commented Mar 19, 2024 •

edited

github-actions bot commented Mar 19, 2024 •

edited

github-actions bot commented Mar 19, 2024 •

edited

github-actions bot commented Mar 19, 2024 •

edited