fix: incompatible storage layout for Bootstrap and ClientChainGateway (…

…ExocoreNetwork#72)

* fix: incompatible storage layout for Bootstrap and ClientChainGateway

* feat: ci for storage

* fix(typo): rename the step for grammar

* fix(ci): add back the snapshot

* docs: remove storage layout files

---------

Co-authored-by: MaxMustermann2 <[email protected]>

.github/workflows/compare_storage_layout.py

@@ -0,0 +1,68 @@
+#!/usr/bin/env python
+
+import pandas as pd
+import os
+
+def parse_layout(file_path):
+    expected_headers = ['Unnamed: 0', 'Name', 'Type', 'Slot', 'Offset', 'Bytes', 'Contract', 'Unnamed: 7']
+
+    if not os.path.isfile(file_path):
+        raise FileNotFoundError(f"Error: File {file_path} does not exist.")
+
+    # Read the file using pandas, with '|' as the delimiter
+    df = pd.read_csv(file_path, delimiter='|', engine='python', header=0)
+
+    # Trim leading/trailing whitespace from all columns
+    df.columns = [col.strip() for col in df.columns]
+    df = df.apply(lambda x: x.strip() if isinstance(x, str) else x)
+
+    # Check headers
+    if not all([df.columns[i] == expected_headers[i] for i in range(len(expected_headers))]):
+        raise ValueError(f"Error: Headers in {file_path} do not match expected headers.")
+
+    # Drop the second row (assuming it's a separator row)
+    df = df.drop(df.index[1])
+
+    # Combine relevant columns into a single string for comparison
+    df['Combined'] = df[['Name', 'Type', 'Slot', 'Offset', 'Bytes']].apply(lambda row: '|'.join(row.values), axis=1)
+
+    return df['Combined'].tolist()
+
+def compare_layouts(clientChainGateway_entries, bootstrap_entries):
+    mismatches = []
+    length = len(bootstrap_entries)
+
+    if length > len(clientChainGateway_entries):
+        mismatches.append("Error: Bootstrap entries are more than ClientChainGateway entries.")
+        return mismatches
+
+    for i in range(length):
+        if bootstrap_entries[i] != clientChainGateway_entries[i]:
+            mismatches.append(f"Mismatch at position {i + 1}: {bootstrap_entries[i]} != {clientChainGateway_entries[i]}")
+
+    return mismatches
+
+if __name__ == "__main__":
+    try:
+        clientChainGateway_entries = parse_layout("ClientChainGateway.md")
+        bootstrap_entries = parse_layout("Bootstrap.md")
+
+        if not clientChainGateway_entries:
+            raise ValueError("Error: No valid entries found in ClientChainGateway.md.")
+
+        if not bootstrap_entries:
+            raise ValueError("Error: No valid entries found in Bootstrap.md.")
+
+        mismatches = compare_layouts(clientChainGateway_entries, bootstrap_entries)
+
+        if mismatches:
+            print(f"Mismatches found: {len(mismatches)}")
+            for mismatch in mismatches:
+                print(mismatch)
+            exit(1)
+        else:
+            print("All entries in Bootstrap are present in ClientChainGateway at the correct positions.")
+    except Exception as e:
+        print(e)
+        exit(1)
+

.github/workflows/forge-ci.yml

@@ -1,5 +1,4 @@
----
-name: Forge CI to build, test and format
+name: Forge CI to build, test, format and compare storage layout
 
 on:
   merge_group:
@@ -24,7 +23,8 @@ jobs:
       installation-dir: ${{ needs.setup.outputs.installation-dir }}
       cache-key: ${{ needs.setup.outputs.cache-key }}
     steps:
-      - uses: actions/checkout@v4
+      - name: Checkout repository
+        uses: actions/checkout@v4
         with:
           submodules: recursive
       - name: Restore cached Foundry toolchain
@@ -62,7 +62,8 @@ jobs:
     runs-on: ubuntu-latest
     needs: build
     steps:
-      - uses: actions/checkout@v4
+      - name: Checkout repository
+        uses: actions/checkout@v4
       - name: Restore cached Foundry toolchain
         uses: actions/cache/restore@v3
         with:
@@ -79,9 +80,9 @@ jobs:
             ./cache
             ./broadcast
           key: ${{ runner.os }}-build-${{ github.sha }}
-      - name: Run tests
+      - name: Test
         run: forge test -vvv
-      - name: Run test snapshot
+      - name: Set test snapshot as summary
         run: NO_COLOR=1 forge snapshot >> $GITHUB_STEP_SUMMARY
       - name: Add comment for test failure
         if: failure()
@@ -96,11 +97,12 @@ jobs:
               body: 'The tests have failed. Please check the logs.'
             })
 
-  fmt:
+  format:
     runs-on: ubuntu-latest
     needs: build
     steps:
-      - uses: actions/checkout@v4
+      - name: Checkout repository
+        uses: actions/checkout@v4
       - name: Restore cached Foundry toolchain
         uses: actions/cache/restore@v3
         with:
@@ -119,7 +121,7 @@ jobs:
           key: ${{ runner.os }}-build-${{ github.sha }}
       - name: Check formatting
         run: forge fmt --check
