For Loops in transfer function - lottery management function gas usage

Question

I'm working on a token that has a lottery payout function. I'm currently using for loops to manage address entries in an array that defines a users eligibility for the lottery. The transfer function loops through the array and based on their holdings after token transfer either adds or removes the address to or from the array. Tests work fine with a small number of array items but my concern is as the array of eligible holders increases the contract will break due to gas costs.

mapping (address => bool) isExemptFromFees;
address[] private eligibleForLotto;
    
function _transferNormal(
        address sender,
        address recipient,
        uint256 amount
    ) internal virtual {
        require(sender != address(0), "ERC20: transfer from the zero address");
        require(recipient != address(0), "ERC20: transfer to the zero address");
        require(!isPaused, "Transfers are paused, check official announcements for info");
        require(amount <= maxSupply / 200, "Tx amount cant be more than 0.5% of total supply");
        
            (uint _tAmount, uint _dFee, uint _mFee, uint _lFee) = getTaxAmount(amount);
            require(_balances[recipient] + _tAmount <= maxSupply / 100, "Cant hold more than 1% of maxSupply");

            uint256 senderBalance = _balances[sender];
            require(senderBalance >= amount, "ERC20: transfer amount exceeds balance");
            unchecked {
                _balances[sender] = senderBalance - amount;
            }
            _balances[recipient] += _tAmount;
            _balances[devWallet] += _dFee;
            _balances[managementWallet] += _mFee;
            _balances[address(this)] += _lFee;

            emit Transfer(sender, recipient, _tAmount);
            emit Transfer(sender, devWallet, _dFee);
            emit Transfer(sender, managementWallet, _mFee);
            emit Transfer(sender, address(this), _lFee);

            if (!isExemptFromFees[sender] && _balances[sender] < holdRequirementForLottoEligibility) {           
                for (uint i = 0; i < eligibleForLotto.length; i++) {
                    if (eligibleForLotto[i] == sender) {
                        eligibleForLotto[i] = eligibleForLotto[eligibleForLotto.length - 1];
                        eligibleForLotto.pop();
                            break;
                    }
                }
            }

            if (!isExemptFromFees[recipient] && _balances[recipient] >= holdRequirementForLottoEligibility) { 
                bool exists;
                for (uint i = 0; i < eligibleForLotto.length; i++) {
                    if (eligibleForLotto[i] == recipient){
                        exists = true;
                        break;
                    }  
                }
                if (!exists) {
                    eligibleForLotto.push(recipient);
                }
            }

            if (lotteryEnabled) {
                txCounter++;
                if (txCounter == txNumForDraw) {
                    uint jackpot = _balances[address(this)];
                    uint modulus = eligibleForLotto.length * 2;
                    uint rand = uint(keccak256(abi.encodePacked(block.timestamp + _tAmount + jackpot)));
                    uint lottoResult = rand % modulus;

                    if (lottoResult > eligibleForLotto.length) { //Lotto rollover occurs if result does not match an eligible wallet
                        txCounter = 0; //Reset the txCounter for the next lotto draw
                    } else { //distribute jackpot to winner and reset the txCounter
                    address winner = eligibleForLotto[lottoResult]; 
                    _balances[address(this)] -= jackpot;
                    _balances[winner] += jackpot;
                    emit Transfer(address(this), winner, jackpot);
                    }
                }
            }
     txCounter = 0;
     if (autoTxNumForDraw) {
        txNumForDraw = (rand % 10) + 10;
                        }
     
    }

Ideally I'd use mappings as opposed to an array, but can't seem to get my head around implementing it without complicating the lottery draw. Is there an obvious solution I'm overlooking?

Answer 1

Definitely avoid looping over the entire array of entries. And don't add this to the standard transfer function (not entirely sure if this is the case). Keep that simple.

In order to mitigate double-entries, you'd probably want to lock the tokens in some form, similar to staking. If we dumb it down, your implementation could look something like this: (short PoC, not full code)

address winner;
address[] entries;
mapping(address => uint256) deposits;
uint256 TICKET_COST = 1e18;

function enterLottery() public {
    token.transferFrom(msg.sender, address(this), TICKET_COST);
    entries.push(msg.sender);
    deposits[msg.sender] += TICKET_COST;
}

function pickWinner() public requiresRandomNumber whenLotteryOver {
    winner = entries[randomNumber % entries.length];
}

function reclaimTokens() public whenLotteryOver {
    token.transferFrom(address(this), msg.sender, deposits[msg.sender]);
    deposits[msg.sender] = 0;
}

This way, you could have multiple entries by locking up more tokens. Since you'll likely want to let the user withdraw their tokens if they didn't win, you'll also want to add them to a deposits mapping that they can claim their tokens from.

You could also easily adapt this to be for multiple rounds:

mapping(uint256 => address[]) entries;
mapping(uint256 => mapping(address => uint256)) deposits;

PS: You should be careful when using random numbers that can be influenced by the user, since they can simply:

enterLottery(); 
if(didntWin()) revert();

Use something like Chainlink or have a pseudo-random number be generated from some trusted party, decoupled from the user entry transaction.