[WP-M6] Inappropriate support of EIP-2981

[code423n4]

Lines of code

https://github.com/code-423n4/2022-02-foundation/blob/4d8c8931baffae31c7506872bf1100e1598f2754/contracts/mixins/NFTMarketCreators.sol#L65-L82

Vulnerability details

https://github.com/code-423n4/2022-02-foundation/blob/4d8c8931baffae31c7506872bf1100e1598f2754/contracts/mixins/NFTMarketCreators.sol#L65-L82

if (nftContract.supportsERC165Interface(type(IRoyaltyInfo).interfaceId)) {
  try IRoyaltyInfo(nftContract).royaltyInfo{ gas: READ_ONLY_GAS_LIMIT }(tokenId, BASIS_POINTS) returns (
    address receiver,
    uint256 /* royaltyAmount */
  ) {
    if (receiver != address(0)) {
      recipients = new address payable[](1);
      recipients[0] = payable(receiver);
      // splitPerRecipientInBasisPoints is not relevant when only 1 recipient is defined
      if (receiver == seller) {
        return (recipients, splitPerRecipientInBasisPoints, true);
      }
    }
  } catch // solhint-disable-next-line no-empty-blocks
  {
    // Fall through
  }
}

The current implementation of EIP-2981 support will always pass a constant BASIS_POINTS as the _salePrice.

As a result, the recipients that are supposed to receive less than 1 BPS of the salePrice may end up not receiving any royalties.

Furthermore, for the NFTs with the total royalties rate set less than 10% for some reason, the current implementation will scale it up to 10%.

Recommendation

1. Instead of passing a constant of 10,000 as the _salePrice, we suggest using the actual _salePrice, so there the royalties can be paid for recipients with less than 1 BPS of the royalties.
2. When the total royalties cut is lower than 10%, it should be honored. it's capped at 10% only when the total royalties cut is higher than 10%.

[HardlyDifficult]

Yes, these are valid points and something we will consider revisiting in the future.

RE recommendations: 1) This can only impact < 0.01% of the payment so not a concern ATM. It may be more appropriate to better honor exact amounts, but it's a non-trivial change to an important code path so we will leave it as-is for now. With payments that small, it's probably more appropriate to be using a contract to manage the payouts - e.g. https://www.0xsplits.xyz/ could handle this well. 2) I agree. ATM we always enforce exactly 10% so that there is a consistent experience with our market and on our website. We will revisit this in the future, and the idea of capping it to 10% but accepting lower is a great one.