Base file I used to use for my mainnet contracts to be compatible with OpenSea

README.md

Warning !!!

Recent events have shown that the auto-approval for user proxies is way too dangerous.

Security of users' NFTs should come before the convenience of auto approving collection for trading.

This is why I decided to remove the files in this gist.

Security risk

Since a few weeks, people are loosing NFTs because of approvals on OpenSea.

It's in no way OpenSea's fault, it's because some bad people are phishing signatures of sale orders.

The only way to protect against this, is for users to revoke the approval they gave to OpenSea, using tools like etherscan approval checker or other revoke apps.

However this is not possible if there is no event emitted. Those website can not know that an approval has been given. Users are not even aware that their NFTs are approved on OpenSea, but still if they get phished, might see them disappear anyway.

Personal thoughts

This pattern seemed convenient a few months back, but experience have just shown that it's too dangerous to have it used, especially without any way to revoke it.

Users wanting to sell on Marketplaces are expecting to make a profit. Therefore my opinion is that they should take the cost of approval into account when they calculate their possible profit, and include that in their sale price.

It's a small price compared to the security of their tokens if they don't try to sell it.

I haven't check SeaPort implementation, but isn't only a new operator addr ? or is it a logic update ?
In case it's only an addr update then the exploit is probably still the same unless the contract doesn't update the openSeaProxyRegistryAddress variable (from code above).

Thinking about the problem I came out with this solution.
With a boolean in the "mint" function, we can ask the user if they want to approve the collection at mint time for popular platforms.
Then instead of override "isApprovedForAll" we call _setApprovalForAll so :

• user is aware approvals
• he choose the approvals
• the ApprovalForAll is emit so 3rd party app can read it and propose a disapprove

@dievardump what do you think about it ?

    function mint(bool _approvalOS, bool _approvalLR, bool _approvalX2Y2) public {
        [...]
    
        _mint(msg.sender, _tokenIds.current());

        if (_approvalOS){
        _setApprovalForAll(msg.sender, OS_OPERATOR, true);
        }
        if (_approvalLR){
        _setApprovalForAll(msg.sender, LR_OPERATOR, true);
        }
        if (_approvalX2Y2){
        _setApprovalForAll(msg.sender, X2Y2_OPERATOR, true);
        }
    }

I haven't check SeaPort implementation, but isn't only a new operator addr ? or is it a logic update ? In case it's only an addr update then the exploit is probably still the same unless the contract doesn't update the openSeaProxyRegistryAddress variable (from code above).

Thinking about the problem I came out with this solution. With a boolean in the "mint" function, we can ask the user if they want to approve the collection at mint time for popular platforms. Then instead of override "isApprovedForAll" we call _setApprovalForAll so :

• user is aware approvals
• he choose the approvals
• the ApprovalForAll is emit so 3rd party app can read it and propose a disapprove

@dievardump what do you think about it ?

    function mint(bool _approvalOS, bool _approvalLR, bool _approvalX2Y2) public {
        [...]
    
        _mint(msg.sender, _tokenIds.current());

        if (_approvalOS){
        _setApprovalForAll(msg.sender, OS_OPERATOR, true);
        }
        if (_approvalLR){
        _setApprovalForAll(msg.sender, LR_OPERATOR, true);
        }
        if (_approvalX2Y2){
        _setApprovalForAll(msg.sender, X2Y2_OPERATOR, true);
        }
    }

As I understand, this should cost extra gas similar to call setApprovalForAll afterward. But yes it should save a little bit of gas comparing to call it separately.