Skip to content
This repository has been archived by the owner on Jun 24, 2022. It is now read-only.

Fix walletconnect double qr code #2547

Closed
wants to merge 4 commits into from
Closed

Fix walletconnect double qr code #2547

wants to merge 4 commits into from

Conversation

nenadV91
Copy link
Contributor

Summary

Fixes #1787

This is new PR that has all of the implementation from #1945 and also additional fix for 1Inch wallet
Since that PR was created long time ago I am opening this one instead of having to rebase and merge.

@nenadV91 nenadV91 requested review from a team March 15, 2022 12:35
@github-actions
Copy link
Contributor

  • 🔭 GP Swap: CoW Protocol v2 Swap UI

anxolin
anxolin approved these changes Mar 15, 2022
View reviewed changes
Copy link
Contributor

@anxolin anxolin left a comment

Choose a reason for hiding this comment

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

I canot stress enough, great detective work Nenad! Your perseverance made it happen

I didn't test it in the app, I only reviewed the code, I give my approval but it should pass the hardest test of all, @elena-zh !

src/custom/components/WalletModal/WalletModalMod.tsx Outdated Show resolved Hide resolved
src/custom/hooks/web3.ts Outdated Show resolved Hide resolved
src/custom/hooks/web3.ts Outdated
localStorage.removeItem(STORAGE_KEY_LAST_PROVIDER)

if (error) {
throw error
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if the on function receives an error? u are bubling up, is this intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure where I was going with this one but I've changed it to just console log if there is an error otherwise execute rest of the that block code.

@elena-zh
Copy link

Hi @nenadV91 , great changes!
WC works fine, I no longer see double QR codes, etc.
As for 1Inch wallet, it works fine in desktop version. However, I have found issues when signing orders/transactions in Mobile app: instead of sending request to sign a Tx, the app navigates a user to install the app.
See the video:

mobile.1inch.mp4

Could you please take a look at it?
Thanks

@elena-zh
Copy link

Some updates: I'm able to sign an order in the mobile app when connected to 1Inch wallet. But again, the app opens 'Connect to 1Inch' wallet page when get back to the Cowswap app after an order signing.

Still not able to sign wrap/unwrap/approve token transactions (behavior is the same as on the video above)

@nenadV91 nenadV91 mentioned this pull request Apr 4, 2022
Closed
@elena-zh elena-zh mentioned this pull request Apr 25, 2022
Merged
@stale
Copy link

stale bot commented Jun 12, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you think it shouldn't be closed, speak now or forever hold your peace.

@stale stale bot added the wontfix Stale issue label Jun 12, 2022
@stale stale bot closed this Jun 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@anxolin anxolin anxolin approved these changes

Assignees
No one assigned
Labels
wontfix Stale issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[autoconnect] - wallet is not correctly connected and QR code remains
3 participants
@nenadV91 @elena-zh @anxolin