2

Can someone help me figure out what is missing with the contract below the functions mint, listToken, buyNFT work fine

Although, when it comes to putUpForSale, the transaction keeps reverting with the error : ERC721: caller is not token owner or approved

I am guessing the error happens when the ERC721 internal function is called _isApprovedOrOwner(address spender, uint256 tokenId)

  • How come though ? spender == owner should return true. the msg.sender is the token' owner and spender is _msgSender (= msg.sender)

I have been using to test locally on VM I have not had much luck with the debug feature.

the file below with 2 smart contracts : NFT and NFTMarketplace

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

import "@openzeppelin/contracts/utils/Counters.sol";
import "@openzeppelin/contracts/token/ERC721/extensions/ERC721URIStorage.sol";
import "@openzeppelin/contracts/token/ERC721/ERC721.sol";
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";

contract NFT is ERC721URIStorage {
    using Counters for Counters.Counter;
    Counters.Counter private _tokenIds;
    address marketplaceContract;
    event NFTMinted(uint256 tokenId);

    constructor(address _marketplaceContract) ERC721("NFTMarketplace", "NFTM") {
        marketplaceContract = _marketplaceContract;
    }

    function mint(string memory _tokenURI) public {
        uint256 currentTokenId = _tokenIds.current();
        _tokenIds.increment(); 
        _safeMint(msg.sender, currentTokenId);
        _setTokenURI(currentTokenId, _tokenURI);
        setApprovalForAll(marketplaceContract, true);

        emit NFTMinted(currentTokenId);
    }

    function approveUser(address _user) public {
        setApprovalForAll(_user, true);
    }

    function getCurrentToken() public view returns (uint256) {
        return _tokenIds.current();
    }
}

contract NFTMarketplace is ReentrancyGuard {

    using Counters for Counters.Counter;
    Counters.Counter private _itemsSold;
    Counters.Counter private _itemsCount;
    uint256 public LISTING_FEE = 0.01 ether;
    address payable private _marketOwner;
    mapping(uint256 => Token) private _idToToken;

    constructor() {
        _marketOwner = payable(msg.sender);
    }

    //The structure to store info about a listed token
    struct Token {
        address nftContract;
        uint256 tokenId;
        address payable seller;
        address payable owner;
        string tokenURI;
        string name;
        uint256 price;
        bool isListed;
    }

    event TokenListed(
        address nftContract,
        uint256 tokenId,
        address seller,
        address owner,
        uint256 price
    );
    event TokenSold(
        address nftContract,
        uint256 tokenId,
        address seller,
        address owner,
        uint256 price
    );

    //the event emitted when a token is successfully listed
    event TokenListedSuccess (
        uint256 indexed tokenId,
        address nftContract,
        address owner,
        uint256 price,
        bool isListed
    );



    //The first time a token is created, it is listed here
    // make it payable with money -- add require
    function listToken(address _nftContract, uint256 _tokenId, string memory _tokenURI, string memory name) public  {

        // require(msg.value > 0, "Price must be at least 1 wei");
        // require(msg.value == LISTING_FEE, "Price must be equal to listing price");
      
        IERC721(_nftContract).transferFrom(msg.sender, address(this), _tokenId);

        _idToToken[_tokenId] = Token(
            _nftContract,
            _tokenId,
            payable(msg.sender),
            payable(address(this)),
            _tokenURI,
            name,
            LISTING_FEE, 
            true
        );

        _itemsCount.increment();
        emit TokenListed(
            _nftContract,
            _tokenId,
            msg.sender,
            address(this),
            LISTING_FEE
        );
    }

    function buyNFT(address _nftContract, uint256 _tokenId) public payable nonReentrant {
        Token storage token = _idToToken[_tokenId];
        address payable buyer = payable(msg.sender);
        address payable seller = payable(token.seller);

        require(msg.value > 0, "You need to send some ether");
        require(msg.value >= token.price, "Not enough ether to cover asking price");
        // require(buyer != seller,"You already own this nft");
        
       IERC721(_nftContract).setApprovalForAll(buyer, true);
IERC721(_nftContract).approve(buyer, _tokenId);
        IERC721(_nftContract).transferFrom(address(this), buyer, _tokenId);
      

        payable(seller).transfer(msg.value);
        _itemsSold.increment();
        token.isListed = false;

        emit TokenSold(_nftContract, token.tokenId, address(this), msg.sender, msg.value);

        token.seller = token.owner;
        token.owner = payable(address(this));
    }

    function putUpForSale(address _nftContract, uint256 _tokenId) public payable {
        address owner = ERC721(_nftContract).ownerOf(_tokenId);
        require(msg.value > 0, "Price must be at least 1 wei");
        require(msg.value == LISTING_FEE, "Not enough ether for listing fee");
        require(owner == msg.sender, "you are not the owner of the token");
    
        IERC721(_nftContract).safeTransferFrom(owner, address(this), _tokenId);

        payable(address(this)).transfer(msg.value);
    
        Token storage token = _idToToken[_tokenId];
        token.seller = payable(msg.sender);
        token.owner = payable(address(this));
        token.isListed = true;
    
        _itemsSold.decrement();
        emit TokenListed(_nftContract, _tokenId, msg.sender, address(this), LISTING_FEE);
    }

    function getListPrice() public view returns (uint256) {
        return LISTING_FEE;
    }
    function getLatestIdToToken(uint256 _tokenId) public view returns (Token memory) {
        return _idToToken[_tokenId];
    }
    function getTokenForId(uint256 _tokenId) public view returns (Token memory) {
        return _idToToken[_tokenId];
    }

     function getOwnerOf(address _nftContract, uint256 _tokenId) public view returns(address) {
        return IERC721(_nftContract).ownerOf(_tokenId);
    }
   
    function getMarketPlaceBalance() public view returns(uint256) {
        //https://docs.soliditylang.org/en/develop/units-and-global-variables.html#address-related
        return address(this).balance;
    }

    function getItemsCount() public view returns(uint256) {
        return _itemsCount.current();
    }
    function getItemsSold() public view returns(uint256) {
        return _itemsSold.current();
    }

    //We might add a resell token function in the future
    //In that case, tokens won't be listed by default but users can send a request to actually list a token
    //Currently NFTs are listed by default
}
4
  • spender isn't owner but NFTMarketplace. The owner must first approve the marketplace as spender, by calling, in a separate transaction, NFT.approve(NFTMarketplaceAddress, tokenId) or NFT.setApprovalForAll(NFTMarketplaceAddress, true)
    – 0xSanson
    Oct 6, 2022 at 14:07
  • I am setApprovalForAll(marketplaceContract, true) when I mint the token from the NFT smart contract ? the marketplace is already approved then ? Oct 7, 2022 at 9:09
  • I did update the question. I have also tried to approve the buyer and new owner. Why the new buyer (owner) cannot resell to marketplace. This where I keep failing Oct 7, 2022 at 9:27
  • You can't do it from the smart contract. Only with a separate transaction from the user. I added a full answer, hopefully it helps
    – 0xSanson
    Oct 7, 2022 at 11:06

