Assignment - ERC721 Fulfillment transferFrom

function transferFrom(address _from, address _to, uint256 _tokenId) external {
        require(msg.sender == _from || kittyIndexToApproved[_tokenId] == msg.sender ||  _operatorApprovals[_from][msg.sender], "msg.sender is current owner, or the authorized operator, or the approved address for this NFT");
        require(_owns(_from,_tokenId), "_from is not the current owner");
        require(_to != address(0), "_to is the zero address");
        require(_tokenId > kitties.length, "_tokenId is not a valid NFT");

        _transfer(_from,_to,_tokenId);
    }
2 Likes

My code:
I had to comment the 2 safeTransferFrom functions inside IERC721.sol since not yet implemented in my Kittycontract.sol to compile en deploy errorless.

Also function isApprovedForAll() is set from external to public so it can be accessed from the transferFrom function too.

Added code:

  function transferFrom(address _from, address _to, uint256 _tokenId) external {
    require(_owns(_from, _tokenId), "Must be owner the token Id");
    require(_owns(msg.sender, _tokenId)
          || _approvedFor(msg.sender, _tokenId)
          || isApprovedForAll( _from, msg.sender),
          "Must be owner, be operator or approved of the token Id");
    require(_to != address(0), "Cannot transfer to zero address");
    require(_tokenId < totalSupply(), "token Id must exist");

    _transfer(_from, _to, _tokenId);
  }

  function _approvedFor(address _claimant, uint256 _tokenId) internal view returns (bool) {
    return kittyIndexToApproved[_tokenId] == _claimant; // is claimant approved for token Id?
  }
2 Likes
function transferFrom(
        address _from,
        address _to,
        uint256 _tokenId
    ) external {
        require(
            _owns(_from, _tokenId),
            "Owner address is not connected to this token!"
        );
        require(
            msg.sender == _from ||
                msg.sender == kittyIndexToApproved[_tokenId] ||
                _operatorApprovals[_from][msg.sender],
            "Only Owner, Operator or Approved Addresses can transfer!"
        );
        require(_to != address(0), "Transfer to zero-address is not possible!");
        require(_tokenId < kitties.length, "This token does not exist!");
        _transfer(_from, _to, _tokenId);
    }
2 Likes
function transferFrom(address _from, address _to, uint256 _tokenId) public {
        require(_to != address(0));
        require(msg.sender == _from || _approvedFor(msg.sender, _tokenId) || isApprovedForAll(_from, msg.sender));
        require(_owns(_from, _tokenId));
        require(tokenId < Kitties.length);

        _transfer(_from, _to, _tokenId);

    }
2 Likes
function transferFrom(address _from, address _to, uint256 _tokenId) external override {
        require( msg.sender == _from || isApprovedForAll(_from, msg.sender) || kittyIndexToApproved[_tokenId] == msg.sender);
        require(_owns(_from, _tokenId) && _to != address(0) && _tokenId < kitties.length);
        
        _transfer(_from, _to, _tokenId);
    }

Here ya go

function transferFrom(address _from, address _to, uint256 _tokenId) external{
        require(_owns(_from, _tokenId));
        require(_to != address(0));
        require(msg.sender == _from || _operatorApprovals[_from][msg.sender] == true || kittyIndexToApproved[_tokenId] == msg.sender);
        require(tokenId < kittens.length);

        _transfer(msg.sender, to, tokenId);
    }
2 Likes

Transfer From Function

function transferFrom(address _from, address _to, uint256 _tokenId) external{
    require(_owns(msg.sender,_tokenId)||
            isApprovedForAll(_from, msg.sender) == true ||
            getApproved(_tokenId) == msg.sender);
    require(_owns(_from,_tokenId));
    require(_to != address(0));
    require(_tokenId < kitties.length);

    _transfer(_from,_to,_tokenId);

}
2 Likes

