Assignment - ERC721

1. What is the time complexity of creating new cats? (constant or linear w nr of cats).
Creating 1 cat costs: 115.603 in gas.
The second cat costs: 85.603.
The third cat costs: 85.603.
The fourth cat costs: 85.603.
So this is constant except for creating it because we need to make up space in the blockchain to put the kitties in. But every time a new kitty is added the same computing power is used.

2. What is the time complexity of the getAllCatsFor function? (constant or linear w nr of cats).
Calling this function once costs: 12.479.
When the function is called again it costs: 14.898.
When the function is called again it costs: 17.317.
Wich is a difference of 2.419 every time a cat is added and this function gets called.
So this is linear.

3. How could the storage design be changed to make the getAllCats function constant? Discuss benefits and drawbacks with your implementation.

pragma solidity ^0.8.0;

contract Kittycontract {

    string public constant name = "TestKitties";
    string public constant symbol = "TK";
    
    event Transfer(address indexed from, address indexed to, uint256 indexed tokenId);

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

    struct Kitty {
        uint256 genes;
        uint64 birthTime;
        uint32 mumId;
        uint32 dadId;
        uint16 generation;
    }

    Kitty[] kitties;

    mapping (uint256 => address) public kittyIndexToOwner;
    
    //change the original mapping to a mapping with the ids of the cats that a address owns.
    mapping (address => uint256[]) OwnedCats;

    

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

    function totalSupply() public view returns (uint) {
        return kitties.length;
    }

    function ownerOf(uint256 _tokenId) external view returns (address)
    {
        return kittyIndexToOwner[_tokenId];
    }

    function transfer(address _to,uint256 _tokenId) external
    {
        require(_to != address(0));
        require(_to != address(this));
        require(_owns(msg.sender, _tokenId));

        _transfer(msg.sender, _to, _tokenId);
    }
    
    function getAllCatsFor(address _owner) external view returns (uint[] memory cats){
        //return directly the mapping of the caller in the function.
        cats = OwnedCats[_owner];
        }
    
    function createKittyGen0(uint256 _genes) public returns (uint256) {
        return _createKitty(0, 0, 0, _genes, msg.sender);
    }

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

        uint256 newKittenId = kitties.length - 1;

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

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

        return newKittenId;

    }

    function _transfer(address _from, address _to, uint256 _tokenId) internal {
        
        // maping the cat of the owner.
        kittyIndexToOwner[_tokenId] = _to;
        
        //push the id of the cat to the list if ids for the user.
        OwnedCats[_to].push(_tokenId);
        
        //remove the id of the cat from the list of the previous order if it is not a new one. 
        if (_from != address(0)) {
            _removeOwnership(_from, _tokenId);
        }

        // Emit the transfer event.
        emit Transfer(_from, _to, _tokenId);
    }
    
    //remove function to remove the cat from the previous owner list.
    function _removeOwnership(address _owner, uint256 _tokenId) internal {
        //loop through the owned cats of the given address
        for (uint i = 0; i < OwnedCats[_owner].length; i++) {
            
            //if the cat/nft was not found
            if(OwnedCats[_owner][i] == _tokenId) {
                //replace the cat by copying the last cat to this position
                OwnedCats[_owner][i] = OwnedCats[_owner][OwnedCats[_owner].length-1];
                
                //this removes the duplicate
                OwnedCats[_owner].pop();
                
                // and lastly return the function to save on gas 
                return;
            }
        }
    }

    function _owns(address _claimant, uint256 _tokenId) internal view returns (bool) {
      return kittyIndexToOwner[_tokenId] == _claimant;
  }

}

pros:
The function GetAllCatsFor now saves a lot of gas because it is constant it doesn’t have to loop over the whole array.

cons
this brings another linear function but this is less gas costly then the other function.

1 Like

1.Constant, a cat will be created no matter the volume.
2.Linear, because we are looping through the array.
3. You could index the cat based on the owner address

pragma solidity ^0.8.0;

contract Kittycontract {

    string public constant name = "TestKitties";
    string public constant symbol = "TK";
    
    event Transfer(address indexed from, address indexed to, uint256 indexed tokenId);

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

    struct Kitty {
        uint256 genes;
        uint64 birthTime;
        uint32 mumId;
        uint32 dadId;
        uint16 generation;
    }

    Kitty[] kitties;

    mapping (uint256 => address) public kittyIndexToOwner;
    mapping (address => uint256) ownershipTokenCount;

    

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

    function totalSupply() public view returns (uint) {
        return kitties.length;
    }

    function ownerOf(uint256 _tokenId) external view returns (address)
    {
        return kittyIndexToOwner[_tokenId];
    }

    function transfer(address _to,uint256 _tokenId) external
    {
        require(_to != address(0));
        require(_to != address(this));
        require(_owns(msg.sender, _tokenId));

        _transfer(msg.sender, _to, _tokenId);
    }
    
    function getAllCatsFor(address _owner) external view returns (uint[] memory cats){
        //Solution
cats = catsOwned[_owner];
    }
    
    function createKittyGen0(uint256 _genes) public returns (uint256) {
        return _createKitty(0, 0, 0, _genes, msg.sender);
    }

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

        uint256 newKittenId = kitties.length - 1;

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

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

        return newKittenId;

    }

    function _transfer(address _from, address _to, uint256 _tokenId) internal {
        ownershipTokenCount[_to]++;

        kittyIndexToOwner[_tokenId] = _to;

        if (_from != address(0)) {
            ownershipTokenCount[_from]--;
        }

        // Emit the transfer event.
        emit Transfer(_from, _to, _tokenId);
    }

    function _owns(address _claimant, uint256 _tokenId) internal view returns (bool) {
      return kittyIndexToOwner[_tokenId] == _claimant;
  }

}

