approve + safeTransferFrom ERC721 from existing smart contract

Question

I am trying to call ERC721 from another smart contract, while calling function approveAndTransfer ,I am getting "ERC721: transfer caller is not owner nor approved", not sure reason behind it,

As require(nft.ownerOf(_tokenId) == msg.sender, "NOT OWNER"); is working as expected and not throwing any error, but source of error is _tranfer function which gets called from nft.safeTransferFrom(msg.sender, _to,_tokenId);

not sure what mistake I am making. I have NFT in my wallet, but I am not owner of external smart contract.

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

import "https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC721/IERC721.sol";

import "https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC721/IERC721Receiver.sol";

contract Tat is IERC721Receiver {
    
constructor() public {}


function approveAndTransfer(IERC721 nft, uint256 _tokenId, address _from, address _to) public {
    require(nft.ownerOf(_tokenId) == msg.sender, "NOT OWNER");
    nft.isApprovedForAll(_from, address(this));
    nft.safeTransferFrom(msg.sender, _to,_tokenId);
}

function ownerOf(IERC721 nft, uint256 tokenId) external view returns (address owner) {
    return nft.ownerOf(tokenId);
}


function test() public view returns(address sc,address own){
    return (address(this),msg.sender);
}


function onERC721Received(address _operator, address _from, uint256 _tokenId, bytes memory _data) public override returns(bytes4) {
       return 0x150b7a02;
 }

}

Thanks in advance.

Answer 1

It looks like one of your function calls needs to be changed -let's look at the approveAndTransfer function from your contract:

function approveAndTransfer(IERC721 nft, uint256 _tokenId, address _from, address _to) public {
    require(nft.ownerOf(_tokenId) == msg.sender, "NOT OWNER");
    nft.isApprovedForAll(_from, address(this));
    nft.safeTransferFrom(msg.sender, _to,_tokenId);
}

It looks like the goal of the function is to transfer NFTs from the _from address to the _to address. In order to do so, the flow needs to be:

• approve the contract to send _from's NFT
• contract sends to _to

After the require, though, the contract calls isApprovedForAll on the NFT contract. Looking at OpenZeppelin's ERC721 docs, we can see that this only tells us if an address is approved for another address's NFTs - it doesn't actually do the approving.

It looks like you probably wanted setApprovalForAll, but if we've gotten this far in the conversation, it's worth pointing out why this is probably not the right choice for you either.

Firstly, setApprovalForAll would approve the contract on all of _from's NFTs on this contract, even though the presence of a single _tokenId in the args indicates that the function will only transfer one NFT at a time. It isn't desirable to over-approve like that. Instead, you'd probably have wanted to use the approve function, which takes a tokenId.

Secondly, and maybe this should have even been first, is that you can't approve yourself on someone else's assets. Or, in this case, the contract can't approve itself over msg.sender's NFT. The reasoning is simple - if I could approve myself over someone else's NFTs, then I could just do that and take them whenever I wanted for no reason (also known as stealing). This is why getting a user to approve happens outside of smart contracts in any scenario that I'm aware of - you can use JavaScript to prompt a user to approve, but you can't have a smart contract approve for them.

Hope this helps!