function transferFrom

    function transferFrom(address from, address to, uint256 tokenId) public {
        require(_owns(msg.sender, tokenId) || isApprovedForAll(from, msg.sender) == true || getApproved(tokenId) == msg.sender, "Not authorized to send transaction");
        require(_owns(from, tokenId), "msg.sender is not the token owner!");
        require(to != address(0), "Must not be sent to 0 address");
        require(tokenId < bears.length, "tokenId does not exist");

        _transfer(from, to, tokenId);
    }
1 Like

Happy new year! :partying_face:

Here is my code for the transferFrom() function:

    /// @notice Transfer ownership of an NFT -- THE CALLER IS RESPONSIBLE
    ///  TO CONFIRM THAT `_to` IS CAPABLE OF RECEIVING NFTS OR ELSE
    ///  THEY MAY BE PERMANENTLY LOST
    /// @dev Throws unless `msg.sender` is the current owner, an authorized
    ///  operator, or the approved address for this NFT. Throws if `_from` is
    ///  not the current owner. Throws if `_to` is the zero address. Throws if
    ///  `_tokenId` is not a valid NFT.
    /// @param _from The current owner of the NFT
    /// @param _to The new owner
    /// @param _tokenId The NFT to transfer
    function transferFrom(address _from, address _to, uint256 _tokenId) external{
        _transferFrom(_from, _to, _tokenId);
    }    

    function _transferFrom(address _from, address _to, uint256 _tokenId) private{
        //msg.sender must be the owner or an existing operator of tokenId
        require(_to != address(0), "invalid _to address");
        require(_tokenId < kitties.length, "Invalid token id");

        address tokenOwner = tokenowners[_tokenId];
        require(_from == tokenOwner, "_from should be the token owner");

        address approver = tokenApprovers[_tokenId];    
        require( (tokenOwner == msg.sender 
        || ownerApprovers[tokenOwner][msg.sender] == true
        || approver == msg.sender) , 'Caller is not owner of the token or an operator of the token owner or an approver of the token'); 

        _transfer(_from, _to, _tokenId);

    }

Kittycontract.sol
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "./IERC721.sol";
import "./Ownable.sol";