One major benefit would be a major reduction in gas price by calling a specific address instead of looping through an entire array. Although having a constant function that is also view(free), you would still need to create a linear function for deleting a cat.

1 Like

1. What is the time complexity of creating new cats? (constant or linear w nr of cats).
It is constant. This is because the _createKitty() which is a private function that takes arguments from the caller and creates a new kitty and pushes it to the end of the array. So, if other arguments are passed to create a new cat, it does exactly the same thing i.e create and push to the end of the array.
2. What is the time complexity of the getAllCatsFor function? (constant or linear w nr of cats).
This is Linear, as it depends on the length of the array. If we have just one cat in the array we get one back if we have 10 we get 10 baack and so on.
3. How could the storage design be changed to make the getAllCats function constant?
The storage design can be changed by mapping from an Address to an Array to make the time complexity constant as shown below.

mapping (address => uint256[]) catsOwned;
function getAllCatsFor(address _owner) external view returns (uint256[] memory cats) {
    cats = catsOwned[_owner];
}

Discuss benefits and drawbacks with your implementation.
The benefits are, (a) Quick Look Ups, input an address into the mapping you get an array of cats owned by that address. (b) Gas cost savings.
Drawbacks: The complexity of removing a cat from the array of cats owned by an address through the use of another function that has to loop through the array.

3 Likes
  1. What is the time complexity of creating new cats? (constant or linear w nr of cats).
    Constant because we use the push() function that appends any new cat created to the end of the array.

  2. What is the time complexity of the getAllCatsFor function? (constant or linear w nr of cats).
    Using the getAllCatsFor() function could result in linear time complexity because as time goes by, the length of the array would be added incrementally by many users.

  3. How could the storage design be changed to make the getAllCats function constant? Discuss benefits and drawbacks with your implementation.
    We could use double mapping to ownershipTokenCount mapping as follow:

mapping (address => (uint256 => address)) ownershipTokenCount;
1 Like

MY ANSWERS:

  1. Constant amount of time complexity (and therefore gas) to create a new cat.
    This is because there are only direct access into mappings and pushing new cat
    to an array. There is no looping that depends upon the number of cats already
    created.

  2. The getAllCatsFor function has a linearly increasing amount of operations
    because the code needs to iterate through the whole array of cats. The more
    cats that exist the bigger this array and the more iterations are required to
    check each cat to see if it belongs to the specified owner (so linera time complexity).
    If this view function was called externally, the gas cost would also increase linearly
    with the number of cats.

  3. The getAllCatsFor function could have its time complexity reduced by introducing
    a new mapping that links an address to an array of owned cat ids. In this way, the
    function would not have to iterate through the array of all cats and its time complexity
    (and if external called, the gas cost) would become constant. If the number of cats in
    existence is very large this function would become inefficient.

See KittyContract2 for my solution. Where I’ve introduced a new mapping:
mapping (address => uint256[]) ownersCats;
And then maintained this mapping in the _transfer function. This allows my
amended version of the getAllCatsFor function to simply return the array of
cat ids (by a simple lookup in the ownersCats mapping).
The disadvantage is that the _transfer function, upon a cat being transferred
from an existing owner to a new owner, now needs to loop through the array of the
transferor’s cats to remove the tokenId from their array of owned cats (in ownersCats
mapping). It does this by calling a new function _removeCat. Whilst this makes the
_transfer function execution somewhat non-constant, it only needs to iterate
through the previous owner’s cats - so the execution cost is will linearly increase
but this will be based on the number of cats owned by the transferor (not the total
number of cats). However, this is not a view function and so the gas prices would be
higher for all users of the _transfer function - which may be more of a negative than the
benefit we gained with the getAllCatsFor function!!

I’m now thinking there could be an even better way by using a double mapping…
I will think about this further! Hrmmm…

In any case, my solution code is here, in the KittyContract2.sol file:
https://github.com/msuscens/Smart-Contract-Programming-201/tree/master/ERC721%20Assignment

