Merge pull request #6096 from ethereum-optimism/refcell/first/stylestack

fix(ctb): Triple Slash Natspec Port

packages/contracts-bedrock/contracts/L1/L1CrossDomainMessenger.sol

@@ -6,42 +6,32 @@ import { OptimismPortal } from "./OptimismPortal.sol";
 import { CrossDomainMessenger } from "../universal/CrossDomainMessenger.sol";
 import { Semver } from "../universal/Semver.sol";
 
-/**
- * @custom:proxied
- * @title L1CrossDomainMessenger
- * @notice The L1CrossDomainMessenger is a message passing interface between L1 and L2 responsible
- *         for sending and receiving data on the L1 side. Users are encouraged to use this
- *         interface instead of interacting with lower-level contracts directly.
- */
+/// @custom:proxied
+/// @title L1CrossDomainMessenger
+/// @notice The L1CrossDomainMessenger is a message passing interface between L1 and L2 responsible
+///         for sending and receiving data on the L1 side. Users are encouraged to use this
+///         interface instead of interacting with lower-level contracts directly.
 contract L1CrossDomainMessenger is CrossDomainMessenger, Semver {
-    /**
-     * @notice Address of the OptimismPortal.
-     */
+    /// @notice Address of the OptimismPortal.
     OptimismPortal public immutable PORTAL;
 
-    /**
-     * @custom:semver 1.4.0
-     *
-     * @param _portal Address of the OptimismPortal contract on this network.
-     */
+    /// @custom:semver 1.4.1
+    /// @notice Constructs the L1CrossDomainMessenger contract.
+    /// @param _portal Address of the OptimismPortal contract on this network.
     constructor(OptimismPortal _portal)
-        Semver(1, 4, 0)
+        Semver(1, 4, 1)
         CrossDomainMessenger(Predeploys.L2_CROSS_DOMAIN_MESSENGER)
     {
         PORTAL = _portal;
         initialize();
     }
 
-    /**
-     * @notice Initializer.
-     */
+    /// @notice Initializes the contract.
     function initialize() public initializer {
         __CrossDomainMessenger_init();
     }
 
-    /**
-     * @inheritdoc CrossDomainMessenger
-     */
+    /// @inheritdoc CrossDomainMessenger
     function _sendMessage(
         address _to,
         uint64 _gasLimit,
@@ -51,16 +41,12 @@ contract L1CrossDomainMessenger is CrossDomainMessenger, Semver {
         PORTAL.depositTransaction{ value: _value }(_to, _value, _gasLimit, false, _data);
     }
 
-    /**
-     * @inheritdoc CrossDomainMessenger
-     */
+    /// @inheritdoc CrossDomainMessenger
     function _isOtherMessenger() internal view override returns (bool) {
         return msg.sender == address(PORTAL) && PORTAL.l2Sender() == OTHER_MESSENGER;
     }
 
-    /**
-     * @inheritdoc CrossDomainMessenger
-     */
+    /// @inheritdoc CrossDomainMessenger
     function _isUnsafeTarget(address _target) internal view override returns (bool) {
         return _target == address(this) || _target == address(PORTAL);
     }

packages/contracts-bedrock/contracts/L1/L1ERC721Bridge.sol

@@ -6,43 +6,34 @@ import { IERC721 } from "@openzeppelin/contracts/token/ERC721/IERC721.sol";
 import { L2ERC721Bridge } from "../L2/L2ERC721Bridge.sol";
 import { Semver } from "../universal/Semver.sol";
 
-/**
- * @title L1ERC721Bridge
- * @notice The L1 ERC721 bridge is a contract which works together with the L2 ERC721 bridge to
- *         make it possible to transfer ERC721 tokens from Ethereum to Optimism. This contract
- *         acts as an escrow for ERC721 tokens deposited into L2.
- */
+/// @title L1ERC721Bridge
+/// @notice The L1 ERC721 bridge is a contract which works together with the L2 ERC721 bridge to
+///         make it possible to transfer ERC721 tokens from Ethereum to Optimism. This contract
+///         acts as an escrow for ERC721 tokens deposited into L2.
 contract L1ERC721Bridge is ERC721Bridge, Semver {
-    /**
-     * @notice Mapping of L1 token to L2 token to ID to boolean, indicating if the given L1 token
-     *         by ID was deposited for a given L2 token.
-     */
+    /// @notice Mapping of L1 token to L2 token to ID to boolean, indicating if the given L1 token
+    ///         by ID was deposited for a given L2 token.
     mapping(address => mapping(address => mapping(uint256 => bool))) public deposits;
 
-    /**
-     * @custom:semver 1.1.1
-     *
-     * @param _messenger   Address of the CrossDomainMessenger on this network.
-     * @param _otherBridge Address of the ERC721 bridge on the other network.
-     */
+    /// @custom:semver 1.1.2
+    /// @notice Constructs the L1ERC721Bridge contract.
+    /// @param _messenger   Address of the CrossDomainMessenger on this network.
+    /// @param _otherBridge Address of the ERC721 bridge on the other network.
     constructor(address _messenger, address _otherBridge)
-        Semver(1, 1, 1)
+        Semver(1, 1, 2)
         ERC721Bridge(_messenger, _otherBridge)
     {}
 
-    /**
-     * @notice Completes an ERC721 bridge from the other domain and sends the ERC721 token to the
-     *         recipient on this domain.
-     *
-     * @param _localToken  Address of the ERC721 token on this domain.
-     * @param _remoteToken Address of the ERC721 token on the other domain.
-     * @param _from        Address that triggered the bridge on the other domain.
-     * @param _to          Address to receive the token on this domain.
-     * @param _tokenId     ID of the token being deposited.
-     * @param _extraData   Optional data to forward to L2. Data supplied here will not be used to
-     *                     execute any code on L2 and is only emitted as extra data for the
-     *                     convenience of off-chain tooling.
-     */
+    /// @notice Completes an ERC721 bridge from the other domain and sends the ERC721 token to the
+    ///         recipient on this domain.
+    /// @param _localToken  Address of the ERC721 token on this domain.
+    /// @param _remoteToken Address of the ERC721 token on the other domain.
+    /// @param _from        Address that triggered the bridge on the other domain.
+    /// @param _to          Address to receive the token on this domain.
+    /// @param _tokenId     ID of the token being deposited.
+    /// @param _extraData   Optional data to forward to L2.
+    ///                     Data supplied here will not be used to execute any code on L2 and is
+    ///                     only emitted as extra data for the convenience of off-chain tooling.
     function finalizeBridgeERC721(
         address _localToken,
         address _remoteToken,
@@ -71,9 +62,7 @@ contract L1ERC721Bridge is ERC721Bridge, Semver {
         emit ERC721BridgeFinalized(_localToken, _remoteToken, _from, _to, _tokenId, _extraData);
     }
 
-    /**
-     * @inheritdoc ERC721Bridge
-     */
+    /// @inheritdoc ERC721Bridge
     function _initiateBridgeERC721(
         address _localToken,
         address _remoteToken,

packages/contracts-bedrock/contracts/L1/ResourceMetering.sol

@@ -6,48 +6,40 @@ import { Math } from "@openzeppelin/contracts/utils/math/Math.sol";
 import { Burn } from "../libraries/Burn.sol";
 import { Arithmetic } from "../libraries/Arithmetic.sol";
 
-/**
- * @custom:upgradeable
- * @title ResourceMetering
- * @notice ResourceMetering implements an EIP-1559 style resource metering system where pricing
- *         updates automatically based on current demand.
- */
+/// @custom:upgradeable
+/// @title ResourceMetering
+/// @notice ResourceMetering implements an EIP-1559 style resource metering system where pricing
+///         updates automatically based on current demand.
 abstract contract ResourceMetering is Initializable {
-    /**
-     * @notice Represents the various parameters that control the way in which resources are
-     *         metered. Corresponds to the EIP-1559 resource metering system.
-     *
-     * @custom:field prevBaseFee   Base fee from the previous block(s).
-     * @custom:field prevBoughtGas Amount of gas bought so far in the current block.
-     * @custom:field prevBlockNum  Last block number that the base fee was updated.
-     */
+    /// @notice Represents the various parameters that control the way in which resources are
+    ///         metered. Corresponds to the EIP-1559 resource metering system.
+    /// @custom:field prevBaseFee   Base fee from the previous block(s).
+    /// @custom:field prevBoughtGas Amount of gas bought so far in the current block.
+    /// @custom:field prevBlockNum  Last block number that the base fee was updated.
     struct ResourceParams {
         uint128 prevBaseFee;
         uint64 prevBoughtGas;
         uint64 prevBlockNum;
     }
 
-    /**
-     * @notice Represents the configuration for the EIP-1559 based curve for the deposit gas
-     *         market. These values should be set with care as it is possible to set them in
-     *         a way that breaks the deposit gas market. The target resource limit is defined as
-     *         maxResourceLimit / elasticityMultiplier. This struct was designed to fit within a
-     *         single word. There is additional space for additions in the future.
-     *
-     * @custom:field maxResourceLimit             Represents the maximum amount of deposit gas that
-     *                                            can be purchased per block.
-     * @custom:field elasticityMultiplier         Determines the target resource limit along with
-     *                                            the resource limit.
-     * @custom:field baseFeeMaxChangeDenominator  Determines max change on fee per block.
-     * @custom:field minimumBaseFee               The min deposit base fee, it is clamped to this
-     *                                            value.
-     * @custom:field systemTxMaxGas               The amount of gas supplied to the system
-     *                                            transaction. This should be set to the same number
-     *                                            that the op-node sets as the gas limit for the
-     *                                            system transaction.
-     * @custom:field maximumBaseFee               The max deposit base fee, it is clamped to this
-     *                                            value.
-     */
+    /// @notice Represents the configuration for the EIP-1559 based curve for the deposit gas
+    ///         market. These values should be set with care as it is possible to set them in
+    ///         a way that breaks the deposit gas market. The target resource limit is defined as
+    ///         maxResourceLimit / elasticityMultiplier. This struct was designed to fit within a
+    ///         single word. There is additional space for additions in the future.
+    /// @custom:field maxResourceLimit             Represents the maximum amount of deposit gas that
+    ///                                            can be purchased per block.
+    /// @custom:field elasticityMultiplier         Determines the target resource limit along with
+    ///                                            the resource limit.
+    /// @custom:field baseFeeMaxChangeDenominator  Determines max change on fee per block.
+    /// @custom:field minimumBaseFee               The min deposit base fee, it is clamped to this
+    ///                                            value.
+    /// @custom:field systemTxMaxGas               The amount of gas supplied to the system
+    ///                                            transaction. This should be set to the same
+    ///                                            number that the op-node sets as the gas limit
+    ///                                            for the system transaction.
+    /// @custom:field maximumBaseFee               The max deposit base fee, it is clamped to this
+    ///                                            value.
     struct ResourceConfig {
         uint32 maxResourceLimit;
         uint8 elasticityMultiplier;
@@ -57,21 +49,14 @@ abstract contract ResourceMetering is Initializable {
         uint128 maximumBaseFee;
     }
 
-    /**
-     * @notice EIP-1559 style gas parameters.
-     */
+    /// @notice EIP-1559 style gas parameters.
     ResourceParams public params;
 
-    /**
-     * @notice Reserve extra slots (to a total of 50) in the storage layout for future upgrades.
-     */
+    /// @notice Reserve extra slots (to a total of 50) in the storage layout for future upgrades.
     uint256[48] private __gap;
 
-    /**
-     * @notice Meters access to a function based an amount of a requested resource.
-     *
-     * @param _amount Amount of the resource requested.
-     */
+    /// @notice Meters access to a function based an amount of a requested resource.
+    /// @param _amount Amount of the resource requested.
     modifier metered(uint64 _amount) {
         // Record initial gas amount so we can refund for it later.
         uint256 initialGas = gasleft();
@@ -83,12 +68,9 @@ abstract contract ResourceMetering is Initializable {
         _metered(_amount, initialGas);
     }
 
-    /**
-     * @notice An internal function that holds all of the logic for metering a resource.
-     *
-     * @param _amount     Amount of the resource requested.
-     * @param _initialGas The amount of gas before any modifier execution.
-     */
+    /// @notice An internal function that holds all of the logic for metering a resource.
+    /// @param _amount     Amount of the resource requested.
+    /// @param _initialGas The amount of gas before any modifier execution.
     function _metered(uint64 _amount, uint256 _initialGas) internal {
         // Update block number and base fee if necessary.
         uint256 blockDiff = block.number - params.prevBlockNum;
@@ -163,18 +145,14 @@ abstract contract ResourceMetering is Initializable {
         }
     }
 
-    /**
-     * @notice Virtual function that returns the resource config. Contracts that inherit this
-     *         contract must implement this function.
-     *
-     * @return ResourceConfig
-     */
+    /// @notice Virtual function that returns the resource config.
+    ///         Contracts that inherit this contract must implement this function.
+    /// @return ResourceConfig
     function _resourceConfig() internal virtual returns (ResourceConfig memory);
 
-    /**
-     * @notice Sets initial resource parameter values. This function must either be called by the
-     *         initializer function of an upgradeable child contract.
-     */
+    /// @notice Sets initial resource parameter values.
+    ///         This function must either be called by the initializer function of an upgradeable
+    ///         child contract.
     // solhint-disable-next-line func-name-mixedcase
     function __ResourceMetering_init() internal onlyInitializing {
         params = ResourceParams({

packages/contracts-bedrock/contracts/test/CommonTest.t.sol

@@ -124,6 +124,26 @@ contract L2OutputOracle_Initializer is CommonTest {
         vm.warp(oracle.computeL2Timestamp(_nextBlockNumber) + 1);
     }
 
+    /// @dev Helper function to propose an output.
+    function proposeAnotherOutput() public {
+        bytes32 proposedOutput2 = keccak256(abi.encode());
+        uint256 nextBlockNumber = oracle.nextBlockNumber();
+        uint256 nextOutputIndex = oracle.nextOutputIndex();
+        warpToProposeTime(nextBlockNumber);
+        uint256 proposedNumber = oracle.latestBlockNumber();
+
+        // Ensure the submissionInterval is enforced
+        assertEq(nextBlockNumber, proposedNumber + submissionInterval);
+
+        vm.roll(nextBlockNumber + 1);
+
+        vm.expectEmit(true, true, true, true);
+        emit OutputProposed(proposedOutput2, nextOutputIndex, nextBlockNumber, block.timestamp);
+
+        vm.prank(proposer);
+        oracle.proposeL2Output(proposedOutput2, nextBlockNumber, 0, 0);
+    }
+
     function setUp() public virtual override {
         super.setUp();
         guardian = makeAddr("guardian");

packages/contracts-bedrock/contracts/test/L1CrossDomainMessenger.t.sol

 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.15;
 
-/* Testing utilities */
+// Testing utilities
 import { Messenger_Initializer, Reverter, ConfigurableCaller } from "./CommonTest.t.sol";
 import { L2OutputOracle_Initializer } from "./L2OutputOracle.t.sol";
 
-/* Libraries */
+// Libraries
 import { AddressAliasHelper } from "../vendor/AddressAliasHelper.sol";
 import { Predeploys } from "../libraries/Predeploys.sol";
 import { Hashing } from "../libraries/Hashing.sol";
 import { Encoding } from "../libraries/Encoding.sol";
 
-/* Target contract dependencies */
+// Target contract dependencies
 import { L2OutputOracle } from "../L1/L2OutputOracle.sol";
 import { OptimismPortal } from "../L1/OptimismPortal.sol";
 
-/* Target contract */
+// Target contract
 import { L1CrossDomainMessenger } from "../L1/L1CrossDomainMessenger.sol";
 
 contract L1CrossDomainMessenger_Test is Messenger_Initializer {
-    // Receiver address for testing
+    /// @dev The receiver address
     address recipient = address(0xabbaacdc);
 
-    // Storage slot of the l2Sender
+    /// @dev The storage slot of the l2Sender
     uint256 constant senderSlotIndex = 50;
 
-    // the version is encoded in the nonce
+    /// @dev Tests that the version can be decoded from the message nonce.
     function test_messageVersion_succeeds() external {
         (, uint16 version) = Encoding.decodeVersionedNonce(L1Messenger.messageNonce());
         assertEq(version, L1Messenger.MESSAGE_VERSION());
     }
 
-    // sendMessage: should be able to send a single message
-    // TODO: this same test needs to be done with the legacy message type
-    // by setting the message version to 0
+    /// @dev Tests that the sendMessage function is able to send a single message.
+    /// TODO: this same test needs to be done with the legacy message type
+    ///       by setting the message version to 0
     function test_sendMessage_succeeds() external {
         // deposit transaction on the optimism portal should be called
         vm.expectCall(
@@ -86,7 +86,8 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
         L1Messenger.sendMessage(recipient, hex"ff", uint32(100));
     }
 
-    // sendMessage: should be able to send the same message twice
+    /// @dev Tests that the sendMessage function is able to send
+    ///      the same message twice.
     function test_sendMessage_twice_succeeds() external {
         uint256 nonce = L1Messenger.messageNonce();
         L1Messenger.sendMessage(recipient, hex"aa", uint32(500_000));
@@ -95,11 +96,14 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
         assertEq(nonce + 2, L1Messenger.messageNonce());
     }
 
+    /// @dev Tests that the xDomainMessageSender reverts when not set.
     function test_xDomainSender_notSet_reverts() external {
         vm.expectRevert("CrossDomainMessenger: xDomainMessageSender is not set");
         L1Messenger.xDomainMessageSender();
     }
 
+    /// @dev Tests that the relayMessage function reverts when
+    ///      the message version is not 0 or 1.
     function test_relayMessage_v2_reverts() external {
         address target = address(0xabcd);
         address sender = Predeploys.L2_CROSS_DOMAIN_MESSENGER;
@@ -124,7 +128,8 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
         );
     }
 
-    // relayMessage: should send a successful call to the target contract
+    /// @dev Tests that the relayMessage function is able to relay a message
+    ///      successfully by calling the target contract.
     function test_relayMessage_succeeds() external {
         address target = address(0xabcd);
         address sender = Predeploys.L2_CROSS_DOMAIN_MESSENGER;
@@ -163,7 +168,8 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
         assertEq(L1Messenger.failedMessages(hash), false);
     }
 
-    // relayMessage: should revert if attempting to relay a message sent to an L1 system contract
+    /// @dev Tests that relayMessage reverts if attempting to relay a message
+    ///      sent to an L1 system contract.
     function test_relayMessage_toSystemContract_reverts() external {
         // set the target to be the OptimismPortal
         address target = address(op);
@@ -193,7 +199,8 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
         );
     }
 
-    // relayMessage: should revert if eth is sent from a contract other than the standard bridge
+    /// @dev Tests that the relayMessage function reverts if eth is
+    ///      sent from a contract other than the standard bridge.
     function test_replayMessage_withValue_reverts() external {
         address target = address(0xabcd);
         address sender = Predeploys.L2_CROSS_DOMAIN_MESSENGER;
@@ -212,7 +219,8 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
         );
     }
 
-    // relayMessage: the xDomainMessageSender is reset to the original value
+    /// @dev Tests that the xDomainMessageSender is reset to the original value
+    ///      after a message is relayed.
     function test_xDomainMessageSender_reset_succeeds() external {
         vm.expectRevert("CrossDomainMessenger: xDomainMessageSender is not set");
         L1Messenger.xDomainMessageSender();
@@ -234,8 +242,9 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
         L1Messenger.xDomainMessageSender();
     }
 
-    // relayMessage: should send a successful call to the target contract after the first message
-    // fails and ETH gets stuck, but the second message succeeds
+    /// @dev Tests that relayMessage should successfully call the target contract after
+    ///      the first message fails and ETH is stuck, but the second message succeeds
+    ///      with a version 1 message.
     function test_relayMessage_retryAfterFailure_succeeds() external {
         address target = address(0xabcd);
         address sender = Predeploys.L2_CROSS_DOMAIN_MESSENGER;
@@ -291,6 +300,9 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
         assertEq(L1Messenger.failedMessages(hash), true);
     }
 
+    /// @dev Tests that relayMessage should successfully call the target contract after
+    ///      the first message fails and ETH is stuck, but the second message succeeds
+    ///      with a legacy message.
     function test_relayMessage_legacy_succeeds() external {
         address target = address(0xabcd);
         address sender = Predeploys.L2_CROSS_DOMAIN_MESSENGER;
@@ -332,6 +344,7 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
         assertEq(L1Messenger.failedMessages(hash), false);
     }
 
+    /// @dev Tests that relayMessage should revert if the message is already replayed.
     function test_relayMessage_legacyOldReplay_reverts() external {
         address target = address(0xabcd);
         address sender = Predeploys.L2_CROSS_DOMAIN_MESSENGER;
@@ -375,6 +388,7 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
         assertEq(L1Messenger.failedMessages(hash), false);
     }
 
+    /// @dev Tests that relayMessage can be retried after a failure with a legacy message.
     function test_relayMessage_legacyRetryAfterFailure_succeeds() external {
         address target = address(0xabcd);
         address sender = Predeploys.L2_CROSS_DOMAIN_MESSENGER;
@@ -450,6 +464,7 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
         assertEq(L1Messenger.failedMessages(hash), true);
     }
 
+    /// @dev Tests that relayMessage cannot be retried after success with a legacy message.
     function test_relayMessage_legacyRetryAfterSuccess_reverts() external {
         address target = address(0xabcd);
         address sender = Predeploys.L2_CROSS_DOMAIN_MESSENGER;
@@ -509,6 +524,7 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
         );
     }
 
+    /// @dev Tests that relayMessage cannot be called after a failure and a successful replay.
     function test_relayMessage_legacyRetryAfterFailureThenSuccess_reverts() external {
         address target = address(0xabcd);
         address sender = Predeploys.L2_CROSS_DOMAIN_MESSENGER;

packages/contracts-bedrock/contracts/test/L1ERC721Bridge.t.sol

 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.15;
 
-import { ERC721 } from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
+// Testing utilities
 import { Messenger_Initializer } from "./CommonTest.t.sol";
-import { L1ERC721Bridge } from "../L1/L1ERC721Bridge.sol";
+import { ERC721 } from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
+
+// Target contract dependencies
 import { L2ERC721Bridge } from "../L2/L2ERC721Bridge.sol";
 
+// Target contract
+import { L1ERC721Bridge } from "../L1/L1ERC721Bridge.sol";
+
+/// @dev Test ERC721 contract.
 contract TestERC721 is ERC721 {
     constructor() ERC721("Test", "TST") {}
 
@@ -39,6 +45,7 @@ contract L1ERC721Bridge_Test is Messenger_Initializer {
         bytes extraData
     );
 
+    /// @dev Sets up the testing environment.
     function setUp() public override {
         super.setUp();
 
@@ -58,6 +65,7 @@ contract L1ERC721Bridge_Test is Messenger_Initializer {
         localToken.approve(address(bridge), tokenId);
     }
 
+    /// @dev Tests that the constructor sets the correct values.
     function test_constructor_succeeds() public {
         assertEq(address(bridge.MESSENGER()), address(L1Messenger));
         assertEq(address(bridge.OTHER_BRIDGE()), otherBridge);
@@ -65,6 +73,7 @@ contract L1ERC721Bridge_Test is Messenger_Initializer {
         assertEq(address(bridge.otherBridge()), otherBridge);
     }
 
+    /// @dev Tests that the ERC721 can be bridged successfully.
     function test_bridgeERC721_succeeds() public {
         // Expect a call to the messenger.
         vm.expectCall(
@@ -109,6 +118,7 @@ contract L1ERC721Bridge_Test is Messenger_Initializer {
         assertEq(localToken.ownerOf(tokenId), address(bridge));
     }
 
+    /// @dev Tests that the ERC721 bridge reverts for non externally owned accounts.
     function test_bridgeERC721_fromContract_reverts() external {
         // Bridge the token.
         vm.etch(alice, hex"01");
@@ -121,6 +131,7 @@ contract L1ERC721Bridge_Test is Messenger_Initializer {
         assertEq(localToken.ownerOf(tokenId), alice);
     }
 
+    /// @dev Tests that the ERC721 bridge reverts for a zero address local token.
     function test_bridgeERC721_localTokenZeroAddress_reverts() external {
         // Bridge the token.
         vm.prank(alice);
@@ -132,6 +143,7 @@ contract L1ERC721Bridge_Test is Messenger_Initializer {
         assertEq(localToken.ownerOf(tokenId), alice);
     }
 
+    /// @dev Tests that the ERC721 bridge reverts for a zero address remote token.
     function test_bridgeERC721_remoteTokenZeroAddress_reverts() external {
         // Bridge the token.
         vm.prank(alice);
@@ -143,6 +155,7 @@ contract L1ERC721Bridge_Test is Messenger_Initializer {
         assertEq(localToken.ownerOf(tokenId), alice);
     }
 
+    /// @dev Tests that the ERC721 bridge reverts for an incorrect owner.
     function test_bridgeERC721_wrongOwner_reverts() external {
         // Bridge the token.
         vm.prank(bob);
@@ -154,6 +167,8 @@ contract L1ERC721Bridge_Test is Messenger_Initializer {
         assertEq(localToken.ownerOf(tokenId), alice);
     }
 
+    /// @dev Tests that the ERC721 bridge successfully sends a token
+    ///      to a different address than the owner.
     function test_bridgeERC721To_succeeds() external {
         // Expect a call to the messenger.
         vm.expectCall(
@@ -198,6 +213,8 @@ contract L1ERC721Bridge_Test is Messenger_Initializer {
         assertEq(localToken.ownerOf(tokenId), address(bridge));
     }
 
+    /// @dev Tests that the ERC721 bridge reverts for non externally owned accounts
+    ///      when sending to a different address than the owner.
     function test_bridgeERC721To_localTokenZeroAddress_reverts() external {
         // Bridge the token.
         vm.prank(alice);
@@ -209,6 +226,8 @@ contract L1ERC721Bridge_Test is Messenger_Initializer {
         assertEq(localToken.ownerOf(tokenId), alice);
     }
 
+    /// @dev Tests that the ERC721 bridge reverts for a zero address remote token
+    ///      when sending to a different address than the owner.
     function test_bridgeERC721To_remoteTokenZeroAddress_reverts() external {
         // Bridge the token.
         vm.prank(alice);
@@ -220,6 +239,8 @@ contract L1ERC721Bridge_Test is Messenger_Initializer {
         assertEq(localToken.ownerOf(tokenId), alice);
     }
 
+    /// @dev Tests that the ERC721 bridge reverts for an incorrect owner
+    ////     when sending to a different address than the owner.
     function test_bridgeERC721To_wrongOwner_reverts() external {
         // Bridge the token.
         vm.prank(bob);
@@ -238,6 +259,7 @@ contract L1ERC721Bridge_Test is Messenger_Initializer {
         assertEq(localToken.ownerOf(tokenId), alice);
     }
 
+    /// @dev Tests that the ERC721 bridge successfully finalizes a withdrawal.
     function test_finalizeBridgeERC721_succeeds() external {
         // Bridge the token.
         vm.prank(alice);
@@ -275,6 +297,8 @@ contract L1ERC721Bridge_Test is Messenger_Initializer {
         assertEq(localToken.ownerOf(tokenId), alice);
     }
 
+    /// @dev Tests that the ERC721 bridge finalize reverts when not called
+    ///      by the remote bridge.
     function test_finalizeBridgeERC721_notViaLocalMessenger_reverts() external {
         // Finalize a withdrawal.
         vm.prank(alice);
@@ -289,6 +313,8 @@ contract L1ERC721Bridge_Test is Messenger_Initializer {
         );
     }
 
+    /// @dev Tests that the ERC721 bridge finalize reverts when not called
+    ///      from the remote messenger.
     function test_finalizeBridgeERC721_notFromRemoteMessenger_reverts() external {
         // Finalize a withdrawal.
         vm.mockCall(
@@ -308,6 +334,8 @@ contract L1ERC721Bridge_Test is Messenger_Initializer {
         );
     }
 
+    /// @dev Tests that the ERC721 bridge finalize reverts when the local token
+    ///      is set as the bridge itself.
     function test_finalizeBridgeERC721_selfToken_reverts() external {
         // Finalize a withdrawal.
         vm.mockCall(
@@ -327,6 +355,8 @@ contract L1ERC721Bridge_Test is Messenger_Initializer {
         );
     }
 
+    /// @dev Tests that the ERC721 bridge finalize reverts when the remote token
+    ///      is not escrowed in the L1 bridge.
     function test_finalizeBridgeERC721_notEscrowed_reverts() external {
         // Finalize a withdrawal.
         vm.mockCall(

packages/contracts-bedrock/contracts/test/ResourceMetering.t.sol

 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.15;
 
+// Testing utilities
 import { Test } from "forge-std/Test.sol";
-import { ResourceMetering } from "../L1/ResourceMetering.sol";
-import { Proxy } from "../universal/Proxy.sol";
+
+// Libraries
 import { Constants } from "../libraries/Constants.sol";
 
+// Target contract dependencies
+import { Proxy } from "../universal/Proxy.sol";
+
+// Target contract
+import { ResourceMetering } from "../L1/ResourceMetering.sol";
+
 contract MeterUser is ResourceMetering {
     ResourceMetering.ResourceConfig public innerConfig;
 
@@ -50,20 +57,20 @@ contract MeterUser is ResourceMetering {
     }
 }
 
-/**
- * @title ResourceConfig
- * @notice The tests are based on the default config values. It is expected that
- *         the config values used in these tests are ran in production.
- */
+/// @title ResourceMetering_Test
+/// @dev Tests are based on the default config values.
+///      It is expected that these config values are used in production.
 contract ResourceMetering_Test is Test {
     MeterUser internal meter;
     uint64 initialBlockNum;
 
+    /// @dev Sets up the test contract.
     function setUp() public {
         meter = new MeterUser();
         initialBlockNum = uint64(block.number);
     }
 
+    /// @dev Tests that the initial resource params are set correctly.
     function test_meter_initialResourceParams_succeeds() external {
         (uint128 prevBaseFee, uint64 prevBoughtGas, uint64 prevBlockNum) = meter.params();
         ResourceMetering.ResourceConfig memory rcfg = meter.resourceConfig();
@@ -73,6 +80,7 @@ contract ResourceMetering_Test is Test {
         assertEq(prevBlockNum, initialBlockNum);
     }
 
+    /// @dev Tests that updating the resource params to the same values works correctly.
     function test_meter_updateParamsNoChange_succeeds() external {
         meter.use(0); // equivalent to just updating the base fee and block number
         (uint128 prevBaseFee, uint64 prevBoughtGas, uint64 prevBlockNum) = meter.params();
@@ -84,6 +92,7 @@ contract ResourceMetering_Test is Test {
         assertEq(postBlockNum, prevBlockNum);
     }
 
+    /// @dev Tests that updating the initial block number sets the meter params correctly.
     function test_meter_updateOneEmptyBlock_succeeds() external {
         vm.roll(initialBlockNum + 1);
         meter.use(0);
@@ -94,6 +103,7 @@ contract ResourceMetering_Test is Test {
         assertEq(prevBlockNum, initialBlockNum + 1);
     }
 
+    /// @dev Tests that updating the initial block number sets the meter params correctly.
     function test_meter_updateTwoEmptyBlocks_succeeds() external {
         vm.roll(initialBlockNum + 2);
         meter.use(0);
@@ -104,6 +114,7 @@ contract ResourceMetering_Test is Test {
         assertEq(prevBlockNum, initialBlockNum + 2);
     }
 
+    /// @dev Tests that updating the initial block number sets the meter params correctly.
     function test_meter_updateTenEmptyBlocks_succeeds() external {
         vm.roll(initialBlockNum + 10);
         meter.use(0);
@@ -114,6 +125,7 @@ contract ResourceMetering_Test is Test {
         assertEq(prevBlockNum, initialBlockNum + 10);
     }
 
+    /// @dev Tests that updating the gas delta sets the meter params correctly.
     function test_meter_updateNoGasDelta_succeeds() external {
         ResourceMetering.ResourceConfig memory rcfg = meter.resourceConfig();
         uint256 target = uint256(rcfg.maxResourceLimit) / uint256(rcfg.elasticityMultiplier);
@@ -125,6 +137,7 @@ contract ResourceMetering_Test is Test {
         assertEq(prevBlockNum, initialBlockNum);
     }
 
+    /// @dev Tests that the meter params are set correctly for the maximum gas delta.
     function test_meter_useMax_succeeds() external {
         ResourceMetering.ResourceConfig memory rcfg = meter.resourceConfig();
         uint64 target = uint64(rcfg.maxResourceLimit) / uint64(rcfg.elasticityMultiplier);
@@ -141,15 +154,9 @@ contract ResourceMetering_Test is Test {
         assertEq(postBaseFee, 2125000000);
     }
 
-    /**
-     * @notice This tests that the metered modifier reverts if
-     *         the ResourceConfig baseFeeMaxChangeDenominator
-     *         is set to 1.
-     *         Since the metered modifier internally calls
-     *         solmate's powWad function, it will revert
-     *         with the error string "UNDEFINED" since the
-     *         first parameter will be computed as 0.
-     */
+    /// @dev Tests that the metered modifier reverts if the baseFeeMaxChangeDenominator is set to 1.
+    ///      Since the metered modifier internally calls solmate's powWad function, it will revert
+    ///      with the error string "UNDEFINED" since the first parameter will be computed as 0.
     function test_meter_denominatorEq1_reverts() external {
         ResourceMetering.ResourceConfig memory rcfg = meter.resourceConfig();
         uint64 target = uint64(rcfg.maxResourceLimit) / uint64(rcfg.elasticityMultiplier);
@@ -167,6 +174,7 @@ contract ResourceMetering_Test is Test {
         meter.use(0);
     }
 
+    /// @dev Tests that the metered modifier reverts if the value is greater than allowed.
     function test_meter_useMoreThanMax_reverts() external {
         ResourceMetering.ResourceConfig memory rcfg = meter.resourceConfig();
         uint64 target = uint64(rcfg.maxResourceLimit) / uint64(rcfg.elasticityMultiplier);
@@ -176,8 +184,7 @@ contract ResourceMetering_Test is Test {
         meter.use(target * elasticityMultiplier + 1);
     }
 
-    // Demonstrates that the resource metering arithmetic can tolerate very large gaps between
-    // deposits.
+    /// @dev Tests that resource metering can handle large gaps between deposits.
     function testFuzz_meter_largeBlockDiff_succeeds(uint64 _amount, uint256 _blockDiff) external {
         // This test fails if the following line is commented out.
         // At 12 seconds per block, this number is effectively unreachable.
@@ -193,11 +200,9 @@ contract ResourceMetering_Test is Test {
     }
 }
 
-/**
- * @title CustomMeterUser
- * @notice A simple wrapper around `ResourceMetering` that allows the initial
- *         params to be set in the constructor.
- */
+/// @title CustomMeterUser
+/// @notice A simple wrapper around `ResourceMetering` that allows the initial
+///         params to be set in the constructor.
 contract CustomMeterUser is ResourceMetering {
     uint256 public startGas;
     uint256 public endGas;
@@ -230,15 +235,13 @@ contract CustomMeterUser is ResourceMetering {
     }
 }
 
-/**
- * @title ArtifactResourceMetering_Test
- * @notice A table test that sets the state of the ResourceParams and then requests
- *         various amounts of gas. This test ensures that a wide range of values
- *         can safely be used with the `ResourceMetering` contract.
- *         It also writes a CSV file to disk that includes useful information
- *         about how much gas is used and how expensive it is in USD terms to
- *         purchase the deposit gas.
- */
+/// @title ArtifactResourceMetering_Test
+/// @notice A table test that sets the state of the ResourceParams and then requests
+///         various amounts of gas. This test ensures that a wide range of values
+///         can safely be used with the `ResourceMetering` contract.
+///         It also writes a CSV file to disk that includes useful information
+///         about how much gas is used and how expensive it is in USD terms to
+///         purchase the deposit gas.
 contract ArtifactResourceMetering_Test is Test {
     uint128 internal minimumBaseFee;
     uint128 internal maximumBaseFee;
@@ -257,10 +260,7 @@ contract ArtifactResourceMetering_Test is Test {
     bytes32 internal emptyReturnData =
         0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470;
 
-    /**
-     * @notice Sets up the tests by getting constants from the ResourceMetering
-     *         contract.
-     */
+    /// @dev Sets up the tests with constants from the ResourceMetering contract.
     function setUp() public {
         vm.roll(1_000_000);
 
@@ -275,11 +275,8 @@ contract ArtifactResourceMetering_Test is Test {
         try vm.removeFile(outfile) {} catch {}
     }
 
-    /**
-     * @notice Generate a CSV file. The call to `meter` should be called with at
-     *         most the L1 block gas limit. Without specifying the amount of
-     *         gas, it can take very long to execute.
-     */
+    /// @dev Generates a CSV file. No more than the L1 block gas limit should
+    ///      be supplied to the `meter` function to avoid long execution time.
     function test_meter_generateArtifact_succeeds() external {
         vm.writeLine(
             outfile,

packages/contracts-bedrock/contracts/test/SystemConfig.t.sol

 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.15;
 
+// Testing utilities
 import { CommonTest } from "./CommonTest.t.sol";
-import { SystemConfig } from "../L1/SystemConfig.sol";
-import { ResourceMetering } from "../L1/ResourceMetering.sol";
+
+// Libraries
 import { Constants } from "../libraries/Constants.sol";
 
+// Target contract dependencies
+import { ResourceMetering } from "../L1/ResourceMetering.sol";
+
+// Target contract
+import { SystemConfig } from "../L1/SystemConfig.sol";
+
 contract SystemConfig_Init is CommonTest {
     SystemConfig sysConf;
 
@@ -34,6 +41,7 @@ contract SystemConfig_Init is CommonTest {
 }
 
 contract SystemConfig_Initialize_TestFail is SystemConfig_Init {
+    /// @dev Tests that initialization reverts if the gas limit is too low.
     function test_initialize_lowGasLimit_reverts() external {
         uint64 minimumGasLimit = sysConf.minimumGasLimit();
 
@@ -60,32 +68,39 @@ contract SystemConfig_Initialize_TestFail is SystemConfig_Init {
 }
 
 contract SystemConfig_Setters_TestFail is SystemConfig_Init {
+    /// @dev Tests that `setBatcherHash` reverts if the caller is not the owner.
     function test_setBatcherHash_notOwner_reverts() external {
         vm.expectRevert("Ownable: caller is not the owner");
         sysConf.setBatcherHash(bytes32(hex""));
     }
 
+    /// @dev Tests that `setGasConfig` reverts if the caller is not the owner.
     function test_setGasConfig_notOwner_reverts() external {
         vm.expectRevert("Ownable: caller is not the owner");
         sysConf.setGasConfig(0, 0);
     }
 
+    /// @dev Tests that `setGasLimit` reverts if the caller is not the owner.
     function test_setGasLimit_notOwner_reverts() external {
         vm.expectRevert("Ownable: caller is not the owner");
         sysConf.setGasLimit(0);
     }
 
+    /// @dev Tests that `setUnsafeBlockSigner` reverts if the caller is not the owner.
     function test_setUnsafeBlockSigner_notOwner_reverts() external {
         vm.expectRevert("Ownable: caller is not the owner");
         sysConf.setUnsafeBlockSigner(address(0x20));
     }
 
+    /// @dev Tests that `setResourceConfig` reverts if the caller is not the owner.
     function test_setResourceConfig_notOwner_reverts() external {
         ResourceMetering.ResourceConfig memory config = Constants.DEFAULT_RESOURCE_CONFIG();
         vm.expectRevert("Ownable: caller is not the owner");
         sysConf.setResourceConfig(config);
     }
 
+    /// @dev Tests that `setResourceConfig` reverts if the min base fee
+    ///      is greater than the maximum allowed base fee.
     function test_setResourceConfig_badMinMax_reverts() external {
         ResourceMetering.ResourceConfig memory config = ResourceMetering.ResourceConfig({
             maxResourceLimit: 20_000_000,
@@ -100,6 +115,8 @@ contract SystemConfig_Setters_TestFail is SystemConfig_Init {
         sysConf.setResourceConfig(config);
     }
 
+    /// @dev Tests that `setResourceConfig` reverts if the baseFeeMaxChangeDenominator
+    ///      is zero.
     function test_setResourceConfig_zeroDenominator_reverts() external {
         ResourceMetering.ResourceConfig memory config = ResourceMetering.ResourceConfig({
             maxResourceLimit: 20_000_000,
@@ -114,6 +131,7 @@ contract SystemConfig_Setters_TestFail is SystemConfig_Init {
         sysConf.setResourceConfig(config);
     }
 
+    /// @dev Tests that `setResourceConfig` reverts if the gas limit is too low.
     function test_setResourceConfig_lowGasLimit_reverts() external {
         uint64 gasLimit = sysConf.gasLimit();
 
@@ -130,6 +148,8 @@ contract SystemConfig_Setters_TestFail is SystemConfig_Init {
         sysConf.setResourceConfig(config);
     }
 
+    /// @dev Tests that `setResourceConfig` reverts if the elasticity multiplier
+    ///      and max resource limit are configured such that there is a loss of precision.
     function test_setResourceConfig_badPrecision_reverts() external {
         ResourceMetering.ResourceConfig memory config = ResourceMetering.ResourceConfig({
             maxResourceLimit: 20_000_000,
@@ -152,6 +172,7 @@ contract SystemConfig_Setters_Test is SystemConfig_Init {
         bytes data
     );
 
+    /// @dev Tests that `setBatcherHash` updates the batcher hash successfully.
     function testFuzz_setBatcherHash_succeeds(bytes32 newBatcherHash) external {
         vm.expectEmit(true, true, true, true);
         emit ConfigUpdate(0, SystemConfig.UpdateType.BATCHER, abi.encode(newBatcherHash));
@@ -161,6 +182,7 @@ contract SystemConfig_Setters_Test is SystemConfig_Init {
         assertEq(sysConf.batcherHash(), newBatcherHash);
     }
 
+    /// @dev Tests that `setGasConfig` updates the overhead and scalar successfully.
     function testFuzz_setGasConfig_succeeds(uint256 newOverhead, uint256 newScalar) external {
         vm.expectEmit(true, true, true, true);
         emit ConfigUpdate(
@@ -175,6 +197,7 @@ contract SystemConfig_Setters_Test is SystemConfig_Init {
         assertEq(sysConf.scalar(), newScalar);
     }
 
+    /// @dev Tests that `setGasLimit` updates the gas limit successfully.
     function testFuzz_setGasLimit_succeeds(uint64 newGasLimit) external {
         uint64 minimumGasLimit = sysConf.minimumGasLimit();
         newGasLimit = uint64(
@@ -189,6 +212,7 @@ contract SystemConfig_Setters_Test is SystemConfig_Init {
         assertEq(sysConf.gasLimit(), newGasLimit);
     }
 
+    /// @dev Tests that `setUnsafeBlockSigner` updates the block signer successfully.
     function testFuzz_setUnsafeBlockSigner_succeeds(address newUnsafeSigner) external {
         vm.expectEmit(true, true, true, true);
         emit ConfigUpdate(