Simple Refactoring with Solidity
The classic movie line goes something like “I love the smell of refactoring in the morning” right? That’s how I remember it.
I was looking into a customer’s code base recently and although my task was simply to assure the code worked as intended, I couldn’t stop but think that a little refactoring would go a long way here to make the code more concise, cheaper to run and easier to read.
Below is a (very) sanitized version of the initial code.
pragma solidity ^0.4.22;contract Voting { address[] public candidates;
mapping(address => uint) public votesReceived; function Voting(address[] candidateNames) public { //330717 gas w/ 5 candidates candidates = candidateNames;
} function voteForCandidate(address candidate) public { //21404 gas
require(isValidCandidate(candidate));
votesReceived[candidate] += 1;
} function totalVotesFor(address candidate) view public returns (uint) {
require(isValidCandidate(candidate));
return votesReceived[candidate];
} function isValidCandidate(address candidate) view public returns (bool) {
for(uint i = 0; i < candidates.length; i++) {
if (candidates[i] == candidate) {
return true;
}
}
return false;
}
}
The first thing that called my attention was the use of the now deprecated syntax for the constructor function. In Solidity, the constructor function of a contract used to be defined by having the exact same name as the contract itself. That is, if the contract was called “Voting”, then a function named “Voting” would be understood to be its constructor, a special function that is only run once, at deployment time, but a function named “voting” (small caps) would be a normal, universally accessible, function. This syntactic construct was the source of a number of costly mistakes with deployed contracts left un-initialised and sensitive “admin” functions left open for any devops199 out there… Now we use a function named “constructor”, it’s harder to miss.
Next it was the data structures. Storing an array with the addresses of all valid candidates on-chain can be expensive. I mean, the ideal cost is a function of the utility of having the data accessible for the contract to manipulate, so we need to see what does the contract do with it before making a judgement. Here, it was used for input sanitation with the use of the “isValidCandidate” function and the “require” keyword. Yikes, thats definitely expensive.
Writing smart-contracts is closer to developing for embedded devices than it is to web development because every computation is calculated and costed appropriately via the “gas” mechanism. This makes loops and excessive writing and reading from state ill advised because even a call to a read-only “view” function will cost an amount of Ether if invoked from a different smart-contract, which is good because that way a bad developer will cost way more than a good one :-)
Knowing that Solidity gives us free getter functions for data types identified as “public”, the “totalVotesFor” and the “isValidCandidate” functions are duplicating functionality already available by simply calling “candidates[_candidateAddr].votes” and “candidates[_candidateAddr].isValid” respectively, so I decided to remove them.
pragma solidity ^0.4.25;contract RefactoredVoting { struct CandidateStruct {
bool isValid;
uint64 votes;
}
mapping(address => CandidateStruct) public candidates; constructor(address[] _candidates) public { //205108 gas w/ 5 candidates
for (uint i = 0; i < _candidates.length; i++) {
candidates[_candidates[i]].isValid = true;
}
} function voteForCandidate(address _candidate) public { //5910 gas
candidates[_candidate].votes += 1;
}
}
Running a simple test with 5 candidate addresses showed some good progress in terms gas costs. Deployment now costs ~205k gas, down from ~330k and the “voteForCandidate” function requires only ~6k gas, down from ~21k in the original code.
It’s possible to reduce gas on deployment by keeping the votes as uint256 (Ethereum uses 32-byte words) but the vote function balloons to ~20k gas and, honestly, I don’t think anyone will ever need more than 9223372036854775807 votes. :-)