1 Like
  1. What is the time complexity of creating new cats? (constant or linear w nr of cats).
    O(1) since regardless of how many are currently in the array, youre just appending to the end of the array
  2. What is the time complexity of the getAllCatsFor function? (constant or linear w nr of cats).
    O(n) since you iterate through every element in the array
  3. How could the storage design be changed to make the getAllCats function constant? Discuss benefits and drawbacks with your implementation.
    We could check after each loop through the for loop, whether the counter is equal to the number of tokens that the address holds (ownershipTokenCount[_owner] since once we reach that threshold, it would be redundant to continue iterating through the for loop
    function getAllCatsFor(address _owner) external view returns (uint[] memory cats){
        uint[] memory result = new uint[](ownershipTokenCount[_owner]);
        uint counter = 0;
        for (uint i = 0; i < kitties.length; i++) {
            if (kittyIndexToOwner[i] == _owner) {
                result[counter] = i;
                counter++;
            }
            if(counter>=ownershipTokenCount[_owner]){
                break;
            }
        }
        return result;
    }
1 Like

Hello everyone.

I’m having a hard time with getting a runtime error to cease. Below are the answers to the assignment prompts, and then I’ll post my code and the error I’m getting. I’ve changed so many things here and there with the code to try to make it happy that I need some guidance.

  1. The _createKitty function has a constant time complexity. It doesn’t have to iterate through any dynamically-sized structures to accomplish it’s purpose, so it takes the same amount of time each time it’s executed.

  2. The getAllCatsFor function execution time depends on the value of kitties.length for a particular user. Therefore, this function has a linearly increasing time constant with respects to number of cats owned by a user.

  3. Ok, here’s where I get stumped. What I’m trying to implement includes a new array of structs called MasterIndex. MasterIndex is an array of struct type MyCats. The contents of the MyCats struct is an array containing the indexes of all of the cats currently owned by MyCats[userId - 1].

As new users are introduced into the system, they’re given unique user IDs, which are stored in a mapping tied to the new user’s ethereum address. The mapping can be used to access a users unique ID number, and then used by the MasterIndex array to pull up a list of all of the indexes of the currently-owned cats. Below is the runtime error and the code. Please advise, and thank you!
(Getting back into coding is super exciting. Ha, can’t wait to get past this error! :joy: )

[vm]

from: 0x4B2…C02db

to: Kittycontract.createKittyGen0(uint256) 0x9dA…Defe6

value: 0 wei

data: 0xd5a…00020

logs: 0

hash: 0x615…3591a

Debug

transact to Kittycontract.createKittyGen0 errored: VM error: revert. revert The transaction has been reverted to the initial state. Note: The called function should be payable if you send value and the value you send should be less than your current balance. Debug the transaction to get more information.

pragma solidity ^0.8.0;

contract Kittycontract {
//maybe create mapping from address to an array of indexes of owned kittens.  Drawback is that it makes the constant time complexity
//of transfer function longer with individual cat collections due to the new remove_from_ownership function


    string public constant name = "TestKitties";
    string public constant symbol = "TK";
    uint public numberOfUsers;
    
    event Transfer(address indexed from, address indexed to, uint256 indexed tokenId);

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

    struct Kitty {
        uint256 genes;
        uint64 birthTime;
        uint32 mumId;
        uint32 dadId;
        uint16 generation;
    }

    struct MyCats{
        uint[] ownedCatsIndex;
    }

    Kitty[] kitties;
    
    MyCats[] MasterIndex;  //An array containing lists of all users cats, indexed by userId
    
    mapping (address => uint) public userId;  //if during the create function the address doesn't point to an existing user, create a new ID for them

    mapping (address => MyCats) userCats;   //mapping that points addresses to an array of indexes of owned cats.  
    
    mapping (uint256 => address) public kittyIndexToOwner; //do kittyIndexToOwner[_index] = _to; makes mapping map to new owner address

    mapping (address => uint256) ownershipTokenCount;

    

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

    function totalSupply() public view returns (uint) {
        return kitties.length;
    }

    function ownerOf(uint256 _tokenId) external view returns (address)
    {
        return kittyIndexToOwner[_tokenId];
    }

    function transfer(address _to,uint256 _tokenId) external
    {
        require(_to != address(0));
        require(_to != address(this));
        require(_owns(msg.sender, _tokenId));

        _transfer(msg.sender, _to, _tokenId);
    }
    
    function getAllCatsFor(address _owner) public view returns(MyCats memory){//external removed for debugging, public added.  need to understand why this has to happen
        return userCats[_owner];

        //time complexity of getAllCatsFor function is linearly proportional to number of cats
//        uint[] memory result = new uint[](ownershipTokenCount[_owner]);
//        uint counter = 0;
//        for (uint i = 0; i < kitties.length; i++) {
//            if (kittyIndexToOwner[i] == _owner) {
//                result[counter] = i;
//                counter++;
//            }
//        }
//        return result;
    }
    
    function createKittyGen0(uint256 _genes) public returns (uint256) {
        return _createKitty(0, 0, 0, _genes, msg.sender);
    }

    function _createKitty( 
    //time complexity of _createKitty function is constant
        uint256 _mumId,
        uint256 _dadId,
        uint256 _generation,
        uint256 _genes,
        address _owner
    ) private returns (uint256) {
        Kitty memory _kitty = Kitty({
            genes: _genes,
            birthTime: uint64(block.timestamp),
            mumId: uint32(_mumId),
            dadId: uint32(_dadId),
            generation: uint16(_generation)
        });
        
        kitties.push(_kitty);

        uint256 newKittenId = kitties.length - 1;

        emit Birth(_owner, newKittenId, _mumId, _dadId, _genes);
    
        //if our user index mapping has no value for the owner address, we'll create one now
        if(userId[_owner] == 0){
            numberOfUsers ++;
            userId[_owner] = numberOfUsers;
            //map ID to MasterIndex array
            MyCats memory new_user;//make separate function for establishing new user
            new_user.ownedCatsIndex[0] = newKittenId;
            MasterIndex.push(new_user);
        }
        _transfer(address(0), _owner, newKittenId);

        return newKittenId;

    }
    function show_Cust_ID(address cust) public view returns(uint){
        return userId[cust];
    }
    function _transfer(address _from, address _to, uint256 _tokenId) internal {  //with the modified storage design the transfer functions time complexity 
        //becomes a function of how many cats the transferring party already owns in their array
        //we know the _from address will have a userID, but if the receipient doesn't, we'll get them into the system now
        if(userId[_to] == 0){
            numberOfUsers ++;
            userId[_to] = numberOfUsers;
            MyCats memory new_user;//make separate function for establishing new user
            new_user.ownedCatsIndex[0] = _tokenId;
            MasterIndex.push(new_user);
        }
        //now we need to set the users MyCats struct with the new cat-owner in the master index, and the
        //remove the cat from the _from address
        
        give_new_owner_cat(_to, _tokenId);
        remove_from_ownership(_from, _tokenId);
        
        //still need to update the other mappings        
        ownershipTokenCount[_to]++;

        kittyIndexToOwner[_tokenId] = _to;

        if (_from != address(0)) {
            ownershipTokenCount[_from]--;
        }

        // Emit the transfer event.
        emit Transfer(_from, _to, _tokenId);
    }

    function give_new_owner_cat(address _to, uint _tokenId) internal returns(bool){
        MasterIndex[userId[_to] -1].ownedCatsIndex.push(_tokenId);
        return true;
    }

    function remove_from_ownership(address _from, uint _tokenId) internal returns(bool){
        uint collectionSize = MasterIndex[userId[_from] -1].ownedCatsIndex.length;
//stopped working after I made getAllCatsFor return MyCats        uint collectionSize = getAllCatsFor(_from).ownedCatsIndex.length;
        uint i = 0;
        while(i <= collectionSize){
            if(MasterIndex[userId[_from] -1].ownedCatsIndex[i] == _tokenId){
                //copy last item of array into spot currently holding _tokenId and then pop array
                MasterIndex[userId[_from]].ownedCatsIndex[i] = MasterIndex[userId[_from]].ownedCatsIndex[collectionSize - 1];
                MasterIndex[userId[_from]].ownedCatsIndex.pop;
            }
            i++;
        }
        uint updated_collection_size = getAllCatsFor(_from).ownedCatsIndex.length;
        if(collectionSize -1 == updated_collection_size){
            return true;
        }
        return false;
    }
    
    function _owns(address _claimant, uint256 _tokenId) internal view returns (bool) {
      return kittyIndexToOwner[_tokenId] == _claimant;
  }

}

//had to change the getAllCatsFor function to public to get my give_new_owner_cat and remove_from_ownership functions to work

1 Like
    function getAllCatsFor(address _owner) external view returns (uint[] memory cats){
        uint[] memory result = new uint[](ownershipTokenCount[_owner]);
        uint counter = 0;
        for (uint i = 0; i < kitties.length; i++) {
            if (kittyIndexToOwner[i] == _owner) {
                result[counter] = i;
                counter++;
            }
            if(counter>=ownershipTokenCount[_owner]){
                break;
            }
        }
        return result;
    }

1: function itself has the time complexity O(1) because createKittyGen0 itself is private.

2: You need to check all the positions in the array so therefore its time complexity is 0(n).

3: Double mapping would be an option. ie) mapping (address => (uint256 => address)) ownershipTokenCount;

