umbra-protocol icon indicating copy to clipboard operation
umbra-protocol copied to clipboard

Add ERC20 and ERC721 warnings for withdrawl

Open alexkeating opened this issue 3 years ago • 4 comments

Issue

#310

Description

Added a check for an ERC721 and ERC20

Screencast from 10-02-2022 11:58:42 PM.webm

alexkeating avatar Oct 03 '22 04:10 alexkeating

Deploy request for jolly-shaw-20fe62 pending review.

Visit the deploys page to approve it

Name Link
Latest commit 7624c80e2f2bbe1335a914a52eb6b42ae7d55b08

netlify[bot] avatar Oct 03 '22 04:10 netlify[bot]

Removed distinction

alexkeating avatar Oct 10 '22 19:10 alexkeating

That's a good point, all other checks are privacy related whereas this would be lost funds. FWIW this is how we spec'd it out in #310, so my suggestion would be to merge as-is since it's better than nothing like we currently have, then we can open a separate issue/PR to make this more prominent

mds1 avatar Oct 12 '22 18:10 mds1

This should be good, I also need to add the joi fix after rebaseing

alexkeating avatar Oct 17 '22 15:10 alexkeating

Hey @alexkeating, sorry for one more nit, but here's something I noticed in testing see_no_evil

If the user clicks the checkbox and does the approval, the modal does not dismiss after the withdrawal. The warning modal, by comparison, dismisses before showing the subsequent fee modal.

Good catch pushed up a fix

alexkeating avatar Oct 21 '22 19:10 alexkeating