-      - name: Add comment for format failure
+      - name: Add comment for format check failure
         if: failure()
         uses: actions/github-script@v6
         with:
@@ -131,3 +133,50 @@ jobs:
               repo: context.repo.repo,
               body: 'The code is not formatted correctly. Please run `forge fmt` and push the changes.'
             })
+
+  compare-storage-layout:
+    runs-on: ubuntu-latest
+    needs: build
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+      - name: Restore cached Foundry toolchain
+        uses: actions/cache/restore@v3
+        with:
+          path: ${{ needs.build.outputs.installation-dir }}
+          key: ${{ needs.build.outputs.cache-key }}
+      - name: Add Foundry to PATH
+        run: echo "${{ needs.build.outputs.installation-dir }}" >> $GITHUB_PATH
+      - name: Restore build artifacts
+        uses: actions/cache/restore@v3
+        with:
+          path: |
+            ./lib
+            ./out
+            ./cache
+            ./broadcast
+          key: ${{ runner.os }}-build-${{ github.sha }}
+      - name: Run forge inspect storage layout on ClientChainGateway
+        run: forge inspect --pretty src/core/ClientChainGateway.sol:ClientChainGateway storageLayout > ClientChainGateway.md
+      - name: Run forge inspect storage layout on Bootstrap
+        run: forge inspect --pretty src/core/Bootstrap.sol:Bootstrap storageLayout > Bootstrap.md
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: '3.12.4'
+      - name: Install pandas
+        run: pip install --root-user-action=ignore pandas==2.2.2
+      - name: Run the comparison script
+        run: python .github/workflows/compare_storage_layout.py
+      - name: Add comment for storage layout mismatch failure
+        if: failure()
+        uses: actions/github-script@v6
+        with:
+          github-token: ${{ secrets.GITHUB_TOKEN }}
+          script: |
+            github.rest.issues.createComment({
+              issue_number: context.issue.number,
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              body: 'The storage layout of Bootstrap and ClientChainGateway is not the same. Please check the logs.'
+            })

.github/workflows/foundry-setup.yml

@@ -36,7 +36,7 @@ jobs:
           installation_path=$(which forge)
           installation_dir=$(dirname $installation_path)
           echo "installation-dir=$installation_dir" >> "$GITHUB_OUTPUT"
-      - name: Cached Foundry toolchain
+      - name: Cache the Foundry toolchain
         uses: actions/cache/save@v3
         with:
           path: ${{ steps.find-path.outputs.installation-dir }}

src/core/BaseRestakingController.sol

@@ -8,6 +8,8 @@ import {MessagingFee, MessagingReceipt, OAppSenderUpgradeable} from "../lzApp/OA
 import {ClientChainGatewayStorage} from "../storage/ClientChainGatewayStorage.sol";
 
 import {OptionsBuilder} from "@layerzero-v2/oapp/contracts/oapp/libs/OptionsBuilder.sol";
+
+import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
 import {PausableUpgradeable} from "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol";
 import {ReentrancyGuardUpgradeable} from "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
 
@@ -18,6 +20,7 @@ import {ReentrancyGuardUpgradeable} from "@openzeppelin/contracts-upgradeable/se
 /// Bootstrap.
 abstract contract BaseRestakingController is
     PausableUpgradeable,
+    OwnableUpgradeable,
     ReentrancyGuardUpgradeable,
     OAppSenderUpgradeable,
     IBaseRestakingController,

src/core/LSTRestakingController.sol

@@ -6,6 +6,7 @@ import {ILSTRestakingController} from "../interfaces/ILSTRestakingController.sol
 import {IVault} from "../interfaces/IVault.sol";
 import {BaseRestakingController} from "./BaseRestakingController.sol";
 
+import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
 import {PausableUpgradeable} from "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol";
 import {ReentrancyGuardUpgradeable} from "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
 
@@ -14,6 +15,7 @@ import {ReentrancyGuardUpgradeable} from "@openzeppelin/contracts-upgradeable/se
 /// @notice Implementation of ILSTRestakingController, used to restake tokens.
 abstract contract LSTRestakingController is
     PausableUpgradeable,
+    OwnableUpgradeable,
     ReentrancyGuardUpgradeable,
     ILSTRestakingController,
     BaseRestakingController

src/core/NativeRestakingController.sol

@@ -8,6 +8,8 @@ import {ValidatorContainer} from "../libraries/ValidatorContainer.sol";
 import {BaseRestakingController} from "./BaseRestakingController.sol";
 
 import {Errors} from "../libraries/Errors.sol";
+
+import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
 import {PausableUpgradeable} from "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol";
 import {ReentrancyGuardUpgradeable} from "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
 import {Create2} from "@openzeppelin/contracts/utils/Create2.sol";
@@ -19,6 +21,7 @@ import {Create2} from "@openzeppelin/contracts/utils/Create2.sol";
 /// @dev This contract is abstract because it does not call the base constructor.
 abstract contract NativeRestakingController is
     PausableUpgradeable,
+    OwnableUpgradeable,
     ReentrancyGuardUpgradeable,
     INativeRestakingController,
     BaseRestakingController