1 Like
  1. The time complexity of creating new cats is constant. Each cration of a cat is similar, using the same parameters.
  2. The time complexity of the GetAllCatsFor function is linear, as we loop through the array and read data from it.
  3. To make getAllCats function constant I tried to write it as the MappedStructWithIndex was written (inthe Storage costs section) as I cannot come to another solution with my knowledge. The getAddAddressCount will return the array.
    I will only post code til the balanceOf() function of the sample given - till I made changes.
    However I got errors here, but it’s the best I was able to do.

Kittycontract.sol

pragma solidity ^0.8.0;

contract Kittycontract {

    string public constant name = "TestKitties";
    string public constant symbol = "TK";
    
    event Transfer(address indexed from, address indexed to, uint256 indexed tokenId);

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

    struct Kitty {
        uint256 genes;
        uint64 birthTime;
        uint32 mumId;
        uint32 dadId;
        uint16 generation;
        uint256 listPointer;
    }

    Kitty[] kitties;

    mapping (uint256 => address) public kittyIndexToOwner;
    mapping (address => uint256) ownershipTokenCount;


    mapping (address => Kitty) ownershipOfKitties;

    address[] public ownersList;
    
    function AddAddressToOwnerList(address _owner, uint256) public returns(bool success){
        if(isAddressEntered(_owner)) revert();
        ownershipOfKitties[_owner].genes = genes;
        ownershipOfKitties[_owner].birthTime = birthTime;
        ownershipOfKitties[_owner].mumId = mumId;
        ownershipOfKitties[_owner].dadId = dadId;
        ownershipOfKitties[_owner].generation = generation;
        ownersList.push(_owner);
        ownershipOfKitties[_owner].listPointer == ownerslist.length - 1;
        return true;
    }
    
    
    function isAddressEntered(address _owner) public view returns (bool entered){
        if(ownersList.length == 0) return false;
        return (ownerslist[ownershipOfKitties[_owner].listPointer == _owner]);
    }
    
    function getAddAddressCount() public view returns(uint getAddAddressCount){
        return ownersList.lengh;
    }

1 Like

Hey @Melshman, hope you are well.

The revert error could be triggered by one of your require statements, I advice you to assign an error message on each of them so you can know which one is being triggered.

Example:

    function transfer(address _to,uint256 _tokenId) external
    {
        require(_to != address(0), "address cant be zero");
        require(_to != address(this), "cant transfer to contract address");
        require(_owns(msg.sender, _tokenId), "is not token owner");

        _transfer(msg.sender, _to, _tokenId);
    }

Basically some kind of message that help you identify easily what condition is not correct.

Also would be great to know when this error is being show, at deployment or compiling?

Which commands did you use, or maybe you can share the unit test, could be a typo error.

Carlos Z

1 Like
  1. Creating new cats is a constant because the newborn cat is created the same way on the contract that cannot be changed. This is a very simple function as far as time because the information to create a cat must be held on the contract as the same constant procedure.

2.The getAllCatsFor function is a linear time complexity because the function requires the search through a mapping using a loop. The number is ever-changing as long as new cats are born. This in turn affects the time it takes to loop through the mapping as it gets bigger.

  1. Coding Solution:
pragma solidity ^0.8.0;

contract Kittycontract {

    string public constant name = "TestKitties";
    string public constant symbol = "TK";
    
    event Transfer(address indexed from, address indexed to, uint256 indexed tokenId);

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

    struct Kitty {
        uint256 genes;
        uint64 birthTime;
        uint32 mumId;
        uint32 dadId;
        uint16 generation;
    }

    Kitty[] kitties;

    mapping (uint256 => address) public kittyIndexToOwner;
    mapping (address => uint256[]) ownershipTokenCount;

    

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

    function totalSupply() public view returns (uint) {
        return kitties.length;
    }

    function ownerOf(uint256 _tokenId) external view returns (address)
    {
        return kittyIndexToOwner[_tokenId];
    }

    function transfer(address _to,uint256 _tokenId) external
    {
        require(_to != address(0));
        require(_to != address(this));
        require(_owns(msg.sender, _tokenId));

        _transfer(msg.sender, _to, _tokenId);
    }
    
    function getAllCatsFor(address _owner) external view returns (uint[] memory cats){
        cats = ownershipTokenCount[_owner];
    }
    
    function createKittyGen0(uint256 _genes) public returns (uint256) {
        return _createKitty(0, 0, 0, _genes, msg.sender);
    }

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

        uint256 newKittenId = kitties.length - 1;

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

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

        return newKittenId;

    }

    function _transfer(address _from, address _to, uint256 _tokenId) internal {
        ownershipTokenCount[_to].push(_tokenId);

        kittyIndexToOwner[_tokenId] = _to;

        if (_from != address(0)) {
            _deleteCat(_from, _tokenId);
        }

        // Emit the transfer event.
        emit Transfer(_from, _to, _tokenId);
    }

    function _owns(address _claimant, uint256 _tokenId) internal view returns (bool) {
      return kittyIndexToOwner[_tokenId] == _claimant;
    }
    
    function _deleteCat(address _owner, uint256 _tokenId) internal {
        for(uint n = 0; n < ownershipTokenCount[_owner].length; ++n){
            if(ownershipTokenCount[_owner][n] == _tokenId){
                ownershipTokenCount[_owner][n] = ownershipTokenCount[_owner][ownershipTokenCount[_owner].length-1];
                ownershipTokenCount[_owner].pop();
                return;
            }
        }
        
    }

}

