hats-finance / Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf

Circles Protocol contracts
https://aboutcircles.com
GNU Affero General Public License v3.0
0 stars 0 forks source link

`else-block` is not needed #44

Open hats-bug-reporter[bot] opened 1 week ago

hats-bug-reporter[bot] commented 1 week ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x629c958c70e31f1d657cd004841d47d952d5365345756c1559ac94514dffe875 Severity: low

Description:

Description

Inside Demurrage.sol, there are two functions for calculating the demurrage factor:

    function _calculateDemurrageFactor(uint256 _dayDifference) internal view returns (int128) {
        if (_dayDifference <= R_TABLE_LOOKUP) {
            // if the day difference is in the lookup table, return the value from the table
            int128 demurrageFactor = R[_dayDifference];
            if (demurrageFactor != 0) {
                return demurrageFactor;
            }
        }
        // if the day difference is not in the lookup table, calculate the value
        return Math64x64.pow(GAMMA_64x64, _dayDifference);
    }

    function _calculateDemurrageFactorAndCache(uint256 _dayDifference) internal returns (int128) {
        if (_dayDifference <= R_TABLE_LOOKUP) {
            int128 demurrageFactor = R[_dayDifference];
            if (demurrageFactor == 0) {
                // for proxy ERC20 contracts, the storage does not contain the R table yet
                // so compute it lazily and store it in the table
                demurrageFactor = Math64x64.pow(GAMMA_64x64, _dayDifference);
                R[_dayDifference] = demurrageFactor;
            }
            return demurrageFactor;
        } else {
            return Math64x64.pow(GAMMA_64x64, _dayDifference);
        }
    }

Note that in the first function, the else-block is omitted. This saves gas.

However, in the second function, _calculateDemurrageFactorAndCache(), the else-block is still in there. This can easily be modified such that it just returns if the first if-statement turns out to be false. Removing the else- block will save a few OPCODES from being executed every time _calculateDemurrageFactor is called, and since these two functions are called extensively, optimizing these functions is favored.

Recommendation

    function _calculateDemurrageFactorAndCache(uint256 _dayDifference) internal returns (int128) {
        if (_dayDifference <= R_TABLE_LOOKUP) {
            int128 demurrageFactor = R[_dayDifference];
            if (demurrageFactor == 0) {
                // for proxy ERC20 contracts, the storage does not contain the R table yet
                // so compute it lazily and store it in the table
                demurrageFactor = Math64x64.pow(GAMMA_64x64, _dayDifference);
                R[_dayDifference] = demurrageFactor;
            }
            return demurrageFactor;
-       } else {
-           return Math64x64.pow(GAMMA_64x64, _dayDifference);
-        }
+      return Math64x64.pow(GAMMA_64x64, _dayDifference);
    }
benjaminbollen commented 1 week ago

Thank you for your report on the unnecessary else-block. While this doesn't qualify as a security issue per our competition guidelines, we appreciate your attention to code optimization.

You've identified a valid gas optimization in a high-frequency code path. We'll discuss with the competition organizers and internally about how to address such valuable optimization suggestions.

We appreciate your careful examination of our code for efficiency opportunities. Thank you for your diligence and commitment to code quality.