refactor(L2): tighten BaseTime NatSpec and tests

interfaces/L2/IBaseTime.sol

@@ -10,15 +10,28 @@ interface IBaseTime is ISemver {
     /// @notice Thrown when a caller other than the protocol depositor attempts to update BaseTime.
     error BaseTime_NotDepositor();
 
-    /// @notice Thrown when the millisecond component is not aligned to a 200 millisecond interval.
+    /// @notice Thrown when the millisecond component is above 800 or not a multiple of 200.
     error BaseTime_InvalidTimestampMillisPart();
 
     /// @notice Returns the millisecond component of the current L2 block timestamp.
+    ///
+    /// @return The millisecond component, one of 0, 200, 400, 600, or 800.
     function timestampMillisPart() external view returns (uint16);
 
     /// @notice Returns the current L2 block timestamp in milliseconds.
-    function timestampMs() external view returns (uint64 timestampMs_);
+    ///
+    /// @dev Until the block's `tx[1]` BaseTime update deposit executes (e.g. during the L1-info deposit at `tx[0]`),
+    ///      combines the current block's seconds with the previous block's millisecond component.
+    ///
+    /// @return The current L2 block timestamp in milliseconds.
+    function timestampMs() external view returns (uint64);
 
     /// @notice Updates the millisecond component of the current L2 block timestamp.
+    ///
+    /// @dev Reverts with `BaseTime_NotDepositor` when the caller is not the depositor account.
+    /// @dev Reverts with `BaseTime_InvalidTimestampMillisPart` when `_timestampMillisPart` is above 800 or not a
+    ///      multiple of 200.
+    ///
+    /// @param _timestampMillisPart The millisecond component of the current L2 block timestamp.
     function setTimestampMillisPart(uint16 _timestampMillisPart) external;
 }

snapshots/abi/BaseTime.json

@@ -31,7 +31,7 @@
     "outputs": [
       {
         "internalType": "uint64",
-        "name": "timestampMs_",
+        "name": "",
         "type": "uint64"
       }
     ],

snapshots/semver-lock.json

