QA Report

[code423n4]

QA Report for OpenSea by Perito Flores

[L-01] INSECURE ERC721 TRANSFERFROM

Impact

Tokens can be locked forever in the receiving contract

PoC

The function _performERC721Transfer() (reference and optimized) is only checking that the token is a contract. This check seems weak to me and the most important would be to check if the receiving contract is able to handle NFT (implements ERC721TokenReceiver interface as recommended in the standard). If not, the sender is responsible for this check. If this is the case you should warn users in the doc.
I added this report in my QA but this could be medium ( code-423n4/2021-06-pooltogether-findings#115).

function _performERC721Transfer(
    address token,
    address from,
    address to,
    uint256 identifier
    ) internal {
    if (token.code.length == 0) {
       revert NoContract(token);
   }
ERC721Interface(token).transferFrom(from, to, identifier);  @audit 

}

Recommended

Check if the receiving contract is able to handle NFTS.

[L-02] _performERC20Transfer can incorrectly succeed in rare edge cases

I could have put this issue in the gas report. However I believe that exist a very unlikely edge case where this can fail.
First of all there is a tautology here (ReferenceTokenTransferrerer#L50).
the first check (data.length!=0 is redundant) so you could remove

if (data.length != 0 && data.length >= 32) { 
        if (!abi.decode(data, (bool))) {
            revert BadReturnValueFromERC20OnTransfer(
                token,
                from,
                to,
                amount
            );
        }
    }

However, I believe that you should remove the second statement (data.length >= 32). The edge case would be if the returned value is 16 bytes (or 8) then the transaction will always pass. I believe that you should avoid it because by doing you will save gas.
There is a interesting post here that could help
OpenZeppelin/openzeppelin-contracts#2577

Recommended

Remove data.length >= 32 to save gas and avoid edge cases. This will be similar to OZ SafeTransfer.

[HardlyDifficult]

INSECURE ERC721 TRANSFERFROM

Using safeTransferFrom is a common best practice to recommend. In this project it was intentionally avoided -- see the sponsor comment here #173 (comment) and #19 (comment)

_performERC20Transfer can incorrectly succeed in rare edge cases

This is an interesting consideration, I agree it may be appropriate to revert in this scenario. Although if data.length != 0 wouldn't it always be >= 32? Maybe it's just a redundant check / no-op and potential gas savings.