contract Kittycontract is IERC721, Ownable {
    struct Kitty {
        uint256 genes;
        uint64 birthTime;
        uint32 mumId;
        uint32 dadId;
        uint16 generation;
    }

    event Birth(
        address owner,
        uint256 tokenId,
        uint32 dadId,
        uint32 mumId,
        uint256 genes
    );

    Kitty[] kitties;

    //Associates an EOA with its balance
    mapping(address => uint256) private balances;

    //Associates a token id with its owner
    mapping(uint256 => address) private tokenowners;

    //Approvers of a token id
    mapping(uint256 => address) private tokenApprovers;
    mapping(address => mapping(address => bool)) private ownerApprovers;

    function balanceOf(address owner) external view returns (uint256 balance) {
        return balances[owner];
    }

    function totalSupply() external view returns (uint256 total) {
        return kitties.length;
    }

    function name() external pure returns (string memory tokenName) {
        return "ScaredKitty";
    }

    function symbol() external pure returns (string memory tokenSymbol) {
        return "SCARKIT";
    }

    // Reverts if a token id does not exists?
    function ownerOf(uint256 tokenid) external view returns (address owner) {
        owner = tokenowners[tokenid];

        require(owner != address(0), "Token id does not exist");
    }

    function transfer(address to, uint256 tokenId) external {
        require(to != address(0), "invalid to address");
        require(to != address(this), "to cannot be the contract address");
        require(tokenId < kitties.length, "Invalid token id");

        address owner = tokenowners[tokenId];

        require(owner != address(0), "Token id does not exist");
        require(msg.sender == owner || tokenApprovers[tokenId] == msg.sender || ownerApprovers[owner][msg.sender] == true, "only owner/approver can make a transfer");

        _transfer(owner, to, tokenId);
    }

    function _transfer(
        address _from,
        address _to,
        uint256 _tokenId
    ) internal {
        if (_from != address(0)) {
            assert(balances[_from] > 0);

            balances[_from]--;
        }

        tokenowners[_tokenId] = _to;
        balances[_to]++;
        // Account approved on this token should be revoked
        delete tokenApprovers[_tokenId];

        emit Transfer(_from, _to, _tokenId);
    }

    function createKittyGen0(uint256 _genes) public onlyOwner {
        _createKitty(0, 0, 0, _genes, msg.sender);
    }

    function getKitty(uint256 _id)
        public
        view
        returns (
            uint256 genes,
            uint256 birthTime,
            uint32 mumId,
            uint32 dadId,
            uint16 generation
        )
    {
        require(_id < kitties.length, "Invalid cat id");

        Kitty storage kitty = kitties[_id];

        genes = kitty.genes;
        birthTime = kitty.birthTime;
        mumId = kitty.mumId;
        dadId = kitty.dadId;
        generation = kitty.generation;
    }

    function _createKitty(
        uint32 _mumId,
        uint32 _dadId,
        uint16 _generation,
        uint256 _genes,
        address _owner
    ) internal returns (uint256) {
        Kitty memory _kitty = Kitty({
            genes: _genes,
            birthTime: uint64(block.timestamp),
            mumId: _mumId,
            dadId: _dadId,
            generation: _generation
        });

        kitties.push(_kitty);
        uint256 newKittyTokenId = kitties.length - 1;

        _transfer(address(0), _owner, newKittyTokenId);

        emit Birth(_owner, newKittyTokenId, _dadId, _mumId, _genes);

        return newKittyTokenId;
    }

    function approve(address _approved, uint256 _tokenId) external{

        require(_tokenId < kitties.length, "Token id does not exist");

        //msg.sender must be the owner or an existing operator of tokenId
        address tokenOwner = tokenowners[_tokenId];

        require( (tokenOwner == msg.sender || ownerApprovers[tokenOwner][msg.sender] == true) , 'Caller is not owner of the token or an approver of the token owner'); 

        _approve(_approved, _tokenId);        

    }

    /// @notice Change or reaffirm the approved address for an NFT
    /// @dev The zero address indicates there is no approved address.
    ///  Throws unless `msg.sender` is the current NFT owner, or an authorized
    ///  operator of the current owner.
    /// @param _approved The new approved NFT controller
    /// @param _tokenId The NFT to approve
    function _approve(address _approved, uint256 _tokenId) private{

        tokenApprovers[_tokenId] = _approved;     
    }

    /// @notice Enable or disable approval for a third party ("operator") to manage
    ///  all of `msg.sender`'s assets
    /// @dev Emits the ApprovalForAll event. The contract MUST allow
    ///  multiple operators per owner.
    /// @param _operator Address to add to the set of authorized operators
    /// @param _approved True if the operator is approved, false to revoke approval
    function setApprovalForAll(address _operator, bool _approved) external{
        
        ownerApprovers[msg.sender][_operator] = _approved;

        emit ApprovalForAll(msg.sender, _operator, _approved);
    }

    /// @notice Get the approved address for a single NFT
    /// @dev Throws if `_tokenId` is not a valid NFT.
    /// @param _tokenId The NFT to find the approved address for
    /// @return The approved address for this NFT, or the zero address if there is none
    function getApproved(uint256 _tokenId) external view returns (address){
        require(_tokenId < kitties.length, "Invalid token id");

        return tokenApprovers[_tokenId];

    }

    /// @notice Query if an address is an authorized operator for another address
    /// @param _owner The address that owns the NFTs
    /// @param _operator The address that acts on behalf of the owner
    /// @return True if `_operator` is an approved operator for `_owner`, false otherwise
    function isApprovedForAll(address _owner, address _operator) external view returns (bool){
        return ownerApprovers[_owner][_operator];
    }

    /// @notice Transfers the ownership of an NFT from one address to another address
    /// @dev Throws unless `msg.sender` is the current owner, an authorized
    ///  operator, or the approved address for this NFT. Throws if `_from` is
    ///  not the current owner. Throws if `_to` is the zero address. Throws if
    ///  `_tokenId` is not a valid NFT. When transfer is complete, this function
    ///  checks if `_to` is a smart contract (code size > 0). If so, it calls
    ///  `onERC721Received` on `_to` and throws if the return value is not
    ///  `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))`.
    /// @param _from The current owner of the NFT
    /// @param _to The new owner
    /// @param _tokenId The NFT to transfer
    /// @param data Additional data with no specified format, sent in call to `_to`
    function safeTransferFrom(address _from, address _to, uint256 _tokenId, bytes calldata data) external{

        _transferFrom(_from, _to, _tokenId);

        //TBD
    }

    /// @notice Transfers the ownership of an NFT from one address to another address
    /// @dev This works identically to the other function with an extra data parameter,
    ///  except this function just sets data to "".
    /// @param _from The current owner of the NFT
    /// @param _to The new owner
    /// @param _tokenId The NFT to transfer
    function safeTransferFrom(address _from, address _to, uint256 _tokenId) external{
    }


    /// @notice Transfer ownership of an NFT -- THE CALLER IS RESPONSIBLE
    ///  TO CONFIRM THAT `_to` IS CAPABLE OF RECEIVING NFTS OR ELSE
    ///  THEY MAY BE PERMANENTLY LOST
    /// @dev Throws unless `msg.sender` is the current owner, an authorized
    ///  operator, or the approved address for this NFT. Throws if `_from` is
    ///  not the current owner. Throws if `_to` is the zero address. Throws if
    ///  `_tokenId` is not a valid NFT.
    /// @param _from The current owner of the NFT
    /// @param _to The new owner
    /// @param _tokenId The NFT to transfer
    function transferFrom(address _from, address _to, uint256 _tokenId) external{
        _transferFrom(_from, _to, _tokenId);
    }    

    function _transferFrom(address _from, address _to, uint256 _tokenId) private{
        //msg.sender must be the owner or an existing operator of tokenId
        require(_to != address(0), "invalid _to address");
        require(_tokenId < kitties.length, "Invalid token id");

        address tokenOwner = tokenowners[_tokenId];
        require(_from == tokenOwner, "_from should be the token owner");

        address approver = tokenApprovers[_tokenId];    
        require( (tokenOwner == msg.sender 
        || ownerApprovers[tokenOwner][msg.sender] == true
        || approver == msg.sender) , 'Caller is not owner of the token or an operator of the token owner or an approver of the token'); 

        _transfer(_from, _to, _tokenId);

    }

}