@@ -57,7 +57,7 @@
   },
   "src/L2/BaseTime.sol:BaseTime": {
     "initCodeHash": "0xfcd48af0cfd907d0ce91f61ea437c81c9a98269ca2071d7d27291f20c483ccc0",
-    "sourceCodeHash": "0x98e246b6ff058ecb8aaeacaa33ce70674f4d3d4a8dcabe91df6f772c6d237667"
+    "sourceCodeHash": "0xd7dd36a7f1ffcb3ae573a943f21b217093b2698170783c9c49178f552516ff5c"
   },
   "src/L2/FeeDisburser.sol:FeeDisburser": {
     "initCodeHash": "0x1278027e3756e2989e80c0a7b513e221a5fe0d3dbd9ded108375a29b2c1f3d57",

src/L2/BaseTime.sol

@@ -20,8 +20,8 @@ contract BaseTime is IBaseTime {
     uint16 public timestampMillisPart;
 
     /// @inheritdoc IBaseTime
-    function timestampMs() external view returns (uint64 timestampMs_) {
-        timestampMs_ = uint64(block.timestamp * 1000 + timestampMillisPart);
+    function timestampMs() external view returns (uint64) {
+        return uint64(block.timestamp * 1000 + timestampMillisPart);
     }
 
     /// @inheritdoc IBaseTime

test/L2/BaseTime.t.sol

@@ -2,86 +2,110 @@
 pragma solidity 0.8.15;
 
 // Testing
-import { Test } from "forge-std/Test.sol";
+import { CommonTest } from "test/setup/CommonTest.sol";
 
 // Libraries
 import { Constants } from "src/libraries/Constants.sol";
 
-// Target contract
-import { BaseTime } from "src/L2/BaseTime.sol";
+// Interfaces
 import { IBaseTime } from "interfaces/L2/IBaseTime.sol";
 
 /// @title BaseTime_TestInit
 /// @notice Reusable test initialization for BaseTime tests.
-abstract contract BaseTime_TestInit is Test {
-    BaseTime internal baseTime;
+abstract contract BaseTime_TestInit is CommonTest {
+    /// @notice Largest valid millisecond component.
+    uint16 internal constant MAX_TIMESTAMP_MILLIS_PART = 800;
 
-    /// @notice Sets up the test suite.
-    function setUp() public {
-        baseTime = new BaseTime();
-    }
+    /// @notice Interval the millisecond component must be aligned to.
+    uint16 internal constant TIMESTAMP_MILLIS_PART_INTERVAL = 200;
 
     /// @notice Sets the millisecond component as the protocol depositor.
     function setTimestampMillisPart(uint16 _timestampMillisPart) internal {
         vm.prank(Constants.DEPOSITOR_ACCOUNT);
         baseTime.setTimestampMillisPart(_timestampMillisPart);
     }
-}
 
-/// @title BaseTime_TimestampMillisPart_Test
-/// @notice Tests BaseTime's initial value.
-contract BaseTime_TimestampMillisPart_Test is BaseTime_TestInit {
-    /// @notice Tests that the millisecond component initially equals zero.
-    function test_initialValue_succeeds() external view {
-        assertEq(baseTime.timestampMillisPart(), 0);
+    /// @notice Maps a fuzzed slot to a valid millisecond component.
+    function validTimestampMillisPart(uint256 _slot) internal pure returns (uint16) {
+        return uint16(
+            bound(_slot, 0, MAX_TIMESTAMP_MILLIS_PART / TIMESTAMP_MILLIS_PART_INTERVAL) * TIMESTAMP_MILLIS_PART_INTERVAL
+        );
+    }
+
+    /// @notice Returns whether a millisecond component is invalid.
+    function isInvalidTimestampMillisPart(uint16 _timestampMillisPart) internal pure returns (bool) {
+        return
+            _timestampMillisPart > MAX_TIMESTAMP_MILLIS_PART
+                || _timestampMillisPart % TIMESTAMP_MILLIS_PART_INTERVAL != 0;
     }
 }
 
 /// @title BaseTime_SetTimestampMillisPart_Test
 /// @notice Tests updating BaseTime's millisecond component.
 contract BaseTime_SetTimestampMillisPart_Test is BaseTime_TestInit {
-    /// @notice Tests every valid millisecond component.
-    function test_setTimestampMillisPart_succeeds() external {
-        uint16[5] memory validValues = [uint16(0), 200, 400, 600, 800];
-
-        for (uint256 i; i < validValues.length; i++) {
-            setTimestampMillisPart(validValues[i]);
-            assertEq(baseTime.timestampMillisPart(), validValues[i]);
-        }
+    /// @notice Tests that callers other than the protocol depositor are rejected.
+    function testFuzz_setTimestampMillisPart_notDepositor_reverts(address _caller, uint256 _slot) external {
+        vm.assume(_caller != Constants.DEPOSITOR_ACCOUNT);
+        uint16 part = validTimestampMillisPart(_slot);
+
+        vm.expectRevert(IBaseTime.BaseTime_NotDepositor.selector);
+        vm.prank(_caller);
+        baseTime.setTimestampMillisPart(part);
+    }
+
+    /// @notice Tests that the depositor check runs before the value check.
+    function testFuzz_setTimestampMillisPart_notDepositorInvalidValue_reverts(
+        address _caller,
+        uint16 _timestampMillisPart
+    )
+        external
+    {
+        vm.assume(_caller != Constants.DEPOSITOR_ACCOUNT);
+        vm.assume(isInvalidTimestampMillisPart(_timestampMillisPart));
+
+        vm.expectRevert(IBaseTime.BaseTime_NotDepositor.selector);
+        vm.prank(_caller);
+        baseTime.setTimestampMillisPart(_timestampMillisPart);
     }
 
     /// @notice Tests that an invalid millisecond component is rejected.
     function testFuzz_setTimestampMillisPart_invalidValue_reverts(uint16 _timestampMillisPart) external {
-        vm.assume(_timestampMillisPart > 800 || _timestampMillisPart % 200 != 0);
+        vm.assume(isInvalidTimestampMillisPart(_timestampMillisPart));
+
         vm.expectRevert(IBaseTime.BaseTime_InvalidTimestampMillisPart.selector);
         vm.prank(Constants.DEPOSITOR_ACCOUNT);
         baseTime.setTimestampMillisPart(_timestampMillisPart);
     }
 
-    /// @notice Tests that callers other than the protocol depositor are rejected.
-    function testFuzz_setTimestampMillisPart_notDepositor_reverts(address _caller) external {
-        vm.assume(_caller != Constants.DEPOSITOR_ACCOUNT);
-        vm.expectRevert(IBaseTime.BaseTime_NotDepositor.selector);
-        vm.prank(_caller);
-        baseTime.setTimestampMillisPart(200);
+    /// @notice Tests that the depositor can set any valid millisecond component.
+    function testFuzz_setTimestampMillisPart_validValue_succeeds(uint256 _slot) external {
+        uint16 part = validTimestampMillisPart(_slot);
+
+        setTimestampMillisPart(part);
+
+        assertEq(baseTime.timestampMillisPart(), part);
     }
 
     /// @notice Tests that the millisecond component occupies slot zero as a uint16.
     function test_setTimestampMillisPart_usesSlotZero_succeeds() external {
-        setTimestampMillisPart(600);
+        uint16 part = 600;
+
+        setTimestampMillisPart(part);
 
-        assertEq(vm.load(address(baseTime), bytes32(0)), bytes32(uint256(600)));
+        assertEq(vm.load(address(baseTime), bytes32(0)), bytes32(uint256(part)));
     }
 }
 
 /// @title BaseTime_TimestampMs_Test
 /// @notice Tests BaseTime's full millisecond timestamp getter.
 contract BaseTime_TimestampMs_Test is BaseTime_TestInit {
     /// @notice Tests that timestampMs combines block.timestamp with the millisecond component.
-    function test_timestampMs_succeeds() external {
-        vm.warp(1725);
-        setTimestampMillisPart(600);
+    function testFuzz_timestampMs_succeeds(uint256 _timestamp, uint256 _slot) external {
+        uint256 timestamp = bound(_timestamp, 0, type(uint64).max / 1000 - 1);
+        uint16 part = validTimestampMillisPart(_slot);
+        vm.warp(timestamp);
+        setTimestampMillisPart(part);
 
-        assertEq(baseTime.timestampMs(), 1_725_600);
+        assertEq(baseTime.timestampMs(), timestamp * 1000 + part);
     }
 }

test/setup/Setup.sol

@@ -46,6 +46,7 @@ import { IL1FeeVault } from "interfaces/L2/IL1FeeVault.sol";
 import { IOperatorFeeVault } from "interfaces/L2/IOperatorFeeVault.sol";
 import { IGasPriceOracle } from "interfaces/L2/IGasPriceOracle.sol";
 import { IL1Block } from "interfaces/L2/IL1Block.sol";
+import { IBaseTime } from "interfaces/L2/IBaseTime.sol";
 import { IProxyAdmin } from "interfaces/universal/IProxyAdmin.sol";
 import { IWETH98 } from "interfaces/universal/IWETH98.sol";
 import { TEEProverRegistry } from "src/L1/proofs/tee/TEEProverRegistry.sol";
@@ -119,6 +120,7 @@ abstract contract Setup is FeatureFlags {
     IGasPriceOracle gasPriceOracle = IGasPriceOracle(Predeploys.GAS_PRICE_ORACLE);
     IL1Block l1Block = IL1Block(Predeploys.L1_BLOCK_ATTRIBUTES);
     IWETH98 weth = IWETH98(payable(Predeploys.WETH));
+    IBaseTime baseTime = IBaseTime(Predeploys.BASE_TIME);
     INitroValidator nitroValidator;
     TEEProverRegistry teeProverRegistry;