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.

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(permit): disable permit for SC wallets #3213

Merged
merged 1 commit into from
Oct 12, 2023

Conversation

alfetopito
Copy link
Collaborator

Summary

Fixes #3207

Disable permit for SC wallets

To Test

  1. With a SC wallet, pick as sell a permittable token on Goerli or Mainnet
  2. Make sure there's no allowance for it
  3. Place a sell order
  • If it's a Safe wallet, you'll be able to bundle the approval in one go
  • Otherwise, you'll have to send an approval tx, then place the order
  1. Open the placed order in the explorer
  • It should not contain hook data in the appData field
    image
  1. Now with an EOA wallet, repeat the process (sell permittable token, no allowance etc, etc)
  • Permit flow should work as usual

@vercel
Copy link

vercel bot commented Oct 12, 2023

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

Name Status Preview Updated (UTC)
swap-dev ✅ Ready (Inspect) Visit Preview Oct 12, 2023 0:45am

🌃 Cosmos ↗︎

@alfetopito alfetopito added the RELEASE Included in the release that is being closed label Oct 12, 2023
@alfetopito alfetopito self-assigned this Oct 12, 2023
@alfetopito alfetopito requested a review from a team October 12, 2023 12:44
@alfetopito
Copy link
Collaborator Author

Optimistically merging, please post merge review

@alfetopito alfetopito merged commit 29e5178 into release/1.48.0 Oct 12, 2023
9 checks passed
@alfetopito alfetopito deleted the fix/3207_disable-sc-permit branch October 12, 2023 14:38
@github-actions github-actions bot locked and limited conversation to collaborators Oct 12, 2023
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.

post merge approved 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
RELEASE Included in the release that is being closed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants