Volt Protocol contest - rfa's results

#1 Not using Safest.toInt128 on currentMonth and previousMonth casting to int128 https://github.com/code-423n4/2022-03-volt/blob/cec24b859c69d1397ce4048b6e9b8e96410b31dd/contracts/oracle/ScalingPriceOracle.sol#L123 ScalingPriceOracle.sol currently is using SafeCast lib. Its a bad practice to not using it when casting a uint var to int128.

#2 No check that previousMonth and currentMonth != 0 https://github.com/code-423n4/2022-03-volt/blob/cec24b859c69d1397ce4048b6e9b8e96410b31dd/contracts/oracle/ScalingPriceOracle.sol#L71-L93 If previousMonth == 0, the return value of getMonthlyAPR() won't valid because of division by zero. And currentMonth value will be the previousMonth value at _addNewMonth L219 function

gas

#1 Caching int128(previousMonth) can save gas https://github.com/code-423n4/2022-03-volt/blob/cec24b859c69d1397ce4048b6e9b8e96410b31dd/contracts/oracle/ScalingPriceOracle.sol#L123-L124 By storing int128(previousMonth) to memory can prevent SLOAD and running int128() function twice:

	int128 _previousMonth =  int128(previousMonth);
        int256 delta = int128(currentMonth) - _previousMonth;
        percentageChange = (delta * Constants.BP_INT) / _previousMonth;

#2 Using custom error https://github.com/code-423n4/2022-03-volt/blob/cec24b859c69d1397ce4048b6e9b8e96410b31dd/contracts/oracle/ScalingPriceOracle.sol#L141 https://github.com/code-423n4/2022-03-volt/blob/cec24b859c69d1397ce4048b6e9b8e96410b31dd/contracts/oracle/ScalingPriceOracle.sol#L177 Declared by error statement and using revert(customError) to throw the revert message can save gas instead of using long revert message in require()

#3 Using unchecked on calculating delta value https://github.com/code-423n4/2022-03-volt/blob/cec24b859c69d1397ce4048b6e9b8e96410b31dd/contracts/oracle/ScalingPriceOracle.sol#L123 The caluulation won't underflow because it can ha negative value. Using unchecked can save gas

#4 Using storage instead of memory to store request struct https://github.com/code-423n4/2022-03-volt/blob/cec24b859c69d1397ce4048b6e9b8e96410b31dd/contracts/oracle/ScalingPriceOracle.sol#L144 request is just called once in the contract. Reading it directly from storage can save gas instead of chacing it into memory

#5 Unnecessary MSTORE currentPrice https://github.com/code-423n4/2022-03-volt/blob/main/contracts/oracle/OraclePassThrough.sol#L38 By passing scalingPriceOracle.getCurrentOraclePrice() directly to L40 (from function) can save gas by not doIng MSTORE (currentPrice is just called once in the function)

#6 Prevent SLOAD multiple times by MSTORE decimalsNormalizer https://github.com/code-423n4/2022-03-volt/blob/cec24b859c69d1397ce4048b6e9b8e96410b31dd/contracts/refs/OracleRef.sol#L110-L114 decimalsNormalizer was called multiple times in readOracle() function. Using MSTORE can save gas:

	int256 _decimalsNormalizer = decimalsNormalizer;
        if (_decimalsNormalizer < 0) {
            scalingFactor = 10**(-1 * _decimalsNormalizer).toUint256();
            _peg = _peg.div(scalingFactor);
        } else {
            scalingFactor = 10**_decimalsNormalizer.toUint256();
            _peg = _peg.mul(scalingFactor);

#7 Preventing calculation of _peg if decimalsNormalizer == 0 https://github.com/code-423n4/2022-03-volt/blob/cec24b859c69d1397ce4048b6e9b8e96410b31dd/contracts/refs/OracleRef.sol#L114-L115 By checking that decimalsNormalizer != 0 before doing calculation of _peg value in else statement can prevent unnecessary gas consumed because it will automatically return 0 value:

else {
	if(decimalsNormalizer != 0){
            scalingFactor = 10**decimalsNormalizer.toUint256();
            _peg = _peg.mul(scalingFactor);
	}
}

#8 Gas improvement on using Safecast lib https://github.com/code-423n4/2022-03-volt/blob/cec24b859c69d1397ce4048b6e9b8e96410b31dd/contracts/refs/OracleRef.sol#L14 By removing L14, and call SafeCast lib directly on the function can save gas. Its also can applied on another library (highly recommended on SafeCast) For instance

scalingFactor = Safecast.toUint256(10**(-1 * decimalsNormalizer))

#9 Using custom error https://github.com/code-423n4/2022-03-volt/blob/cec24b859c69d1397ce4048b6e9b8e96410b31dd/contracts/utils/RateLimited.sol#L64 Declared by error statement and using revert() to throw the error message. It can save gas instead of using a long string to throw the error message

#10 Using unchecked on calculating elapsed value https://github.com/code-423n4/2022-03-volt/blob/cec24b859c69d1397ce4048b6e9b8e96410b31dd/contracts/utils/RateLimited.sol#L83 The calculation won't underflow because lastBufferUsedTime value was set by previous block.timestamp (its impossible that lastBufferUsedTime > block.timestamp)

#11 Using && in require cost more gas https://github.com/code-423n4/2022-03-volt/blob/main/contracts/volt/Volt.sol#L91 Using && in for validating in require cost more gas than just using multiple require:

require(
            recoveredAddress != address(0),
            "Fei: INVALID_SIGNATURE"
        );
require(
            recoveredAddress == owner,
            "Fei: INVALID_SIGNATURE"
        );

#12 Some function can set to external instead of public https://github.com/code-423n4/2022-03-volt/blob/cec24b859c69d1397ce4048b6e9b8e96410b31dd/contracts/refs/OracleRef.sol#L94-L123 updateOracle and readOracle function is not called in the same contract. Set them external