Changing the mapping to point to an array of the number of cats owned rather than single catIDs can make the function a constant one instead of linear as the loop is eliminated when calling it. However, this just prolongs the problem as a linear function is added later to remove an id from the array in the mapping.

1 Like

After reviewing the answer by Filip I also failed to mention that the changes with making the getAllCatsFor function is pointless in the fact that it is a view function that doesn’t need to be recorded on the blockchain and cost gas while making the transfer function that does much more complicated.

1 Like
  1. Constant
  2. Linear
  3. We can add another mapping(address => uint[]) that tracks all the specific tokens that a certain address holds. The benefit would be that it makes the getAllCats function constant. But drawbacks are we are writing more to storage, and it makes transferring cats(and deleting them from previous owners array) more complex.
pragma solidity ^0.8.0;

contract Kittycontract {

    string public constant name = "TestKitties";
    string public constant symbol = "TK";
    
    event Transfer(address indexed from, address indexed to, uint256 indexed tokenId);

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

    struct Kitty {
        uint256 genes;
        uint64 birthTime;
        uint32 mumId;
        uint32 dadId;
        uint16 generation;
        
    }

    Kitty[] kitties;

    mapping (uint256 => address) public kittyIndexToOwner;
    mapping (address => uint256) ownershipTokenCount;
    
    mapping (address => uint256[]) ownerToCats;
    

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

    function totalSupply() public view returns (uint) {
        return kitties.length;
    }

    function ownerOf(uint256 _tokenId) external view returns (address)
    {
        return kittyIndexToOwner[_tokenId];
    }

    function transfer(address _to,uint256 _tokenId) external
    {
        require(_to != address(0));
        require(_to != address(this));
        require(_owns(msg.sender, _tokenId));

        _transfer(msg.sender, _to, _tokenId);
    }
    
    function getAllCatsFor(address _owner) external view returns (uint[] memory cats){
        return ownerToCats[_owner];
    }
    
    function createKittyGen0(uint256 _genes) public returns (uint256) {
        return _createKitty(0, 0, 0, _genes, msg.sender);
    }

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

        uint256 newKittenId = kitties.length - 1;

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

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

        return newKittenId;

    }

    function _transfer(address _from, address _to, uint256 _tokenId) internal {
        ownershipTokenCount[_to]++;

        kittyIndexToOwner[_tokenId] = _to;
        ownerToCats[_to].push(_tokenId);

        if (_from != address(0)) {
            ownershipTokenCount[_from]--;
            _removeTokenIdFromOwner(_from, _tokenId);
        }

        // Emit the transfer event.
        emit Transfer(_from, _to, _tokenId);
    }
    
    function _removeTokenIdFromOwner(address _owner, uint256 _tokenId) internal{
        uint lastTokenId = ownerToCats[_owner][ownerToCats[_owner].length - 1];
        for (uint i = 0; i < ownerToCats[_owner].length; i++){
            if (ownerToCats[_owner][i]== _tokenId){
                ownerToCats[_owner][i] = lastTokenId;
                ownerToCats[_owner].pop();

            }
        }
    }

    function _owns(address _claimant, uint256 _tokenId) internal view returns (bool) {
      return kittyIndexToOwner[_tokenId] == _claimant;
  }

}
1 Like
  1. Constant
  2. Linear
