Pick Winner Lottery test throws; Error: Transaction has been reverted by the EVM

Question

I'm writing a Lottery contract that's supposed to accept players, pick a winner and send the funds collected to the winner's address, then start over again. After running npm run test, the first 5 tests are passing but the "sends money to the winner & resets the players array" test fails, with the error message below.

1) lottery Contract
   sends money to the winner & resets the players array:
 Error: Transaction has been reverted by the EVM:

Personally, I think the problem is at file Lottery.test.js, on this code snippet because up to this point, everything is running well;

await lottery.methods.pickWinner().send({
        from: accounts[0]
    });

Other projects files are pasted below to help with the debugging.

My Lottery.sol file;

pragma solidity ^0.8.13;

contract Lottery {
    address public manager;
    address payable[] public players;

    function myLottery() public {
        manager = msg.sender;
    }

    function enter() public payable {
        require(msg.value > .001 ether);
        players.push(payable(msg.sender));
    }

    function random() private view returns(uint){
        return uint(keccak256(abi.encodePacked(block.difficulty, block.timestamp, players)));
    }

    function pickWinner() public restricted{
        uint index = random() % players.length;
        players[index].transfer(address(this).balance);  
        players = new address payable[](0);
    }

    modifier restricted() {
        require(msg.sender == manager);
        _;
    }

    function getPlayers() public view returns (address payable[] memory) {
        return players;
    }
}

compile.js

const path = require("path");
const fs = require("fs");
const solc = require("solc");

const lotteryPath = path.resolve(__dirname, "contracts", "Lottery.sol");
const source = fs.readFileSync(lotteryPath, "utf8");

let input = {
  language: "Solidity",
  sources: {
    [lotteryPath]: {
      content: source,
    },
  },

  settings: {
    outputSelection: {
      "*": {
        "*": ["*"],
      },
    },
  },
};

var output = JSON.parse(solc.compile(JSON.stringify(input)));

module.exports = {
  abi: output.contracts[[lotteryPath]]["Lottery"].abi,
  bytecode: output.contracts[[lotteryPath]]["Lottery"].evm.bytecode.object,
};

deploy.js

const HDWalletProvider = require('truffle-hdwallet-provider');
const Web3 = require('web3');
const { abi, bytecode } = require("./compile");

const provider = new HDWalletProvider(
    '****myMneumonic*****',
    '****myInfuraNode****'
);

const web3 = new Web3 (provider);

const deploy = async () => {
    const accounts = await web3.eth.getAccounts();

    console.log('Attempting to deploy from account;', accounts[0]);

    const result = await new web3.eth.Contract(abi)
    .deploy({ data: bytecode })
    .send({ from: accounts[0], gas: '10000000', gasPrice: '2000000000'});

    console.log('Contract deployed to;', result.options.address);
};
deploy();

Lottery.test.js

const assert = require("assert");
const ganache = require("ganache");
const Web3 = require("web3");
const web3 = new Web3(ganache.provider());
const { abi, bytecode } = require("../compile");

let accounts;
let lottery;

beforeEach(async () => {
  //get a list of all accounts
  accounts = await web3.eth.getAccounts();

  lottery = await new web3.eth.Contract(abi)
    .deploy({ data: bytecode })
    .send({ from: accounts[0], gas: "1000000" });
});

describe('lottery Contract', () => {
    it ('deploys a contract', () => {
        assert.ok(lottery.options.address);
    });

    it('allows one account to enter', async () => {
        await lottery.methods.enter().send({
            from: accounts[0],
            value: web3.utils.toWei('0.002', 'ether')
        });

        const players = await lottery.methods.getPlayers().call({
            from: accounts [0]
        });

        assert.equal(accounts[0], players);
        assert.equal(1, players.length);
    });

    it('allows multiple accounts to enter', async () => {
        await lottery.methods.enter().send({
            from: accounts[0],
            value: web3.utils.toWei('0.002', 'ether')
        });

        await lottery.methods.enter().send({
            from: accounts[1],
            value: web3.utils.toWei('0.002', 'ether')
        });

        await lottery.methods.enter().send({
            from: accounts[2],
            value: web3.utils.toWei('0.002', 'ether')
        });

        const players = await lottery.methods.getPlayers().call({
            from: accounts[0]
        });

        assert.equal(accounts[0], players[0]);
        assert.equal(accounts[1], players[1]);
        assert.equal(accounts[2], players[2]);
        assert.equal(3, players.length);
    });

    it('requires a minimum amount of ether to enter', async () => {
        try {
            await lottery.methods.enter().send({
                from: accounts[0],
                value: 200
            });
            assert(false);
        } catch (err) {
            assert(err);
        }
    });

    it ('only manager can call pickWinner', async () => {
        try {
            await lottery.methods.pickWinner().send({
                from: accounts[1],
                value: 200
            });
            assert(false);
        } catch (err) {
            assert(err);
        }
    });

    it('sends money to the winner & resets the players array', async () =>{
        await lottery.methods.enter().send({
            from: accounts[0],
            value: web3.utils.toWei('1', 'ether')
        });

        const initialBalance = await web3.eth.getBalance(accounts[0]);

        await lottery.methods.pickWinner().send({
            from: accounts[0]
        });

        const finalBalance = await web3.eth.getBalance(accounts[0]);

        const difference = finalBalance - initialBalance;
        console.log(difference);
        assert(difference > web3.utils.toWei('0.8', 'ether'));
    });
}); 

Answer 1

The problem was that I never set the manager role before I tried picking the winner. It's failing because at the time of calling pickWinner() method, manager address is null, and this would fail from the require statement in the restricted modifier.

I would also note that allowing the manager role to be set by anyone is probably not something you would want to do. Instead you should set the manager role inside the contracts constructor upon deploying your contract like so:

constructor(){
  manager = msg.sender;
}

Then you can get rid of myLottery() all together. Hope this helped.

Answer credits to; @_syndk8 on Twitter.

Answer 2

random()'s output is an integer with too many digits.

You can check that by setting random() to public and see what it returns.

In other words, if n is the player's number, and x is the random number, as a general condition for your function to work properly, x must be equal or less than n. In your code, this condition is less likely to be fulfilled.

Answer 3

The previous comment was a good observation, indeed. However, if you did not solve the issue, I might have a solution for you. I changed the pickWinner function as per the following:

function pickWinner() external restricted {
    //generating a uint number using the random function modulo array size
    uint index = random() % players.length;
    //send ethers to the winner at the position = index
    payable(players[index]).transfer(address(this).balance);
    //clean up the players' array initializing it again
    players = new address payable[](0);
}

So, changing visibility from public to external fixed the issue, probably.