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

No 'Insufficient allowance' warning for permittable tokens in Safe #4076

Closed
elena-zh opened this issue Mar 21, 2024 · 2 comments · Fixed by #5207
Closed

No 'Insufficient allowance' warning for permittable tokens in Safe #4076

elena-zh opened this issue Mar 21, 2024 · 2 comments · Fixed by #5207
Assignees
Labels
app:CowSwap CowSwap app Bug Something isn't working Medium Severity indicator. It causes some undesirable behavior, but the system is still functional Regression

Comments

@elena-zh
Copy link

elena-zh commented Mar 21, 2024

Currently, permittable tokens are not supported in SC wallets. So every time when allowance becomes 0, and a limit order is created with this token as a Sell token, we need to show 'Insuffisient allowance' warning for these orders.

To reproduce:

  1. Open the app inside the Safe wallet
  2. Select a sell token with no allowance to sell
  3. Specify allowance amount < than the order amount
  4. Place a partially fillable order.
  5. Wait till the order is partially filled (all approved amount is executed)
  6. Check the order

AR: status says 'Open'
image

ER: it should say 'Unfillable' and show the 'Insufficient allowance' warning (when approved amount is <0.05%)
image

@elena-zh elena-zh added app:CowSwap CowSwap app Bug Something isn't working Low Severity indicator for defects. It won't cause any major break-down of the system Regression labels Mar 21, 2024
@anxolin anxolin added Medium Severity indicator. It causes some undesirable behavior, but the system is still functional and removed Low Severity indicator for defects. It won't cause any major break-down of the system labels Nov 25, 2024
@anxolin
Copy link
Contributor

anxolin commented Nov 25, 2024

@elena-zh @shoom3301 any clue why this is related to the Safe? Why do you say this is related to permitable tokens? I mean, we always do the allowance check, the only difference for permit orders is that if there's a permit as a pre-hook then we can count the order as OPEN, but here is not the case.

I think this one we should fix cause makes people think the protocol is not working

@elena-zh
Copy link
Author

@elena-zh @shoom3301 any clue why this is related to the Safe? Why do you say this is related to permitable tokens? I mean, we always do the allowance check, the only difference for permit orders is that if there's a permit as a pre-hook then we can count the order as OPEN, but here is not the case.

I think this one we should fix cause makes people think the protocol is not working

I say that it is related to permittable tokens because the issue is not reproducible with regular tokens. IDK why this happens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app:CowSwap CowSwap app Bug Something isn't working Medium Severity indicator. It causes some undesirable behavior, but the system is still functional Regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants