github.com/ethereum-optimism/optimism@v1.7.2/packages/contracts-bedrock/STYLE_GUIDE.md (about)

     1  # Smart Contract Style Guide
     2  
     3  This document provides guidance on how we organize and write our smart contracts. For cases where
     4  this document does not provide guidance, please refer to existing contracts for guidance,
     5  with priority on the `L2OutputOracle` and `OptimismPortal`.
     6  
     7  ## Standards and Conventions
     8  
     9  ### Style
    10  
    11  #### Comments
    12  
    13  Optimism smart contracts follow the triple-slash [solidity natspec comment style](https://docs.soliditylang.org/en/develop/natspec-format.html#documentation-example)
    14  with additional rules. These are:
    15  
    16  - Always use `@notice` since it has the same general effect as `@dev` but avoids confusion about when to use one over the other.
    17  - Include a newline between `@notice` and the first `@param`.
    18  - Include a newline between `@param` and the first `@return`.
    19  - Use a line-length of 100 characters.
    20  
    21  We also have the following custom tags:
    22  
    23  - `@custom:proxied`: Add to a contract whenever it's meant to live behind a proxy.
    24  - `@custom:upgradeable`: Add to a contract whenever it's meant to be inherited by an upgradeable contract.
    25  - `@custom:semver`: Add to `version` variable which indicate the contracts semver.
    26  - `@custom:legacy`: Add to an event or function when it only exists for legacy support.
    27  - `@custom:network-specific`: Add to state variables which vary between OP Chains.
    28  
    29  #### Errors
    30  
    31  - Use `require` statements when making simple assertions.
    32  - Use `revert(string)` if throwing an error where an assertion is not being made (no custom errors).
    33    See [here](https://github.com/ethereum-optimism/optimism/blob/861ae315a6db698a8c0adb1f8eab8311fd96be4c/packages/contracts-bedrock/contracts/L2/OVM_ETH.sol#L31)
    34    for an example of this in practice.
    35  - Error strings MUST have the format `"{ContractName}: {message}"` where `message` is a lower case string.
    36  
    37  #### Function Parameters
    38  
    39  - Function parameters should be prefixed with an underscore.
    40  
    41  #### Function Return Arguments
    42  
    43  - Arguments returned by functions should be suffixed with an underscore.
    44  
    45  #### Event Parameters
    46  
    47  - Event parameters should NOT be prefixed with an underscore.
    48  
    49  #### Immutable variables
    50  
    51  Immutable variables:
    52  
    53  - should be in `SCREAMING_SNAKE_CASE`
    54  - should be `internal`
    55  - should have a hand written getter function
    56  
    57  This approach clearly indicates to the developer that the value is immutable, without exposing
    58  the non-standard casing to the interface. It also ensures that we don’t need to break the ABIs if
    59  we switch between values being in storage and immutable.
    60  
    61  #### Spacers
    62  
    63  We use spacer variables to account for old storage slots that are no longer being used.
    64  The name of a spacer variable MUST be in the format `spacer_<slot>_<offset>_<length>` where
    65  `<slot>` is the original storage slot number, `<offset>` is the original offset position
    66  within the storage slot, and `<length>` is the original size of the variable.
    67  Spacers MUST be `private`.
    68  
    69  ### Proxy by Default
    70  
    71  All contracts should be assumed to live behind proxies (except in certain special circumstances).
    72  This means that new contracts MUST be built under the assumption of upgradeability.
    73  We use a minimal [`Proxy`](./contracts/universal/Proxy.sol) contract designed to be owned by a
    74  corresponding [`ProxyAdmin`](./contracts/universal/ProxyAdmin.sol) which follow the interfaces
    75  of OpenZeppelin's `Proxy` and `ProxyAdmin` contracts, respectively.
    76  
    77  Unless explicitly discussed otherwise, you MUST include the following basic upgradeability
    78  pattern for each new implementation contract:
    79  
    80  1. Extend OpenZeppelin's `Initializable` base contract.
    81  2. Include a `uint8 public constant VERSION = X` at the TOP of your contract.
    82  3. Include a function `initialize` with the modifier `reinitializer(VERSION)`.
    83  4. In the `constructor`, set any `immutable` variables and call the `initialize` function for setting mutables.
    84  
    85  ### Versioning
    86  
    87  All (non-library and non-abstract) contracts MUST extend the `Semver` base contract which
    88  exposes a `version()` function that returns a semver-compliant version string.
    89  
    90  Contracts must have a `Semver` of `1.0.0` or greater to be production ready. Contracts
    91  with `Semver` values less than `1.0.0` should only be used locally or on devnets.
    92  
    93  Additionally, contracts MUST use the following versioning scheme:
    94  
    95  - `patch` releases are to be used only for changes that do NOT modify contract bytecode (such as updating comments).
    96  - `minor` releases are to be used for changes that modify bytecode OR changes that expand the contract ABI provided that these changes do NOT break the existing interface.
    97  - `major` releases are to be used for changes that break the existing contract interface OR changes that modify the security model of a contract.
    98  
    99  #### Exceptions
   100  
   101  We have made an exception to the `Semver` rule for the `WETH` contract to avoid
   102  making changes to a well-known, simple, and recognizable contract.
   103  
   104  ### Dependencies
   105  
   106  Where basic functionality is already supported by an existing contract in the OpenZeppelin library,
   107  we should default to using the Upgradeable version of that contract.
   108  
   109  ### Tests
   110  
   111  Tests are written using Foundry.
   112  
   113  All test contracts and functions should be organized and named according to the following guidelines.
   114  
   115  These guidelines are also encoded in a script which can be run with:
   116  
   117  ```
   118  tsx scripts/checks/check-test-names.ts
   119  ```
   120  
   121  #### Expect Revert with Low Level Calls
   122  
   123  There is a non-intuitive behavior in foundry tests, which is documented [here](https://book.getfoundry.sh/cheatcodes/expect-revert?highlight=expectrevert#expectrevert).
   124  When testing for a revert on a low-level call, please use the `revertsAsExpected` pattern suggested there.
   125  
   126  _Note: This is a work in progress, not all test files are compliant with these guidelines._
   127  
   128  #### Organizing Principles
   129  
   130  - Solidity `contract`s are used to organize the test suite similar to how mocha uses describe.
   131  - Every non-trivial state changing function should have a separate contract for happy and sad path
   132    tests. This helps to make it very obvious where there are not yet sad path tests.
   133  - Simpler functions like getters and setters are grouped together into test contracts.
   134  
   135  #### Test function naming convention
   136  
   137  Test function names are split by underscores, into 3 or 4 parts. An example function name is `test_onlyOwner_callerIsNotOwner_reverts()`.
   138  
   139  The parts are: `[method]_[FunctionName]_[reason]_[status]`, where:
   140  
   141  - `[method]` is either `test`, `testFuzz`, or `testDiff`
   142  - `[FunctionName]` is the name of the function or higher level behavior being tested.
   143  - `[reason]` is an optional description for the behavior being tested.
   144  - `[status]` must be one of:
   145    - `succeeds`: used for most happy path cases
   146    - `reverts`: used for most sad path cases
   147    - `works`: used for tests which include a mix of happy and sad assertions (these should be broken up if possible)
   148    - `fails`: used for tests which 'fail' in some way other than reverting
   149    - `benchmark`: used for tests intended to establish gas costs
   150  
   151  #### Contract Naming Conventions
   152  
   153  Test contracts should be named one of the following according to their use:
   154  
   155  - `TargetContract_Init` for contracts that perform basic setup to be reused in other test contracts.
   156  - `TargetContract_Function_Test` for contracts containing happy path tests for a given function.
   157  - `TargetContract_Function_TestFail` for contracts containing sad path tests for a given function.
   158  
   159  To minimize clutter, getter functions can be grouped together into a single test contract,
   160    ie. `TargetContract_Getters_Test`.
   161  
   162  ## Withdrawing From Fee Vaults
   163  
   164  See the file `scripts/FeeVaultWithdrawal.s.sol` to withdraw from the L2 fee vaults. It includes
   165  instructions on how to run it. `foundry` is required.