pragma solidity ^0.8.0;

contract Kittycontract {

    string public constant name = "TestKitties";
    string public constant symbol = "TK";
    
    event Transfer(address indexed from, address indexed to, uint256 indexed tokenId);

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

    struct Kitty {
        uint256 genes;
        uint64 birthTime;
        uint32 mumId;
        uint32 dadId;
        uint16 generation;
    }

    Kitty[] kitties;

    mapping (uint256 => address) public kittyIndexToOwner;
    mapping (address => uint256) ownershipTokenCount;
    // mapping (address => mapping(uint256 => bool)) ownershipToken;  // owner address => (kitty id => true / false) )
    mapping (address => uint256[]) ownershipToken;  

    

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

    function totalSupply() public view returns (uint) {
        return kitties.length;
    }

    function ownerOf(uint256 _tokenId) external view returns (address)
    {
        return kittyIndexToOwner[_tokenId];
    }

    function transfer(address _to,uint256 _tokenId) external
    {
        require(_to != address(0));
        require(_to != address(this));
        require(_owns(msg.sender, _tokenId));

        _transfer(msg.sender, _to, _tokenId);
    }
    
    function getAllCatsFor(address _owner) external view returns (uint[] memory cats){
        return ownershipToken[_owner];
    }
    
    function createKittyGen0(uint256 _genes) public returns (uint256) {
        return _createKitty(0, 0, 0, _genes, msg.sender);
    }

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

        uint256 newKittenId = kitties.length - 1;

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

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

        return newKittenId;

    }

    function _transfer(address _from, address _to, uint256 _tokenId) internal {
        ownershipTokenCount[_to]++;
        ownershipToken[_to].push(_tokenId);

        kittyIndexToOwner[_tokenId] = _to;

        if (_from != address(0)) {
            ownershipTokenCount[_from]--;
            for(uint256 i = 0; i<ownershipToken[_from].length; i++) {
                if(ownershipToken[_from][i] == _tokenId) {
                    ownershipToken[_from][i] = ownershipToken[_from][ownershipToken[_from].length - 1];
                    ownershipToken[_from].pop();
                    break;
                }
            }
        }

        // Emit the transfer event.
        emit Transfer(_from, _to, _tokenId);
    }

    function _owns(address _claimant, uint256 _tokenId) internal view returns (bool) {
      return kittyIndexToOwner[_tokenId] == _claimant;
  }

}
1 Like
  1. It has a constant time complexity.
  2. It is linear.
  3. You add a mapping that has a key address and a uint array value and return that.
    It makes the getAllCats function constant, but it does increase complexity.
Code
pragma solidity ^0.8.0;

contract Kittycontract {

    string public constant name = "TestKitties";
    string public constant symbol = "TK";
    
    event Transfer(address indexed from, address indexed to, uint256 indexed tokenId);

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

    struct Kitty {
        uint256 genes;
        uint64 birthTime;
        uint32 mumId;
        uint32 dadId;
        uint16 generation;
    }
    
    Kitty[] kitties;

    mapping (uint256 => address) public kittyIndexToOwner;
    mapping (address => uint256) ownershipTokenCount;
    mapping (address => uint[]) kittyIdInventories;

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

    function totalSupply() public view returns (uint) {
        return kitties.length;
    }

    function ownerOf(uint256 _tokenId) external view returns (address)
    {
        return kittyIndexToOwner[_tokenId];
    }

    // 32014 + 13406
    function transfer(address _to,uint256 _tokenId) external
    {
        require(_to != address(0));
        require(_to != address(this));
        require(_owns(msg.sender, _tokenId));
        
        _transfer(msg.sender, _to, _tokenId);
    }
    
//27902 + 5222
    function getAllCatsFor(address _owner) external view returns (uint[] memory cats){
        
        return kittyIdInventories[_owner];
    }
    
    //107131 + 85603
    function createKittyGen0(uint256 _genes) public returns (uint256) {
        return _createKitty(0, 0, 0, _genes, msg.sender);
    }

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

        

        uint256 newKittenId = kitties.length - 1;

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

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

        return newKittenId;

    }

    function _transfer(address _from, address _to, uint256 _tokenId) internal {
        ownershipTokenCount[_to]++;
        kittyIndexToOwner[_tokenId] = _to;
        kittyIdInventories[_to].push(_tokenId);

        if (_from != address(0)) {
            
            ownershipTokenCount[_from]--;
            
           _removeIdFromInventory(_from, _tokenId);

        }

        // Emit the transfer event.
        emit Transfer(_from, _to, _tokenId);
    }

    function _removeIdFromInventory(address inventoryOwner, uint tokenId) private{
        
        for (uint i=0; i<kittyIdInventories[inventoryOwner].length; i++){
        
            if (kittyIdInventories[inventoryOwner][i] == tokenId){
                    
                uint temp = kittyIdInventories[inventoryOwner][kittyIdInventories[inventoryOwner].length - 1];
                kittyIdInventories[inventoryOwner][kittyIdInventories[inventoryOwner].length - 1] = kittyIdInventories[inventoryOwner][i];
                kittyIdInventories[inventoryOwner][i] = temp;
                    
                kittyIdInventories[inventoryOwner].pop();
            }


        }
    }

    function _owns(address _claimant, uint256 _tokenId) internal view returns (bool) {
      return kittyIndexToOwner[_tokenId] == _claimant;
  }

}
1 Like
  1. Creating a kitty is a constant time complexity because it creates one kitty and adds it to the end of the kitty array.

