erc721r / ERC721R

An ERC721 base contract that mints tokens in a pseudo-random order. Because the token is revealed in the same transaction as the mint itself, this contract creates a fun but not fully secure experience.
MIT License
96 stars 22 forks source link

_mintRandom ERC721REnumerable #5

Open 0xfcks opened 2 years ago

0xfcks commented 2 years ago

Hey! I love this 721 feature and its accompanied documentation! Thank you for all the work done in brining this to the Ethereum developer community.

I am wanting to use the ERC721REnumerable in an upcoming project but having issues in using the tokenOfOwnerByIndex. Specifically, only the 0-index owner's token is showing a randomly generated index value.

I'm checking to see if I am implementing the Enumerable contract correctly. Here is the basic contract structure Im using for tests:

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "./ERC721REnumerable.sol";

contract TEST is ERC721rEnumerable {    

    constructor(string memory name_, string memory symbol_, uint maxSupply_) ERC721r(name_, symbol_, maxSupply_) {}

    function mint(uint256 quantity) public {
        _mintRandom(msg.sender, quantity);
    }

}

And here is my mocha.js test:

const assert = require('assert');
const ganache = require('ganache-cli');
const HDWalletProvider = require('truffle-hdwallet-provider');
const Web3 = require('web3');
const web3 = new Web3(ganache.provider(HDWalletProvider));
const { abi, bytecode } = require('../../build/contracts/TEST.json')

let accounts;
let contract;

beforeEach( async () => {

    accounts = await web3.eth.getAccounts();
    contract = await new web3.eth.Contract(abi).deploy({data:bytecode, arguments: ['TEST', 'TEST', 777]}).send({from:accounts[0], gas:5000000})

})

describe( 'Mint Tests', () => {    

    it.skip("Contract Deploys", () => {
        assert(contract)
    })

    // this test passes
    it('Mint 1 Token', async () => {
        await contract.methods.mint(1).send({from: accounts[0], gas:1000000})
        const tokens = await contract.methods.balanceOf(accounts[0]).call()
        assert(tokens,1)
    })

    // this test passes
    it.skip('Mint 1 Token & Call Random TokenIdx', async () => {
        await contract.methods.mint(1).send({from: accounts[0], gas:1000000})
        const tokenIdx = await contract.methods.tokenOfOwnerByIndex(accounts[0],0).call()
        console.log(`Randomly Assigned Token Index: ${tokenIdx}`)
        assert.notEqual(0,tokenIdx)
    })

    // this test FAILS (see below)
    it('Mint Multiple Tokens & Call Random TokenIdx of all Tokens', async (numMints=10) => {                

        await contract.methods.mint(numMints).send({from: accounts[0], gas:1000000})
        const tokens = await contract.methods.balanceOf(accounts[0]).call()
        assert.equal(tokens, numMints)

        for (let i=0; i< numMints; i++ ){
            let tokenIdx = await contract.methods.tokenOfOwnerByIndex(accounts[0],i).call()
            console.log(`Randomly Assigned Token Index: ${tokenIdx}`)
            assert.notEqual(0,tokenIdx)
        }        

    })

})

The output for the last test is as follows:

  Mint Tests
    - Contract Deploys
    ✔ Mint 1 Token (108ms)
Randomly Assigned Token Index: 513
    ✔ Mint 1 Token & Call Random TokenIdx (82ms)
Randomly Assigned Token Index: 442
Randomly Assigned Token Index: 0
    1) Mint Multiple Tokens & Call Random TokenIdx of all Tokens

  2 passing (779ms)
  1 pending
  1 failing

  1) Mint Tests
       Mint Multiple Tokens & Call Random TokenIdx of all Tokens:
     AssertionError [ERR_ASSERTION]: 0 != '0'
      at Context.<anonymous> (test/mint.test.js:46:20)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)

Any suggestions/feedback would be greatly appreciated!

0xfcks commented 2 years ago

Side Note: it looks like the tokenOfOwnerByIndex works as expected when individual tokens are minted. For example, the test below passes (i.e. logs 3 different non-0 values*):

   ...
   it('Mint Multiple Tokens Individually & Call Random TokenIdx For Each Token', async () => {                

        await contract.methods.mint(1).send({from: accounts[0], gas:1000000})
        let tokenIdx = await contract.methods.tokenOfOwnerByIndex(accounts[0],0).call()
        console.log(`Randomly Assigned Token Index: ${tokenIdx}`)

        await contract.methods.mint(1).send({from: accounts[0], gas:1000000})     
        tokenIdx = await contract.methods.tokenOfOwnerByIndex(accounts[0],1).call()
        console.log(`Randomly Assigned Token Index: ${tokenIdx}`)

        await contract.methods.mint(1).send({from: accounts[0], gas:1000000})     
        tokenIdx = await contract.methods.tokenOfOwnerByIndex(accounts[0],2).call()
        console.log(`Randomly Assigned Token Index: ${tokenIdx}`)

    })

*I understand that the getRandomAvailableTokenId can return 0 but its highly unlikely in tests cases (given large TokenSupply) and since the default value for non-existent token indexes by token owner is 0 I am choosing to use it in the test assertions