How to allow each voter to vote only once? [Very Simple Voting Contract]

Question

first, I apologize for this very very very simple question. I'm starting my studies on smart contracts, and after many attempts, I have not been able to solve this problem.

Objectively, this is a simple voting contract, with two candidates, A and B. The buttons add votes for each one.

 //Vote for candidate A or B 
function VoteA() public {
    CandidateA++;
}

function VoteB() public {
    CandidateB++;
}

I would like to know how to ensure that each address can only vote once for each candidate. Only that!

I imagine that the solution goes through the mapping and array, but, all my attempts resulted in errors.

I apologize for the very basic question. Unfortunately, I am unable to resolve this issue even though I have consulted some similar topics.

Thanks!!!! Full contract follows

contract Election {

uint CandidateA;
uint CandidateB;
string Result;


//Vote for candidate A or B 
function VoteA() public {
    CandidateA++;
}

function VoteB() public {
    CandidateB++;
}

//Function shows how many votes each candidate had
function getVotesA() public view returns (uint) {
    return CandidateA;
}

function getVotesB() public view returns (uint) {
    return CandidateB;
}

//Here the electoral commission determines the result
function CoutingVotes() public {
    if(CandidateA > CandidateB) {Result = "A Won";} else {
        Result = "B Won";} 
}  

//Here the electoral commission declares the result
function ResultElection() public view returns (string memory) { 
    return Result;

}
     

}

Answer 1

There are many ways to do this, you might want to try adding something like the following:

mapping (address => bool) private voted;

function VoteA() public {
    require(voted[msg.sender] == false, 
        "user already voted");
    CandidateA++;
    voted[msg.sender] == true;
}

function VoteB() public {
    require(voted[msg.sender] == false, 
        "user already voted");
    CandidateB++;
    voted[msg.sender] == true;
}

Let's talk quickly about what this is doing.

First we introduce a mapping (voted) which will keep track of which addresses have already voted.

Second the require at the beginning of VoteA and VoteB checks if this address (msg.sender means the address sending the tx) has already voted or not.

Lastly after a successful vote, the address is registered as having voted in the voted mapping so they won't be able to vote again.

Future Improvements

This is fine for trying to get familiar with smart contracts, but let's talk about why you might not want to deploy a contract like this, too. I hope you don't mind - you mentioned that you're studying smart contracts, and we thought these might be some useful points:

Sybil attacks: If you've begun to get familiar with wallets, you may have noticed that in many of them one user can generate many addresses. In the case of this contract, an attacker could create as many addresses as they like, and vote from all of them. Attacks like these, where one user/entity is able to use multiple addresses in order to exploit looking like many, is called a Sybil attack.

Only one election: In the current structure of the contract, it cannot be reused for multiple elections - there's no way to differentiate between one election and another. Similarly, there is no start time or end time.

Refactoring: There are some ways you might want to try your hand at refactoring this contract.

• Can you make VoteA and VoteB into one function?
• If CountingVotes is called after someone else has already called it, it looks like the second caller will still have to pay for a transaction to write Result into the same thing it is already. What might you do to avoid that problem?

Reentrancy: This one is a bit more advanced than the other items on the list, but it is worth bringing up anyway. Take a look at the solution we offered again. What would happen if the voter's transaction ran out of gas after CandidateA or CandidateB was incremented, but before voted[msg.sender] was set to true? The voter would be able to vote a second time. There's a kind of attack where an attacker can leverage the fact that the vote is registered before the voter is set to having voted called Reentrancy, which basically allows them to call the function in a loop like this. In this case, it would mean they could vote many times without the voted mapping for their address being set to true.

Just to be clear, this item is about our implementation in the answer, not your code in the question. We answered, then realized that our answer was vulnerable to reentrancy, then decided to leave it there and talk about it.

A very simple solution would be have voted[msg.sender] set to true before incrementing CandidateA or CandidateB.

If you want to learn more about Reentrancy, just search for "reentrancy ethereum". Here are two links, one from ConsenSys and another from Quantstamp. Don't worry if you don't understand them now - Reentrancy can be a bit hard to get the first few times you encounter it. Just remember that in cases where you want to make sure an update (like the one to the voted mapping here) happens, do it before the state change (incrementing the Candidate here) wherever possible. There is another solution for reentrancy which involves using a mutex. Here is OpenZeppelin's implementation of a solution using that pattern.

Style Guide: Here is a link to the Solidity Style Guide, which details some thoughts on how to capitalize variables and things like that. While it isn't an obligation to style your code and variables like the Guide, it is a generally accepted convention, and makes the code easier for others to read. A tl;dr would be that variables are usually not capitalized in the beginning (so candidateA, candidateB, result), and same for functions (and modifiers, but events are capitalized), so voteA, voteB, and resultElection. The if would also be formatted a bit differently - see the link for more details on that.

Hope this helps!