2.The getAllCatsFor function is linear in its time complexity because you have to loop through the kitty array and there for as the array grows the time complexity grows.

  1. In order to reduce time complexity bottle necking you could implement a mapping that holds an array of all id’s owned by owner. This would make the operation more constant but the main trade off is the extra storage cost to hold this extra data.
pragma solidity ^0.8.0;

contract Kittycontract {

    string public constant name = "TestKitties";
    string public constant symbol = "TK";
    
    event Transfer(address indexed from, address indexed to, uint256 indexed tokenId);

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

    struct Kitty {
        uint256 genes;
        uint64 birthTime;
        uint32 mumId;
        uint32 dadId;
        uint16 generation;
    }

    Kitty[] kitties;

    mapping (uint256 => address) public kittyIndexToOwner;
    mapping (address => uint256) ownershipTokenCount;
    mapping (address => uint[]) allCatsPerAddress;
    

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

    function totalSupply() public view returns (uint) {
        return kitties.length;
    }

    function ownerOf(uint256 _tokenId) external view returns (address)
    {
        return kittyIndexToOwner[_tokenId];
    }

    function transfer(address _to,uint256 _tokenId) external
    {
        require(_to != address(0));
        require(_to != address(this));
        require(_owns(msg.sender, _tokenId));

        _transfer(msg.sender, _to, _tokenId);
    }
    
    function getAllCatsFor(address _owner) external view returns (uint[] memory cats){
       
        return allCatsPerAddress[_owner];
    }
    
    function createKittyGen0(uint256 _genes) public returns (uint256) {
        return _createKitty(0, 0, 0, _genes, msg.sender);
    }

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

        uint256 newKittenId = kitties.length - 1;

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

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

        return newKittenId;

    }

    function _transfer(address _from, address _to, uint256 _tokenId) internal {
        ownershipTokenCount[_to]++;

        kittyIndexToOwner[_tokenId] = _to;
        
        allCatsPerAddress[_to].push(_tokenId);

        if (_from != address(0)) {
            ownershipTokenCount[_from]--;
        }

        // Emit the transfer event.
        emit Transfer(_from, _to, _tokenId);
    }

    function _owns(address _claimant, uint256 _tokenId) internal view returns (bool) {
      return kittyIndexToOwner[_tokenId] == _claimant;
  }

}
1 Like

Can someone just break down in simple terms what these questions are even asking? I don’t want to shoot straight to the assignment solution or look at other’s answers before I give it a try, but I don’t even remember learning about “time complexity,” let alone if it’s constant or linear.

  1. Creating kitty is Constant time complexity.
  2. The getAllCatsFor function is incremental iterative in nature so we can conclude it as a linear.
  3. In order to reduce the time complexity we can try to replace an array with the mapping. It will help us to get quick result but may lead to increase in storage.

Here is my solution:

pragma solidity ^0.8.0;

