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.

I've also requested this of OpenZeppelin ,

📝 Details
I recommend editing ERC721A contract 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 an inherited contract showing how a block list might be implemented. I'm sure there are better ways, but it would be great to see this added to the core contract

This code wasn't meant to be taken literally and was just meant to start a conversation, again I know its not optimized

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);
}

Thanks for the consideration

[Vectorized]

From an efficiency point of view, this can be better implemented as a centralized on-chain registry, or multiple centralized on-chain registries (like ad-block lists), that any ERC721/ERC1155/ERC20 can query and use to block transfers.

It will also offer the advantage keeping the data up to date -- a DAO can be created to allow users to add new addresses collaboratively in a timely fashion.

If this is implemented as an extension or in the core ERC721A contract, users will have to write many addresses (e.g. OFAC sanctions list) to storage for every newly published contract. That costs a lot of gas.

As this is out-of-scope to the core goals of ERC721A, and can be implemented way more efficiently with another method, we are not in favor of adding this.

Thanks!

[peetzweg]

How about tackling this issue the other way around with an allowlist. Both solutions are not optimal anyways, but at least the collection owner just needs to take care of the allowed platforms instead of constantly adding newly deployed marketplaces to a blocklist.

[sterlingcrispin]

users will have to write many addresses (e.g. OFAC sanctions list) to storage for every newly published contract. That costs a lot of gas.

Isn't this is another good reason to have it within each ERC721A contract? Because a sanction in Country A may not be valid in Country B. So you'd want the creator to be able to make their own block list and update it over time.

Can you please elaborate on why you are against this feature request? Beyond the registry issue there are a lot of reasons why a creator might need to block an address

How about tackling this issue the other way around with an allowlist. Both solutions are not optimal anyways, but at least the collection owner just needs to take care of the allowed platforms instead of constantly adding newly deployed marketplaces to a blocklist.

No, because you'd have to add every ETH wallet that could ever exist into the allow list

[myssynglynx]

No, because you'd have to add every ETH wallet that could ever exist into the allow list

A basic contract check (e.g. _msgSenderERC721A().code.length == 0) could be used to ensure any external contract call for the transfer is executed by an allowlisted contract, bypassing the allowlist check if the caller is a wallet. This would increase gas cost if implemented, but it would ensure safety for any wallet-to-wallet transfer, while also ensuring that any AMM or Atomic Swap function called by a contract in the allowlisted registry is also safe.

EIP2981 can be ignored, but attempting to bypass it would require trusted transactions, since an external contract that isn't on the allowlist couldn't facilitate a trustless transaction without reverting. If ignoring EIP2981 has to be split into two trusted transactions, then the ETH sender could just rug them after receiving the token (or the token sender could rug the ETH sender after receiving the ETH).

[sterlingcrispin]

One argument I've seen on twitter 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.

[Vectorized]

Integrating a centralized registry can be done in just a few lines.

For maintainability, we won't accept implementations that can be trivially implemented in a couple of lines. See: #246
Otherwise, we will start collecting an entire suite of few-liner extensions.

function _beforeTokenTransfers(
    address from,
    address to,
    uint256 startTokenId,
    uint256 quantity
) internal virtual override(ERC721A) {
    // You can even supplement the registry with a mapping of banned addresses.
    if (isBannedAddress[msg.sender] || registry.isBanned(msg.sender)) revert();
    super._beforeTokenTransfers(from, to, startTokenId, quantity);
}

The implementation of the registry is not within the scope of ERC721A.

Feel free to make a separate project for the registry.

[sterlingcrispin]

Why wouldn't that registry just be an array within the contract?

Denying a feature due to the complexity of its implementation is a poor reason. This has large scale user value.

Ask any NFT creator, "Would you like people to deny paying you a royalty? Would you like more control over how people are allowed to operate on your NFTs?" They will overwhelming support this feature

[Vectorized]

Your implementation scans through an entire array.

Each element scanned takes 2100 gas.

---

There are many things with large scale user value -- NFT marketplaces, cancer research, solving world hunger.
Unfortunately, they are not under the scope of efficient ERC721 batch minting.
Including them will dilute the focus of this repo.

[sterlingcrispin]

I am 100% in agreement you should not use my implementation, it was just a conversation starter. I'm unclear the best way to implement this.

NFT Marketplaces, cancer research, etc are outside the scope of this repo.

Having a deny list during transfers, is within the scope of this repo. It's not a huge sprawling new direction its a very small change

[edsonayllon]

Why wouldn't that registry just be an array within the contract?

Denying a feature due to the complexity of its implementation is a poor reason. This has large scale user value.

Ask any NFT creator, "Would you like people to deny paying you a royalty? Would you like more control over how people are allowed to operate on your NFTs?" They will overwhelming support this feature

I don't think using the reasoning of royalty bypass is sufficient reasoning for adding an alow list. Allow lists are very centralized. The owner could block addresses without needing a reason, increasing censorship.

If royalties are a concern, it should be dealt by creating on chain royalties. Instead of choosing favorites for marketplaces in the open market.

I don't think royalty chasing is sufficient justification for including a mechanism that greatly increases financial censorship

[sterlingcrispin]

Yeah you're right, 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. Sorry about that.

[deanpress]

I don't think using the reasoning of royalty bypass is sufficient reasoning for adding an alow list. Allow lists are very centralized. The owner could block addresses without needing a reason, increasing censorship.

If marketplaces deliberately circumvent the existing royalty system, which was made specifically for a convenient user experience, then I think it's fair that they can expect a solid multisig DAO, operated by several reputable NFT projects, to vote that contract's address into a filter mapping (that's not part of any standard, just a standalone contract that offers an external function).

The DAO contract would expose a function that simply reverts if a checked address matches the filter, which NFT contracts can integrate if they so choose (by running the filter function on the to address in _beforeTokenTransfer()). If a protocol then does add royalties later, they could also be removed from the list.

This should obviously not be integrated into any standard and instead be offered as an external call function that any contract can integrate in their _beforeTokenTransfer() function.

The existence of such a filter list alone, along with a very small number of projects supporting it, should be enough for most serious protocols to stop circumventing royalties for the sake of delegating more fees to themselves. It would no longer be worth the risk.

It being multisig DAO-operated also solves the issue of NFT project owners otherwise freely adding/removing addresses at a whim.

Food for thought. IMO the decentralization and open ethos of the smart contract ecosystem with this system is upheld as anyone can freely check whether an NFT contract integrated this filter and make their decision on whether or not to obtain it if they specifically want to interact with royalty bypassing contracts.

[deanpress]

If anyone would like to discuss the potential solution of a DAO-operated contract extension, I drafted a contract + set of extensions here: https://github.com/deanpress/OCF-DAO