Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(permit): improve permittable detection #3226

Merged
merged 9 commits into from
Oct 16, 2023
Merged

fix(permit): improve permittable detection #3226

merged 9 commits into from
Oct 16, 2023

Conversation

alfetopito
Copy link
Collaborator

@alfetopito alfetopito commented Oct 13, 2023 •
edited
Loading

Summary

Fixes #3221

WC network switching was not changing the provider network

To Test

  1. Delete the permittableTokens localStorage key if you have it
  2. Connect via WCv2 to Goerli or Gchain
  3. Filter the console logs by permit
  4. Switch between different tokens
  • On gchain none will be permittable and you'll see something like:
    [checkTokenIsPermittable] Not a permittable token 0x91056d4a53e1faa1a84306d4deaec71085394bc8 nonce not supported
  • On Goerli you should see the above only for unsupported. Supported should not print anything. You can check the permittableTokens localStorage key to see the token info
  1. Switch to mainnet and try different tokens
  • Same result as on Goerli. Permittable tokens will not log anything, only visible in the localStorage

@alfetopito alfetopito self-assigned this Oct 13, 2023
@vercel
Copy link

vercel bot commented Oct 13, 2023 •
edited by github-actions bot
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
swap-dev ✅ Ready (Inspect) Visit Preview Oct 16, 2023 10:48am

🌃 Cosmos ↗︎

@socket-security
Copy link

New, updated, and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
@walletconnect/ethereum-provider 2.10.2 None +11 15.5 MB gancho_walletconnect
@web3-react/empty 8.2.0...8.2.3 None +2/-2 381 kB towering-elsie
@web3-react/gnosis-safe 8.2.1...8.2.4 None +2/-2 393 kB towering-elsie
@web3-react/walletconnect-v2 8.3.7...8.5.1 None +14/-14 16 MB towering-elsie
@web3-react/url 8.2.0...8.2.3 None +2/-2 387 kB towering-elsie
@web3-react/eip1193 8.2.0...8.2.3 None +2/-2 388 kB towering-elsie
@web3-react/coinbase-wallet 8.2.0...8.2.3 None +2/-2 404 kB towering-elsie
@web3-react/network 8.2.0...8.2.3 None +2/-2 395 kB towering-elsie
@web3-react/types 8.2.0...8.2.3 None +1/-1 344 kB towering-elsie
@web3-react/metamask 8.2.1...8.2.4 None +2/-2 405 kB towering-elsie
@web3-react/core 8.2.0...8.2.3 None +3/-3 456 kB towering-elsie

🚮 Removed packages: @web3-react/[email protected]

@alfetopito alfetopito force-pushed the fix/3221_improve-permittable-detection branch from c33c5b2 to abe22f6 Compare October 13, 2023 18:18
alfetopito
alfetopito commented Oct 13, 2023
View reviewed changes
package.json
"@web3-react/network": "^8.2.0",
"@web3-react/types": "^8.2.0",
"@web3-react/url": "^8.2.0",
"@web3-react/walletconnect": "^8.2.0",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No need for this anymore, right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep

@github-actions
Copy link
Contributor

CLA Assistant Lite bot:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.

I have read the CLA Document and I hereby sign the CLA

1 out of 2 committers have signed the CLA.
✅ @shoom3301
❌ @alfetopito
You can retrigger this bot by commenting recheck in this Pull Request

alfetopito
alfetopito commented Oct 16, 2023
View reviewed changes
libs/wallet/src/web3-react/connection/asyncConnector.ts
Comment on lines +14 to +18

// Update provider when network is changed on wallet side
connector.provider?.on('chainChanged', (chainIdHex: string) => {
initCustomProvider(self, connector, +chainIdHex)
})
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the actual fix!

Thanks Sasha!

Everything else is not necessary but good to have as improvements.

@alfetopito alfetopito marked this pull request as ready for review October 16, 2023 10:46
@alfetopito alfetopito added the RELEASE Included in the release that is being closed label Oct 16, 2023
@alfetopito alfetopito merged commit 79cb48a into release/1.48.0 Oct 16, 2023
6 of 7 checks passed
@alfetopito alfetopito deleted the fix/3221_improve-permittable-detection branch October 16, 2023 10:51
@github-actions github-actions bot locked and limited conversation to collaborators Oct 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@shoom3301 shoom3301 shoom3301 approved these changes

Assignees

@alfetopito alfetopito

Labels
RELEASE Included in the release that is being closed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@alfetopito @shoom3301