salvorio / salvor-contracts

0 stars 0 forks source link

[AMR-01M] Inexistent Usage of EIP-2981 Royalty #4

Open HKskn opened 4 months ago

HKskn commented 4 months ago

AMR-01M: Inexistent Usage of EIP-2981 Royalty

Type Severity Location
Standard Conformity AssetManager.sol:L342

Description:

The AssetManager::_sendRoyalty function will query whether an NFT supports the EIP-2981 interface yet will solely utilize the intended royaltyReceiver, ignoring the yielded royalty amount meant to be charged.

Impact:

Although non-standard behaviour, this may be a desirable trait of the protocol and thus has been marked as unknown in severity.

Example:

/**
 * @notice Calculates and sends the royalty payment for an NFT transaction.
 * @param _nftContractAddress The address of the NFT contract.
 * @param _tokenId The ID of the NFT.
 * @param _seller The address of the seller.
 * @param price The price of the NFT.
 * @return The amount of royalty paid.
 */
function _sendRoyalty(address _nftContractAddress, uint256 _tokenId, address _seller, uint256 price) internal returns (uint256) {
    Royalty memory royalty = royalties[_nftContractAddress];
    if (royalty.isEnabled) {
        address royaltyReceiver = royalty.receiver;
        uint256 royaltyAmount = _getPortionOfBid(price, royalty.percentage);
        if (royaltyReceiver != _seller && royaltyReceiver != address(0)) {
            emit RoyaltyReceived(_nftContractAddress, _tokenId, _seller, royaltyReceiver, royaltyAmount);
            _safeTransferTo(payable(royaltyReceiver), royaltyAmount);
            return royaltyAmount;
        }
    } else {
        if (IERC721Upgradeable(_nftContractAddress).supportsInterface(_INTERFACE_ID_EIP2981) && defaultRoyalty > 0) {
            uint256 royaltyAmount = _getPortionOfBid(price, defaultRoyalty);
            (address royaltyReceiver,) = IRoyalty(_nftContractAddress).royaltyInfo(_tokenId, price);
            if (royaltyReceiver != _seller && royaltyReceiver != address(0)) {
                emit RoyaltyReceived(_nftContractAddress, _tokenId, _seller, royaltyReceiver, royaltyAmount);
                _safeTransferTo(payable(royaltyReceiver), royaltyAmount);
                return royaltyAmount;
            }
        }
    }

    return 0;
}

Recommendation:

We advise this to be re-evaluated as it contradicts the EIP-2981 standard and thus renders the codebase non-compliant with it.

HKskn commented 4 months ago

Fixed. We have removed the _INTERFACE_ID_EIP2981 support from the code.