-
Notifications
You must be signed in to change notification settings - Fork 97
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: minimum_balance for placing orders set to 1 atom for FOK #2558
feat: minimum_balance for placing orders set to 1 atom for FOK #2558
Conversation
d8f12b7
to
8cf01f7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An e2e test would be nice:
- create order where user only has 1 wei of the sell token
- fund the user account
- await order getting settled
But I can also add that to the PR if you like.
That's way beyond my current capacity and patience levels
Pretty please? 🥺 |
69608d2
to
5ed9d35
Compare
Co-authored-by: Martin Beckmann <martin.beckmann@protonmail.com>
5ed9d35
to
6badecb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG
Description
The frontend wants to enable placement of LIMIT orders without requiring the full balance.
It can be tested here cowprotocol/cowswap#3742
It is already possible for Partially fillable orders due to the partial fill nature, but Fill or kill orders still require the full balance.
The arguments for the feature is:
The argument to remove the Fill or kill limitation is:
Due to spam protection, 1 atom of the sell token balance is still required, and that is not being removed.
Changes
minimum_balance
function into a constant of value1
.How to test
This change relies on existing unit tests
Notes
Please do let me know where best to put the constant, whether there is need to add any specific unit test and so on