Fix up styling

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

@@ -12,6 +12,7 @@ import { Semver } from "../universal/Semver.sol";
 ///         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.
     OptimismPortal public immutable PORTAL;
 

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

@@ -11,6 +11,7 @@ import { Semver } from "../universal/Semver.sol";
 ///         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.
     mapping(address => mapping(address => mapping(uint256 => bool))) public deposits;
@@ -26,7 +27,6 @@ contract L1ERC721Bridge is ERC721Bridge, Semver {
 
     /// @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.

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

@@ -16,6 +16,7 @@ import { Semver } from "../universal/Semver.sol";
 ///         of some token types that may not be properly supported by this contract include, but are
 ///         not limited to: tokens with transfer fees, rebasing tokens, and tokens with blocklists.
 contract L1StandardBridge is StandardBridge, Semver {
+
     /// @custom:legacy
     /// @notice Emitted whenever a deposit of ETH from L1 into L2 is initiated.
     /// @param from      Address of the depositor.

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

@@ -11,6 +11,7 @@ import { Types } from "../libraries/Types.sol";
 ///         commitment to the state of the L2 chain. Other contracts like the OptimismPortal use
 ///         these outputs to verify information about the state of L2.
 contract L2OutputOracle is Initializable, Semver {
+
     /// @notice The interval in L2 blocks at which checkpoints must be submitted.
     ///         Although this is immutable, it can safely be modified by upgrading the
     ///         implementation contract.

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

@@ -19,6 +19,7 @@ import { Semver } from "../universal/Semver.sol";
 ///         and L2. Messages sent directly to the OptimismPortal have no form of replayability.
 ///         Users are encouraged to use the L1CrossDomainMessenger for a higher-level interface.
 contract OptimismPortal is Initializable, ResourceMetering, Semver {
+
     /// @notice Represents a proven withdrawal.
     /// @custom:field outputRoot    Root of the L2 output this was proven against.
     /// @custom:field timestamp     Timestamp at whcih the withdrawal was proven.

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

@@ -11,6 +11,7 @@ import { Arithmetic } from "../libraries/Arithmetic.sol";
 /// @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).

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

@@ -12,6 +12,7 @@ import { ResourceMetering } from "./ResourceMetering.sol";
 ///         All configuration is stored on L1 and picked up by L2 as part of the derviation of
 ///         the L2 chain.
 contract SystemConfig is OwnableUpgradeable, Semver {
+
     /// @notice Enum representing different types of updates.
     /// @custom:value BATCHER              Represents an update to the batcher hash.
     /// @custom:value GAS_CONFIG           Represents an update to txn fee config on L2.

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 +87,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 +97,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 +129,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 +169,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 +200,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 +220,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 +243,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 +301,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 +345,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 +389,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 +465,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 +525,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/L1StandardBridge.t.sol

 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.15;
 
+// Testing utilities
+import { stdStorage, StdStorage } from "forge-std/Test.sol";
+import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
 import { Bridge_Initializer } from "./CommonTest.t.sol";
+
+// Libraries
+import { Predeploys } from "../libraries/Predeploys.sol";
+
+// Target contract dependencies
 import { StandardBridge } from "../universal/StandardBridge.sol";
-import { OptimismPortal } from "../L1/OptimismPortal.sol";
 import { L2StandardBridge } from "../L2/L2StandardBridge.sol";
 import { CrossDomainMessenger } from "../universal/CrossDomainMessenger.sol";
-import { Predeploys } from "../libraries/Predeploys.sol";
 import { AddressAliasHelper } from "../vendor/AddressAliasHelper.sol";
-import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
-import { stdStorage, StdStorage } from "forge-std/Test.sol";
+
+// Target contract
+import { OptimismPortal } from "../L1/OptimismPortal.sol";
 
 contract L1StandardBridge_Getter_Test is Bridge_Initializer {
+    /// @dev Test that the accessors return the correct initialized values.
     function test_getters_succeeds() external {
         assert(L1Bridge.l2TokenBridge() == address(L2Bridge));
         assert(L1Bridge.OTHER_BRIDGE() == L2Bridge);
@@ -22,11 +30,10 @@ contract L1StandardBridge_Getter_Test is Bridge_Initializer {
 }
 
 contract L1StandardBridge_Initialize_Test is Bridge_Initializer {
+    /// @dev Test that the initialize function sets the correct values.
     function test_initialize_succeeds() external {
         assertEq(address(L1Bridge.messenger()), address(L1Messenger));
-
         assertEq(address(L1Bridge.OTHER_BRIDGE()), Predeploys.L2_STANDARD_BRIDGE);
-
         assertEq(address(L2Bridge), Predeploys.L2_STANDARD_BRIDGE);
     }
 }
@@ -34,8 +41,8 @@ contract L1StandardBridge_Initialize_Test is Bridge_Initializer {
 contract L1StandardBridge_Initialize_TestFail is Bridge_Initializer {}
 
 contract L1StandardBridge_Receive_Test is Bridge_Initializer {
-    // receive
-    // - can accept ETH
+
+    /// @dev Tests receive bridges ETH successfully.
     function test_receive_succeeds() external {
         assertEq(address(op).balance, 0);
 
@@ -72,6 +79,8 @@ contract L1StandardBridge_Receive_Test is Bridge_Initializer {
 contract L1StandardBridge_Receive_TestFail {}
 
 contract PreBridgeETH is Bridge_Initializer {
+    /// @dev Asserts the expected calls and events for bridging ETH depending
+    ///      on whether the bridge call is legacy or not.
     function _preBridgeETH(bool isLegacy) internal {
         assertEq(address(op).balance, 0);
         uint256 nonce = L1Messenger.messageNonce();
@@ -165,12 +174,11 @@ contract PreBridgeETH is Bridge_Initializer {
 }
 
 contract L1StandardBridge_DepositETH_Test is PreBridgeETH {
-    // depositETH
-    // - emits ETHDepositInitiated
-    // - emits ETHBridgeInitiated
-    // - calls optimismPortal.depositTransaction
-    // - only EOA
-    // - ETH ends up in the optimismPortal
+    /// @dev Tests that depositing ETH succeeds.
+    ///      Emits ETHDepositInitiated and ETHBridgeInitiated events.
+    ///      Calls depositTransaction on the OptimismPortal.
+    ///      Only EOA can call depositETH.
+    ///      ETH ends up in the optimismPortal.
     function test_depositETH_succeeds() external {
         _preBridgeETH({ isLegacy: true });
         L1Bridge.depositETH{ value: 500 }(50000, hex"dead");
@@ -179,12 +187,11 @@ contract L1StandardBridge_DepositETH_Test is PreBridgeETH {
 }
 
 contract L1StandardBridge_BridgeETH_Test is PreBridgeETH {
-    // BridgeETH
-    // - emits ETHDepositInitiated
-    // - emits ETHBridgeInitiated
-    // - calls optimismPortal.depositTransaction
-    // - only EOA
-    // - ETH ends up in the optimismPortal
+    /// @dev Tests that bridging ETH succeeds.
+    ///      Emits ETHDepositInitiated and ETHBridgeInitiated events.
+    ///      Calls depositTransaction on the OptimismPortal.
+    ///      Only EOA can call bridgeETH.
+    ///      ETH ends up in the optimismPortal.
     function test_bridgeETH_succeeds() external {
         _preBridgeETH({ isLegacy: false });
         L1Bridge.bridgeETH{ value: 500 }(50000, hex"dead");
@@ -193,10 +200,9 @@ contract L1StandardBridge_BridgeETH_Test is PreBridgeETH {
 }
 
 contract L1StandardBridge_DepositETH_TestFail is Bridge_Initializer {
+    /// @dev Tests that depositing ETH reverts if the call is not from an EOA.
     function test_depositETH_notEoa_reverts() external {
-        // turn alice into a contract
         vm.etch(alice, address(L1Token).code);
-
         vm.expectRevert("StandardBridge: function can only be called from an EOA");
         vm.prank(alice);
         L1Bridge.depositETH{ value: 1 }(300, hex"");
@@ -204,6 +210,8 @@ contract L1StandardBridge_DepositETH_TestFail is Bridge_Initializer {
 }
 
 contract PreBridgeETHTo is Bridge_Initializer {
+    /// @dev Asserts the expected calls and events for bridging ETH to a different
+    ///      address depending on whether the bridge call is legacy or not.
     function _preBridgeETHTo(bool isLegacy) internal {
         assertEq(address(op).balance, 0);
         uint256 nonce = L1Messenger.messageNonce();
@@ -299,11 +307,11 @@ contract PreBridgeETHTo is Bridge_Initializer {
 }
 
 contract L1StandardBridge_DepositETHTo_Test is PreBridgeETHTo {
-    // depositETHTo
-    // - emits ETHDepositInitiated
-    // - calls optimismPortal.depositTransaction
-    // - EOA or contract can call
-    // - ETH ends up in the optimismPortal
+    /// @dev Tests that depositing ETH to a different address succeeds.
+    ///      Emits ETHDepositInitiated event.
+    ///      Calls depositTransaction on the OptimismPortal.
+    ///      EOA or contract can call depositETHTo.
+    ///      ETH ends up in the optimismPortal.
     function test_depositETHTo_succeeds() external {
         _preBridgeETHTo({ isLegacy: true });
         L1Bridge.depositETHTo{ value: 600 }(bob, 60000, hex"dead");
@@ -312,12 +320,11 @@ contract L1StandardBridge_DepositETHTo_Test is PreBridgeETHTo {
 }
 
 contract L1StandardBridge_BridgeETHTo_Test is PreBridgeETHTo {
-    // BridgeETHTo
-    // - emits ETHDepositInitiated
-    // - emits ETHBridgeInitiated
-    // - calls optimismPortal.depositTransaction
-    // - only EOA
-    // - ETH ends up in the optimismPortal
+    /// @dev Tests that bridging ETH to a different address succeeds.
+    ///      Emits ETHDepositInitiated and ETHBridgeInitiated events.
+    ///      Calls depositTransaction on the OptimismPortal.
+    ///      Only EOA can call bridgeETHTo.
+    ///      ETH ends up in the optimismPortal.
     function test_bridgeETHTo_succeeds() external {
         _preBridgeETHTo({ isLegacy: false });
         L1Bridge.bridgeETHTo{ value: 600 }(bob, 60000, hex"dead");
@@ -335,6 +342,12 @@ contract L1StandardBridge_DepositERC20_Test is Bridge_Initializer {
     // - emits ERC20DepositInitiated
     // - calls optimismPortal.depositTransaction
     // - only callable by EOA
+
+    /// @dev Tests that depositing ERC20 to the bridge succeeds.
+    ///      Bridge deposits are updated.
+    ///      Emits ERC20DepositInitiated event.
+    ///      Calls depositTransaction on the OptimismPortal.
+    ///      Only EOA can call depositERC20.
     function test_depositERC20_succeeds() external {
         uint256 nonce = L1Messenger.messageNonce();
         uint256 version = 0; // Internal constant in the OptimismPortal: DEPOSIT_VERSION
@@ -429,6 +442,8 @@ contract L1StandardBridge_DepositERC20_Test is Bridge_Initializer {
 }
 
 contract L1StandardBridge_DepositERC20_TestFail is Bridge_Initializer {
+    /// @dev Tests that depositing an ERC20 to the bridge reverts
+    ///      if the caller is not an EOA.
     function test_depositERC20_notEoa_reverts() external {
         // turn alice into a contract
         vm.etch(alice, hex"ffff");
@@ -440,11 +455,12 @@ contract L1StandardBridge_DepositERC20_TestFail is Bridge_Initializer {
 }
 
 contract L1StandardBridge_DepositERC20To_Test is Bridge_Initializer {
-    // depositERC20To
-    // - updates bridge.deposits
-    // - emits ERC20DepositInitiated
-    // - calls optimismPortal.depositTransaction
-    // - callable by a contract
+    /// @dev Tests that depositing ERC20 to the bridge succeeds when
+    ///      sent to a different address.
+    ///      Bridge deposits are updated.
+    ///      Emits ERC20DepositInitiated event.
+    ///      Calls depositTransaction on the OptimismPortal.
+    ///      Contracts can call depositERC20.
     function test_depositERC20To_succeeds() external {
         uint256 nonce = L1Messenger.messageNonce();
         uint256 version = 0; // Internal constant in the OptimismPortal: DEPOSIT_VERSION
@@ -541,9 +557,9 @@ contract L1StandardBridge_DepositERC20To_Test is Bridge_Initializer {
 contract L1StandardBridge_FinalizeETHWithdrawal_Test is Bridge_Initializer {
     using stdStorage for StdStorage;
 
-    // finalizeETHWithdrawal
-    // - emits ETHWithdrawalFinalized
-    // - only callable by L2 bridge
+    /// @dev Tests that finalizing an ETH withdrawal succeeds.
+    ///      Emits ETHWithdrawalFinalized event.
+    ///      Only callable by the L2 bridge.
     function test_finalizeETHWithdrawal_succeeds() external {
         uint256 aliceBalance = alice.balance;
 
@@ -575,10 +591,10 @@ contract L1StandardBridge_FinalizeETHWithdrawal_TestFail is Bridge_Initializer {
 contract L1StandardBridge_FinalizeERC20Withdrawal_Test is Bridge_Initializer {
     using stdStorage for StdStorage;
 
-    // finalizeERC20Withdrawal
-    // - updates bridge.deposits
-    // - emits ERC20WithdrawalFinalized
-    // - only callable by L2 bridge
+    /// @dev Tests that finalizing an ERC20 withdrawal succeeds.
+    ///      Bridge deposits are updated.
+    ///      Emits ERC20WithdrawalFinalized event.
+    ///      Only callable by the L2 bridge.
     function test_finalizeERC20Withdrawal_succeeds() external {
         deal(address(L1Token), address(L1Bridge), 100, true);
 
@@ -625,6 +641,7 @@ contract L1StandardBridge_FinalizeERC20Withdrawal_Test is Bridge_Initializer {
 }
 
 contract L1StandardBridge_FinalizeERC20Withdrawal_TestFail is Bridge_Initializer {
+    /// @dev Tests that finalizing an ERC20 withdrawal reverts if the caller is not the L2 bridge.
     function test_finalizeERC20Withdrawal_notMessenger_reverts() external {
         vm.mockCall(
             address(L1Bridge.messenger()),
@@ -643,6 +660,7 @@ contract L1StandardBridge_FinalizeERC20Withdrawal_TestFail is Bridge_Initializer
         );
     }
 
+    /// @dev Tests that finalizing an ERC20 withdrawal reverts if the caller is not the L2 bridge.
     function test_finalizeERC20Withdrawal_notOtherBridge_reverts() external {
         vm.mockCall(
             address(L1Bridge.messenger()),
@@ -663,6 +681,7 @@ contract L1StandardBridge_FinalizeERC20Withdrawal_TestFail is Bridge_Initializer
 }
 
 contract L1StandardBridge_FinalizeBridgeETH_Test is Bridge_Initializer {
+    /// @dev Tests that finalizing bridged ETH succeeds.
     function test_finalizeBridgeETH_succeeds() external {
         address messenger = address(L1Bridge.messenger());
         vm.mockCall(
@@ -681,6 +700,7 @@ contract L1StandardBridge_FinalizeBridgeETH_Test is Bridge_Initializer {
 }
 
 contract L1StandardBridge_FinalizeBridgeETH_TestFail is Bridge_Initializer {
+    /// @dev Tests that finalizing bridged ETH reverts if the amount is incorrect.
     function test_finalizeBridgeETH_incorrectValue_reverts() external {
         address messenger = address(L1Bridge.messenger());
         vm.mockCall(
@@ -694,6 +714,7 @@ contract L1StandardBridge_FinalizeBridgeETH_TestFail is Bridge_Initializer {
         L1Bridge.finalizeBridgeETH{ value: 50 }(alice, alice, 100, hex"");
     }
 
+    /// @dev Tests that finalizing bridged ETH reverts if the destination is the L1 bridge.
     function test_finalizeBridgeETH_sendToSelf_reverts() external {
         address messenger = address(L1Bridge.messenger());
         vm.mockCall(
@@ -707,6 +728,7 @@ contract L1StandardBridge_FinalizeBridgeETH_TestFail is Bridge_Initializer {
         L1Bridge.finalizeBridgeETH{ value: 100 }(alice, address(L1Bridge), 100, hex"");
     }
 
+    /// @dev Tests that finalizing bridged ETH reverts if the destination is the messenger.
     function test_finalizeBridgeETH_sendToMessenger_reverts() external {
         address messenger = address(L1Bridge.messenger());
         vm.mockCall(

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

 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.15;
 
+// Testing utilities
 import { stdError } from "forge-std/Test.sol";
 import { L2OutputOracle_Initializer, NextImpl } from "./CommonTest.t.sol";
-import { L2OutputOracle } from "../L1/L2OutputOracle.sol";
-import { Proxy } from "../universal/Proxy.sol";
+
+// Libraries
 import { Types } from "../libraries/Types.sol";
 
-contract L2OutputOracleTest is L2OutputOracle_Initializer {
-    bytes32 proposedOutput1 = keccak256(abi.encode(1));
+// Target contract dependencies
+import { Proxy } from "../universal/Proxy.sol";
+
+// Target contract
+import { L2OutputOracle } from "../L1/L2OutputOracle.sol";
 
+contract L2OutputOracle_constructor_Test is L2OutputOracle_Initializer {
+    /// @dev Tests that constructor sets the initial values correctly.
     function test_constructor_succeeds() external {
         assertEq(oracle.PROPOSER(), proposer);
         assertEq(oracle.CHALLENGER(), owner);
@@ -19,6 +25,7 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
         assertEq(oracle.startingTimestamp(), startingTimestamp);
     }
 
+    /// @dev Tests that the constructor reverts if the starting timestamp is invalid.
     function test_constructor_badTimestamp_reverts() external {
         vm.expectRevert("L2OutputOracle: starting L2 timestamp must be less than current time");
 
@@ -34,6 +41,7 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
         });
     }
 
+    /// @dev Tests that the constructor reverts if the l2BlockTime is invalid.
     function test_constructor_l2BlockTimeZero_reverts() external {
         vm.expectRevert("L2OutputOracle: L2 block time must be greater than 0");
         new L2OutputOracle({
@@ -47,6 +55,7 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
         });
     }
 
+    /// @dev Tests that the constructor reverts if the submissionInterval is zero.
     function test_constructor_submissionInterval_reverts() external {
         vm.expectRevert("L2OutputOracle: submission interval must be greater than 0");
         new L2OutputOracle({
@@ -59,12 +68,12 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
             _finalizationPeriodSeconds: 7 days
         });
     }
+}
 
-    /****************
-     * Getter Tests *
-     ****************/
+contract L2OutputOracle_getter_Test is L2OutputOracle_Initializer {
+    bytes32 proposedOutput1 = keccak256(abi.encode(1));
 
-    // Test: latestBlockNumber() should return the correct value
+    /// @dev Tests that `latestBlockNumber` returns the correct value.
     function test_latestBlockNumber_succeeds() external {
         uint256 proposedNumber = oracle.nextBlockNumber();
 
@@ -75,7 +84,7 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
         assertEq(oracle.latestBlockNumber(), proposedNumber);
     }
 
-    // Test: getL2Output() should return the correct value
+    /// @dev Tests that `getL2Output` returns the correct value.
     function test_getL2Output_succeeds() external {
         uint256 nextBlockNumber = oracle.nextBlockNumber();
         uint256 nextOutputIndex = oracle.nextOutputIndex();
@@ -92,7 +101,8 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
         oracle.getL2Output(nextOutputIndex + 1);
     }
 
-    // Test: getL2OutputIndexAfter() returns correct value when input is exact block
+    /// @dev Tests that `getL2OutputIndexAfter` returns the correct value
+    ///      when the input is the exact block number of the proposal.
     function test_getL2OutputIndexAfter_sameBlock_succeeds() external {
         bytes32 output1 = keccak256(abi.encode(1));
         uint256 nextBlockNumber1 = oracle.nextBlockNumber();
@@ -105,7 +115,8 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
         assertEq(index1, 0);
     }
 
-    // Test: getL2OutputIndexAfter() returns correct value when input is previous block
+    /// @dev Tests that `getL2OutputIndexAfter` returns the correct value
+    ///      when the input is the previous block number of the proposal.
     function test_getL2OutputIndexAfter_previousBlock_succeeds() external {
         bytes32 output1 = keccak256(abi.encode(1));
         uint256 nextBlockNumber1 = oracle.nextBlockNumber();
@@ -118,7 +129,7 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
         assertEq(index1, 0);
     }
 
-    // Test: getL2OutputIndexAfter() returns correct value during binary search
+    /// @dev Tests that `getL2OutputIndexAfter` returns the correct value.
     function test_getL2OutputIndexAfter_multipleOutputsExist_succeeds() external {
         bytes32 output1 = keccak256(abi.encode(1));
         uint256 nextBlockNumber1 = oracle.nextBlockNumber();
@@ -157,13 +168,13 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
         assertEq(index3, 3);
     }
 
-    // Test: getL2OutputIndexAfter() reverts when no output exists yet
+    /// @dev Tests that `getL2OutputIndexAfter` reverts when no output exists.
     function test_getL2OutputIndexAfter_noOutputsExis_reverts() external {
         vm.expectRevert("L2OutputOracle: cannot get output as no outputs have been proposed yet");
         oracle.getL2OutputIndexAfter(0);
     }
 
-    // Test: nextBlockNumber() should return the correct value
+    /// @dev Tests that `nextBlockNumber` returns the correct value.
     function test_nextBlockNumber_succeeds() external {
         assertEq(
             oracle.nextBlockNumber(),
@@ -172,34 +183,33 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
         );
     }
 
+    /// @dev Tests that `computeL2Timestamp` returns the correct value.
     function test_computeL2Timestamp_succeeds() external {
         // reverts if timestamp is too low
         vm.expectRevert(stdError.arithmeticError);
         oracle.computeL2Timestamp(startingBlockNumber - 1);
 
-        // returns the correct value...
-        // ... for the very first block
+        // check timestamp for the very first block
         assertEq(oracle.computeL2Timestamp(startingBlockNumber), startingTimestamp);
 
-        // ... for the first block after the starting block
+        // check timestamp for the first block after the starting block
         assertEq(
             oracle.computeL2Timestamp(startingBlockNumber + 1),
             startingTimestamp + l2BlockTime
         );
 
-        // ... for some other block number
+        // check timestamp for some other block number
         assertEq(
             oracle.computeL2Timestamp(startingBlockNumber + 96024),
             startingTimestamp + l2BlockTime * 96024
         );
     }
+}
 
-    /*****************************
-     * Propose Tests - Happy Path *
-     *****************************/
+contract L2OutputOracle_proposeL2Output_Test is L2OutputOracle_Initializer {
 
-    // Test: proposeL2Output succeeds when given valid input, and no block hash and number are
-    // specified.
+    /// @dev Test that `proposeL2Output` succeeds for a valid input
+    ///      and when a block hash and number are not specified.
     function test_proposeL2Output_proposeAnotherOutput_succeeds() public {
         bytes32 proposedOutput2 = keccak256(abi.encode());
         uint256 nextBlockNumber = oracle.nextBlockNumber();
@@ -219,8 +229,8 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
         oracle.proposeL2Output(proposedOutput2, nextBlockNumber, 0, 0);
     }
 
-    // Test: proposeL2Output succeeds when given valid input, and when a block hash and number are
-    // specified for reorg protection.
+    /// @dev Tests that `proposeL2Output` succeeds when given valid input and
+    ///      when a block hash and number are specified for reorg protection.
     function test_proposeWithBlockhashAndHeight_succeeds() external {
         // Get the number and hash of a previous block in the chain
         uint256 prevL1BlockNumber = block.number - 1;
@@ -232,11 +242,8 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
         oracle.proposeL2Output(nonZeroHash, nextBlockNumber, prevL1BlockHash, prevL1BlockNumber);
     }
 
-    /***************************
-     * Propose Tests - Sad Path *
-     ***************************/
-
-    // Test: proposeL2Output fails if called by a party that is not the proposer.
+    /// @dev Tests that `proposeL2Output` reverts when called by a party
+    ///      that is not the proposer.
     function test_proposeL2Output_notProposer_reverts() external {
         uint256 nextBlockNumber = oracle.nextBlockNumber();
         warpToProposeTime(nextBlockNumber);
@@ -246,7 +253,7 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
         oracle.proposeL2Output(nonZeroHash, nextBlockNumber, 0, 0);
     }
 
-    // Test: proposeL2Output fails given a zero blockhash.
+    /// @dev Tests that `proposeL2Output` reverts when given a zero blockhash.
     function test_proposeL2Output_emptyOutput_reverts() external {
         bytes32 outputToPropose = bytes32(0);
         uint256 nextBlockNumber = oracle.nextBlockNumber();
@@ -256,7 +263,8 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
         oracle.proposeL2Output(outputToPropose, nextBlockNumber, 0, 0);
     }
 
-    // Test: proposeL2Output fails if the block number doesn't match the next expected number.
+    /// @dev Tests that `proposeL2Output` reverts when given a block number
+    ///      that does not match the next expected block number.
     function test_proposeL2Output_unexpectedBlockNumber_reverts() external {
         uint256 nextBlockNumber = oracle.nextBlockNumber();
         warpToProposeTime(nextBlockNumber);
@@ -265,7 +273,8 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
         oracle.proposeL2Output(nonZeroHash, nextBlockNumber - 1, 0, 0);
     }
 
-    // Test: proposeL2Output fails if it would have a timestamp in the future.
+    /// @dev Tests that `proposeL2Output` reverts when given a block number
+    ///      that has a timestamp in the future.
     function test_proposeL2Output_futureTimetamp_reverts() external {
         uint256 nextBlockNumber = oracle.nextBlockNumber();
         uint256 nextTimestamp = oracle.computeL2Timestamp(nextBlockNumber);
@@ -275,8 +284,8 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
         oracle.proposeL2Output(nonZeroHash, nextBlockNumber, 0, 0);
     }
 
-    // Test: proposeL2Output fails if a non-existent L1 block hash and number are provided for reorg
-    // protection.
+    /// @dev Tests that `proposeL2Output` reverts when given a block number
+    ///      whose hash does not match the given block hash.
     function test_proposeL2Output_wrongFork_reverts() external {
         uint256 nextBlockNumber = oracle.nextBlockNumber();
         warpToProposeTime(nextBlockNumber);
@@ -292,8 +301,8 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
         );
     }
 
-    // Test: proposeL2Output fails when given valid input, but the block hash and number do not
-    // match.
+    /// @dev Tests that `proposeL2Output` reverts when given a block number
+    ///      whose block hash does not match the given block hash.
     function test_proposeL2Output_unmatchedBlockhash_reverts() external {
         // Move ahead to block 100 so that we can reference historical blocks
         vm.roll(100);
@@ -312,11 +321,11 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
         );
         oracle.proposeL2Output(nonZeroHash, nextBlockNumber, l1BlockHash, l1BlockNumber - 1);
     }
+}
 
-    /*****************************
-     * Delete Tests - Happy Path *
-     *****************************/
+contract L2OutputOracle_deleteOutputs_Test is L2OutputOracle_Initializer {
 
+    /// @dev Tests that `deleteL2Outputs` succeeds for a single output.
     function test_deleteOutputs_singleOutput_succeeds() external {
         test_proposeL2Output_proposeAnotherOutput_succeeds();
         test_proposeL2Output_proposeAnotherOutput_succeeds();
@@ -341,6 +350,7 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
         assertEq(newLatestOutput.timestamp, proposal.timestamp);
     }
 
+    /// @dev Tests that `deleteL2Outputs` succeeds for multiple outputs.
     function test_deleteOutputs_multipleOutputs_succeeds() external {
         test_proposeL2Output_proposeAnotherOutput_succeeds();
         test_proposeL2Output_proposeAnotherOutput_succeeds();
@@ -367,10 +377,7 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
         assertEq(newLatestOutput.timestamp, proposal.timestamp);
     }
 
-    /***************************
-     * Delete Tests - Sad Path *
-     ***************************/
-
+    /// @dev Tests that `deleteL2Outputs` reverts when not called by the challenger.
     function test_deleteL2Outputs_ifNotChallenger_reverts() external {
         uint256 latestBlockNumber = oracle.latestBlockNumber();
 
@@ -378,6 +385,7 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
         oracle.deleteL2Outputs(latestBlockNumber);
     }
 
+    /// @dev Tests that `deleteL2Outputs` reverts for a non-existant output index.
     function test_deleteL2Outputs_nonExistent_reverts() external {
         test_proposeL2Output_proposeAnotherOutput_succeeds();
 
@@ -388,6 +396,8 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
         oracle.deleteL2Outputs(latestBlockNumber + 1);
     }
 
+    /// @dev Tests that `deleteL2Outputs` reverts when trying to delete outputs
+    ///      after the latest output index.
     function test_deleteL2Outputs_afterLatest_reverts() external {
         // Start by proposing three outputs
         test_proposeL2Output_proposeAnotherOutput_succeeds();
@@ -405,6 +415,7 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
         oracle.deleteL2Outputs(latestOutputIndex - 2);
     }
 
+    /// @dev Tests that `deleteL2Outputs` reverts for finalized outputs.
     function test_deleteL2Outputs_finalized_reverts() external {
         test_proposeL2Output_proposeAnotherOutput_succeeds();
 
@@ -428,6 +439,7 @@ contract L2OutputOracleUpgradeable_Test is L2OutputOracle_Initializer {
         proxy = Proxy(payable(address(oracle)));
     }
 
+    /// @dev Tests that the proxy is initialized with the correct values.
     function test_initValuesOnProxy_succeeds() external {
         assertEq(submissionInterval, oracleImpl.SUBMISSION_INTERVAL());
         assertEq(l2BlockTime, oracleImpl.L2_BLOCK_TIME());
@@ -438,16 +450,19 @@ contract L2OutputOracleUpgradeable_Test is L2OutputOracle_Initializer {
         assertEq(owner, oracleImpl.CHALLENGER());
     }
 
+    /// @dev Tests that the proxy cannot be initialized twice.
     function test_initializeProxy_alreadyInitialized_reverts() external {
         vm.expectRevert("Initializable: contract is already initialized");
         L2OutputOracle(payable(proxy)).initialize(startingBlockNumber, startingTimestamp);
     }
 
+    /// @dev Tests that the implementation contract cannot be initialized twice.
     function test_initializeImpl_alreadyInitialized_reverts() external {
         vm.expectRevert("Initializable: contract is already initialized");
         L2OutputOracle(oracleImpl).initialize(startingBlockNumber, startingTimestamp);
     }
 
+    /// @dev Tests that the proxy can be successfully upgraded.
     function test_upgrading_succeeds() external {
         // Check an unused slot before upgrading.
         bytes32 slot21Before = vm.load(address(oracle), bytes32(uint256(21)));