How to loop through array of addresses and make sure msg.sender matches a stored address

Question

I am writing the smart contract below to create a digital registery. Most of the contract is good code and is working, yet I can not seem to get the "AddAttendancePoints" function to work properly.

The body of the function should execute the following:

• start a loop that iterates through the array of saved addresses in a particular "Register". ("Register" is data structure which is created everytime a user wants a new register)

• check if the address which is sending the message is stored in that "Register's" array. <- "This is where my code has a bug, the problem is, the transaction reverts when you have stored 2 addresses or more in the array. The code does not allow for the 2nd or 3rd addresses to add attendance points even when they have been saved."

• At the end of the loop, the code reverts the transaction if the "msg.sender" does not match "[i]" when looping through the "Register's" array of addresses. <- for some reason, every transaction after the first call to "AddAttendancePoints" returns this line of code, even when the msg.sender is saved in the "Register's" array.

// SPDX-License-Identifier: GPL-3.0

pragma solidity ^0.8.4;

contract Register {
    
    uint256 public totalRegisters;
    
    struct completeRegister {
        address[] attendees;
        uint256 attendancePoints;
        uint256 sessionNumber;
        uint256 startTime;
        uint256 finishTime;
        string title;
        bool active;
    }
    
    mapping (uint256 => completeRegister) RegisterToNumber;
    
    
    modifier RegisterActive(uint256 _registerNumber) {
        require (RegisterToNumber[_registerNumber].active == true);
        _;
    }
    

    function NewRegister (address[] memory _attendees, string memory _title) public  {
        RegisterToNumber[totalRegisters] = completeRegister(_attendees, 0, totalRegisters, block.timestamp, 0, _title, true);
        totalRegisters = totalRegisters + 1;
        
    }
    
    function FinaliseRegister (uint256 _registerNumber) public RegisterActive(_registerNumber) returns (bool) {
        RegisterToNumber[_registerNumber].finishTime = block.timestamp;
        RegisterToNumber[_registerNumber].active = false;
        return true;
    }
    
    function AddAttendee(address _attendee, uint256 _registerNumber) public RegisterActive(_registerNumber) {
        RegisterToNumber[_registerNumber].attendees.push(_attendee);
    }
    
    function AddAttendancePoints(uint256 _registerNumber) public RegisterActive(_registerNumber) {
         for (uint256 i = 0; i < RegisterToNumber[_registerNumber].attendees.length; i++) {
            if (RegisterToNumber[_registerNumber].attendees[i] == msg.sender) {
                RegisterToNumber[_registerNumber].attendancePoints = RegisterToNumber[_registerNumber].attendancePoints + 1;
                } else {
                    revert();
                }
            }  
            
    }
    
    
    function GetSession (uint256 _registerNumber) public view returns (address[] memory, uint256, uint256, uint256, uint256, string memory, bool) {
        return (RegisterToNumber[_registerNumber].attendees,
                RegisterToNumber[_registerNumber].attendancePoints,
                RegisterToNumber[_registerNumber].sessionNumber,
                RegisterToNumber[_registerNumber].startTime,
                RegisterToNumber[_registerNumber].finishTime,
                RegisterToNumber[_registerNumber].title,
                RegisterToNumber[_registerNumber].active);
    }
    
}

The function "AddAttendancePoints" is the only function "not" working in the smart contract. If you deploy in remix and play around with the code, you will see that the function below does not allow the script to execute according to the logic written in the body.

function AddAttendancePoints(uint256 _registerNumber) public RegisterActive(_registerNumber) {
         for (uint256 i = 0; i < RegisterToNumber[_registerNumber].attendees.length; i++) {
            if (RegisterToNumber[_registerNumber].attendees[i] == msg.sender) {
                RegisterToNumber[_registerNumber].attendancePoints = RegisterToNumber[_registerNumber].attendancePoints + 1;
                } else {
                    revert();
                }
            }  
            
    }

The code should allow a transaction to execute when the msg.sender is stored in the "Register's" array, the 1st address is okay and works fine, yet, when you add a 2nd and 3rd address to the array and try to call "AddAttendancePoints" the transaction reverts.

I need to know if there is any other way of writing the code, so the smart contract can execute the logic above and check if an address is matched to msg.sender. I was thinking that maybe it could work with a requirement once you performed the loop however we get the same results with the code shown below.

• Instead of using "if" I used "require" and ended up with the same results.

function AddAttendancePoints(uint256 _registerNumber) public RegisterActive(_registerNumber) {
         for (uint256 i = 0; i < RegisterToNumber[_registerNumber].attendees.length; i++) {
            require (RegisterToNumber[_registerNumber].attendees[i] == msg.sender);
            RegisterToNumber[_registerNumber].attendancePoints = RegisterToNumber[_registerNumber].attendancePoints + 1;
            }  
            
    }

To progress forward from this point, the smart contract needs to be able to match a "saved" address to a "msg.sender" when an array has more than 1 entry. Remember the array is stored in a struct and accessing the array could potentially be the problem.

Any help would be fantastic!

Answer 1

You shouldn't iterate an array of arbitrary length if possible. I'll suggest to use a mapping instead.

Anyway the function reverts because you are saying to revert when msg.sender is not equal to another attendee, so it will fail for meetings with more than one people.

Probably you want to revert when msg.sender is none of the attendees.

function AddAttendancePoints(uint256 _registerNumber) public RegisterActive(_registerNumber) {
  for (uint256 i = 0; i < RegisterToNumber[_registerNumber].attendees.length; i++) {
    if (RegisterToNumber[_registerNumber].attendees[i] == msg.sender) {
      RegisterToNumber[_registerNumber].attendancePoints = RegisterToNumber[_registerNumber].attendancePoints + 1;
      return; // stop looking
    }  
  }
  require(false, "msg.sender isn't an attendee");
}

Answer 2

I have fixed the problem with the help given above. Thanks Ismael!

By removing "else" and "revert" and adding a "return" at the end of my loop, I am now able to add additional points to a Register when it is active, the code only allows msg.senders who are saved in the Register's array to add points to storage.

The code is written as:

function AddAttendancePoints(uint256 _registerNumber) public RegisterActive(_registerNumber) returns (bool success) {
         for (uint256 i = 0; i < RegisterToNumber[_registerNumber].attendees.length; i++) {
            if (RegisterToNumber[_registerNumber].attendees[i] == msg.sender) {
                RegisterToNumber[_registerNumber].attendancePoints = RegisterToNumber[_registerNumber].attendancePoints + 1;
                return success;
            } 
    
        }  
            
    }

You can see that I have deleted 2 lines of code and also close the loop with a return function.

Lesson learned: Close loops with a "return" when iterating through an arbitrary length so the EVM notices a stop in the loop and do not add "else" to continue the function if a value does not match another when called.