search

code-423n4 / 2022-05-velodrome-findings

0 stars 0 forks source link

QA Report #183

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

1. !=0 is cheaper than >0

2. Initilization to default values can be omitted.

uint variables are set to zero by default, initializing them to zero (for storage variables) causes extra gas.

3. Use solidity custom errors to save gas

solidity 0.8.4 introduces custom errors which are cheaper than using revert strings in terms of gas Use the custom error patterns to reduce gas cost.

for eg.


  // Before
  require(condition, "Revert strings");

  // After
  error CustomError();
  if (!condition) {
    revert CustomError();
  }

more details can be found here

4. using prefix increment in loops can save some gas

Prefix increment operation(++i) is more gas effiecient than postfix increment(i++), while few instance of postfix increment doesn't makes much difference in overall gas, in loops these increments are called a large number of times resulting in significant gas savings.

// Before
for (uint i = 0; i < _prices.length; i++) {
}

// After
for (uint i = 0; i < _prices.length; ++i) {
}

Also, since the loop increment starts from zero and only increases, there is no chance of overflow. So the increment can also be added inside a unchecked block so save further gas

for (uint i = 0; i < _prices.length;) {
  unchecked {
    ++i;
  }
}

The instances of prefix increment can be found as follows

❯ grep -rn './contracts' -e 'for.*i++'
./contracts/contracts/Pair.sol:257:        for (uint i = 0; i < _prices.length; i++) {
./contracts/contracts/Pair.sol:389:        for (uint i = 0; i < 255; i++) {
./contracts/contracts/VotingEscrow.sol:1146:        for (uint i = 0; i < _tokenIds.length; i++) {
./contracts/contracts/VotingEscrow.sol:1193:        for (uint i = 0; i < _tokenIds.length; i++) {
./contracts/contracts/VotingEscrow.sol:1225:                for (uint i = 0; i < srcRepOld.length; i++) {
./contracts/contracts/VotingEscrow.sol:1249:                for (uint i = 0; i < dstRepOld.length; i++) {
./contracts/contracts/VotingEscrow.sol:1295:                for (uint i = 0; i < srcRepOld.length; i++) {
.
.
.

5. array length in loops can be cached instead of calculating in every iteration

The loop bounds are calculated with array.length which are calculated in every loop iterations which can result in high gas. The array length can be cached instead of calculating in every loop iteration to save gas.

// Before
for (uint i = 0; i < _prices.length; i++) {
}

// After
uint len = _prices.length;
for (uint i = 0; i < len; i++) {
}

The instances where this pattern can be applied is found as follows

❯ grep -rn './contracts' -e 'for.*.length'
./contracts/contracts/Pair.sol:257:        for (uint i = 0; i < _prices.length; i++) {
./contracts/contracts/Pair.sol:276:        for (; i < length; i+=window) {
./contracts/contracts/VotingEscrow.sol:1146:        for (uint i = 0; i < _tokenIds.length; i++) {
./contracts/contracts/VotingEscrow.sol:1193:        for (uint i = 0; i < _tokenIds.length; i++) {
./contracts/contracts/VotingEscrow.sol:1225:                for (uint i = 0; i < srcRepOld.length; i++) {
./contracts/contracts/VotingEscrow.sol:1249:                for (uint i = 0; i < dstRepOld.length; i++) {
./contracts/contracts/VotingEscrow.sol:1295:                for (uint i = 0; i < srcRepOld.length; i++) {
./contracts/contracts/VotingEscrow.sol:1320:                for (uint i = 0; i < dstRepOld.length; i++) {
./contracts/contracts/Minter.sol:57:        for (uint i = 0; i < claimants.length; i++) {
./contracts/contracts/Gauge.sol:353:        for (uint i = 0; i < tokens.length; i++) {
./contracts/contracts/Gauge.sol:426:        for (uint i; i < length; i++) {
.
.
.
GalloDaSballo commented 2 years ago

The warden submitted the report as gas already, marking as invalid