0

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?

1 Answer 1

0

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.

4
  • Thanks for you reply Iain. This does replace the _transfer function. The transfer function separates into _transferNormal and _transferNoFees with this being the former and main function. I wanted the contract to manage the lottery function with each transfer: contract receives a tx fee to fund the lottery, if a set txnum is hit then it distributes the jackpot to the drawn array item.
    – BigDommo
    Mar 15, 2022 at 12:13
  • "PS: You should be careful when using random numbers that can be influenced by the user, since they can simply:..." - I'm being a bit simple, I think. As they wouldn't know their address array number, I couldn't see how they could manipulate the outcome to their favour.
    – BigDommo
    Mar 15, 2022 at 12:33
  • I repeat that, this is 100% not safe, even if they didn't know their address array number of whatever you are referring to. They can simply check their balance. You can even simply read contract storage, even if it is private. Everything is public. If this answer helped, please mark it.
    – phaze
    Mar 15, 2022 at 21:08
  • 1
    100% not safe is 200% not good. Thanks Iain.
    – BigDommo
    Mar 16, 2022 at 6:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.