How to Properly Integrate ERC2981 Royalty Standard into ERC721 NFT Marketplace Contract

NFT
1

I’m struggling to understand how to correctly implement royalties using the ERC2981 standard in my NFT marketplace. After going through various tutorials and documentation, I’m still confused about some key concepts.

My main question is about the feeNumerator parameter in the _setDefaultRoyalty(address recipient, uint96 feeNumerator) function. What exactly should this value represent? Is it the actual NFT price or the royalty percentage?

Here’s my current marketplace contract implementation. Could someone review it and let me know if I’m handling the royalty setup correctly?

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import "@openzeppelin/contracts/access/Ownable.sol";
import "@openzeppelin/contracts/utils/Counters.sol";
import "@openzeppelin/contracts/token/common/ERC2981.sol";
import "@openzeppelin/contracts/token/ERC721/extensions/ERC721URIStorage.sol";

contract AudioNFT is ERC721URIStorage, ERC2981, Ownable {
    using Counters for Counters.Counter;
    Counters.Counter private _itemCounter;
    Counters.Counter private _tokenCounter;

    struct AudioItem {
        address creator;
        address currentOwner;
        string itemTitle;
        string itemDetails;
        uint256 salePrice;
        uint256 royaltyRate;
        uint256 tokenId;
        bool isActive;
    }

    mapping(uint256 => AudioItem) public audioItems;

    constructor() ERC721("Audio NFT Collection", "AUDIO") {}

    function createToken(
        string memory metadataURI,
        string memory _title, 
        string memory _details, 
        uint256 _price, 
        uint256 _royalty) external returns(uint256) {
            
            require(_price > 0, "Price must be greater than zero");
            require(_royalty > 0 && _royalty <= 25, "Royalty must be between 1 and 25 percent");

            _itemCounter.increment();
            _tokenCounter.increment();
            uint256 newTokenId = _tokenCounter.current();

            _safeMint(msg.sender, newTokenId);
            _setTokenURI(newTokenId, metadataURI);

            audioItems[_itemCounter.current()] = AudioItem(
                msg.sender, 
                msg.sender, 
                _title, 
                _details, 
                _price, 
                _royalty,
                newTokenId,
                false
            );

            return newTokenId;
    }

    function activateForSale(uint256 _itemId) external {
        require(msg.sender == audioItems[_itemId].currentOwner, "Only token owner can activate sale");
        audioItems[_itemId].isActive = true;
        approve(address(this), audioItems[_itemId].tokenId);
        setApprovalForAll(address(this), true);
        _setDefaultRoyalty(audioItems[_itemId].creator, uint96(audioItems[_itemId].royaltyRate));
    }
}

Am I calling the royalty function at the right time and with correct parameters? Any guidance would be appreciated!

2

The feeNumerator is the royalty percentage in basis points - 10000 = 100%. So 5% royalty = 500. Your code has a big problem though. You’re setting royalties in activateForSale instead of when minting. Royalties should be set once during token creation and never change. Move _setDefaultRoyalty to your createToken function instead. Since you’re validating 1-25% royalty, just multiply by 100 to convert to basis points. You’ve got redundant storage too. You’re storing royalty data in your AudioItem struct AND using ERC2981’s internal storage. Drop royaltyRate from your struct and just use ERC2981’s royaltyInfo function. Your marketplace contract should call royaltyInfo(tokenId, salePrice) during sales to get the royalty amount and recipient address.

3

you’re overcomplicating this. yeah, feenumator should be basis points like everyone said, but there’s a bigger issue - why are you using _setDefaultRoyalty AND storing royalty in your struct? pick one.

also, calling _setDefaultRoyalty every time someone lists for sale is burning gas for no reason. set it once during mint and you’re done.

4

hey @LeapingFox! i’ve been working with ERC2981 for a while and spotted something in your code that might be causing confusion.

feeNumerator uses basis points, not percentages or prices. want 5% royalty? pass 500 (since 500/10000 = 5%). you’re passing audioItems[_itemId].royaltyRate directly - is this stored as basis points or percentage?

curious why you’re calling _setDefaultRoyalty in activateForSale. wouldn’t it make more sense to set royalty info when minting in createToken? setting it during activation might cause issues if the same token gets activated multiple times.

also noticed you’re using setApprovalForAll(address(this), true). have you thought about the security risks? might want to look into more granular approval mechanisms.

why store royalty info in both your custom AudioItem struct AND use ERC2981? seems redundant unless i’m missing something about your architecture.

btw, don’t forget to override supportsInterface to include ERC2981 - didn’t see that in your snippet but it’s crucial for proper interface detection!