I have also included some unit-tests:

1_approval_test.js
const Kitty = artifacts.require("Kittycontract");
const truffleAssert = require("truffle-assertions");


contract("Test approve function", accounts =>{

    // State is not reset in-between this sequence of tests therefore fix them in sequence and add new tests at the end

    it("only an owner of a token id should be able to nominate approver", async()=>{

        let kitty = await Kitty.deployed();

        await kitty.createKittyGen0(123);
        let tokenId = await kitty.totalSupply()-1;

        assert(kitty.getApproved(tokenId) != accounts[1], accounts[1] + " should not be an approver");

        await truffleAssert.reverts(
            kitty.approve(accounts[1], tokenId, {from: accounts[2]})    
        )

        await truffleAssert.passes(
            kitty.approve(accounts[1], tokenId)    
        )

        let approver = await kitty.getApproved(tokenId);

        assert(approver == accounts[1], "getApproved failed");

    })


    it("an approver of the current token should not be able to call approve", async()=>{
        let kitty = await Kitty.deployed()

        await kitty.createKittyGen0(123);
        let tokenId = await kitty.totalSupply()-1

        await kitty.approve(accounts[3], tokenId, {from: accounts[0]})
        let approver = await kitty.getApproved(tokenId)
        assert(approver == accounts[3], "Pre-requisite: Account 3 should be approver of token " + tokenId)

        let isOperator = await kitty.isApprovedForAll(accounts[0], accounts[3])     
        assert(isOperator == false, "account 3 should not be operator of the token owner")   

        await truffleAssert.reverts(
            kitty.approve(accounts[4], tokenId, {from: accounts[3]})
        )

    })


    it("The approver of a token should be removed when the token is transferred", async()=>{
        let kitty = await Kitty.deployed()

        await kitty.createKittyGen0(123);
        let tokenId = await kitty.totalSupply()-1

        await kitty.approve(accounts[3], tokenId)
        let tokenOperator = await kitty.getApproved(tokenId)
        assert(tokenOperator == accounts[3], "Account 3 should be approved for token id " + tokenId)

        // EOA 0 transfers the token to EOA 4. EOA 3 should not be approved for token any more
        await kitty.transfer(accounts[4], tokenId)

        tokenOperator = await kitty.getApproved(tokenId)
        assert(tokenOperator == 0, tokenOperator + " should not be approved for token 0 any more")
    })
})