contract Kittycontract {

    string public constant name = "TestKitties";
    string public constant symbol = "TK";
    
    event Transfer(address indexed from, address indexed to, uint256 indexed tokenId);

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

    struct Kitty {
        uint256 genes;
        uint64 birthTime;
        uint32 mumId;
        uint32 dadId;
        uint16 generation;
    }

    Kitty[] kitties;

    mapping (uint256 => address) public kittyIndexToOwner;
    mapping (address => uint256) ownershipTokenCount;
    mapping (address => uint[]) getAllOwnedCryptoKitties;

    

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

    function totalSupply() public view returns (uint) {
        return kitties.length;
    }

    function ownerOf(uint256 _tokenId) external view returns (address)
    {
        return kittyIndexToOwner[_tokenId];
    }

    function transfer(address _to,uint256 _tokenId) external
    {
        require(_to != address(0));
        require(_to != address(this));
        require(_owns(msg.sender, _tokenId));

        _transfer(msg.sender, _to, _tokenId);
    }
    
    function getAllCatsFor(address _owner) external view returns (uint[] memory cats){
        
        return getAllOwnedCryptoKitties[_owner];
       
        /*uint[] memory result = new uint[](ownershipTokenCount[_owner]);
        uint counter = 0;
        for (uint i = 0; i < kitties.length; i++) {
            if (kittyIndexToOwner[i] == _owner) {
                result[counter] = i;
                counter++;
            }
        }
        return result;*/
        
    }
    
    function createKittyGen0(uint256 _genes) public returns (uint256) {
        return _createKitty(0, 0, 0, _genes, msg.sender);
    }

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

        uint256 newKittenId = kitties.length - 1;

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

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

        return newKittenId;

    }

    function _transfer(address _from, address _to, uint256 _tokenId) internal {
        ownershipTokenCount[_to]++;

        kittyIndexToOwner[_tokenId] = _to;
        getAllOwnedCryptoKitties[_to].push(_tokenId);
        

        if (_from != address(0)) {
            ownershipTokenCount[_from]--;
        }

        // Emit the transfer event.
        emit Transfer(_from, _to, _tokenId);
    }

    function _owns(address _claimant, uint256 _tokenId) internal view returns (bool) {
      return kittyIndexToOwner[_tokenId] == _claimant;
  }

}


1 Like
  1. The time complexity of creating new cats is constant… as once you plug in the required inputs, the execution time/cost will always remain constant.

  2. getAllCatsFor function is a linear time complexity. This depends on how many cat ID’s address X owns. The function creates a variable (result) that represents an integer that points to the mapping that holds the token count of the address inputted. After that, you’re setting a counter variable and running it through an iteration process that is run through the length of the array that holds the created Struct objects named ‘Kitty.’ If the CAT ID (specific index in the array where the NFT resides) aligns with the address you originally inputted to run the function, then the function will add it to the result counter variable. Depending on how many times an ID matches with the owner is linear in time complexity, because that is unique to how many ID’s the particular address owns.

  3. I simply kept the ownershipTokenCount mapping and created a third mapping called:
    mapping (address => uint256[]) ownedCats;

Pros: This was easy, I got rid of all the code in the getallCatsFor() and returned the new ownedCats mapping. I also just had to push the new tokenID in the _transfer() to the new ownedCats mapping, so that each created cat would get added to my new mapping as well.

Cons: After trying it out for a little, I noticed the obvious bug. I created the mapping that had an address point to the indexes in the array of cat ID’s he owned. So far, so good. But, after transferring these NFT’s to another address - and checking the getAllCatsFor() - the array wouldn’t change.

That’s because, you CAN’T delete things out of the middle of an array. So, this then sent me on the path of having to remember that you have to essentially “copy” the LAST catID (in the ownedCats array) the owner owns and paste it to the index of the tokenID you are transferring out. Then, you have to .pop() the last ID off of the array.

This was getting to crazy for me, so I just incorporated the .pop in the _transfer function if (_from != address(0)) { ownershipTokenCount[_from]--; ownedCats[_from].pop(); }

This just gets rid of the last Index in the array…i knowwwwww

1 Like
pragma solidity ^0.8.0;

contract Kittycontract {

    string public constant name = "TestKitties";
    string public constant symbol = "TK";
    
    event Transfer(address indexed from, address indexed to, uint256 indexed tokenId);

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

    struct Kitty {
        uint256 genes;
        uint64 birthTime;
        uint32 mumId;
        uint32 dadId;
        uint16 generation;
    }

    Kitty[] kitties;

    mapping (uint256 => address) public kittyIndexToOwner;
    mapping (address => uint256) ownershipTokenCount;
    mapping (address => uint256[]) ownedCats;
    

    

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

    function totalSupply() public view returns (uint) {
        return kitties.length;
    }

    function ownerOf(uint256 _tokenId) external view returns (address)
    {
        return kittyIndexToOwner[_tokenId];
    }

    function transfer(address _to,uint256 _tokenId) external
    {
        require(_to != address(0));
        require(_to != address(this));
        require(_owns(msg.sender, _tokenId));

        _transfer(msg.sender, _to, _tokenId);
    }
    
    function getAllCatsFor(address _owner) external view returns (uint[] memory cats){
        return ownedCats[_owner];
        //uint[] memory result = new uint[](ownershipTokenCount[_owner]);
        //uint counter = 0;
        //for (uint i = 0; i < kitties.length; i++) {
            //if (kittyIndexToOwner[i] == _owner) {
                //result[counter] = i;
                //counter++;
            //}
        //}
        //return result;
    }
    
    function createKittyGen0(uint256 _genes) public returns (uint256) {
        return _createKitty(0, 0, 0, _genes, msg.sender);
    }

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

        uint256 newKittenId = kitties.length - 1;

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

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

        return newKittenId;

    }

    function _transfer(address _from, address _to, uint256 _tokenId) internal {
        ownershipTokenCount[_to]++;
        
        ownedCats[_to].push(_tokenId);

        kittyIndexToOwner[_tokenId] = _to;
        

        if (_from != address(0)) {
            ownershipTokenCount[_from]--;
            ownedCats[_from].pop();
        }

        // Emit the transfer event.
        emit Transfer(_from, _to, _tokenId);
    }

    function _owns(address _claimant, uint256 _tokenId) internal view returns (bool) {
      return kittyIndexToOwner[_tokenId] == _claimant;
  }

}
1 Like