In the contract NFTLoanFacilitator.sol there is a mix of using IERC721.safeTransferFrom() and IERC721.transferFrom(). The methods repayAndCloseLoan() and seizeCollateral() uses the safeTransferFrom(), where the methods createLoan() and closeLoan() uses the transferFrom().
For the method closeLoan() it should just be changed, since it transfers the collateral from the contract to another address just like the methods repayAndCloseLoan() and seizeCollateral(), which both uses the safeTransferFrom().
However, in order to change the method createLoan() to use the safeTransferFrom() it will require the contract to implement the IERC721Receive.onERC721Received() method.
Notice! Currently the method closeLoan() does not make use of the safeTransferFrom() and this means that if a user provided a contract address as the parameter sendCollateralTo and this contract does not implement proper handling of ERC721, then the token would be lost. But since I see this as very unlikely, I choose to consider this as a non-critical instead of a low.
QA report
Non-critical
Use safeTransferFrom for ERC721 consistency
In the contract
NFTLoanFacilitator.sol
there is a mix of usingIERC721.safeTransferFrom()
andIERC721.transferFrom()
. The methodsrepayAndCloseLoan()
andseizeCollateral()
uses thesafeTransferFrom()
, where the methodscreateLoan()
andcloseLoan()
uses thetransferFrom()
.For the method
closeLoan()
it should just be changed, since it transfers the collateral from the contract to another address just like the methodsrepayAndCloseLoan()
andseizeCollateral()
, which both uses thesafeTransferFrom()
.However, in order to change the method
createLoan()
to use thesafeTransferFrom()
it will require the contract to implement theIERC721Receive.onERC721Received()
method.Notice! Currently the method
closeLoan()
does not make use of thesafeTransferFrom()
and this means that if a user provided a contract address as the parametersendCollateralTo
and this contract does not implement proper handling ofERC721
, then the token would be lost. But since I see this as very unlikely, I choose to consider this as anon-critical
instead of alow
.