contract("Test setApproveForAll function", accounts =>{
    it("isApprovedForAll should reflect changes made with setApprovedForAll", async()=>{
        let kitty = await Kitty.deployed();     
        
        let approved = await kitty.isApprovedForAll(accounts[0], accounts[1]);
        assert(approved == false, "account 1 should not be an approver of account 0")

        // account 0 sets account 1 as one of the approvers
        await kitty.setApprovalForAll(accounts[1], true, {from: accounts[0]});

        approved = await kitty.isApprovedForAll(accounts[0], accounts[1]);
        assert(approved == true, "account 1 should be an approver of account 0");
    })

    it("A named approver should be able to transfer an owner's asset", async()=>{
        let kitty = await Kitty.deployed();

        // EOA 0 owns token id 0
        await kitty.createKittyGen0(123, {from: accounts[0]})

        // EOA 2 is approver of EOA 0
        await kitty.setApprovalForAll(accounts[2], true, {from: accounts[0]})

        let approved = await kitty.isApprovedForAll(accounts[0], accounts[2])
        assert(approved == true, "Account 2 should be approver of account 0")


        // EOA 2 should be able to transfer token id 0 to another address
        await truffleAssert.passes(
            kitty.transfer(accounts[3], 0, {from: accounts[2]})
        )

        let owner = await kitty.ownerOf(0)
        assert(owner == accounts[3], "Account 3 should be the new owner")

    })

    it("an authorized operator of the current owner should be able to call approve on an owner's token", async()=>{
        let kitty = await Kitty.deployed()

        let tokenId = await kitty.totalSupply()-1
        let owner = await kitty.ownerOf(tokenId)        
        assert(owner == accounts[3], "Pre-requisite: Account 3 should be owner of token " + tokenId)

        await kitty.setApprovalForAll(accounts[4], true, {from: accounts[3]})
        let approved = await kitty.isApprovedForAll(accounts[3], accounts[4])
        assert(approved == true, "Pre-requisite: Account 4 should be operator of Account 3")

        // Account 4 should be able to approve Account 5
        await truffleAssert.passes(
            kitty.approve(accounts[5], tokenId, {from: accounts[4]})
        )

        let approver = await kitty.getApproved(tokenId)
        assert(approver == accounts[5], "Account 5 should be approver of token " + tokenId)

    })

//     it("should emit ApprovalForAll event", async()=>{
//         let kitty = await Kitty.deployed();

//         // EOA 0 owns token id 0
//         await kitty.createKittyGen0(123)

//         await kitty.setApprovalForAll(accounts[1], true, {from: accounts[0]})



//         // EOA 1 is approver of EOA 0
//         await truffleAssert.eventEmitted(
//             kitty.setApprovalForAll(accounts[1], true, {from: accounts[0]})
//         )

//    })


})


