chore: sync develop

.circleci/config.yml

@@ -121,6 +121,15 @@ commands:
   install-contracts-dependencies:
     description: "Install the dependencies for the smart contracts"
     steps:
+      - run:
+          name: Install SVM compilers
+          command: |
+            # Have to wipe out the SVM directory to avoid issues with the SVM version
+            rm -rf ~/.svm/*
+            svm install 0.8.15
+            svm install 0.8.19
+            svm install 0.8.25
+            svm install 0.8.28
       - run:
           name: Install dependencies
           command: |
@@ -342,8 +351,6 @@ jobs:
             - "packages/contracts-bedrock/cache"
             - "packages/contracts-bedrock/artifacts"
             - "packages/contracts-bedrock/forge-artifacts"
-            - "packages/contracts-bedrock/deploy-config/devnetL1.json"
-            - "packages/contracts-bedrock/deployments/devnetL1"
       - notify-failures-on-develop
 
   check-kontrol-build:
@@ -793,7 +800,8 @@ jobs:
           no_output_timeout: <<parameters.test_timeout>>
       - run:
           name: Print failed test traces
-          command: just test-rerun
+          command: |
+            just test-rerun | tee failed-test-traces.log
           environment:
             FOUNDRY_PROFILE: <<parameters.test_profile>>
             ETH_RPC_URL: https://ci-mainnet-l1-archive.optimism.io
@@ -803,10 +811,25 @@ jobs:
           disable_search: true
           files: ./packages/contracts-bedrock/lcov-all.info
           flags: contracts-bedrock-tests
+      - store_artifacts:
+          path: packages/contracts-bedrock/failed-test-traces.log
+          when: on_fail
       - notify-failures-on-develop
 
   contracts-bedrock-tests-upgrade:
     circleci_ip_ranges: true
+    parameters:
+      fork_op_chain:
+        description: Fork OP Chain
+        type: string
+        default: "op"
+      fork_base_chain:
+        description: Fork Base Chain
+        type: string
+        default: "mainnet"
+      fork_base_rpc:
+        description: Fork Base RPC
+        type: string
     docker:
       - image: <<pipeline.parameters.default_docker_image>>
     resource_class: large
@@ -833,28 +856,35 @@ jobs:
           command: |
             just print-pinned-block-number > ./pinnedBlockNumber.txt
             cat pinnedBlockNumber.txt
+          environment:
+            FORK_BASE_CHAIN: <<parameters.fork_base_chain>>
           working_directory: packages/contracts-bedrock
       - restore_cache:
           name: Restore forked state
-          key: forked-state-contracts-bedrock-tests-upgrade-{{ checksum "packages/contracts-bedrock/pinnedBlockNumber.txt" }}
+          key: forked-state-contracts-bedrock-tests-upgrade-<<parameters.fork_op_chain>>-<<parameters.fork_base_chain>>-{{ checksum "packages/contracts-bedrock/pinnedBlockNumber.txt" }}
       - run:
           name: Run tests
           command: just test-upgrade
           environment:
             FOUNDRY_FUZZ_SEED: 42424242
             FOUNDRY_FUZZ_RUNS: 1
             FOUNDRY_PROFILE: ci
-            ETH_RPC_URL: https://ci-mainnet-l1-archive.optimism.io
+            ETH_RPC_URL: <<parameters.fork_base_rpc>>
+            FORK_OP_CHAIN: <<parameters.fork_op_chain>>
+            FORK_BASE_CHAIN: <<parameters.fork_base_chain>>
           working_directory: packages/contracts-bedrock
           no_output_timeout: 15m
       - run:
           name: Print failed test traces
-          command: just test-upgrade-rerun
+          command: |
+            just test-upgrade-rerun | tee failed-test-traces.log
           environment:
             FOUNDRY_FUZZ_SEED: 42424242
             FOUNDRY_FUZZ_RUNS: 1
             FOUNDRY_PROFILE: ci
-            ETH_RPC_URL: https://ci-mainnet-l1-archive.optimism.io
+            ETH_RPC_URL: <<parameters.fork_base_rpc>>
+            FORK_OP_CHAIN: <<parameters.fork_op_chain>>
+            FORK_BASE_CHAIN: <<parameters.fork_base_chain>>
           working_directory: packages/contracts-bedrock
           when: on_fail
       - save_cache:
@@ -864,10 +894,13 @@ jobs:
             - "/root/.cache/go-build"
       - save_cache:
           name: Save forked state
-          key: forked-state-contracts-bedrock-tests-upgrade-{{ checksum "packages/contracts-bedrock/pinnedBlockNumber.txt" }}
+          key: forked-state-contracts-bedrock-tests-upgrade-<<parameters.fork_op_chain>>-<<parameters.fork_base_chain>>-{{ checksum "packages/contracts-bedrock/pinnedBlockNumber.txt" }}
           when: always
           paths:
             - "/root/.foundry/cache"
+      - store_artifacts:
+          path: packages/contracts-bedrock/failed-test-traces.log
+          when: on_fail
       - notify-failures-on-develop
 
   contracts-bedrock-checks:
@@ -900,6 +933,8 @@ jobs:
           command: snapshots-check-no-build
       - run-contracts-check:
           command: interfaces-check-no-build
+      - run-contracts-check:
+          command: reinitializer-check-no-build
       - run-contracts-check:
           command: size-check
       - run-contracts-check:
@@ -921,7 +956,7 @@ jobs:
           command: sudo apt-get install -y ripgrep
       - run:
           name: Check TODO issues
-          command: ./ops/scripts/todo-checker.sh --verbose <<#parameters.check_closed>> --check-closed <</parameters.check_closed>>
+          command: ./ops/scripts/todo-checker.sh --verbose --strict <<#parameters.check_closed>> --check-closed <</parameters.check_closed>>
       - notify-failures-on-develop
 
   fuzz-golang:
@@ -1184,7 +1219,12 @@ jobs:
     steps:
       - utils/checkout-with-mise
       - setup_remote_docker
-      - run: make -C op-program verify-reproducibility
+      - run:
+          name: Verify reproducibility
+          command: make -C op-program verify-reproducibility
+      - store_artifacts:
+          path: ./op-program/temp/logs
+          when: always
       - notify-failures-on-develop:
           mentions: "@proofs-team"
 
@@ -1446,6 +1486,9 @@ workflows:
             - contracts-bedrock-build
       - contracts-bedrock-tests-upgrade:
           name: contracts-bedrock-tests-upgrade
+          fork_op_chain: op
+          fork_base_chain: mainnet
+          fork_base_rpc: https://ci-mainnet-l1-archive.optimism.io
       - contracts-bedrock-checks:
           requires:
             - contracts-bedrock-build
@@ -1506,6 +1549,7 @@ workflows:
             op-e2e/actions
             op-e2e/faultproofs
             packages/contracts-bedrock/scripts/checks
+            packages/contracts-bedrock/scripts/verify
             op-dripper
           requires:
             - contracts-bedrock-build

.github/CODEOWNERS

@@ -34,3 +34,6 @@
 
 # Contracts
 /packages/contracts-bedrock     @ethereum-optimism/contract-reviewers
+
+# Security docs
+/docs @ethereum-optimism/evm-safety

.gitignore

@@ -22,7 +22,7 @@ cache
 
 !op-deployer/pkg/deployer/artifacts
 
-packages/contracts-bedrock/deployments/devnetL1
+
 packages/contracts-bedrock/deployments/anvil
 
 # vim

.semgrep/rules/sol-rules.yaml

@@ -247,7 +247,7 @@ rules:
   - id: sol-safety-proper-initializer
     languages: [solidity]
     severity: ERROR
-    message: Proxied contracts must have an initialize function with the initializer modifier and external visibility
+    message: Proxied contracts must have an initialize function with the initializer or reinitializer modifier and external or public visibility
     patterns:
       - pattern-regex: "///\\s*@custom:proxied\\s+true(?P<CONTRACT>[\\s\\S]*)"
       - focus-metavariable: $CONTRACT
@@ -259,6 +259,34 @@ rules:
           function initialize(...) external initializer {
             ...
           }
+      - pattern-not: |
+          function initialize(...) public initializer {
+            ...
+          }
+      - pattern-not: |
+          function initialize(...) external reinitializer(...) {
+            ...
+          }
+      - pattern-not: |
+          function initialize(...) public reinitializer(...) {
+            ...
+          }
     paths:
       exclude:
-        - packages/contracts-bedrock/src/L1/SystemConfig.sol
+        - packages/contracts-bedrock/src/L1/SystemConfigInterop.sol
+
+  - id: sol-safety-proper-upgrade-function
+    languages: [solidity]
+    severity: ERROR
+    message: Upgrade functions must be external and have the reinitializer modifier
+    patterns:
+      - pattern-regex: "///\\s*@custom:proxied\\s+true(?P<CONTRACT>[\\s\\S]*)"
+      - focus-metavariable: $CONTRACT
+      - pattern: |
+          function upgrade(...) {
+            ...
+          }
+      - pattern-not: |
+          function upgrade(...) external reinitializer(...) {
+            ...
+          }

.semgrep/tests/sol-rules.sol-safety-proper-initializer.t.sol

@@ -32,13 +32,28 @@ contract SemgrepTest__sol_safety_proper_initializer {
         // ...
     }
 
-    // ruleid: sol-safety-proper-initializer
-    function initialize() external {
+    // ok: sol-safety-proper-initializer
+    function initialize() public initializer {
+        // ...
+    }
+
+    // ok: sol-safety-proper-initializer
+    function initialize() external reinitializer(1) {
+        // ...
+    }
+
+    // ok: sol-safety-proper-initializer
+    function initialize() external reinitializer(1) {
+        // ...
+    }
+
+    // ok: sol-safety-proper-initializer
+    function initialize() public reinitializer(2) {
         // ...
     }
 
     // ruleid: sol-safety-proper-initializer
-    function initialize() public initializer {
+    function initialize() internal {
         // ...
     }
 

.semgrep/tests/sol-rules.sol-safety-proper-upgrade-function.t.sol

@@ -0,0 +1,64 @@
+// Semgrep tests for Solidity rules are defined in this file.
+// Semgrep tests do not need to be valid Solidity code but should be syntactically correct so that
+// Semgrep can parse them. You don't need to be able to *run* the code here but it should look like
+// the code that you expect to catch with the rule.
+//
+// Semgrep testing 101
+// Use comments like "ruleid: <rule-id>" to assert that the rule catches the code.
+// Use comments like "ok: <rule-id>" to assert that the rule does not catch the code.
+
+/// NOTE: Semgrep limitations mean that the rule for this check is defined as a relatively loose regex that searches the
+/// remainder of the file after the `@custom:proxied` natspec tag is detected. This means that we must test the case
+/// without this natspec tag BEFORE the case with the tag or the rule will apply to the remainder of the file.
+
+// If no proxied natspec, upgrade functions can have no upgrade modifier and be public or external
+contract SemgrepTest__sol_safety_proper_upgrade_function_1 {
+    // ok: sol-safety-proper-upgrade-function
+    function upgrade() external {
+        // ...
+    }
+
+    // ok: sol-safety-proper-upgrade-function
+    function upgrade() public {
+        // ...
+    }
+}
+
+/// NOTE: the proxied natspec below is valid for all contracts after this one
+/// @custom:proxied true
+contract SemgrepTest__sol_safety_proper_upgrade_function_2 {
+    // ok: sol-safety-proper-upgrade-function
+    function upgrade() external reinitializer(1) {
+        // ...
+    }
+
+    // ok: sol-safety-proper-upgrade-function
+    function upgrade() external reinitializer(1) {
+        // ...
+    }
+
+    // ruleid: sol-safety-proper-upgrade-function
+    function upgrade() public reinitializer(2) {
+        // ...
+    }
+
+    // ruleid: sol-safety-proper-upgrade-function
+    function upgrade() external initializer {
+        // ...
+    }
+
+    // ruleid: sol-safety-proper-upgrade-function
+    function upgrade() public initializer {
+        // ...
+    }
+
+    // ruleid: sol-safety-proper-upgrade-function
+    function upgrade() external {
+        // ...
+    }
+
+    // ruleid: sol-safety-proper-upgrade-function
+    function upgrade() public {
+        // ...
+    }
+}