-
Notifications
You must be signed in to change notification settings - Fork 11
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
fix(transfer): allow zero transfers #329
Conversation
Rubilmax
commented
Oct 25, 2023
- Fixes Transfer reverts on zero amount #297
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.
Should we add a comments since it's a bit different from the standard that we use in other bundlers
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.
The problem is that an erc20 can revert on 0 transfer. We have a specific usecase to transfer(0), for slippage undertaken amount (if we reach the slippage tolerance). So I would suggest to have an early return and to not call the safeTransfer is 0, or to add a skim function that is returning on 0 amount.
Maybe we should add a |
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.
IMO, we have to reference somewhere that the 0 amount is not calling the transfer (and thus not triggering a Transfer event). This is a specific behavior different from the one defined by the EIP20, where the end user can expect to have a Transfer event with a 0 amount
why merging it? |
…nsfer"" This reverts commit 1aa7ed8.
Revert "Merge pull request #329 from morpho-labs/fix/zero-transfer"
…nsfer"" This reverts commit 1aa7ed8.