2_transfer_test.js
const Kitty = artifacts.require('Kittycontract');
const truffleAssert = require('truffle-assertions');


contract("Test transferFrom function", accounts =>{
    it("should throw if msg.sender is not owner, operator or token approver", async()=>{
        let kitty = await Kitty.deployed()

        await kitty.createKittyGen0(123)
        
        let owner = await kitty.ownerOf(0)
        assert(owner != accounts[1], "Pre-requisite: msg.sender is not the owner")
        
        let operator = await kitty.isApprovedForAll(owner, accounts[1])
        assert(operator == false, "Pre-requisite: msg.sender is not operator")

        let approver = await kitty.getApproved(0)
        assert(approver != accounts[1], "Pre-requisite: msg.sender is not approver")

        await truffleAssert.reverts(
            kitty.transferFrom(accounts[0], accounts[2], 0, {from: accounts[1]})
        )
    })


    it("should pass if msg.sender is owner", async()=>{
        let kitty = await Kitty.deployed()

        await kitty.createKittyGen0(123)
        let tokenid = await kitty.totalSupply()-1

        await truffleAssert.passes(
            kitty.transferFrom(accounts[0], accounts[1], tokenid, {from: accounts[0]}),
            "should pass if msg.sender is the owner"
        )

        let newOwner = await kitty.ownerOf(tokenid)
        assert(newOwner == accounts[1], "Ownership should have been transferred to account1")

    })

    it("should pass if msg.sender is operator ", async()=>{
        let kitty = await Kitty.deployed()

        await kitty.createKittyGen0(123)
        let tokenid = await kitty.totalSupply()-1

        await kitty.setApprovalForAll(accounts[1], true)

        await truffleAssert.passes(
            kitty.transferFrom(accounts[0], accounts[2], tokenid, {from: accounts[1]})
        )

        let newOwner = await kitty.ownerOf(tokenid)
        assert(newOwner == accounts[2], "Ownership should have been transferred to account2")

    })

    it("should pass if msg.sender in an approver of a token id", async()=>{
        let kitty = await Kitty.deployed();

        await kitty.createKittyGen0(123)
        let tokenId = await kitty.totalSupply()-1;

        await kitty.approve(accounts[3], tokenId)
        let approver = await kitty.getApproved(tokenId)
        assert(approver == accounts[3], "Pre-requisite: account3 should be approver of token id "  + tokenId)

        let isOperator = await kitty.isApprovedForAll(accounts[0], accounts[3])
        assert(isOperator == false, "Pre-requisite: Account3 is not operator of account0")

        // EOA3 is approver of token id and should be able to transfer it to someone else 
        await truffleAssert.passes(
            kitty.transferFrom(accounts[0], accounts[4], tokenId, {from: accounts[3]})
        )

        let newOwner = await kitty.ownerOf(tokenId)
        assert(newOwner == accounts[4], "Ownership should have been transferred to account4")

    })

    it("should throw if _from is not the current owner", async()=>{
        let kitty = await Kitty.deployed();

        await kitty.createKittyGen0(123)
        let tokenId = await kitty.totalSupply()-1;

        await truffleAssert.reverts(
            kitty.transferFrom(accounts[1], accounts[4], tokenId, {from: accounts[0]})
        )

    })    

    it("should fail if _to is the 0 address", async()=>{
        let kitty = await Kitty.deployed();

        await kitty.createKittyGen0(123)
        let tokenId = await kitty.totalSupply()-1;

        await truffleAssert.fails(
            // the ABI raises an error
            kitty.transferFrom(accounts[0], 0, tokenId, {from: accounts[0]})
        )
    })

})




The full project is on github:
https://github.com/CodingInLondon/moralisacademy-nftmarketplace

Cheers,
Matt

1 Like