Summary
The lack of naming conventions like the one used by Chainlink, it makes difficult for developers differentiate state variables and memory variables and make gas optimizations.
Impact
Low - Doesn't impact in the correct use of the protocol
Recommendations
When using a state variables more than 2 times, like in a for loop, it may be wise to load the variable into memory first and use that variable to minize the gas consumption.
I recommend this naming convention for state and memory variables:
If it's a state variable the name will be the same as we see in the Solidity Documentation, camelcase but it will be preceded by 's_variableName', and memory variables simple camelcase 'variableName'.
With this change in can be easier for developers in where to do the optimization, and by adopting this pattern, (prioritize the use of memory variables instead).
PoC
Let's have this example, let's suppose we want to add 200 players into the raffle, let's rewrite the enterRaffle() function to use memory variables, and let's see the gas consumption of each test.
Paste this function into PuppyRaffle
function enterRaffleOptimized(address[] calldata newPlayers) public payable {
require(msg.value == entranceFee * newPlayers.length, "PuppyRaffle: Must send enough to enter raffle");
for (uint256 i = 0; i < newPlayers.length; i++) {
require(!newPlayers[i].isContract(), "Contracts are not permited in the raffle");
players.push(newPlayers[i]);
}
address[] memory tmpPlayers = players;
for (uint256 i = 0; i < tmpPlayers.length - 1; i++) {
for (uint256 j = i + 1; j < tmpPlayers.length; j++) {
require(tmpPlayers[i] != tmpPlayers[j], "PuppyRaffle: Duplicate player");
}
}
emit RaffleEnter(newPlayers);
}
Paste these functions in PuppyRaffleTest.t.sol
function testGasUseEnterRaffle() public {
addNumberPlayersToRaffle(puppyRaffle, 200);
}
function testGasUseEnterRaffleOptimized() public {
addNumberPlayersToRaffleOptimized(puppyRaffle, 200);
}
function addNumberPlayersToRaffleOptimized(PuppyRaffle _puppyRaffle, uint n) public {
address[] memory players = new address[](n);
for (uint i = 0; i < n; i++) {
players[i] = address(i + 10);
}
_puppyRaffle.enterRaffleOptimized{value: entranceFee * n}(players);
}
function addNumberPlayersToRaffle(PuppyRaffle _puppyRaffle, uint n) public {
address[] memory players = new address[](n);
for (uint i = 0; i < n; i++) {
players[i] = address(i + 10);
}
_puppyRaffle.enterRaffle{value: entranceFee * n}(players);
}
Run in the following command in the terminal at the root of the project:
forge snapshot --match-test testGasUseRaffle
We should have the following output:
Running 2 tests for test/PuppyRaffleTest.t.sol:PuppyRaffleTest
[PASS] testGasUseEnterRaffle() (gas: 20427396)
[PASS] testGasUseEnterRaffleOptimized() (gas: 9352736)
Test result: ok. 2 passed; 0 failed; 0 skipped; finished in 67.88ms
The optimized enterRaffle() consumes 45% of what the first enterRaffle() does. This disparity it increases when passing more players into the function, when passing 300 players, these are the outputs:
Running 2 tests for test/PuppyRaffleTest.t.sol:PuppyRaffleTest
[PASS] testGasUseEnterRaffle() (gas: 42457434)
[PASS] testGasUseEnterRaffleOptimized() (gas: 17100325)
Making that the gas consumption is so great in the first one that it won't be possible to run in the Ethereum blockchain, which at this time the gas limit of nodes is set to 30_000_000. We can check this with this command:
forge snapshot --match-test testGasUseEnterRaffle --gas-limit 30000000
Running 2 tests for test/PuppyRaffleTest.t.sol:PuppyRaffleTest
[FAIL. Reason: EvmError: Revert] testGasUseEnterRaffle() (gas: 29512007)
[PASS] testGasUseEnterRaffleOptimized() (gas: 17100325)
Test result: FAILED. 1 passed; 1 failed; 0 skipped; finished in 86.90ms
Ran 1 test suites: 1 tests passed, 1 failed, 0 skipped (2 total tests)
Failing tests:
Encountered 1 failing test in test/PuppyRaffleTest.t.sol:PuppyRaffleTest
[FAIL. Reason: EvmError: Revert] testGasUseEnterRaffle() (gas: 29512007)
And the EvmError if we inspect it, it's the error "EvmError: OutOfGas"
