Summary

Use safeTransfer()/safeTransferFrom() instead of transfer()/transferFrom()

Vulnerability Details

It is a good idea to add a require() statement that checks the return value of ERC20 token transfers or to use something like OpenZeppelin’s safeTransfer()/safeTransferFrom() unless one is sure the given token reverts in case of a failure. Failure to do so will cause silent failures of transfers and affect token accounting in contract.

However, using require() to check transfer return values could lead to issues with non-compliant ERC20 tokens which do not return a boolean value. Therefore, it's highly advised to use OpenZeppelin’s safeTransfer()/safeTransferFrom().

Impact

This can make some successful token transfer got reverted because of not returning true

Tools Used

Manual review

Recommendations

Consider using safeTransfer()/safeTransferFrom() instead of transfer()/transferFrom().