-
Notifications
You must be signed in to change notification settings - Fork 457
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat: native WalletConnect #2535
Conversation
Branch preview✅ Deploy successful! |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
8273d8e
to
99fb684
Compare
* Feat: add extra tracking events * Fix tests * Fix: debounce input error tracking
* Refactor: on close callback for WalletConnect * Use safeAddress + chainId instead of raw query * Close modal on path change
ESLint Summary View Full Report
Report generated by eslint-plus-action |
This ticket will serve as the RC, so issues found during regression will be listed here: 1 - When signing a tx, the alert that " you will leave the tx form " shows. Even tho signing will take you out of the tx form as expected is also expected that the alert doesn't show up: Queue a tx |
Fixed, @francovenica! ✅ |
bb76267
to
a011fe4
Compare
The issues with the alert were fixed I tried again 1inch, the issue I had with the tx not popping up and now it does pop up. The regression went fine: Also tested "Safe as owner" with the new feature. Works fine I tried a few more dapps (cowswap, Yearn, balance) with a Eth safe 1/x with with those work fine Still have the issue of some dapps remaining in a pending status after the tx was fully executed in the safe. I still not sure if those are from the dapps side or ours. I think is impossible to know if every dapp outhere will work fine or not, so let's make sure after the deployment that the feature toggle works fine. |
WalletConnect is now a first-class citizen in the app.
?wc=
query param in the URLSpecs: https://www.notion.so/safe-global/WalletConnect-as-first-class-citizen-c8578e20f9ac464e9c6f8cb3a973193b?pvs=4