Omniscia 0xPhase Audit

SystemClockV1 Code Style Findings

SystemClockV1 Code Style Findings

SCV-01C: Inefficient Variable Usage

TypeSeverityLocation
Gas OptimizationSystemClockV1.sol:L16

Description:

The referenced return statement will read the lastTime value from storage, however, the result of SystemClockV1::getTime (curTime) is guaranteed to be what lastTime will represent at the statement's evaluation.

Example:

clock/SystemClockV1.sol
8/// @inheritdoc ISystemClock
9function time() external override returns (uint256) {
10 uint256 curTime = getTime();
11
12 if (curTime > lastTime) {
13 lastTime = curTime;
14 }
15
16 return lastTime;
17}
18
19/// @inheritdoc ISystemClock
20function getTime() public view override returns (uint256) {
21 return MathLib.max(block.timestamp, lastTime);
22}

Recommendation:

We advise the curTime value to be utilized instead, optimizing the code's gas cost.

Alleviation (3dd3d7bf0c2693b2f9c23bacedfa420393f7ea84):

The code was refactored to compare the lastTime variable as well as the current block.timestamp, minimizing redundant storage reads and thus optimizing the codebase.

SCV-02C: Potentially Redundant Evaluation of Time

TypeSeverityLocation
Gas OptimizationSystemClockV1.sol:L20-L22

Description:

The SystemClockV1::getTime function will yield the maximum between block.timestamp and lastTime, however, the lastTime value which is solely assigned in SystemClockV1::time will always point to the highest SystemClockV1::getTime evaluation. As such, SystemClockV1::getTime will always logically evaluate to block.timestamp.

Example:

clock/SystemClockV1.sol
8/// @inheritdoc ISystemClock
9function time() external override returns (uint256) {
10 uint256 curTime = getTime();
11
12 if (curTime > lastTime) {
13 lastTime = curTime;
14 }
15
16 return lastTime;
17}
18
19/// @inheritdoc ISystemClock
20function getTime() public view override returns (uint256) {
21 return MathLib.max(block.timestamp, lastTime);
22}

Recommendation:

We advise the MathLib import and maximum operator to be omitted from the code as it is ineffectual.

Alleviation (3dd3d7bf0c2693b2f9c23bacedfa420393f7ea84):

The code of SystemClockV1::time was updated to compare the lastTime variable with the current block.timestamp, minimizing redundant evaluations. Additionally, the 0xPhase team stated that per the zkSync Era developer team a more reliable way to query time will be introduced at a later point in the blockchain's lifetime meaning that projects should be upgradeable in the way they query time.

As such, we consider this exhibit alleviated in full as the SystemClockV1::getTime function should remain in the codebase and the SystemClockV1::time function was optimized.