Justifying the design choice of using transferFrom instead of safeTransferFrom

[dleonard00]

This is an excerpt taken from an internal discussion authored by @scorpion9979:

Ok, I just had another look into the Pooled NFT code:
Since we're requiring NFT contract that is given to the factory to implement the IERC721Metadata interface, then it is a given that it should contain the safe transfer functionality.
It should be possible to replace transferFrom with safeTransferFrom in the implementation of Pool since safeTransferFrom is part of the standard ERC721 interface, but I do not see any value in that for the following reasons:
The reason why safeTransferFrom exists is to ensure that the NFT recipient is a smart contract that is compatible with ERC721 NFTs so that those transferred NFTs don't get locked in it forever. The only difference between the two methods is in how they're implemented. safeTransferFrom invokes the callback function _checkOnERC721Received after the transfer, which basically invokes a function in the sender of safeTransferFrom itself to make sure it knows how to deal with ERC721s. We already know that the created Pool knows how to handle ERC721s, since we are asserting that the given NFT contract implements a certain interface. Also, the Pool is implemented with the assumption that it is only dealing with ERC721s.
Using safeTransferFrom instead of transferFrom does not protect from the attack where the NFT contract itself is coded maliciously. If the NFT contract is malicious, the attacker could simply not include the _checkOnERC721Received callback in their implementation for safeTransferFrom.
This is not analogous to ERC20's safeTransferFrom and I felt that there was some misconception here: ERC20's transferFrom returns a boolean value, and the ERC20 safeTransferFrom exists in order to assert that the boolean value returned is true, since there exist some implementations of ERC20 (i.e., the 0x token) where transferFrom doesn't revert and simply returns false when the token transfer doesn't go through, and so the entire call would execute successfully even though the tokens weren't received. On the other hand, ERC721's transferFrom doesn't return a boolean in the function signature and should always revert when the transfer doesn't go through.
Here are some notes describing how safeTransferFrom works in ERC721:
https://github.com/OpenZeppelin/openzeppelin-contracts/blob/75ef7b8b27cf8d5ad8874d23754b0ff0dc256794/contracts/token/ERC721/IERC721.sol#L75