[MEDIUM-01] - Use safeTransfer and safeTransferFrom instead of transfer and transferFrom

[ethereumdegen]

The codebase transfers tokens in multiple parts of the code. In some of them, SafeERC20 library is used. However, other parts of the code directly perform unsafe ERC20 transfers and approvals. Concretely, the contracts that incorrectly transfer tokens are:

CollateralManager
TellerV2 (only in _sendOrEscrowFunds)
Recommendation
It is recommended to use SafeERC20 library consistently to ensure token transfers ( via safeTransfer/safeTransferFrom) and approvals (via forceApprove) are correctly handled.

[ethereumdegen]

also i have a technical issue with adding safeTransferFrom to the sendOrEscrow funds :(

function _sendOrEscrowFunds(uint256 _bidId, Payment memory _payment)
internal
{
Bid storage bid = bids[_bidId];
address lender = getLoanLender(_bidId);

    uint256 _paymentAmount = _payment.principal + _payment.interest;

    try 

        bid.loanDetails.lendingToken.transferFrom{ gas: 100000 }(
            _msgSenderForMarket(bid.marketplaceId),
            lender,
            _paymentAmount
        )
        
    {} catch {

If i change that to safeTransferFrom then solidity gives me an error saying

Function call options can only be set on external function calls or contract creations.

so just FYI i am stuck on this and working thru it

[ethereumdegen]

partial fix in #64

[ethereumdegen]

0xadrii:

Regarding this, safeTransfer is still needed as for example the transaction won’t actually revert for tokens like USDT, so it will never enter the catch statement. Maybe another option is to trigger a low-level call, and handle the success return value. The only problem with this is that the callback can returnbomb, but this pattern fixes that: https://github.com/nomad-xyz/ExcessivelySafeCall

Then, if there’s return data, something like this should be done:    https://github.com/transmissions11/solmate/blob/c93f7716c9909175d45f6ef80a34a650e2d24e56/src/utils/SafeTransferLib.sol#L53 . As a summary, if there’s return data we check it (for example, if USDT fails it will return some data with a false). 

In the end, instead of requiring success, you can trigger the flow that is currently in the catch logic

[ethereumdegen]

fully fixed by #64 now, approved by 0xAdri

There's a small thing that needs to be changed. dataIsSuccess needs to be initialized as true, otherwise if the token does not return any data (most of tokens), even if the call succeeded, callSuccess && dataIsSuccess will be false.

In addition, I would change the namecallData to returnData as it's actually more approprate for this scenario, but overall the fix logic looks good