1 Answer 1

1

So, the problem is that the marketplace doesn't have permission to take the NFT out of the users' wallets.

A user needs to grant permission by making a call to the NFT contract before. Using ethers.js:

const tx = await NFTContract.setApprovalForAll(marketplaceContract, true);
await tx.wait();

Only after this transaction the marketplace can successfully call:

IERC721(_nftContract).safeTransferFrom(owner, address(this), _tokenId);

otherwise it will fail because it's not approved. Notice that _nftContract will see the marketplace as spender, not the owner. Indeed every time a contract calls another contract, the msg.sender changes to the caller contract (I think you were confused by this).


The part regarding buyNFT works correctly because when you call it like this:

IERC721(_nftContract).transferFrom(address(this), buyer, _tokenId);

the spender and the owner are both the marketplace (think about it). In this case no approval is needed (cause you're definitely allowed to transfer your own tokens how you please). So the previous two lines aren't needed and should be eliminated:

IERC721(_nftContract).setApprovalForAll(buyer, true);
IERC721(_nftContract).approve(buyer, _tokenId);
5
  • Once you transfer ownership from the marketplace to the buyer. The same buyer wants to sell. but I get the error : ERC721: caller is not token owner or approved Oct 7, 2022 at 22:19
  • FYI, I do call await NFTContract.setApprovalForAll(marketplaceContract, true) from the NFT contract. Upon deployment I pass the marketplace address as a argument to the constructor if you can see in the first smart contract Oct 7, 2022 at 22:20
  • I am still confused with the function putUpForSale. Who is the owner ? it should be the user who last bought the token from the marketplace, though ? What am I missing ? Oct 8, 2022 at 1:17
  • @user9958772 You can't call setApprovalForAll from the NFT contract, it's totally useless. You have to call it in a separate transaction from the user! In the putUpForSale function, the owner is the user but the spender is the marketplace. That's why the user has to grant approval.
    – 0xSanson
    Oct 8, 2022 at 10:06
  • gotcha ! makes sense now. thanks !! Oct 8, 2022 at 14:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.