feat(ctp): update style for L1 contracts (#2704)

Updates and standardizes the style for the L1 contracts. Adds a few
comments where things were missing.
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

.changeset/shaggy-zebras-juggle.md

+---
+'@eth-optimism/contracts-bedrock': patch
+---
+
+Update comments and style for L1 contracts

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

@@ -6,28 +6,22 @@ import { OptimismPortal } from "./OptimismPortal.sol";
 import { CrossDomainMessenger } from "../universal/CrossDomainMessenger.sol";
 
 /**
+ * @custom:proxied
  * @title L1CrossDomainMessenger
- * @dev The L1 Cross Domain Messenger contract sends messages from L1 to L2, and relays messages
- * from L2 onto L1.
- * This contract should be deployed behind an upgradable proxy
+ * @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 {
-    /*************
-     * Variables *
-     *************/
-
     /**
      * @notice Address of the OptimismPortal.
      */
     OptimismPortal public portal;
 
-    /********************
-     * Public Functions *
-     ********************/
-
     /**
-     * @notice Initialize the L1CrossDomainMessenger
-     * @param _portal The OptimismPortal
+     * @notice Initializes the L1CrossDomainMessenger.
+     *
+     * @param _portal Address of the OptimismPortal to send and receive messages through.
      */
     function initialize(OptimismPortal _portal) external {
         portal = _portal;
@@ -38,21 +32,22 @@ contract L1CrossDomainMessenger is CrossDomainMessenger {
         _initialize(Lib_PredeployAddresses.L2_CROSS_DOMAIN_MESSENGER, blockedSystemAddresses);
     }
 
-    /**********************
-     * Internal Functions *
-     **********************/
-
     /**
-     * @notice Ensure that the L1CrossDomainMessenger can only be called
-     * by the OptimismPortal and the L2 sender is the L2CrossDomainMessenger.
+     * @notice Checks whether the message being sent from the other messenger.
+     *
+     * @return True if the message was sent from the messenger, false otherwise.
      */
     function _isSystemMessageSender() internal view override returns (bool) {
         return msg.sender == address(portal) && portal.l2Sender() == otherMessenger;
     }
 
     /**
-     * @notice Sending a message in the L1CrossDomainMessenger involves
-     * depositing through the OptimismPortal.
+     * @notice Sends a message via the OptimismPortal contract.
+     *
+     * @param _to       Address of the recipient on L2.
+     * @param _gasLimit Minimum gas limit that the message can be executed with.
+     * @param _value    ETH value to attach to the message and send to the recipient.
+     * @param _data     Data to attach to the message and call the recipient with.
      */
     function _sendMessage(
         address _to,

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

@@ -8,13 +8,24 @@ import { ExcessivelySafeCall } from "../libraries/ExcessivelySafeCall.sol";
 import { ResourceMetering } from "./ResourceMetering.sol";
 
 /**
+ * @custom:proxied
  * @title OptimismPortal
- * This contract should be deployed behind an upgradable proxy.
+ * @notice The OptimismPortal is a low-level contract responsible for passing messages between L1
+ *         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 ResourceMetering {
     /**
-     * Emitted when a Transaction is deposited from L1 to L2. The parameters of this
-     * event are read by the rollup node and used to derive deposit transactions on L2.
+     * @notice Emitted when a transaction is deposited from L1 to L2. The parameters of this event
+     *         are read by the rollup node and used to derive deposit transactions on L2.
+     *
+     * @param from       Address that triggered the deposit transaction.
+     * @param to         Address that the deposit transaction is directed to.
+     * @param mint       Amount of ETH to mint to the sender on L2.
+     * @param value      Amount of ETH to send to the recipient.
+     * @param gasLimit   Minimum gas limit that the message can be executed with.
+     * @param isCreation Whether the message is a contract creation.
+     * @param data       Data to attach to the message and call the recipient with.
      */
     event TransactionDeposited(
         address indexed from,
@@ -27,40 +38,42 @@ contract OptimismPortal is ResourceMetering {
     );
 
     /**
-     * Emitted when a withdrawal is finalized
+     * @notice Emitted when a withdrawal transaction is finalized.
+     *
+     * @param withdrawalHash Hash of the withdrawal transaction.
+     * @param success        Whether the withdrawal transaction was successful.
      */
-    event WithdrawalFinalized(bytes32 indexed, bool success);
+    event WithdrawalFinalized(bytes32 indexed withdrawalHash, bool success);
 
     /**
-     * Value used to reset the l2Sender, this is more efficient than setting it to zero.
+     * @notice Value used to reset the l2Sender, this is more efficient than setting it to zero.
      */
     address internal constant DEFAULT_L2_SENDER = 0x000000000000000000000000000000000000dEaD;
 
     /**
-     * Minimum time that must elapse before a withdrawal can be finalized.
+     * @notice Minimum time (in seconds) that must elapse before a withdrawal can be finalized.
      */
     uint256 public immutable FINALIZATION_PERIOD_SECONDS;
 
     /**
-     * Address of the L2OutputOracle.
+     * @notice Address of the L2OutputOracle.
      */
     L2OutputOracle public immutable L2_ORACLE;
 
     /**
-     * Public variable which can be used to read the address of the L2 account which initiated the
-     * withdrawal. Can also be used to determine whether or not execution is occuring downstream of
-     * a call to finalizeWithdrawalTransaction().
+     * @notice Address of the L2 account which initiated a withdrawal in this transaction. If the
+     *         of this variable is the default L2 sender address, then we are NOT inside of a call
+     *         to finalizeWithdrawalTransaction.
      */
     address public l2Sender = DEFAULT_L2_SENDER;
 
     /**
-     * A list of withdrawal hashes which have been successfully finalized.
-     * Used for replay protection.
+     * @notice A list of withdrawal hashes which have been successfully finalized.
      */
     mapping(bytes32 => bool) public finalizedWithdrawals;
 
     /**
-     * @param _l2Oracle Address of the L2OutputOracle.
+     * @param _l2Oracle                  Address of the L2OutputOracle.
      * @param _finalizationPeriodSeconds Finalization time in seconds.
      */
     constructor(L2OutputOracle _l2Oracle, uint256 _finalizationPeriodSeconds) {
@@ -69,9 +82,9 @@ contract OptimismPortal is ResourceMetering {
     }
 
     /**
-     * Accepts value so that users can send ETH directly to this contract and have the funds be
-     * deposited to their address on L2. This is intended as a convenience function for EOAs.
-     * Contracts should call the depositTransaction() function directly.
+     * @notice Accepts value so that users can send ETH directly to this contract and have the
+     *         funds be deposited to their address on L2. This is intended as a convenience
+     *         function for EOAs. Contracts should call the depositTransaction() function directly.
      */
     receive() external payable {
         depositTransaction(msg.sender, msg.value, 100000, false, bytes(""));
@@ -79,16 +92,15 @@ contract OptimismPortal is ResourceMetering {
 
     /**
      * @notice Accepts deposits of ETH and data, and emits a TransactionDeposited event for use in
-     * deriving deposit transactions. Note that if a deposit is made by a contract, its address will
-     * be aliased when retrieved using `tx.origin` or `msg.sender`. This can lead to loss of funds
-     * in some cases which the depositing contract may not have accounted for. Consider using the
-     * Bridge or CrossDomainMessenger contracts which provide additional safety assurances.
+     *         deriving deposit transactions. Note that if a deposit is made by a contract, its
+     *         address will be aliased when retrieved using `tx.origin` or `msg.sender`. Consider
+     *         using the CrossDomainMessenger contracts for a simpler developer experience.
      *
-     * @param _to The L2 destination address.
-     * @param _value The ETH value to send in the deposit transaction.
-     * @param _gasLimit The L2 gasLimit.
-     * @param _isCreation Whether or not the transaction should be contract creation.
-     * @param _data The input data.
+     * @param _to         Target address on L2.
+     * @param _value      ETH value to send to the recipient.
+     * @param _gasLimit   Minimum L2 gas limit (can be greater than or equal to this value).
+     * @param _isCreation Whether or not the transaction is a contract creation.
+     * @param _data       Data to trigger the recipient with.
      */
     function depositTransaction(
         address _to,
@@ -119,15 +131,15 @@ contract OptimismPortal is ResourceMetering {
     }
 
     /**
-     * Finalizes a withdrawal transaction.
+     * @notice Finalizes a withdrawal transaction.
      *
-     * @param _nonce Nonce for the provided message.
-     * @param _sender Message sender address on L2.
-     * @param _target Target address on L1.
-     * @param _value ETH to send to the target.
-     * @param _gasLimit Gas to be forwarded to the target.
-     * @param _data Data to send to the target.
-     * @param _l2Timestamp L2 timestamp of the outputRoot.
+     * @param _nonce           Nonce for the provided message.
+     * @param _sender          Message sender address on L2.
+     * @param _target          Target address on L1.
+     * @param _value           ETH to send to the target.
+     * @param _gasLimit        Minumum gas to be forwarded to the target.
+     * @param _data            Data to send to the target.
+     * @param _l2Timestamp     L2 timestamp of the outputRoot.
      * @param _outputRootProof Inclusion proof of the withdrawer contracts storage root.
      * @param _withdrawalProof Inclusion proof for the given withdrawal in the withdrawer contract.
      */

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

@@ -9,11 +9,12 @@ import { Burn } from "../libraries/Burn.sol";
 /**
  * @title ResourceMetering
  * @notice ResourceMetering implements an EIP-1559 style resource metering system where pricing
- * updates automatically based on current demand.
+ *         updates automatically based on current demand.
  */
 contract ResourceMetering {
     /**
-     * Struct representing current resource parameters.
+     * @notice Represents the various parameters that control the way in which resources are
+     *         metered. Corresponds to the EIP-1559 resource metering system.
      */
     struct ResourceParams {
         uint128 prevBaseFee;
@@ -22,42 +23,42 @@ contract ResourceMetering {
     }
 
     /**
-     * Along with the resource limit, determines the target resource limit.
+     * @notice Maximum amount of the resource that can be used within this block.
      */
-    int256 public constant ELASTICITY_MULTIPLIER = 4;
+    int256 public constant MAX_RESOURCE_LIMIT = 8_000_000;
 
     /**
-     * Denominator that determines max change on fee per block.
+     * @notice Along with the resource limit, determines the target resource limit.
      */
-    int256 public constant BASE_FEE_MAX_CHANGE_DENOMINATOR = 8;
+    int256 public constant ELASTICITY_MULTIPLIER = 4;
 
     /**
-     * Maximum amount of deposit gas that can be used within this block.
+     * @notice Target amount of the resource that should be used within this block.
      */
-    int256 public constant MAX_RESOURCE_LIMIT = 8_000_000;
+    int256 public constant TARGET_RESOURCE_LIMIT = MAX_RESOURCE_LIMIT / ELASTICITY_MULTIPLIER;
 
     /**
-     * Target amount of deposit gas that should be used within this block.
+     * @notice Denominator that determines max change on fee per block.
      */
-    int256 public constant TARGET_RESOURCE_LIMIT = MAX_RESOURCE_LIMIT / ELASTICITY_MULTIPLIER;
+    int256 public constant BASE_FEE_MAX_CHANGE_DENOMINATOR = 8;
 
     /**
-     * Minimum base fee value, cannot go lower than this.
+     * @notice Minimum base fee value, cannot go lower than this.
      */
     int256 public constant MINIMUM_BASE_FEE = 10_000;
 
     /**
-     * Initial base fee value.
+     * @notice Initial base fee value.
      */
     uint128 public constant INITIAL_BASE_FEE = 1_000_000_000;
 
     /**
-     * EIP-1559 style gas parameters.
+     * @notice EIP-1559 style gas parameters.
      */
     ResourceParams public params;
 
     /**
-     * Sets the initial resource values.
+     * @notice Sets initial resource parameter values.
      */
     constructor() {
         params = ResourceParams({
@@ -68,7 +69,7 @@ contract ResourceMetering {
     }
 
     /**
-     * Meters access to a function based an amount of a requested resource.
+     * @notice Meters access to a function based an amount of a requested resource.
      *
      * @param _amount Amount of the resource requested.
      */