Feature Request: ERC-721 and 1155 Transfer Block-Lists for certain contracts

[sterlingcrispin]

I'm leaving this up for posterity but I'm obviously wrong on this one, I posted this in an emotional state reacting to what I felt like was an unjust situation, but long term this would case more problems than it solves and create more unjust situations.

my bad

Also, I'm getting roasted on twitter (maybe fair) but a core ERC721A dev had already come to the same conclusion I did and wrote their own implementation as an example. This might be ideologically the wrong direction, but I'm a lone crazy person for coming to this conclusion

🧐 Motivation
Recent actions by SudoSwap and now X2Y2 marketplaces are aggressively moving towards circumventing the NFT royalty system, which has largely been an honor system up until this point. Creators of NFTs should be given tools to easily cut off bad actors and control how their tokens are used.

EIP-2981 is one step towards addressing this problem, but royalties will remain optional. Because of this, I suggest a transfer-level block list should become an adopted standard.

Beyond the royalty issue, generally speaking, creators should be given more control over who is allowed to operate on their tokens. The concept of permissionless tokens is nice in theory, but bad actors do exist, and giving creators control to cut them off is important.

📝 Details
I recommend editing the Open Zeppelin ERC-721 and ERC-1155 contracts to easily allow creators to define block-listed addresses. For example, when beforeTokenTransfer is called, the function would iterate through blocked addresses and require that the msg.sender was not equal to one of these blocked addresses.

Here's one example in a contract inheriting from OpenZeppelin's ERC-721 contract, showing how a block list might be implemented. I'm sure there are better ways, but I really think this needs to become a part of the core OpenZeppelin contracts

This code isn't mean to be a literal solution but just a conversation starter, I know it isn't the optimal code

address[] public blockedAddrs;

function populateBlockedAddrs(address[] memory addrs) public onlyOwner{
    blockedAddrs = addrs;
}

function _beforeTokenTransfer(
    address from,
    address to,
    uint256 tokenId
) internal virtual override(ERC721, ERC721Enumerable) {
    uint256 limit = blockedAddrs.length;
    uint256 i = 0;
    while(i < limit){
        require(msg.sender != blockedAddrs[i], "blocked address");
        unchecked {
            i++;
        }
    }
    super._beforeTokenTransfer(from, to, tokenId);
}

Here's another suggestion someone made along the same lines

https://twitter.com/cygaar_dev/status/1563186676771405830

Thanks for the consideration

[sterlingcrispin]

One argument I've seen against this is that it shouldn't be always-on by default. I totally agree with that. There should be a way to specify as the deployer of a contract if you want this functionality or not.

Maybe it becomes a true-false variable during deployment in the constructor or something.

[gwendall]

Very needed - I actually wrote an extension for that a few weeks ago:
https://github.com/gwendall/erc721-transfer-control/blob/main/contracts/extensions/ERC721TransferControl.sol

[frangio]

As you've both shown, it's already possible and easy to extend a token with this functionality, so it's not necessary to include it as a core feature of the library.

[sterlingcrispin]

Yeah I totally get that, you're right.

For posterity I think that artist created NFTs with royalties, and NFTs that are fully unrestricted bearer assets, are potentially diverging things. There's so many contexts for an ERC-721 token that baking this context into a core feature of a library doesn't really make sense.

I posted this feature request in an emotional reaction and in hindsight this isn't the right solution

[johnnyshankman]

@sterlingcrispin great response. just wanted to mention, we'd have to do more than just block transfers to avoid listings. We also have to block AND remove their status as approved ie override setApprovalForAll etc.

[johnnyshankman]

@gwendall that applies to your extension as well. you'll want to block the approvals for those specific blacklisted addresses to avoid listings ever appearing in the first place. it's a horrible UX to have your collector not realize a listing is impossible to buy until they actually try to buy it (and the tx fails).

i've implemented this before in soulbound contracts like below:
https://etherscan.io/address/0x1b23d0f0f6dc3547c1b6945152acbfd6eaad85b0#code

[sterlingcrispin]

@johnnyshankman yeah totally -- Cygaar had another approach and I've seen a dozen other people imagine other solutions to this , so I'm not alone in thinking this might be desirable. Although at this point I think block lists is not the correct direction. The solution is probably a mix of stuff like EIP-2981 that makes royalties more supported in code, and game theory stuff that incentivizes cooperation.

[johnnyshankman]

@sterlingcrispin totally agreed longterm about the incentivized approach, as well as preferring this functionality an extension instead of built into the standard. interesting convo here! glad you proposed this, got my gears turning.